From 5f87736966c0a2ef4d5d4a3c5c541d771d45bcb3 Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Tue, 30 Aug 2011 18:17:27 -0700 Subject: [PATCH] lacp: Clean up LACP module interface. There's no particular reason to force users of the LACP module to be aware of the lacp_pdu structure. This patch hides that information in the LACP module implementation. This results in slightly cleaner code which is more consistent with the CFM module. --- lib/lacp.c | 75 +++++++++++++++++++++++++++++++++++++----- lib/lacp.h | 58 ++------------------------------ ofproto/ofproto-dpif.c | 14 ++++---- 3 files changed, 75 insertions(+), 72 deletions(-) diff --git a/lib/lacp.c b/lib/lacp.c index eaf01c3c..2504e6b3 100644 --- a/lib/lacp.c +++ b/lib/lacp.c @@ -32,6 +32,55 @@ VLOG_DEFINE_THIS_MODULE(lacp); +/* Masks for lacp_info state member. */ +#define LACP_STATE_ACT 0x01 /* Activity. Active or passive? */ +#define LACP_STATE_TIME 0x02 /* Timeout. Short or long timeout? */ +#define LACP_STATE_AGG 0x04 /* Aggregation. Is the link is bondable? */ +#define LACP_STATE_SYNC 0x08 /* Synchronization. Is the link in up to date? */ +#define LACP_STATE_COL 0x10 /* Collecting. Is the link receiving frames? */ +#define LACP_STATE_DIST 0x20 /* Distributing. Is the link sending frames? */ +#define LACP_STATE_DEF 0x40 /* Defaulted. Using default partner info? */ +#define LACP_STATE_EXP 0x80 /* Expired. Using expired partner info? */ + +#define LACP_FAST_TIME_TX 1000 /* Fast transmission rate. */ +#define LACP_SLOW_TIME_TX 30000 /* Slow transmission rate. */ +#define LACP_RX_MULTIPLIER 3 /* Multiply by TX rate to get RX rate. */ + +#define LACP_INFO_LEN 15 +struct lacp_info { + ovs_be16 sys_priority; /* System priority. */ + uint8_t sys_id[ETH_ADDR_LEN]; /* System ID. */ + ovs_be16 key; /* Operational key. */ + ovs_be16 port_priority; /* Port priority. */ + ovs_be16 port_id; /* Port ID. */ + uint8_t state; /* State mask. See LACP_STATE macros. */ +} __attribute__((packed)); +BUILD_ASSERT_DECL(LACP_INFO_LEN == sizeof(struct lacp_info)); + +#define LACP_PDU_LEN 110 +struct lacp_pdu { + uint8_t subtype; /* Always 1. */ + uint8_t version; /* Always 1. */ + + uint8_t actor_type; /* Always 1. */ + uint8_t actor_len; /* Always 20. */ + struct lacp_info actor; /* LACP actor information. */ + uint8_t z1[3]; /* Reserved. Always 0. */ + + uint8_t partner_type; /* Always 2. */ + uint8_t partner_len; /* Always 20. */ + struct lacp_info partner; /* LACP partner information. */ + uint8_t z2[3]; /* Reserved. Always 0. */ + + uint8_t collector_type; /* Always 3. */ + uint8_t collector_len; /* Always 16. */ + ovs_be16 collector_delay; /* Maximum collector delay. Set to UINT16_MAX. */ + uint8_t z3[64]; /* Combination of several fields. Always 0. */ +} __attribute__((packed)); +BUILD_ASSERT_DECL(LACP_PDU_LEN == sizeof(struct lacp_pdu)); + +/* Implementation. */ + enum slave_status { LACP_CURRENT, /* Current State. Partner up to date. */ LACP_EXPIRED, /* Expired State. Partner out of date. */ @@ -90,7 +139,7 @@ static void lacp_unixctl_show(struct unixctl_conn *, const char *args, void *aux); /* Populates 'pdu' with a LACP PDU comprised of 'actor' and 'partner'. */ -void +static void compose_lacp_pdu(const struct lacp_info *actor, const struct lacp_info *partner, struct lacp_pdu *pdu) { @@ -116,7 +165,7 @@ compose_lacp_pdu(const struct lacp_info *actor, * returns NULL if 'b' is malformed, or does not represent a LACP PDU format * supported by OVS. Otherwise, it returns a pointer to the lacp_pdu contained * within 'b'. */ -const struct lacp_pdu * +static const struct lacp_pdu * parse_lacp_packet(const struct ofpbuf *b) { const struct lacp_pdu *pdu; @@ -202,16 +251,24 @@ lacp_is_active(const struct lacp *lacp) return lacp->active; } -/* Processes 'pdu', a parsed LACP packet received on 'slave_'. This function - * should be called on all packets received on 'slave_' with Ethernet Type - * ETH_TYPE_LACP and parsable by parse_lacp_packet(). */ +/* Processes 'packet' which was received on 'slave_'. This function should be + * called on all packets received on 'slave_' with Ethernet Type ETH_TYPE_LACP. + */ void -lacp_process_pdu(struct lacp *lacp, const void *slave_, - const struct lacp_pdu *pdu) +lacp_process_packet(struct lacp *lacp, const void *slave_, + const struct ofpbuf *packet) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); struct slave *slave = slave_lookup(lacp, slave_); + const struct lacp_pdu *pdu; long long int tx_rate; + pdu = parse_lacp_packet(packet); + if (!pdu) { + VLOG_WARN_RL(&rl, "%s: received an unparsable LACP PDU.", lacp->name); + return; + } + switch (lacp->lacp_time) { case LACP_TIME_FAST: tx_rate = LACP_FAST_TIME_TX; @@ -371,7 +428,6 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) } HMAP_FOR_EACH (slave, node, &lacp->slaves) { - struct lacp_pdu pdu; struct lacp_info actor; if (!slave_may_tx(slave)) { @@ -383,10 +439,11 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) if (timer_expired(&slave->tx) || !info_tx_equal(&actor, &slave->ntt_actor)) { long long int duration; + struct lacp_pdu pdu; slave->ntt_actor = actor; compose_lacp_pdu(&actor, &slave->partner, &pdu); - send_pdu(slave->aux, &pdu); + send_pdu(slave->aux, &pdu, sizeof pdu); if (lacp->lacp_time == LACP_TIME_CUSTOM) { duration = lacp->custom_time; diff --git a/lib/lacp.h b/lib/lacp.h index 0fb797e8..27dc174e 100644 --- a/lib/lacp.h +++ b/lib/lacp.h @@ -21,58 +21,6 @@ #include #include "packets.h" -/* Masks for lacp_info state member. */ -#define LACP_STATE_ACT 0x01 /* Activity. Active or passive? */ -#define LACP_STATE_TIME 0x02 /* Timeout. Short or long timeout? */ -#define LACP_STATE_AGG 0x04 /* Aggregation. Is the link is bondable? */ -#define LACP_STATE_SYNC 0x08 /* Synchronization. Is the link in up to date? */ -#define LACP_STATE_COL 0x10 /* Collecting. Is the link receiving frames? */ -#define LACP_STATE_DIST 0x20 /* Distributing. Is the link sending frames? */ -#define LACP_STATE_DEF 0x40 /* Defaulted. Using default partner info? */ -#define LACP_STATE_EXP 0x80 /* Expired. Using expired partner info? */ - -#define LACP_FAST_TIME_TX 1000 /* Fast transmission rate. */ -#define LACP_SLOW_TIME_TX 30000 /* Slow transmission rate. */ -#define LACP_RX_MULTIPLIER 3 /* Multiply by TX rate to get RX rate. */ - -#define LACP_INFO_LEN 15 -struct lacp_info { - ovs_be16 sys_priority; /* System priority. */ - uint8_t sys_id[ETH_ADDR_LEN]; /* System ID. */ - ovs_be16 key; /* Operational key. */ - ovs_be16 port_priority; /* Port priority. */ - ovs_be16 port_id; /* Port ID. */ - uint8_t state; /* State mask. See LACP_STATE macros. */ -} __attribute__((packed)); -BUILD_ASSERT_DECL(LACP_INFO_LEN == sizeof(struct lacp_info)); - -#define LACP_PDU_LEN 110 -struct lacp_pdu { - uint8_t subtype; /* Always 1. */ - uint8_t version; /* Always 1. */ - - uint8_t actor_type; /* Always 1. */ - uint8_t actor_len; /* Always 20. */ - struct lacp_info actor; /* LACP actor information. */ - uint8_t z1[3]; /* Reserved. Always 0. */ - - uint8_t partner_type; /* Always 2. */ - uint8_t partner_len; /* Always 20. */ - struct lacp_info partner; /* LACP partner information. */ - uint8_t z2[3]; /* Reserved. Always 0. */ - - uint8_t collector_type; /* Always 3. */ - uint8_t collector_len; /* Always 16. */ - ovs_be16 collector_delay; /* Maximum collector delay. Set to UINT16_MAX. */ - uint8_t z3[64]; /* Combination of several fields. Always 0. */ -} __attribute__((packed)); -BUILD_ASSERT_DECL(LACP_PDU_LEN == sizeof(struct lacp_pdu)); - -void compose_lacp_pdu(const struct lacp_info *actor, - const struct lacp_info *partner, struct lacp_pdu *); - -const struct lacp_pdu *parse_lacp_packet(const struct ofpbuf *); - /* LACP Protocol Implementation. */ enum lacp_time { @@ -98,8 +46,8 @@ void lacp_destroy(struct lacp *); void lacp_configure(struct lacp *, const struct lacp_settings *); bool lacp_is_active(const struct lacp *); -void lacp_process_pdu(struct lacp *, const void *slave, - const struct lacp_pdu *); +void lacp_process_packet(struct lacp *, const void *slave, + const struct ofpbuf *packet); bool lacp_negotiated(const struct lacp *); struct lacp_slave_settings { @@ -118,7 +66,7 @@ uint16_t lacp_slave_get_port_id(const struct lacp *, const void *slave); bool lacp_slave_is_current(const struct lacp *, const void *slave_); /* Callback function for lacp_run() for sending a LACP PDU. */ -typedef void lacp_send_pdu(void *slave, const struct lacp_pdu *); +typedef void lacp_send_pdu(void *slave, const void *pdu, size_t pdu_size); void lacp_run(struct lacp *, lacp_send_pdu *); void lacp_wait(struct lacp *); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index f09c230d..478768f0 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -1111,7 +1111,7 @@ bundle_remove(struct ofport *port_) } static void -send_pdu_cb(void *port_, const struct lacp_pdu *pdu) +send_pdu_cb(void *port_, const void *pdu, size_t pdu_size) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 10); struct ofport_dpif *port = port_; @@ -1120,13 +1120,14 @@ send_pdu_cb(void *port_, const struct lacp_pdu *pdu) error = netdev_get_etheraddr(port->up.netdev, ea); if (!error) { - struct lacp_pdu *packet_pdu; struct ofpbuf packet; + void *packet_pdu; ofpbuf_init(&packet, 0); packet_pdu = eth_compose(&packet, eth_addr_lacp, ea, ETH_TYPE_LACP, - sizeof *packet_pdu); - *packet_pdu = *pdu; + pdu_size); + memcpy(packet_pdu, pdu, pdu_size); + error = netdev_send(port->up.netdev, &packet); if (error) { VLOG_WARN_RL(&rl, "port %s: sending LACP PDU on iface %s failed " @@ -1622,10 +1623,7 @@ process_special(struct ofproto_dpif *ofproto, const struct flow *flow, } else if (flow->dl_type == htons(ETH_TYPE_LACP)) { struct ofport_dpif *port = get_ofp_port(ofproto, flow->in_port); if (packet && port && port->bundle && port->bundle->lacp) { - const struct lacp_pdu *pdu = parse_lacp_packet(packet); - if (pdu) { - lacp_process_pdu(port->bundle->lacp, port, pdu); - } + lacp_process_packet(port->bundle->lacp, port, packet); } return true; } -- 2.30.2