========
On certain Fortville NICs it has been observed that PHY UP detection can
get delayed (sometimes up to 4-5 secs). When the driver fails to fetch
PHY status as UP even though its actually UP, LACP packets can get
exchanged and LACP slave brought UP. In such a case, the remote end
would start sending traffic on that slave, but OVS drops it, as the
bond-slave is not yet enabled due to PHY DOWN.
Fix:
====
The main intention here is delay LACP negotiation until carrier (PHY)
status is read as UP.
1. In port_run()/bundle_run(), cache the carrier status in
"struct ofport_dpif"/"struct bond_slave".
2. When carrier state is DOWN, do not send any LACPDUs and
drop any received LACPDUs.
3. When LACP state changes or LACPDU is received, trigger re-checking
of carrier-state (in port_run()) by incrementing the connectivity
sequence number to find out the true carrier state as fast as
possible.
Signed-off-by: Manohar Krishnappa Chidambaraswamy <manukc@gmail.com>
Co-authored-by: Manohar Krishnappa Chidambaraswamy <manukc@gmail.com>
Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com>
---
lib/lacp.c | 44 +++++++++++++++++++++++++++++++++++---------
lib/lacp.h | 8 ++++----
ofproto/bond.c | 30 ++++++++++++++++++++++++++++++
ofproto/bond.h | 3 +++
ofproto/ofproto-dpif-xlate.c | 1 +
ofproto/ofproto-dpif.c | 10 +++++++---
6 files changed, 80 insertions(+), 16 deletions(-)
@@ -33,6 +33,7 @@
#include "unixctl.h"
#include "openvswitch/vlog.h"
#include "util.h"
+#include "ofproto/bond.h"
VLOG_DEFINE_THIS_MODULE(lacp);
@@ -148,7 +149,7 @@ static void slave_get_actor(struct slave *, struct lacp_info *actor)
OVS_REQUIRES(mutex);
static void slave_get_priority(struct slave *, struct lacp_info *priority)
OVS_REQUIRES(mutex);
-static bool slave_may_tx(const struct slave *)
+static bool slave_may_tx(const void *bond, const struct slave *)
OVS_REQUIRES(mutex);
static struct slave *slave_lookup(const struct lacp *, const void *slave)
OVS_REQUIRES(mutex);
@@ -326,14 +327,15 @@ lacp_is_active(const struct lacp *lacp) OVS_EXCLUDED(mutex)
* called on all packets received on 'slave_' with Ethernet Type ETH_TYPE_LACP.
*/
bool
-lacp_process_packet(struct lacp *lacp, const void *slave_,
- const struct dp_packet *packet)
+lacp_process_packet(struct lacp *lacp, const void *bond,
+ const void *slave_, const struct dp_packet *packet)
OVS_EXCLUDED(mutex)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
const struct lacp_pdu *pdu;
long long int tx_rate;
struct slave *slave;
+ bool carrier_up = false;
bool lacp_may_enable = false;
lacp_lock();
@@ -350,6 +352,21 @@ lacp_process_packet(struct lacp *lacp, const void *slave_,
goto out;
}
+ /*
+ * On some NICs L1 state reporting is slow. In case LACP packets are
+ * received while carrier (L1) state is still down, drop the LACPDU and
+ * trigger re-checking of L1 state.
+ */
+ carrier_up = bond_slave_get_carrier(bond, slave->aux);
+ if (!carrier_up) {
+ seq_change(connectivity_seq_get());
+
+ VLOG_INFO_RL(&rl, "carrier still DOWN - conn seq changed for %s, "
+ "dropping packet\n", slave->name);
+
+ goto out;
+ }
+
slave->status = LACP_CURRENT;
tx_rate = lacp->fast ? LACP_FAST_TIME_TX : LACP_SLOW_TIME_TX;
timer_set_duration(&slave->rx, LACP_RX_MULTIPLIER * tx_rate);
@@ -529,7 +546,8 @@ lacp_slave_is_current(const struct lacp *lacp, const void *slave_)
/* This function should be called periodically to update 'lacp'. */
void
-lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) OVS_EXCLUDED(mutex)
+lacp_run(struct lacp *lacp, const void *bond,
+ lacp_send_pdu *send_pdu) OVS_EXCLUDED(mutex)
{
struct slave *slave;
@@ -559,7 +577,7 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) OVS_EXCLUDED(mutex)
HMAP_FOR_EACH (slave, node, &lacp->slaves) {
struct lacp_info actor;
- if (!slave_may_tx(slave)) {
+ if (!slave_may_tx(bond, slave)) {
continue;
}
@@ -588,13 +606,13 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) OVS_EXCLUDED(mutex)
/* Causes poll_block() to wake up when lacp_run() needs to be called again. */
void
-lacp_wait(struct lacp *lacp) OVS_EXCLUDED(mutex)
+lacp_wait(struct lacp *lacp, const void *bond) OVS_EXCLUDED(mutex)
{
struct slave *slave;
lacp_lock();
HMAP_FOR_EACH (slave, node, &lacp->slaves) {
- if (slave_may_tx(slave)) {
+ if (slave_may_tx(bond, slave)) {
timer_wait(&slave->tx);
}
@@ -818,9 +836,17 @@ slave_get_priority(struct slave *slave, struct lacp_info *priority)
}
static bool
-slave_may_tx(const struct slave *slave) OVS_REQUIRES(mutex)
+slave_may_tx(const void *bond, const struct slave *slave)
+ OVS_REQUIRES(mutex)
{
- return slave->lacp->active || slave->status != LACP_DEFAULTED;
+ bool carrier_up = true;
+
+ if (bond) {
+ carrier_up = bond_slave_get_carrier(bond, slave->aux);
+ }
+
+ return carrier_up &&
+ (slave->lacp->active || slave->status != LACP_DEFAULTED);
}
static struct slave *
@@ -46,8 +46,8 @@ struct lacp *lacp_ref(const struct lacp *);
void lacp_configure(struct lacp *, const struct lacp_settings *);
bool lacp_is_active(const struct lacp *);
-bool lacp_process_packet(struct lacp *, const void *slave,
- const struct dp_packet *packet);
+bool lacp_process_packet(struct lacp *, const void *bond,
+ const void *slave, const struct dp_packet *packet);
enum lacp_status lacp_status(const struct lacp *);
struct lacp_slave_settings {
@@ -67,8 +67,8 @@ 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 void *pdu, size_t pdu_size);
-void lacp_run(struct lacp *, lacp_send_pdu *);
-void lacp_wait(struct lacp *);
+void lacp_run(struct lacp *, const void *, lacp_send_pdu *);
+void lacp_wait(struct lacp *, const void *);
struct lacp_slave_stats {
/* id */
@@ -93,6 +93,7 @@ struct bond_slave {
/* Link status. */
bool enabled; /* May be chosen for flows? */
bool may_enable; /* Client considers this slave bondable. */
+ bool carrier_up; /* Carrier state from NIC port */
long long delay_expires; /* Time after which 'enabled' may change. */
/* Rebalancing info. Used only by bond_rebalance(). */
@@ -634,6 +635,35 @@ bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable)
ovs_rwlock_unlock(&rwlock);
}
+void
+bond_slave_set_carrier(struct bond *bond, void *slave_, bool carrier_up)
+{
+ struct bond_slave *slave = NULL;
+
+ ovs_rwlock_wrlock(&rwlock);
+ slave = bond_slave_lookup(bond, slave_);
+ if (slave) {
+ slave->carrier_up = carrier_up;
+ }
+ ovs_rwlock_unlock(&rwlock);
+}
+
+bool
+bond_slave_get_carrier(const struct bond *bond, void *slave_)
+{
+ bool carrier_up = false;
+ struct bond_slave *slave;
+
+ ovs_rwlock_rdlock(&rwlock);
+ slave = bond_slave_lookup((struct bond *)bond, slave_);
+ if (slave) {
+ carrier_up = slave->carrier_up;
+ }
+ ovs_rwlock_unlock(&rwlock);
+
+ return carrier_up;
+}
+
/* Performs periodic maintenance on 'bond'.
*
* Returns true if the caller should revalidate its flows.
@@ -79,6 +79,9 @@ void bond_wait(struct bond *);
void bond_slave_set_may_enable(struct bond *, void *slave_, bool may_enable);
+void bond_slave_set_carrier(struct bond *bond, void *slave_, bool carrier_up);
+bool bond_slave_get_carrier(const struct bond *bond, void *slave_);
+
/* Special MAC learning support for SLB bonding. */
bool bond_should_send_learning_packets(struct bond *);
struct dp_packet *bond_compose_learning_packet(struct bond *,
@@ -3329,6 +3329,7 @@ process_special(struct xlate_ctx *ctx, const struct xport *xport)
&& flow->dl_type == htons(ETH_TYPE_LACP)) {
if (packet) {
lacp_may_enable = lacp_process_packet(xport->xbundle->lacp,
+ xport->xbundle->bond,
xport->ofport, packet);
/* Update LACP status in bond-slave to avoid packet-drops until
* LACP state machine is run by the main thread. */
@@ -137,6 +137,7 @@ struct ofport_dpif {
struct cfm *cfm; /* Connectivity Fault Management, if any. */
struct bfd *bfd; /* BFD, if any. */
struct lldp *lldp; /* lldp, if any. */
+ bool carrier_up; /* Carrier state from NIC */
bool is_tunnel; /* This port is a tunnel. */
long long int carrier_seq; /* Carrier status changes. */
struct ofport_dpif *peer; /* Peer if patch port. */
@@ -3324,12 +3325,13 @@ static void
bundle_run(struct ofbundle *bundle)
{
if (bundle->lacp) {
- lacp_run(bundle->lacp, send_pdu_cb);
+ lacp_run(bundle->lacp, bundle->bond, send_pdu_cb);
}
if (bundle->bond) {
struct ofport_dpif *port;
LIST_FOR_EACH (port, bundle_node, &bundle->ports) {
+ bond_slave_set_carrier(bundle->bond, port, port->carrier_up);
bond_slave_set_may_enable(bundle->bond, port, port->up.may_enable);
}
@@ -3347,7 +3349,7 @@ static void
bundle_wait(struct ofbundle *bundle)
{
if (bundle->lacp) {
- lacp_wait(bundle->lacp);
+ lacp_wait(bundle->lacp, bundle->bond);
}
if (bundle->bond) {
bond_wait(bundle->bond);
@@ -3563,8 +3565,10 @@ ofport_update_peer(struct ofport_dpif *ofport)
static bool
may_enable_port(struct ofport_dpif *ofport)
{
+ ofport->carrier_up = netdev_get_carrier(ofport->up.netdev);
+
/* Carrier must be up. */
- if (!netdev_get_carrier(ofport->up.netdev)) {
+ if (!ofport->carrier_up) {
return false;
}