Hi, > -----Original Message----- > From: Ben Pfaff [mailto:[email protected]] > Sent: Saturday, March 02, 2019 12:15 AM > To: Nitin Katiyar <[email protected]> > Cc: [email protected]; Manohar Krishnappa Chidambaraswamy > <[email protected]> > Subject: Re: [ovs-dev] [PATCH v4 2/2] Avoid packet drop on LACP bond after > link up > > I don't entirely understand the problem. It seems like a driver bug. > Why isn't the bug being fixed? I agree it is driver/firmware bug but OVS implementation also doesn't account for link state which causes traffic drop in case of bond. > > Patches 1 and 2 have the same title. It would be better if they were > different. Both issues had surfaced in same scenario so we decided to have 2 patches. But we can send it as 2 different patches next time. > > [more below] > > On Thu, Feb 28, 2019 at 01:56:20PM +0000, Nitin Katiyar wrote: > > + /* > > + * 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); > > It's going to be hard for a user to understand what this means, especially the > "conn seq changed" part. The user also doesn't know the context, so it's not > clear what "carrier still DOWN" means--still down after what? Can you please > rephrase the log message so that it's clear what happened and why it's > unusual? Sure, I will rephrase it. > > Please put the name of the lacp object at the beginning of the message. > > Please don't add \n at the end of a log message; the logger does that itself. > > In bond_slave_get_carrier(), please use CONST_CAST for casting away const. I will update all these three in next patch.
Thanks, Nitin _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
