| Message ID | 20250919052929.498183-1-changliang.wu@smartx.com |
|---|---|
| State | Not Applicable |
| Delegated to: | aaron conole |
| Headers | show |
| Series | [ovs-dev] lacp: Restore lacp status when service reload manually. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/cirrus-robot | success | cirrus build: passed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
Hi Changliang, Changliang Wu <changliang.wu@smartx.com> writes: > On most switches, if STP is enabled and the port connected to OVS > is not in edge mode, when OVS service restart and negotiate LACP, > which will trigger STP recalculation. > > During this time, traffic on the LACP interface will be blocked > for several seconds(up to 5s+). > > Test in Mellanox Switch > OvS LACP: Fast Mode > Switch LACP | STP Mode | Downtime > ------------|-------------|--------- > Fast/Slow | on - normal | 3.1s > Fast/Slow | off / edge | 0.2s > > This patch propose a solution, similar to flow restore, > 1. save LACP negotiation status > 2. pause LACP process > 3. restart OvS service > 4. inject LACP status into vswitchd > 5. resume LACP process > > Test Results with this patch > Switch LACP | STP Mode | Downtime > ------------|-------------|--------- > Fast | on - normal | 0.3s > Slow | on - normal | 0.0s > Fast/Slow | off / edge | 0.0s > > Signed-off-by: Changliang Wu <changliang.wu@smartx.com> > --- This is quite a hack, but I don't know that it is appropriate. For example, we don't know how long the vswitchd service will be down. Unlike flows, which are a user configuration, LACP negotiation is a process between two peers. Restoring state doesn't make much sense to me. > lib/lacp.c | 272 +++++++++++++++++++++++++++++++++++++++++ > ofproto/ofproto-dpif.c | 5 +- > ofproto/ofproto.c | 14 +++ > ofproto/ofproto.h | 2 + > utilities/ovs-lib.in | 46 ++++++- > utilities/ovs-save | 159 ++++++++++++++++++++++++ > vswitchd/bridge.c | 5 + > 7 files changed, 499 insertions(+), 4 deletions(-) In the future, if you introduce commands and user visible behavior, please add NEWS entry. This kind of behavior would be visible to the user, and these commands are likewise visible. > diff --git a/lib/lacp.c b/lib/lacp.c > index 3252f17eb..251f63dbe 100644 > --- a/lib/lacp.c > +++ b/lib/lacp.c > @@ -25,6 +25,7 @@ > #include "dp-packet.h" > #include "ovs-atomic.h" > #include "packets.h" > +#include "openvswitch/ofp-parse.h" > #include "openvswitch/poll-loop.h" > #include "seq.h" > #include "openvswitch/shash.h" > @@ -168,6 +169,7 @@ static bool member_may_enable__(struct member *) OVS_REQUIRES(mutex); > > static unixctl_cb_func lacp_unixctl_show; > static unixctl_cb_func lacp_unixctl_show_stats; > +static unixctl_cb_func lacp_unixctl_set; > > /* Populates 'pdu' with a LACP PDU comprised of 'actor' and 'partner'. */ > static void > @@ -229,6 +231,8 @@ lacp_init(void) > lacp_unixctl_show, NULL); > unixctl_command_register("lacp/show-stats", "[port]", 0, 1, > lacp_unixctl_show_stats, NULL); > + unixctl_command_register("lacp/set", "[port]", 3, INT_MAX, > + lacp_unixctl_set, NULL); Indentation here is not correct. > } FYI, I didn't review the code beyond this point. I think the approach here isn't acceptable. Perhaps we could look at how the LACP initialization phase may be improved. > > static void > @@ -940,6 +944,34 @@ lacp_find(const char *name) OVS_REQUIRES(mutex) > return NULL; > } > > +static struct member * > +lacp_member_find(struct lacp *lacp, const char *name) OVS_REQUIRES(mutex) > +{ > + struct member *member; > + > + HMAP_FOR_EACH_SAFE (member, node, &lacp->members) { > + if (!strcmp(member->name, name)) { > + return member; > + } > + } > + > + return NULL; > +} > + > +static struct member * > +lacp_member_find_by_key(struct lacp *lacp, uint16_t key) OVS_REQUIRES(mutex) > +{ > + struct member *member; > + > + HMAP_FOR_EACH_SAFE (member, node, &lacp->members) { > + if (member->port_id == key) { > + return member; > + } > + } > + > + return NULL; > +} > + > static void > ds_put_lacp_state(struct ds *ds, uint8_t state) > { > @@ -1219,5 +1251,245 @@ lacp_get_member_stats(const struct lacp *lacp, const void *member_, > } > ovs_mutex_unlock(&mutex); > return ret; > +} > + > +static void > +lacp_port_set(struct unixctl_conn *conn, struct lacp *lacp, const char *parms) > + OVS_REQUIRES(mutex) > +{ > + char *value; > + char *key; > + char **p = (char **)&parms; > + while (ofputil_parse_key_value(p, &key, &value)) { > + lacp->update = false; > + uint32_t tmp; > + if (nullable_string_is_equal(key, "status")) { > + if (strstr(value, "active")) { > + lacp->active = true; > + } > + if (strstr(value, "negotiated")) { > + lacp->negotiated = true; > + } > + > + } else if (nullable_string_is_equal(key, "sys_id")) { > + if (!eth_addr_from_string(value, &lacp->sys_id)) { > + unixctl_command_reply_error(conn, "invalid port sys_id"); > + return; > + } > + } else if (nullable_string_is_equal(key, "sys_priority")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid port sys_prority"); > + return; > + } > + lacp->sys_priority = tmp; > + } else if (nullable_string_is_equal(key, "key")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid port key"); > + return; > + } > + lacp->key_member = lacp_member_find_by_key(lacp, tmp); > + } > + } > +} > + > +static uint8_t > +lacp_state_from_str(const char *s) > +{ > + uint8_t state = 0; > + if (strstr(s, "activity")) { > + state |= LACP_STATE_ACT; > + } > + if (strstr(s, "timeout")) { > + state |= LACP_STATE_TIME; > + } > + if (strstr(s, "aggregation")) { > + state |= LACP_STATE_AGG; > + } > + if (strstr(s, "synchronized")) { > + state |= LACP_STATE_SYNC; > + } > + if (strstr(s, "collecting")) { > + state |= LACP_STATE_COL; > + } > + if (strstr(s, "distributing")) { > + state |= LACP_STATE_DIST; > + } > + if (strstr(s, "defaulted")) { > + state |= LACP_STATE_DEF; > + } > + if (strstr(s, "expired")) { > + state |= LACP_STATE_EXP; > + } > + return state; > +} > > +static void > +lacp_member_set(struct unixctl_conn *conn, struct member *member, > + const char *op, const char *parms) OVS_REQUIRES(mutex) > +{ > + char *value; > + char *key; > + char **p = (char **)&parms; > + if (nullable_string_is_equal(op, "member")) { > + while (ofputil_parse_key_value(p, &key, &value)) { > + uint32_t tmp; > + if (nullable_string_is_equal(key, "port_id")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "port_id"); > + return; > + } > + member->port_id = tmp; > + } else if (nullable_string_is_equal(key, "port_priority")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "port_priority"); > + return; > + } > + member->port_priority = tmp; > + } else if (nullable_string_is_equal(key, "status")) { > + if (strstr(value, "current")) { > + member->status = LACP_CURRENT; > + member->carrier_up = true; > + } else if (strstr(value, "expired")) { > + member->status = LACP_EXPIRED; > + } else if (strstr(value, "defaulted")) { > + member->status = LACP_DEFAULTED; > + } > + > + if (strstr(value, "attached")) { > + member->attached = true; > + } > + } > + } > + } else if (nullable_string_is_equal(op, "actor")) { > + while (ofputil_parse_key_value(p, &key, &value)) { > + uint32_t tmp; > + if (nullable_string_is_equal(key, "sys_id")) { > + struct eth_addr addr; > + if (!eth_addr_from_string(value, &addr)) { > + unixctl_command_reply_error(conn, "invalid member " > + "actor sys_id"); > + return; > + } > + member->ntt_actor.sys_id = addr; > + } else if (nullable_string_is_equal(key, "sys_priority")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "actor sys_priority"); > + return; > + } > + member->ntt_actor.sys_priority = htons(tmp); > + } else if (nullable_string_is_equal(key, "port_id")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "actor port_id"); > + return; > + } > + member->ntt_actor.port_id = htons(tmp); > + } else if (nullable_string_is_equal(key, "port_priority")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "actor port_priority"); > + return; > + } > + member->ntt_actor.port_priority = htons(tmp); > + } else if (nullable_string_is_equal(key, "key")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "actor key"); > + return; > + } > + member->ntt_actor.key = htons(tmp); > + } else if (nullable_string_is_equal(key, "state")) { > + member->ntt_actor.state = lacp_state_from_str(value); > + VLOG_INFO("member->ntt_actor.state (%p) (%d)", > + &(member->ntt_actor.state), member->ntt_actor.state); > + } > + } > + } else if (nullable_string_is_equal(op, "partner")) { > + while (ofputil_parse_key_value(p, &key, &value)) { > + uint32_t tmp; > + if (nullable_string_is_equal(key, "sys_id")) { > + struct eth_addr addr; > + if (!eth_addr_from_string(value, &addr)) { > + unixctl_command_reply_error(conn, "invalid member " > + "partner sys_id"); > + return; > + } > + member->partner.sys_id = addr; > + } else if (nullable_string_is_equal(key, "sys_priority")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "partner sys_priority"); > + return; > + } > + member->partner.sys_priority = htons(tmp); > + } else if (nullable_string_is_equal(key, "port_id")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "partner port_id"); > + return; > + } > + member->partner.port_id = htons(tmp); > + } else if (nullable_string_is_equal(key, "port_priority")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "partner port_priority"); > + return; > + } > + member->partner.port_priority = htons(tmp); > + } else if (nullable_string_is_equal(key, "key")) { > + if (!str_to_uint(value, 10, &tmp)) { > + unixctl_command_reply_error(conn, "invalid member " > + "partner key"); > + return; > + } > + member->partner.key = htons(tmp); > + } else if (nullable_string_is_equal(key, "state")) { > + member->partner.state = lacp_state_from_str(value); > + } > + } > + } else { > + unixctl_command_reply_error(conn, "invalid member op"); > + } > + timer_set_duration(&member->tx, LACP_FAST_TIME_TX); > + timer_set_duration(&member->rx, LACP_FAST_TIME_TX); > +} > + > +static void > +lacp_unixctl_set(struct unixctl_conn *conn, int argc, const char *argv[], > + void *aux OVS_UNUSED) OVS_EXCLUDED(mutex) > +{ > + struct member *member; > + struct lacp *lacp; > + > + lacp_lock(); > + lacp = lacp_find(argv[1]); > + if (!lacp) { > + unixctl_command_reply_error(conn, "lacp port not found"); > + goto out; > + } > + > + if (nullable_string_is_equal(argv[2], "port")) { > + lacp_port_set(conn, lacp, argv[3]); > + } else if (nullable_string_is_equal(argv[2], "member")) { > + member = lacp_member_find(lacp, argv[3]); > + if (!member) { > + unixctl_command_reply_error(conn, "lacp member not found"); > + goto out; > + } > + if (argc < 5) { > + unixctl_command_reply_error(conn, "invalid member parms"); > + goto out; > + } > + lacp_member_set(conn, member, argv[4], argv[5]); > + } else { > + unixctl_command_reply_error(conn, "invalid op type"); > + goto out; > + } > + > +out: > + unixctl_command_reply(conn, NULL); > + lacp_unlock(); > } > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index ed9e44ce2..28657f0e8 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -41,6 +41,7 @@ > #include "nx-match.h" > #include "odp-util.h" > #include "odp-execute.h" > +#include "ofproto/ofproto.h" > #include "ofproto/ofproto-dpif.h" > #include "ofproto/ofproto-provider.h" > #include "ofproto-dpif-ipfix.h" > @@ -3702,7 +3703,6 @@ send_pdu_cb(void *port_, const void *pdu, size_t pdu_size) > struct ofport_dpif *port = port_; > struct eth_addr ea; > int error; > - > error = netdev_get_etheraddr(port->up.netdev, &ea); > if (!error) { > struct dp_packet packet; > @@ -3781,6 +3781,9 @@ bundle_send_learning_packets(struct ofbundle *bundle) > static void > bundle_run(struct ofbundle *bundle) > { > + if (bundle->lacp && ofproto_get_lacp_restore_wait()) { > + return; > + } > if (bundle->lacp) { > lacp_run(bundle->lacp, send_pdu_cb); > } > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index 6fa18228b..6f587c21f 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -326,6 +326,8 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > > /* The default value of true waits for flow restore. */ > static bool flow_restore_wait = true; > +/* The default value of true waits for lacp restore. */ > +static bool lacp_restore_wait = true; > > /* Must be called to initialize the ofproto library. > * > @@ -997,6 +999,18 @@ ofproto_get_flow_restore_wait(void) > return flow_restore_wait; > } > > +void > +ofproto_set_lacp_restore_wait(bool lacp_restore_wait_db) > +{ > + lacp_restore_wait = lacp_restore_wait_db; > +} > + > +bool > +ofproto_get_lacp_restore_wait(void) > +{ > + return lacp_restore_wait; > +} > + > /* Retrieve datapath capabilities. */ > void > ofproto_get_datapath_cap(const char *datapath_type, struct smap *dp_cap) > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h > index 3f85509a1..e8e69b0da 100644 > --- a/ofproto/ofproto.h > +++ b/ofproto/ofproto.h > @@ -382,6 +382,8 @@ int ofproto_set_local_sample(struct ofproto *ofproto, > size_t n_options); > void ofproto_set_flow_restore_wait(bool flow_restore_wait_db); > bool ofproto_get_flow_restore_wait(void); > +void ofproto_set_lacp_restore_wait(bool lacp_restore_wait_db); > +bool ofproto_get_lacp_restore_wait(void); > int ofproto_set_stp(struct ofproto *, const struct ofproto_stp_settings *); > int ofproto_get_stp_status(struct ofproto *, struct ofproto_stp_status *); > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > index dded0b7c7..a845ac915 100644 > --- a/utilities/ovs-lib.in > +++ b/utilities/ovs-lib.in > @@ -587,12 +587,21 @@ ovs_save () { > [ -z "${bridges}" ] && return 0 > } > > +ovs_save_lacp () { > + $datadir/scripts/ovs-save save-lacp > "${script_lacp}" > + chmod +x "${script_lacp}" > +} > + > save_flows_if_required () { > if test X"$DELETE_BRIDGES" != Xyes; then > action "Saving flows" ovs_save save-flows "${script_flows}" > fi > } > > +save_lacp () { > + action "Saving lacp" ovs_save_lacp > +} > + > save_interfaces () { > "$datadir/scripts/ovs-save" save-interfaces ${ifaces} \ > > "${script_interfaces}" > @@ -604,6 +613,12 @@ flow_restore_wait () { > fi > } > > +lacp_restore_wait () { > + if test X"${OVS_VSWITCHD:-yes}" = Xyes; then > + ovs_vsctl set open_vswitch . other_config:lacp-restore-wait="true" > + fi > +} > + > flow_restore_complete () { > if test X"${OVS_VSWITCHD:-yes}" = Xyes; then > ovs_vsctl --if-exists remove open_vswitch . other_config \ > @@ -611,11 +626,23 @@ flow_restore_complete () { > fi > } > > +lacp_restore_complete () { > + if test X"${OVS_VSWITCHD:-yes}" = Xyes; then > + ovs_vsctl --if-exists remove open_vswitch . other_config \ > + lacp-restore-wait="true" > + fi > +} > + > restore_flows () { > [ -x "${script_flows}" ] && \ > action "Restoring saved flows" "${script_flows}" > } > > +restore_lacp () { > + [ -x "${script_lacp}" ] && \ > + action "Restoring saved lacp" "${script_lacp}" > +} > + > restore_interfaces () { > [ ! -x "${script_interfaces}" ] && return 0 > action "Restoring interface configuration" "${script_interfaces}" > @@ -633,7 +660,8 @@ restore_interfaces () { > init_restore_scripts () { > script_interfaces=`mktemp` > script_flows=`mktemp` > - trap 'rm -f "${script_interfaces}" "${script_flows}"' 0 > + script_lacp=`mktemp` > + trap 'rm -f "${script_interfaces}" "${script_flows}" "${script_lacp}"' 0 > } > > force_reload_kmod () { > @@ -648,6 +676,7 @@ force_reload_kmod () { > > init_restore_scripts > save_flows_if_required > + save_lacp > > # Restart the database first, since a large database may take a > # while to load, and we want to minimize forwarding disruption. > @@ -682,11 +711,16 @@ force_reload_kmod () { > > # Start vswitchd by asking it to wait till flow restore is finished. > flow_restore_wait > + lacp_restore_wait > start_forwarding || return 1 > > - # Restore saved flows and inform vswitchd that we are done. > + # Restore saved resources and inform vswitchd that we are done. > restore_flows > + restore_lacp > + > flow_restore_complete > + lacp_restore_complete > + > add_managers > > restore_interfaces > @@ -704,6 +738,7 @@ restart () { > init_restore_scripts > if test X"${OVS_VSWITCHD:-yes}" = Xyes; then > save_flows_if_required > + save_lacp > fi > fi > > @@ -716,10 +751,15 @@ restart () { > > # Start vswitchd by asking it to wait till flow restore is finished. > flow_restore_wait > + lacp_restore_wait > start_forwarding || return 1 > > - # Restore saved flows and inform vswitchd that we are done. > + # Restore saved resources and inform vswitchd that we are done. > restore_flows > + restore_lacp > + > flow_restore_complete > + lacp_restore_complete > + > add_managers > } > diff --git a/utilities/ovs-save b/utilities/ovs-save > index 67092ecf7..792d5daee 100755 > --- a/utilities/ovs-save > +++ b/utilities/ovs-save > @@ -33,6 +33,8 @@ Commands: > configuration. > save-flows Outputs a shell script on stdout that will restore > OpenFlow flows of each Open vSwitch bridge. > + save-lacp Outputs a shell script on stdout that will restore > + lacp info of each lacp bond port. > This script is meant as a helper for the Open vSwitch init script commands. > EOF > } > @@ -159,6 +161,158 @@ save_flows () { > echo "rm -rf \"$workdir\"" > } > > +save_lacp () { > + if (ovs-appctl --version) > /dev/null 2>&1; then :; else > + echo "$0: ovs-ofctl not found in $PATH" >&2 > + exit 1 > + fi > + > + case `ovs-appctl version | sed 1q` in > + "ovs-vswitchd (Open vSwitch) 1."*.*) > + return > + ;; > + esac > + > + SP="[[:space:]]*" > + BOND_NAME_PATTERN="s/^----${SP}([^[:space:]]+)${SP}----.*/\1/" > + STATUS_PATTERN="s/^status:${SP}(.*)/\1/" > + SYS_ID_PATTERN="s/^sys_id:${SP}(.*)/\1/" > + SYS_PRIORITY_PATTERN="s/^sys_priority:${SP}(.*)/\1/" > + AGGREGATION_KEY_PATTERN="s/^aggregation${SP}key:${SP}(.*)/\1/" > + LACP_TIME_PATTERN="s/^lacp_time:${SP}(.*)/\1/" > + MEMBER_PATTERN="s/^(member|slave):${SP}([^:]+):${SP}(.*)/\2/" > + MEMBER_STATUS_PATTERN="s/^(member|slave):${SP}([^:]+):${SP}(.*)/\3/" > + PORT_ID_PATTERN="s/^port_id:${SP}(.*)/\1/" > + PORT_PRIORITY_PATTERN="s/^port_priority:${SP}(.*)/\1/" > + MAY_ENABLE_PATTERN="s/^may_enable:${SP}(.*)/\1/" > + ACTOR_SYS_ID_PATTERN="s/^actor${SP}sys_id:${SP}(.*)/\1/" > + ACTOR_SYS_PRIORITY_PATTERN="s/^actor${SP}sys_priority:${SP}(.*)/\1/" > + ACTOR_PORT_ID_PATTERN="s/^actor${SP}port_id:${SP}(.*)/\1/" > + ACTOR_PORT_PRIORITY_PATTERN="s/^actor${SP}port_priority:${SP}(.*)/\1/" > + ACTOR_KEY_PATTERN="s/^actor${SP}key:${SP}(.*)/\1/" > + ACTOR_STATE_PATTERN="s/^actor${SP}state:${SP}(.*)/\1/" > + PARTNER_SYS_ID_PATTERN="s/^partner${SP}sys_id:${SP}(.*)/\1/" > + PARTNER_SYS_PRIORITY_PATTERN="s/^partner${SP}sys_priority:${SP}(.*)/\1/" > + PARTNER_PORT_ID_PATTERN="s/^partner${SP}port_id:${SP}(.*)/\1/" > + PARTNER_PORT_PRIORITY_PATTERN="s/^partner${SP}port_priority:${SP}(.*)/\1/" > + PARTNER_KEY_PATTERN="s/^partner${SP}key:${SP}(.*)/\1/" > + PARTNER_STATE_PATTERN="s/^partner${SP}state:${SP}(.*)/\1/" > + tmpfile=$(mktemp) > + ovs-appctl lacp/show | while IFS= read -r line; do > + t=$(echo "$line" | sed -E 's/^[[:space:]]+//') > + case "$t" in > + ----*----*) > + BOND_NAME=$(echo "$t" | sed -E "$BOND_NAME_PATTERN") > + ;; > + status:${SP}) > + STATUS=$(echo "$t" | sed -E "$STATUS_PATTERN" | sed 's/ /+/g') > + ;; > + sys_id:${SP}) > + SYS_ID=$(echo "$t" | sed -E "$SYS_ID_PATTERN") > + ;; > + sys_priority:${SP}) > + SYS_PRIORITY=$(echo "$t" | sed -E "$SYS_PRIORITY_PATTERN") > + ;; > + aggregation${SP}key:${SP}) > + AGGREGATION_KEY=$(echo "$t" | > + sed -E "$AGGREGATION_KEY_PATTERN") > + ;; > + lacp_time:${SP}) > + LACP_TIME=$(echo "$t" | sed -E "$LACP_TIME_PATTERN") > + cmd="ovs-appctl lacp/set $BOND_NAME port status=$STATUS," > + cmd="${cmd}sys_id=$SYS_ID,sys_priority=$SYS_PRIORITY," > + cmd="${cmd}key=$AGGREGATION_KEY,lacp_time=$LACP_TIME" > + echo "$cmd" >> "$tmpfile" > + ;; > + member:${SP}|slave:${SP}) > + # Extract member name and status > + MEMBER_NAME=$(echo "$t" | sed -E "$MEMBER_PATTERN") > + MEMBER_STATUS=$(echo "$t" | > + sed -E "$MEMBER_STATUS_PATTERN" | sed 's/ /+/g') > + ;; > + port_id:${SP}) > + PORT_ID=$(echo "$t" | sed -E "$PORT_ID_PATTERN") > + ;; > + port_priority:${SP}) > + PORT_PRIORITY=$(echo "$t" | sed -E "$PORT_PRIORITY_PATTERN") > + ;; > + may_enable:${SP}) > + MAY_ENABLE=$(echo "$t" | sed -E "$MAY_ENABLE_PATTERN") > + # Print member information > + cmd="ovs-appctl lacp/set $BOND_NAME member $MEMBER_NAME member" > + cmd="${cmd} status=$MEMBER_STATUS,port_id=$PORT_ID," > + cmd="${cmd}port_priority=$PORT_PRIORITY,may_enable=$MAY_ENABLE" > + echo "$cmd" >> "$tmpfile" > + ;; > + actor${SP}sys_id:${SP}) > + ACTOR_SYS_ID=$(echo "$t" | sed -E "$ACTOR_SYS_ID_PATTERN") > + ;; > + actor${SP}sys_priority:${SP}) > + ACTOR_SYS_PRIORITY=$(echo "$t" | > + sed -E "$ACTOR_SYS_PRIORITY_PATTERN") > + ;; > + actor${SP}port_id:${SP}) > + ACTOR_PORT_ID=$(echo "$t" | sed -E "$ACTOR_PORT_ID_PATTERN") > + ;; > + actor${SP}port_priority:${SP}) > + ACTOR_PORT_PRIORITY=$(echo "$t" | > + sed -E "$ACTOR_PORT_PRIORITY_PATTERN") > + ;; > + actor${SP}key:${SP}) > + ACTOR_KEY=$(echo "$t" | sed -E "$ACTOR_KEY_PATTERN") > + ;; > + actor${SP}state:${SP}) > + ACTOR_STATE=$(echo "$t" | > + sed -E "$ACTOR_STATE_PATTERN" | sed 's/ /+/g') > + # Print actor information > + cmd="ovs-appctl lacp/set $BOND_NAME member $MEMBER_NAME actor" > + cmd="${cmd} sys_id=$ACTOR_SYS_ID," > + cmd="${cmd}sys_priority=$ACTOR_SYS_PRIORITY," > + cmd="${cmd}port_id=$ACTOR_PORT_ID," > + cmd="${cmd}port_priority=$ACTOR_PORT_PRIORITY," > + cmd="${cmd}key=$ACTOR_KEY,state=$ACTOR_STATE" > + echo "$cmd" >> "$tmpfile" > + ;; > + partner${SP}sys_id:${SP}) > + PARTNER_SYS_ID=$(echo "$t" | > + sed -E "$PARTNER_SYS_ID_PATTERN") > + ;; > + partner${SP}sys_priority:${SP}) > + PARTNER_SYS_PRIORITY=$(echo "$t" | > + sed -E "$PARTNER_SYS_PRIORITY_PATTERN") > + ;; > + partner${SP}port_id:${SP}) > + PARTNER_PORT_ID=$(echo "$t" | > + sed -E "$PARTNER_PORT_ID_PATTERN") > + ;; > + partner${SP}port_priority:${SP}) > + PARTNER_PORT_PRIORITY=$(echo "$t" | > + sed -E "$PARTNER_PORT_PRIORITY_PATTERN") > + ;; > + partner${SP}key:${SP}) > + PARTNER_KEY=$(echo "$t" | sed -E "$PARTNER_KEY_PATTERN") > + ;; > + partner${SP}state:${SP}) > + PARTNER_STATE=$(echo "$t" | > + sed -E "$PARTNER_STATE_PATTERN" | sed 's/ /+/g') > + # Print partner information > + cmd="ovs-appctl lacp/set $BOND_NAME" > + cmd="${cmd} member $MEMBER_NAME partner" > + cmd="${cmd} sys_id=$PARTNER_SYS_ID," > + cmd="${cmd}sys_priority=$PARTNER_SYS_PRIORITY," > + cmd="${cmd}port_id=$PARTNER_PORT_ID," > + cmd="${cmd}port_priority=$PARTNER_PORT_PRIORITY," > + cmd="${cmd}key=$PARTNER_KEY,state=$PARTNER_STATE" > + echo "$cmd" >> "$tmpfile" > + ;; > + esac > + done > + sed -i '1!G;h;$!d' "$tmpfile" > + cat "$tmpfile" > + rm -f "$tmpfile" > +} > + > + > while [ $# -ne 0 ] > do > case $1 in > @@ -172,6 +326,11 @@ do > save_interfaces "$@" > exit 0 > ;; > + "save-lacp") > + shift > + save_lacp "$@" > + exit 0 > + ;; > -h | --help) > usage > exit 0 > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index 456b784c0..e2c523051 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -3415,6 +3415,11 @@ bridge_run(void) > "flow-restore-wait", false)); > } > > + if (cfg && ofproto_get_lacp_restore_wait()) { > + ofproto_set_lacp_restore_wait(smap_get_bool(&cfg->other_config, > + "lacp-restore-wait", false)); > + } > + > bridge_run__(); > > /* Re-configure SSL/TLS. We do this on every trip through the main loop,
Hi, Aaron On Fri, Oct 10, 2025 at 11:48 PM Aaron Conole <aconole@redhat.com> wrote: > > Hi Changliang, > > Changliang Wu <changliang.wu@smartx.com> writes: > > > On most switches, if STP is enabled and the port connected to OVS > > is not in edge mode, when OVS service restart and negotiate LACP, > > which will trigger STP recalculation. > > > > During this time, traffic on the LACP interface will be blocked > > for several seconds(up to 5s+). > > > > Test in Mellanox Switch > > OvS LACP: Fast Mode > > Switch LACP | STP Mode | Downtime > > ------------|-------------|--------- > > Fast/Slow | on - normal | 3.1s > > Fast/Slow | off / edge | 0.2s > > > > This patch propose a solution, similar to flow restore, > > 1. save LACP negotiation status > > 2. pause LACP process > > 3. restart OvS service > > 4. inject LACP status into vswitchd > > 5. resume LACP process > > > > Test Results with this patch > > Switch LACP | STP Mode | Downtime > > ------------|-------------|--------- > > Fast | on - normal | 0.3s > > Slow | on - normal | 0.0s > > Fast/Slow | off / edge | 0.0s > > > > Signed-off-by: Changliang Wu <changliang.wu@smartx.com> > > --- > > This is quite a hack, but I don't know that it is appropriate. For > example, we don't know how long the vswitchd service will be down. > Unlike flows, which are a user configuration, LACP negotiation is a > process between two peers. Restoring state doesn't make much sense to > me. Yes, this is just an experimental draft. The LACP bond interface disconnects for 3+ seconds when reload, optimizing this downtime may be worthwhile. 'Restoring state' is one possible solution, and modifying the LACP state machine, as mentioned below, is also a possible solution. > > > lib/lacp.c | 272 +++++++++++++++++++++++++++++++++++++++++ > > ofproto/ofproto-dpif.c | 5 +- > > ofproto/ofproto.c | 14 +++ > > ofproto/ofproto.h | 2 + > > utilities/ovs-lib.in | 46 ++++++- > > utilities/ovs-save | 159 ++++++++++++++++++++++++ > > vswitchd/bridge.c | 5 + > > 7 files changed, 499 insertions(+), 4 deletions(-) > > In the future, if you introduce commands and user visible behavior, > please add NEWS entry. This kind of behavior would be visible to the > user, and these commands are likewise visible. Noted with thanks. > > > diff --git a/lib/lacp.c b/lib/lacp.c > > index 3252f17eb..251f63dbe 100644 > > --- a/lib/lacp.c > > +++ b/lib/lacp.c > > @@ -25,6 +25,7 @@ > > #include "dp-packet.h" > > #include "ovs-atomic.h" > > #include "packets.h" > > +#include "openvswitch/ofp-parse.h" > > #include "openvswitch/poll-loop.h" > > #include "seq.h" > > #include "openvswitch/shash.h" > > @@ -168,6 +169,7 @@ static bool member_may_enable__(struct member *) OVS_REQUIRES(mutex); > > > > static unixctl_cb_func lacp_unixctl_show; > > static unixctl_cb_func lacp_unixctl_show_stats; > > +static unixctl_cb_func lacp_unixctl_set; > > > > /* Populates 'pdu' with a LACP PDU comprised of 'actor' and 'partner'. */ > > static void > > @@ -229,6 +231,8 @@ lacp_init(void) > > lacp_unixctl_show, NULL); > > unixctl_command_register("lacp/show-stats", "[port]", 0, 1, > > lacp_unixctl_show_stats, NULL); > > + unixctl_command_register("lacp/set", "[port]", 3, INT_MAX, > > + lacp_unixctl_set, NULL); > > Indentation here is not correct. > > > } > > FYI, I didn't review the code beyond this point. I think the approach > here isn't acceptable. Perhaps we could look at how the LACP > initialization phase may be improved. > > > > > static void > > @@ -940,6 +944,34 @@ lacp_find(const char *name) OVS_REQUIRES(mutex) > > return NULL; > > } > > > > +static struct member * > > +lacp_member_find(struct lacp *lacp, const char *name) OVS_REQUIRES(mutex) > > +{ > > + struct member *member; > > + > > + HMAP_FOR_EACH_SAFE (member, node, &lacp->members) { > > + if (!strcmp(member->name, name)) { > > + return member; > > + } > > + } > > + > > + return NULL; > > +} > > + > > +static struct member * > > +lacp_member_find_by_key(struct lacp *lacp, uint16_t key) OVS_REQUIRES(mutex) > > +{ > > + struct member *member; > > + > > + HMAP_FOR_EACH_SAFE (member, node, &lacp->members) { > > + if (member->port_id == key) { > > + return member; > > + } > > + } > > + > > + return NULL; > > +} > > + > > static void > > ds_put_lacp_state(struct ds *ds, uint8_t state) > > { > > @@ -1219,5 +1251,245 @@ lacp_get_member_stats(const struct lacp *lacp, const void *member_, > > } > > ovs_mutex_unlock(&mutex); > > return ret; > > +} > > + > > +static void > > +lacp_port_set(struct unixctl_conn *conn, struct lacp *lacp, const char *parms) > > + OVS_REQUIRES(mutex) > > +{ > > + char *value; > > + char *key; > > + char **p = (char **)&parms; > > + while (ofputil_parse_key_value(p, &key, &value)) { > > + lacp->update = false; > > + uint32_t tmp; > > + if (nullable_string_is_equal(key, "status")) { > > + if (strstr(value, "active")) { > > + lacp->active = true; > > + } > > + if (strstr(value, "negotiated")) { > > + lacp->negotiated = true; > > + } > > + > > + } else if (nullable_string_is_equal(key, "sys_id")) { > > + if (!eth_addr_from_string(value, &lacp->sys_id)) { > > + unixctl_command_reply_error(conn, "invalid port sys_id"); > > + return; > > + } > > + } else if (nullable_string_is_equal(key, "sys_priority")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid port sys_prority"); > > + return; > > + } > > + lacp->sys_priority = tmp; > > + } else if (nullable_string_is_equal(key, "key")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid port key"); > > + return; > > + } > > + lacp->key_member = lacp_member_find_by_key(lacp, tmp); > > + } > > + } > > +} > > + > > +static uint8_t > > +lacp_state_from_str(const char *s) > > +{ > > + uint8_t state = 0; > > + if (strstr(s, "activity")) { > > + state |= LACP_STATE_ACT; > > + } > > + if (strstr(s, "timeout")) { > > + state |= LACP_STATE_TIME; > > + } > > + if (strstr(s, "aggregation")) { > > + state |= LACP_STATE_AGG; > > + } > > + if (strstr(s, "synchronized")) { > > + state |= LACP_STATE_SYNC; > > + } > > + if (strstr(s, "collecting")) { > > + state |= LACP_STATE_COL; > > + } > > + if (strstr(s, "distributing")) { > > + state |= LACP_STATE_DIST; > > + } > > + if (strstr(s, "defaulted")) { > > + state |= LACP_STATE_DEF; > > + } > > + if (strstr(s, "expired")) { > > + state |= LACP_STATE_EXP; > > + } > > + return state; > > +} > > > > +static void > > +lacp_member_set(struct unixctl_conn *conn, struct member *member, > > + const char *op, const char *parms) OVS_REQUIRES(mutex) > > +{ > > + char *value; > > + char *key; > > + char **p = (char **)&parms; > > + if (nullable_string_is_equal(op, "member")) { > > + while (ofputil_parse_key_value(p, &key, &value)) { > > + uint32_t tmp; > > + if (nullable_string_is_equal(key, "port_id")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "port_id"); > > + return; > > + } > > + member->port_id = tmp; > > + } else if (nullable_string_is_equal(key, "port_priority")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "port_priority"); > > + return; > > + } > > + member->port_priority = tmp; > > + } else if (nullable_string_is_equal(key, "status")) { > > + if (strstr(value, "current")) { > > + member->status = LACP_CURRENT; > > + member->carrier_up = true; > > + } else if (strstr(value, "expired")) { > > + member->status = LACP_EXPIRED; > > + } else if (strstr(value, "defaulted")) { > > + member->status = LACP_DEFAULTED; > > + } > > + > > + if (strstr(value, "attached")) { > > + member->attached = true; > > + } > > + } > > + } > > + } else if (nullable_string_is_equal(op, "actor")) { > > + while (ofputil_parse_key_value(p, &key, &value)) { > > + uint32_t tmp; > > + if (nullable_string_is_equal(key, "sys_id")) { > > + struct eth_addr addr; > > + if (!eth_addr_from_string(value, &addr)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "actor sys_id"); > > + return; > > + } > > + member->ntt_actor.sys_id = addr; > > + } else if (nullable_string_is_equal(key, "sys_priority")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "actor sys_priority"); > > + return; > > + } > > + member->ntt_actor.sys_priority = htons(tmp); > > + } else if (nullable_string_is_equal(key, "port_id")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "actor port_id"); > > + return; > > + } > > + member->ntt_actor.port_id = htons(tmp); > > + } else if (nullable_string_is_equal(key, "port_priority")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "actor port_priority"); > > + return; > > + } > > + member->ntt_actor.port_priority = htons(tmp); > > + } else if (nullable_string_is_equal(key, "key")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "actor key"); > > + return; > > + } > > + member->ntt_actor.key = htons(tmp); > > + } else if (nullable_string_is_equal(key, "state")) { > > + member->ntt_actor.state = lacp_state_from_str(value); > > + VLOG_INFO("member->ntt_actor.state (%p) (%d)", > > + &(member->ntt_actor.state), member->ntt_actor.state); > > + } > > + } > > + } else if (nullable_string_is_equal(op, "partner")) { > > + while (ofputil_parse_key_value(p, &key, &value)) { > > + uint32_t tmp; > > + if (nullable_string_is_equal(key, "sys_id")) { > > + struct eth_addr addr; > > + if (!eth_addr_from_string(value, &addr)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "partner sys_id"); > > + return; > > + } > > + member->partner.sys_id = addr; > > + } else if (nullable_string_is_equal(key, "sys_priority")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "partner sys_priority"); > > + return; > > + } > > + member->partner.sys_priority = htons(tmp); > > + } else if (nullable_string_is_equal(key, "port_id")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "partner port_id"); > > + return; > > + } > > + member->partner.port_id = htons(tmp); > > + } else if (nullable_string_is_equal(key, "port_priority")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "partner port_priority"); > > + return; > > + } > > + member->partner.port_priority = htons(tmp); > > + } else if (nullable_string_is_equal(key, "key")) { > > + if (!str_to_uint(value, 10, &tmp)) { > > + unixctl_command_reply_error(conn, "invalid member " > > + "partner key"); > > + return; > > + } > > + member->partner.key = htons(tmp); > > + } else if (nullable_string_is_equal(key, "state")) { > > + member->partner.state = lacp_state_from_str(value); > > + } > > + } > > + } else { > > + unixctl_command_reply_error(conn, "invalid member op"); > > + } > > + timer_set_duration(&member->tx, LACP_FAST_TIME_TX); > > + timer_set_duration(&member->rx, LACP_FAST_TIME_TX); > > +} > > + > > +static void > > +lacp_unixctl_set(struct unixctl_conn *conn, int argc, const char *argv[], > > + void *aux OVS_UNUSED) OVS_EXCLUDED(mutex) > > +{ > > + struct member *member; > > + struct lacp *lacp; > > + > > + lacp_lock(); > > + lacp = lacp_find(argv[1]); > > + if (!lacp) { > > + unixctl_command_reply_error(conn, "lacp port not found"); > > + goto out; > > + } > > + > > + if (nullable_string_is_equal(argv[2], "port")) { > > + lacp_port_set(conn, lacp, argv[3]); > > + } else if (nullable_string_is_equal(argv[2], "member")) { > > + member = lacp_member_find(lacp, argv[3]); > > + if (!member) { > > + unixctl_command_reply_error(conn, "lacp member not found"); > > + goto out; > > + } > > + if (argc < 5) { > > + unixctl_command_reply_error(conn, "invalid member parms"); > > + goto out; > > + } > > + lacp_member_set(conn, member, argv[4], argv[5]); > > + } else { > > + unixctl_command_reply_error(conn, "invalid op type"); > > + goto out; > > + } > > + > > +out: > > + unixctl_command_reply(conn, NULL); > > + lacp_unlock(); > > } > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > > index ed9e44ce2..28657f0e8 100644 > > --- a/ofproto/ofproto-dpif.c > > +++ b/ofproto/ofproto-dpif.c > > @@ -41,6 +41,7 @@ > > #include "nx-match.h" > > #include "odp-util.h" > > #include "odp-execute.h" > > +#include "ofproto/ofproto.h" > > #include "ofproto/ofproto-dpif.h" > > #include "ofproto/ofproto-provider.h" > > #include "ofproto-dpif-ipfix.h" > > @@ -3702,7 +3703,6 @@ send_pdu_cb(void *port_, const void *pdu, size_t pdu_size) > > struct ofport_dpif *port = port_; > > struct eth_addr ea; > > int error; > > - > > error = netdev_get_etheraddr(port->up.netdev, &ea); > > if (!error) { > > struct dp_packet packet; > > @@ -3781,6 +3781,9 @@ bundle_send_learning_packets(struct ofbundle *bundle) > > static void > > bundle_run(struct ofbundle *bundle) > > { > > + if (bundle->lacp && ofproto_get_lacp_restore_wait()) { > > + return; > > + } > > if (bundle->lacp) { > > lacp_run(bundle->lacp, send_pdu_cb); > > } > > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > > index 6fa18228b..6f587c21f 100644 > > --- a/ofproto/ofproto.c > > +++ b/ofproto/ofproto.c > > @@ -326,6 +326,8 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > > > > /* The default value of true waits for flow restore. */ > > static bool flow_restore_wait = true; > > +/* The default value of true waits for lacp restore. */ > > +static bool lacp_restore_wait = true; > > > > /* Must be called to initialize the ofproto library. > > * > > @@ -997,6 +999,18 @@ ofproto_get_flow_restore_wait(void) > > return flow_restore_wait; > > } > > > > +void > > +ofproto_set_lacp_restore_wait(bool lacp_restore_wait_db) > > +{ > > + lacp_restore_wait = lacp_restore_wait_db; > > +} > > + > > +bool > > +ofproto_get_lacp_restore_wait(void) > > +{ > > + return lacp_restore_wait; > > +} > > + > > /* Retrieve datapath capabilities. */ > > void > > ofproto_get_datapath_cap(const char *datapath_type, struct smap *dp_cap) > > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h > > index 3f85509a1..e8e69b0da 100644 > > --- a/ofproto/ofproto.h > > +++ b/ofproto/ofproto.h > > @@ -382,6 +382,8 @@ int ofproto_set_local_sample(struct ofproto *ofproto, > > size_t n_options); > > void ofproto_set_flow_restore_wait(bool flow_restore_wait_db); > > bool ofproto_get_flow_restore_wait(void); > > +void ofproto_set_lacp_restore_wait(bool lacp_restore_wait_db); > > +bool ofproto_get_lacp_restore_wait(void); > > int ofproto_set_stp(struct ofproto *, const struct ofproto_stp_settings *); > > int ofproto_get_stp_status(struct ofproto *, struct ofproto_stp_status *); > > > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > > index dded0b7c7..a845ac915 100644 > > --- a/utilities/ovs-lib.in > > +++ b/utilities/ovs-lib.in > > @@ -587,12 +587,21 @@ ovs_save () { > > [ -z "${bridges}" ] && return 0 > > } > > > > +ovs_save_lacp () { > > + $datadir/scripts/ovs-save save-lacp > "${script_lacp}" > > + chmod +x "${script_lacp}" > > +} > > + > > save_flows_if_required () { > > if test X"$DELETE_BRIDGES" != Xyes; then > > action "Saving flows" ovs_save save-flows "${script_flows}" > > fi > > } > > > > +save_lacp () { > > + action "Saving lacp" ovs_save_lacp > > +} > > + > > save_interfaces () { > > "$datadir/scripts/ovs-save" save-interfaces ${ifaces} \ > > > "${script_interfaces}" > > @@ -604,6 +613,12 @@ flow_restore_wait () { > > fi > > } > > > > +lacp_restore_wait () { > > + if test X"${OVS_VSWITCHD:-yes}" = Xyes; then > > + ovs_vsctl set open_vswitch . other_config:lacp-restore-wait="true" > > + fi > > +} > > + > > flow_restore_complete () { > > if test X"${OVS_VSWITCHD:-yes}" = Xyes; then > > ovs_vsctl --if-exists remove open_vswitch . other_config \ > > @@ -611,11 +626,23 @@ flow_restore_complete () { > > fi > > } > > > > +lacp_restore_complete () { > > + if test X"${OVS_VSWITCHD:-yes}" = Xyes; then > > + ovs_vsctl --if-exists remove open_vswitch . other_config \ > > + lacp-restore-wait="true" > > + fi > > +} > > + > > restore_flows () { > > [ -x "${script_flows}" ] && \ > > action "Restoring saved flows" "${script_flows}" > > } > > > > +restore_lacp () { > > + [ -x "${script_lacp}" ] && \ > > + action "Restoring saved lacp" "${script_lacp}" > > +} > > + > > restore_interfaces () { > > [ ! -x "${script_interfaces}" ] && return 0 > > action "Restoring interface configuration" "${script_interfaces}" > > @@ -633,7 +660,8 @@ restore_interfaces () { > > init_restore_scripts () { > > script_interfaces=`mktemp` > > script_flows=`mktemp` > > - trap 'rm -f "${script_interfaces}" "${script_flows}"' 0 > > + script_lacp=`mktemp` > > + trap 'rm -f "${script_interfaces}" "${script_flows}" "${script_lacp}"' 0 > > } > > > > force_reload_kmod () { > > @@ -648,6 +676,7 @@ force_reload_kmod () { > > > > init_restore_scripts > > save_flows_if_required > > + save_lacp > > > > # Restart the database first, since a large database may take a > > # while to load, and we want to minimize forwarding disruption. > > @@ -682,11 +711,16 @@ force_reload_kmod () { > > > > # Start vswitchd by asking it to wait till flow restore is finished. > > flow_restore_wait > > + lacp_restore_wait > > start_forwarding || return 1 > > > > - # Restore saved flows and inform vswitchd that we are done. > > + # Restore saved resources and inform vswitchd that we are done. > > restore_flows > > + restore_lacp > > + > > flow_restore_complete > > + lacp_restore_complete > > + > > add_managers > > > > restore_interfaces > > @@ -704,6 +738,7 @@ restart () { > > init_restore_scripts > > if test X"${OVS_VSWITCHD:-yes}" = Xyes; then > > save_flows_if_required > > + save_lacp > > fi > > fi > > > > @@ -716,10 +751,15 @@ restart () { > > > > # Start vswitchd by asking it to wait till flow restore is finished. > > flow_restore_wait > > + lacp_restore_wait > > start_forwarding || return 1 > > > > - # Restore saved flows and inform vswitchd that we are done. > > + # Restore saved resources and inform vswitchd that we are done. > > restore_flows > > + restore_lacp > > + > > flow_restore_complete > > + lacp_restore_complete > > + > > add_managers > > } > > diff --git a/utilities/ovs-save b/utilities/ovs-save > > index 67092ecf7..792d5daee 100755 > > --- a/utilities/ovs-save > > +++ b/utilities/ovs-save > > @@ -33,6 +33,8 @@ Commands: > > configuration. > > save-flows Outputs a shell script on stdout that will restore > > OpenFlow flows of each Open vSwitch bridge. > > + save-lacp Outputs a shell script on stdout that will restore > > + lacp info of each lacp bond port. > > This script is meant as a helper for the Open vSwitch init script commands. > > EOF > > } > > @@ -159,6 +161,158 @@ save_flows () { > > echo "rm -rf \"$workdir\"" > > } > > > > +save_lacp () { > > + if (ovs-appctl --version) > /dev/null 2>&1; then :; else > > + echo "$0: ovs-ofctl not found in $PATH" >&2 > > + exit 1 > > + fi > > + > > + case `ovs-appctl version | sed 1q` in > > + "ovs-vswitchd (Open vSwitch) 1."*.*) > > + return > > + ;; > > + esac > > + > > + SP="[[:space:]]*" > > + BOND_NAME_PATTERN="s/^----${SP}([^[:space:]]+)${SP}----.*/\1/" > > + STATUS_PATTERN="s/^status:${SP}(.*)/\1/" > > + SYS_ID_PATTERN="s/^sys_id:${SP}(.*)/\1/" > > + SYS_PRIORITY_PATTERN="s/^sys_priority:${SP}(.*)/\1/" > > + AGGREGATION_KEY_PATTERN="s/^aggregation${SP}key:${SP}(.*)/\1/" > > + LACP_TIME_PATTERN="s/^lacp_time:${SP}(.*)/\1/" > > + MEMBER_PATTERN="s/^(member|slave):${SP}([^:]+):${SP}(.*)/\2/" > > + MEMBER_STATUS_PATTERN="s/^(member|slave):${SP}([^:]+):${SP}(.*)/\3/" > > + PORT_ID_PATTERN="s/^port_id:${SP}(.*)/\1/" > > + PORT_PRIORITY_PATTERN="s/^port_priority:${SP}(.*)/\1/" > > + MAY_ENABLE_PATTERN="s/^may_enable:${SP}(.*)/\1/" > > + ACTOR_SYS_ID_PATTERN="s/^actor${SP}sys_id:${SP}(.*)/\1/" > > + ACTOR_SYS_PRIORITY_PATTERN="s/^actor${SP}sys_priority:${SP}(.*)/\1/" > > + ACTOR_PORT_ID_PATTERN="s/^actor${SP}port_id:${SP}(.*)/\1/" > > + ACTOR_PORT_PRIORITY_PATTERN="s/^actor${SP}port_priority:${SP}(.*)/\1/" > > + ACTOR_KEY_PATTERN="s/^actor${SP}key:${SP}(.*)/\1/" > > + ACTOR_STATE_PATTERN="s/^actor${SP}state:${SP}(.*)/\1/" > > + PARTNER_SYS_ID_PATTERN="s/^partner${SP}sys_id:${SP}(.*)/\1/" > > + PARTNER_SYS_PRIORITY_PATTERN="s/^partner${SP}sys_priority:${SP}(.*)/\1/" > > + PARTNER_PORT_ID_PATTERN="s/^partner${SP}port_id:${SP}(.*)/\1/" > > + PARTNER_PORT_PRIORITY_PATTERN="s/^partner${SP}port_priority:${SP}(.*)/\1/" > > + PARTNER_KEY_PATTERN="s/^partner${SP}key:${SP}(.*)/\1/" > > + PARTNER_STATE_PATTERN="s/^partner${SP}state:${SP}(.*)/\1/" > > + tmpfile=$(mktemp) > > + ovs-appctl lacp/show | while IFS= read -r line; do > > + t=$(echo "$line" | sed -E 's/^[[:space:]]+//') > > + case "$t" in > > + ----*----*) > > + BOND_NAME=$(echo "$t" | sed -E "$BOND_NAME_PATTERN") > > + ;; > > + status:${SP}) > > + STATUS=$(echo "$t" | sed -E "$STATUS_PATTERN" | sed 's/ /+/g') > > + ;; > > + sys_id:${SP}) > > + SYS_ID=$(echo "$t" | sed -E "$SYS_ID_PATTERN") > > + ;; > > + sys_priority:${SP}) > > + SYS_PRIORITY=$(echo "$t" | sed -E "$SYS_PRIORITY_PATTERN") > > + ;; > > + aggregation${SP}key:${SP}) > > + AGGREGATION_KEY=$(echo "$t" | > > + sed -E "$AGGREGATION_KEY_PATTERN") > > + ;; > > + lacp_time:${SP}) > > + LACP_TIME=$(echo "$t" | sed -E "$LACP_TIME_PATTERN") > > + cmd="ovs-appctl lacp/set $BOND_NAME port status=$STATUS," > > + cmd="${cmd}sys_id=$SYS_ID,sys_priority=$SYS_PRIORITY," > > + cmd="${cmd}key=$AGGREGATION_KEY,lacp_time=$LACP_TIME" > > + echo "$cmd" >> "$tmpfile" > > + ;; > > + member:${SP}|slave:${SP}) > > + # Extract member name and status > > + MEMBER_NAME=$(echo "$t" | sed -E "$MEMBER_PATTERN") > > + MEMBER_STATUS=$(echo "$t" | > > + sed -E "$MEMBER_STATUS_PATTERN" | sed 's/ /+/g') > > + ;; > > + port_id:${SP}) > > + PORT_ID=$(echo "$t" | sed -E "$PORT_ID_PATTERN") > > + ;; > > + port_priority:${SP}) > > + PORT_PRIORITY=$(echo "$t" | sed -E "$PORT_PRIORITY_PATTERN") > > + ;; > > + may_enable:${SP}) > > + MAY_ENABLE=$(echo "$t" | sed -E "$MAY_ENABLE_PATTERN") > > + # Print member information > > + cmd="ovs-appctl lacp/set $BOND_NAME member $MEMBER_NAME member" > > + cmd="${cmd} status=$MEMBER_STATUS,port_id=$PORT_ID," > > + cmd="${cmd}port_priority=$PORT_PRIORITY,may_enable=$MAY_ENABLE" > > + echo "$cmd" >> "$tmpfile" > > + ;; > > + actor${SP}sys_id:${SP}) > > + ACTOR_SYS_ID=$(echo "$t" | sed -E "$ACTOR_SYS_ID_PATTERN") > > + ;; > > + actor${SP}sys_priority:${SP}) > > + ACTOR_SYS_PRIORITY=$(echo "$t" | > > + sed -E "$ACTOR_SYS_PRIORITY_PATTERN") > > + ;; > > + actor${SP}port_id:${SP}) > > + ACTOR_PORT_ID=$(echo "$t" | sed -E "$ACTOR_PORT_ID_PATTERN") > > + ;; > > + actor${SP}port_priority:${SP}) > > + ACTOR_PORT_PRIORITY=$(echo "$t" | > > + sed -E "$ACTOR_PORT_PRIORITY_PATTERN") > > + ;; > > + actor${SP}key:${SP}) > > + ACTOR_KEY=$(echo "$t" | sed -E "$ACTOR_KEY_PATTERN") > > + ;; > > + actor${SP}state:${SP}) > > + ACTOR_STATE=$(echo "$t" | > > + sed -E "$ACTOR_STATE_PATTERN" | sed 's/ /+/g') > > + # Print actor information > > + cmd="ovs-appctl lacp/set $BOND_NAME member $MEMBER_NAME actor" > > + cmd="${cmd} sys_id=$ACTOR_SYS_ID," > > + cmd="${cmd}sys_priority=$ACTOR_SYS_PRIORITY," > > + cmd="${cmd}port_id=$ACTOR_PORT_ID," > > + cmd="${cmd}port_priority=$ACTOR_PORT_PRIORITY," > > + cmd="${cmd}key=$ACTOR_KEY,state=$ACTOR_STATE" > > + echo "$cmd" >> "$tmpfile" > > + ;; > > + partner${SP}sys_id:${SP}) > > + PARTNER_SYS_ID=$(echo "$t" | > > + sed -E "$PARTNER_SYS_ID_PATTERN") > > + ;; > > + partner${SP}sys_priority:${SP}) > > + PARTNER_SYS_PRIORITY=$(echo "$t" | > > + sed -E "$PARTNER_SYS_PRIORITY_PATTERN") > > + ;; > > + partner${SP}port_id:${SP}) > > + PARTNER_PORT_ID=$(echo "$t" | > > + sed -E "$PARTNER_PORT_ID_PATTERN") > > + ;; > > + partner${SP}port_priority:${SP}) > > + PARTNER_PORT_PRIORITY=$(echo "$t" | > > + sed -E "$PARTNER_PORT_PRIORITY_PATTERN") > > + ;; > > + partner${SP}key:${SP}) > > + PARTNER_KEY=$(echo "$t" | sed -E "$PARTNER_KEY_PATTERN") > > + ;; > > + partner${SP}state:${SP}) > > + PARTNER_STATE=$(echo "$t" | > > + sed -E "$PARTNER_STATE_PATTERN" | sed 's/ /+/g') > > + # Print partner information > > + cmd="ovs-appctl lacp/set $BOND_NAME" > > + cmd="${cmd} member $MEMBER_NAME partner" > > + cmd="${cmd} sys_id=$PARTNER_SYS_ID," > > + cmd="${cmd}sys_priority=$PARTNER_SYS_PRIORITY," > > + cmd="${cmd}port_id=$PARTNER_PORT_ID," > > + cmd="${cmd}port_priority=$PARTNER_PORT_PRIORITY," > > + cmd="${cmd}key=$PARTNER_KEY,state=$PARTNER_STATE" > > + echo "$cmd" >> "$tmpfile" > > + ;; > > + esac > > + done > > + sed -i '1!G;h;$!d' "$tmpfile" > > + cat "$tmpfile" > > + rm -f "$tmpfile" > > +} > > + > > + > > while [ $# -ne 0 ] > > do > > case $1 in > > @@ -172,6 +326,11 @@ do > > save_interfaces "$@" > > exit 0 > > ;; > > + "save-lacp") > > + shift > > + save_lacp "$@" > > + exit 0 > > + ;; > > -h | --help) > > usage > > exit 0 > > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > > index 456b784c0..e2c523051 100644 > > --- a/vswitchd/bridge.c > > +++ b/vswitchd/bridge.c > > @@ -3415,6 +3415,11 @@ bridge_run(void) > > "flow-restore-wait", false)); > > } > > > > + if (cfg && ofproto_get_lacp_restore_wait()) { > > + ofproto_set_lacp_restore_wait(smap_get_bool(&cfg->other_config, > > + "lacp-restore-wait", false)); > > + } > > + > > bridge_run__(); > > > > /* Re-configure SSL/TLS. We do this on every trip through the main loop, >
diff --git a/lib/lacp.c b/lib/lacp.c index 3252f17eb..251f63dbe 100644 --- a/lib/lacp.c +++ b/lib/lacp.c @@ -25,6 +25,7 @@ #include "dp-packet.h" #include "ovs-atomic.h" #include "packets.h" +#include "openvswitch/ofp-parse.h" #include "openvswitch/poll-loop.h" #include "seq.h" #include "openvswitch/shash.h" @@ -168,6 +169,7 @@ static bool member_may_enable__(struct member *) OVS_REQUIRES(mutex); static unixctl_cb_func lacp_unixctl_show; static unixctl_cb_func lacp_unixctl_show_stats; +static unixctl_cb_func lacp_unixctl_set; /* Populates 'pdu' with a LACP PDU comprised of 'actor' and 'partner'. */ static void @@ -229,6 +231,8 @@ lacp_init(void) lacp_unixctl_show, NULL); unixctl_command_register("lacp/show-stats", "[port]", 0, 1, lacp_unixctl_show_stats, NULL); + unixctl_command_register("lacp/set", "[port]", 3, INT_MAX, + lacp_unixctl_set, NULL); } static void @@ -940,6 +944,34 @@ lacp_find(const char *name) OVS_REQUIRES(mutex) return NULL; } +static struct member * +lacp_member_find(struct lacp *lacp, const char *name) OVS_REQUIRES(mutex) +{ + struct member *member; + + HMAP_FOR_EACH_SAFE (member, node, &lacp->members) { + if (!strcmp(member->name, name)) { + return member; + } + } + + return NULL; +} + +static struct member * +lacp_member_find_by_key(struct lacp *lacp, uint16_t key) OVS_REQUIRES(mutex) +{ + struct member *member; + + HMAP_FOR_EACH_SAFE (member, node, &lacp->members) { + if (member->port_id == key) { + return member; + } + } + + return NULL; +} + static void ds_put_lacp_state(struct ds *ds, uint8_t state) { @@ -1219,5 +1251,245 @@ lacp_get_member_stats(const struct lacp *lacp, const void *member_, } ovs_mutex_unlock(&mutex); return ret; +} + +static void +lacp_port_set(struct unixctl_conn *conn, struct lacp *lacp, const char *parms) + OVS_REQUIRES(mutex) +{ + char *value; + char *key; + char **p = (char **)&parms; + while (ofputil_parse_key_value(p, &key, &value)) { + lacp->update = false; + uint32_t tmp; + if (nullable_string_is_equal(key, "status")) { + if (strstr(value, "active")) { + lacp->active = true; + } + if (strstr(value, "negotiated")) { + lacp->negotiated = true; + } + + } else if (nullable_string_is_equal(key, "sys_id")) { + if (!eth_addr_from_string(value, &lacp->sys_id)) { + unixctl_command_reply_error(conn, "invalid port sys_id"); + return; + } + } else if (nullable_string_is_equal(key, "sys_priority")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid port sys_prority"); + return; + } + lacp->sys_priority = tmp; + } else if (nullable_string_is_equal(key, "key")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid port key"); + return; + } + lacp->key_member = lacp_member_find_by_key(lacp, tmp); + } + } +} + +static uint8_t +lacp_state_from_str(const char *s) +{ + uint8_t state = 0; + if (strstr(s, "activity")) { + state |= LACP_STATE_ACT; + } + if (strstr(s, "timeout")) { + state |= LACP_STATE_TIME; + } + if (strstr(s, "aggregation")) { + state |= LACP_STATE_AGG; + } + if (strstr(s, "synchronized")) { + state |= LACP_STATE_SYNC; + } + if (strstr(s, "collecting")) { + state |= LACP_STATE_COL; + } + if (strstr(s, "distributing")) { + state |= LACP_STATE_DIST; + } + if (strstr(s, "defaulted")) { + state |= LACP_STATE_DEF; + } + if (strstr(s, "expired")) { + state |= LACP_STATE_EXP; + } + return state; +} +static void +lacp_member_set(struct unixctl_conn *conn, struct member *member, + const char *op, const char *parms) OVS_REQUIRES(mutex) +{ + char *value; + char *key; + char **p = (char **)&parms; + if (nullable_string_is_equal(op, "member")) { + while (ofputil_parse_key_value(p, &key, &value)) { + uint32_t tmp; + if (nullable_string_is_equal(key, "port_id")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "port_id"); + return; + } + member->port_id = tmp; + } else if (nullable_string_is_equal(key, "port_priority")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "port_priority"); + return; + } + member->port_priority = tmp; + } else if (nullable_string_is_equal(key, "status")) { + if (strstr(value, "current")) { + member->status = LACP_CURRENT; + member->carrier_up = true; + } else if (strstr(value, "expired")) { + member->status = LACP_EXPIRED; + } else if (strstr(value, "defaulted")) { + member->status = LACP_DEFAULTED; + } + + if (strstr(value, "attached")) { + member->attached = true; + } + } + } + } else if (nullable_string_is_equal(op, "actor")) { + while (ofputil_parse_key_value(p, &key, &value)) { + uint32_t tmp; + if (nullable_string_is_equal(key, "sys_id")) { + struct eth_addr addr; + if (!eth_addr_from_string(value, &addr)) { + unixctl_command_reply_error(conn, "invalid member " + "actor sys_id"); + return; + } + member->ntt_actor.sys_id = addr; + } else if (nullable_string_is_equal(key, "sys_priority")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "actor sys_priority"); + return; + } + member->ntt_actor.sys_priority = htons(tmp); + } else if (nullable_string_is_equal(key, "port_id")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "actor port_id"); + return; + } + member->ntt_actor.port_id = htons(tmp); + } else if (nullable_string_is_equal(key, "port_priority")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "actor port_priority"); + return; + } + member->ntt_actor.port_priority = htons(tmp); + } else if (nullable_string_is_equal(key, "key")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "actor key"); + return; + } + member->ntt_actor.key = htons(tmp); + } else if (nullable_string_is_equal(key, "state")) { + member->ntt_actor.state = lacp_state_from_str(value); + VLOG_INFO("member->ntt_actor.state (%p) (%d)", + &(member->ntt_actor.state), member->ntt_actor.state); + } + } + } else if (nullable_string_is_equal(op, "partner")) { + while (ofputil_parse_key_value(p, &key, &value)) { + uint32_t tmp; + if (nullable_string_is_equal(key, "sys_id")) { + struct eth_addr addr; + if (!eth_addr_from_string(value, &addr)) { + unixctl_command_reply_error(conn, "invalid member " + "partner sys_id"); + return; + } + member->partner.sys_id = addr; + } else if (nullable_string_is_equal(key, "sys_priority")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "partner sys_priority"); + return; + } + member->partner.sys_priority = htons(tmp); + } else if (nullable_string_is_equal(key, "port_id")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "partner port_id"); + return; + } + member->partner.port_id = htons(tmp); + } else if (nullable_string_is_equal(key, "port_priority")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "partner port_priority"); + return; + } + member->partner.port_priority = htons(tmp); + } else if (nullable_string_is_equal(key, "key")) { + if (!str_to_uint(value, 10, &tmp)) { + unixctl_command_reply_error(conn, "invalid member " + "partner key"); + return; + } + member->partner.key = htons(tmp); + } else if (nullable_string_is_equal(key, "state")) { + member->partner.state = lacp_state_from_str(value); + } + } + } else { + unixctl_command_reply_error(conn, "invalid member op"); + } + timer_set_duration(&member->tx, LACP_FAST_TIME_TX); + timer_set_duration(&member->rx, LACP_FAST_TIME_TX); +} + +static void +lacp_unixctl_set(struct unixctl_conn *conn, int argc, const char *argv[], + void *aux OVS_UNUSED) OVS_EXCLUDED(mutex) +{ + struct member *member; + struct lacp *lacp; + + lacp_lock(); + lacp = lacp_find(argv[1]); + if (!lacp) { + unixctl_command_reply_error(conn, "lacp port not found"); + goto out; + } + + if (nullable_string_is_equal(argv[2], "port")) { + lacp_port_set(conn, lacp, argv[3]); + } else if (nullable_string_is_equal(argv[2], "member")) { + member = lacp_member_find(lacp, argv[3]); + if (!member) { + unixctl_command_reply_error(conn, "lacp member not found"); + goto out; + } + if (argc < 5) { + unixctl_command_reply_error(conn, "invalid member parms"); + goto out; + } + lacp_member_set(conn, member, argv[4], argv[5]); + } else { + unixctl_command_reply_error(conn, "invalid op type"); + goto out; + } + +out: + unixctl_command_reply(conn, NULL); + lacp_unlock(); } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index ed9e44ce2..28657f0e8 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -41,6 +41,7 @@ #include "nx-match.h" #include "odp-util.h" #include "odp-execute.h" +#include "ofproto/ofproto.h" #include "ofproto/ofproto-dpif.h" #include "ofproto/ofproto-provider.h" #include "ofproto-dpif-ipfix.h" @@ -3702,7 +3703,6 @@ send_pdu_cb(void *port_, const void *pdu, size_t pdu_size) struct ofport_dpif *port = port_; struct eth_addr ea; int error; - error = netdev_get_etheraddr(port->up.netdev, &ea); if (!error) { struct dp_packet packet; @@ -3781,6 +3781,9 @@ bundle_send_learning_packets(struct ofbundle *bundle) static void bundle_run(struct ofbundle *bundle) { + if (bundle->lacp && ofproto_get_lacp_restore_wait()) { + return; + } if (bundle->lacp) { lacp_run(bundle->lacp, send_pdu_cb); } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 6fa18228b..6f587c21f 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -326,6 +326,8 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); /* The default value of true waits for flow restore. */ static bool flow_restore_wait = true; +/* The default value of true waits for lacp restore. */ +static bool lacp_restore_wait = true; /* Must be called to initialize the ofproto library. * @@ -997,6 +999,18 @@ ofproto_get_flow_restore_wait(void) return flow_restore_wait; } +void +ofproto_set_lacp_restore_wait(bool lacp_restore_wait_db) +{ + lacp_restore_wait = lacp_restore_wait_db; +} + +bool +ofproto_get_lacp_restore_wait(void) +{ + return lacp_restore_wait; +} + /* Retrieve datapath capabilities. */ void ofproto_get_datapath_cap(const char *datapath_type, struct smap *dp_cap) diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 3f85509a1..e8e69b0da 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -382,6 +382,8 @@ int ofproto_set_local_sample(struct ofproto *ofproto, size_t n_options); void ofproto_set_flow_restore_wait(bool flow_restore_wait_db); bool ofproto_get_flow_restore_wait(void); +void ofproto_set_lacp_restore_wait(bool lacp_restore_wait_db); +bool ofproto_get_lacp_restore_wait(void); int ofproto_set_stp(struct ofproto *, const struct ofproto_stp_settings *); int ofproto_get_stp_status(struct ofproto *, struct ofproto_stp_status *); diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index dded0b7c7..a845ac915 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -587,12 +587,21 @@ ovs_save () { [ -z "${bridges}" ] && return 0 } +ovs_save_lacp () { + $datadir/scripts/ovs-save save-lacp > "${script_lacp}" + chmod +x "${script_lacp}" +} + save_flows_if_required () { if test X"$DELETE_BRIDGES" != Xyes; then action "Saving flows" ovs_save save-flows "${script_flows}" fi } +save_lacp () { + action "Saving lacp" ovs_save_lacp +} + save_interfaces () { "$datadir/scripts/ovs-save" save-interfaces ${ifaces} \ > "${script_interfaces}" @@ -604,6 +613,12 @@ flow_restore_wait () { fi } +lacp_restore_wait () { + if test X"${OVS_VSWITCHD:-yes}" = Xyes; then + ovs_vsctl set open_vswitch . other_config:lacp-restore-wait="true" + fi +} + flow_restore_complete () { if test X"${OVS_VSWITCHD:-yes}" = Xyes; then ovs_vsctl --if-exists remove open_vswitch . other_config \ @@ -611,11 +626,23 @@ flow_restore_complete () { fi } +lacp_restore_complete () { + if test X"${OVS_VSWITCHD:-yes}" = Xyes; then + ovs_vsctl --if-exists remove open_vswitch . other_config \ + lacp-restore-wait="true" + fi +} + restore_flows () { [ -x "${script_flows}" ] && \ action "Restoring saved flows" "${script_flows}" } +restore_lacp () { + [ -x "${script_lacp}" ] && \ + action "Restoring saved lacp" "${script_lacp}" +} + restore_interfaces () { [ ! -x "${script_interfaces}" ] && return 0 action "Restoring interface configuration" "${script_interfaces}" @@ -633,7 +660,8 @@ restore_interfaces () { init_restore_scripts () { script_interfaces=`mktemp` script_flows=`mktemp` - trap 'rm -f "${script_interfaces}" "${script_flows}"' 0 + script_lacp=`mktemp` + trap 'rm -f "${script_interfaces}" "${script_flows}" "${script_lacp}"' 0 } force_reload_kmod () { @@ -648,6 +676,7 @@ force_reload_kmod () { init_restore_scripts save_flows_if_required + save_lacp # Restart the database first, since a large database may take a # while to load, and we want to minimize forwarding disruption. @@ -682,11 +711,16 @@ force_reload_kmod () { # Start vswitchd by asking it to wait till flow restore is finished. flow_restore_wait + lacp_restore_wait start_forwarding || return 1 - # Restore saved flows and inform vswitchd that we are done. + # Restore saved resources and inform vswitchd that we are done. restore_flows + restore_lacp + flow_restore_complete + lacp_restore_complete + add_managers restore_interfaces @@ -704,6 +738,7 @@ restart () { init_restore_scripts if test X"${OVS_VSWITCHD:-yes}" = Xyes; then save_flows_if_required + save_lacp fi fi @@ -716,10 +751,15 @@ restart () { # Start vswitchd by asking it to wait till flow restore is finished. flow_restore_wait + lacp_restore_wait start_forwarding || return 1 - # Restore saved flows and inform vswitchd that we are done. + # Restore saved resources and inform vswitchd that we are done. restore_flows + restore_lacp + flow_restore_complete + lacp_restore_complete + add_managers } diff --git a/utilities/ovs-save b/utilities/ovs-save index 67092ecf7..792d5daee 100755 --- a/utilities/ovs-save +++ b/utilities/ovs-save @@ -33,6 +33,8 @@ Commands: configuration. save-flows Outputs a shell script on stdout that will restore OpenFlow flows of each Open vSwitch bridge. + save-lacp Outputs a shell script on stdout that will restore + lacp info of each lacp bond port. This script is meant as a helper for the Open vSwitch init script commands. EOF } @@ -159,6 +161,158 @@ save_flows () { echo "rm -rf \"$workdir\"" } +save_lacp () { + if (ovs-appctl --version) > /dev/null 2>&1; then :; else + echo "$0: ovs-ofctl not found in $PATH" >&2 + exit 1 + fi + + case `ovs-appctl version | sed 1q` in + "ovs-vswitchd (Open vSwitch) 1."*.*) + return + ;; + esac + + SP="[[:space:]]*" + BOND_NAME_PATTERN="s/^----${SP}([^[:space:]]+)${SP}----.*/\1/" + STATUS_PATTERN="s/^status:${SP}(.*)/\1/" + SYS_ID_PATTERN="s/^sys_id:${SP}(.*)/\1/" + SYS_PRIORITY_PATTERN="s/^sys_priority:${SP}(.*)/\1/" + AGGREGATION_KEY_PATTERN="s/^aggregation${SP}key:${SP}(.*)/\1/" + LACP_TIME_PATTERN="s/^lacp_time:${SP}(.*)/\1/" + MEMBER_PATTERN="s/^(member|slave):${SP}([^:]+):${SP}(.*)/\2/" + MEMBER_STATUS_PATTERN="s/^(member|slave):${SP}([^:]+):${SP}(.*)/\3/" + PORT_ID_PATTERN="s/^port_id:${SP}(.*)/\1/" + PORT_PRIORITY_PATTERN="s/^port_priority:${SP}(.*)/\1/" + MAY_ENABLE_PATTERN="s/^may_enable:${SP}(.*)/\1/" + ACTOR_SYS_ID_PATTERN="s/^actor${SP}sys_id:${SP}(.*)/\1/" + ACTOR_SYS_PRIORITY_PATTERN="s/^actor${SP}sys_priority:${SP}(.*)/\1/" + ACTOR_PORT_ID_PATTERN="s/^actor${SP}port_id:${SP}(.*)/\1/" + ACTOR_PORT_PRIORITY_PATTERN="s/^actor${SP}port_priority:${SP}(.*)/\1/" + ACTOR_KEY_PATTERN="s/^actor${SP}key:${SP}(.*)/\1/" + ACTOR_STATE_PATTERN="s/^actor${SP}state:${SP}(.*)/\1/" + PARTNER_SYS_ID_PATTERN="s/^partner${SP}sys_id:${SP}(.*)/\1/" + PARTNER_SYS_PRIORITY_PATTERN="s/^partner${SP}sys_priority:${SP}(.*)/\1/" + PARTNER_PORT_ID_PATTERN="s/^partner${SP}port_id:${SP}(.*)/\1/" + PARTNER_PORT_PRIORITY_PATTERN="s/^partner${SP}port_priority:${SP}(.*)/\1/" + PARTNER_KEY_PATTERN="s/^partner${SP}key:${SP}(.*)/\1/" + PARTNER_STATE_PATTERN="s/^partner${SP}state:${SP}(.*)/\1/" + tmpfile=$(mktemp) + ovs-appctl lacp/show | while IFS= read -r line; do + t=$(echo "$line" | sed -E 's/^[[:space:]]+//') + case "$t" in + ----*----*) + BOND_NAME=$(echo "$t" | sed -E "$BOND_NAME_PATTERN") + ;; + status:${SP}) + STATUS=$(echo "$t" | sed -E "$STATUS_PATTERN" | sed 's/ /+/g') + ;; + sys_id:${SP}) + SYS_ID=$(echo "$t" | sed -E "$SYS_ID_PATTERN") + ;; + sys_priority:${SP}) + SYS_PRIORITY=$(echo "$t" | sed -E "$SYS_PRIORITY_PATTERN") + ;; + aggregation${SP}key:${SP}) + AGGREGATION_KEY=$(echo "$t" | + sed -E "$AGGREGATION_KEY_PATTERN") + ;; + lacp_time:${SP}) + LACP_TIME=$(echo "$t" | sed -E "$LACP_TIME_PATTERN") + cmd="ovs-appctl lacp/set $BOND_NAME port status=$STATUS," + cmd="${cmd}sys_id=$SYS_ID,sys_priority=$SYS_PRIORITY," + cmd="${cmd}key=$AGGREGATION_KEY,lacp_time=$LACP_TIME" + echo "$cmd" >> "$tmpfile" + ;; + member:${SP}|slave:${SP}) + # Extract member name and status + MEMBER_NAME=$(echo "$t" | sed -E "$MEMBER_PATTERN") + MEMBER_STATUS=$(echo "$t" | + sed -E "$MEMBER_STATUS_PATTERN" | sed 's/ /+/g') + ;; + port_id:${SP}) + PORT_ID=$(echo "$t" | sed -E "$PORT_ID_PATTERN") + ;; + port_priority:${SP}) + PORT_PRIORITY=$(echo "$t" | sed -E "$PORT_PRIORITY_PATTERN") + ;; + may_enable:${SP}) + MAY_ENABLE=$(echo "$t" | sed -E "$MAY_ENABLE_PATTERN") + # Print member information + cmd="ovs-appctl lacp/set $BOND_NAME member $MEMBER_NAME member" + cmd="${cmd} status=$MEMBER_STATUS,port_id=$PORT_ID," + cmd="${cmd}port_priority=$PORT_PRIORITY,may_enable=$MAY_ENABLE" + echo "$cmd" >> "$tmpfile" + ;; + actor${SP}sys_id:${SP}) + ACTOR_SYS_ID=$(echo "$t" | sed -E "$ACTOR_SYS_ID_PATTERN") + ;; + actor${SP}sys_priority:${SP}) + ACTOR_SYS_PRIORITY=$(echo "$t" | + sed -E "$ACTOR_SYS_PRIORITY_PATTERN") + ;; + actor${SP}port_id:${SP}) + ACTOR_PORT_ID=$(echo "$t" | sed -E "$ACTOR_PORT_ID_PATTERN") + ;; + actor${SP}port_priority:${SP}) + ACTOR_PORT_PRIORITY=$(echo "$t" | + sed -E "$ACTOR_PORT_PRIORITY_PATTERN") + ;; + actor${SP}key:${SP}) + ACTOR_KEY=$(echo "$t" | sed -E "$ACTOR_KEY_PATTERN") + ;; + actor${SP}state:${SP}) + ACTOR_STATE=$(echo "$t" | + sed -E "$ACTOR_STATE_PATTERN" | sed 's/ /+/g') + # Print actor information + cmd="ovs-appctl lacp/set $BOND_NAME member $MEMBER_NAME actor" + cmd="${cmd} sys_id=$ACTOR_SYS_ID," + cmd="${cmd}sys_priority=$ACTOR_SYS_PRIORITY," + cmd="${cmd}port_id=$ACTOR_PORT_ID," + cmd="${cmd}port_priority=$ACTOR_PORT_PRIORITY," + cmd="${cmd}key=$ACTOR_KEY,state=$ACTOR_STATE" + echo "$cmd" >> "$tmpfile" + ;; + partner${SP}sys_id:${SP}) + PARTNER_SYS_ID=$(echo "$t" | + sed -E "$PARTNER_SYS_ID_PATTERN") + ;; + partner${SP}sys_priority:${SP}) + PARTNER_SYS_PRIORITY=$(echo "$t" | + sed -E "$PARTNER_SYS_PRIORITY_PATTERN") + ;; + partner${SP}port_id:${SP}) + PARTNER_PORT_ID=$(echo "$t" | + sed -E "$PARTNER_PORT_ID_PATTERN") + ;; + partner${SP}port_priority:${SP}) + PARTNER_PORT_PRIORITY=$(echo "$t" | + sed -E "$PARTNER_PORT_PRIORITY_PATTERN") + ;; + partner${SP}key:${SP}) + PARTNER_KEY=$(echo "$t" | sed -E "$PARTNER_KEY_PATTERN") + ;; + partner${SP}state:${SP}) + PARTNER_STATE=$(echo "$t" | + sed -E "$PARTNER_STATE_PATTERN" | sed 's/ /+/g') + # Print partner information + cmd="ovs-appctl lacp/set $BOND_NAME" + cmd="${cmd} member $MEMBER_NAME partner" + cmd="${cmd} sys_id=$PARTNER_SYS_ID," + cmd="${cmd}sys_priority=$PARTNER_SYS_PRIORITY," + cmd="${cmd}port_id=$PARTNER_PORT_ID," + cmd="${cmd}port_priority=$PARTNER_PORT_PRIORITY," + cmd="${cmd}key=$PARTNER_KEY,state=$PARTNER_STATE" + echo "$cmd" >> "$tmpfile" + ;; + esac + done + sed -i '1!G;h;$!d' "$tmpfile" + cat "$tmpfile" + rm -f "$tmpfile" +} + + while [ $# -ne 0 ] do case $1 in @@ -172,6 +326,11 @@ do save_interfaces "$@" exit 0 ;; + "save-lacp") + shift + save_lacp "$@" + exit 0 + ;; -h | --help) usage exit 0 diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 456b784c0..e2c523051 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -3415,6 +3415,11 @@ bridge_run(void) "flow-restore-wait", false)); } + if (cfg && ofproto_get_lacp_restore_wait()) { + ofproto_set_lacp_restore_wait(smap_get_bool(&cfg->other_config, + "lacp-restore-wait", false)); + } + bridge_run__(); /* Re-configure SSL/TLS. We do this on every trip through the main loop,
On most switches, if STP is enabled and the port connected to OVS is not in edge mode, when OVS service restart and negotiate LACP, which will trigger STP recalculation. During this time, traffic on the LACP interface will be blocked for several seconds(up to 5s+). Test in Mellanox Switch OvS LACP: Fast Mode Switch LACP | STP Mode | Downtime ------------|-------------|--------- Fast/Slow | on - normal | 3.1s Fast/Slow | off / edge | 0.2s This patch propose a solution, similar to flow restore, 1. save LACP negotiation status 2. pause LACP process 3. restart OvS service 4. inject LACP status into vswitchd 5. resume LACP process Test Results with this patch Switch LACP | STP Mode | Downtime ------------|-------------|--------- Fast | on - normal | 0.3s Slow | on - normal | 0.0s Fast/Slow | off / edge | 0.0s Signed-off-by: Changliang Wu <changliang.wu@smartx.com> --- lib/lacp.c | 272 +++++++++++++++++++++++++++++++++++++++++ ofproto/ofproto-dpif.c | 5 +- ofproto/ofproto.c | 14 +++ ofproto/ofproto.h | 2 + utilities/ovs-lib.in | 46 ++++++- utilities/ovs-save | 159 ++++++++++++++++++++++++ vswitchd/bridge.c | 5 + 7 files changed, 499 insertions(+), 4 deletions(-)