> On 7 Nov 2018, at 15:04, Stokes, Ian wrote: > > >>> When the netdev link flags are changed, !NETDEV_UP, the DPDK ports > >>> are not actually going down. This is causing problems for people > >>> trying to bring down a bond member. The bond link is no longer being > >>> used to receive or transmit traffic, however, the other end keeps > >>> sending data as the link remains up. > >>> > >>> With OVS 2.6 the link was brought down, and this was changed with > >>> commit 3b1fb0779. In this commit, it's explicitly mentioned that the > >>> link down/up DPDK APIs are not called as not all PMD devices support > >>> it. > >>> > >>> However, this patch does call the appropriate DPDK APIs and ignoring > >>> errors due to the PMD not supporting it. PMDs not supporting this > >>> should be fixed in DPDK upstream. > >>> > >>> I verified this patch is working correctly using the ovs-appctl > >>> netdev-dpdk/set-admin-state <port> {up|down} and ovs-ofctl mod-port > >>> <bridge> <port> {up|down} commands on a XL710 and 82599ES. > >>> > >>> Fixes: 3b1fb0779b87 ("netdev-dpdk: Don't call rte_dev_stop() in > >>> update_flags().") > >>> Signed-off-by: Eelco Chaudron <echaudro at redhat.com> > >>> --- > >>> lib/netdev-dpdk.c | 16 ++++++++++++++++ > >>> 1 file changed, 16 insertions(+) > >>> > >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > >>> 7e0a593..ee8296b 100644 > >>> --- a/lib/netdev-dpdk.c > >>> +++ b/lib/netdev-dpdk.c > >>> @@ -2942,10 +2942,26 @@ netdev_dpdk_update_flags__(struct > >>> netdev_dpdk > >> *dev, > >>> } > >>> > >>> if (dev->type == DPDK_DEV_ETH) { > >>> + int err; > >>> + > >>> + if (dev->flags & NETDEV_UP) { > >>> + err = rte_eth_dev_set_link_up(dev->port_id); > >>> + if (err < 0 && err != -ENOTSUP) { > >>> + return -err; > >> > >> Should we restore the flags before returning the error? > >> Otherwise on the next equal call we'll return 0 without any real > >> changes. > >> > > Yes I agree on error we should do dev->flags = *old_flags > > > Yes, as well as that I think we should flag when the operation is not > > supported and handle it accordingly. > > I think if ENOTSUP is returned we should silently ignore the physical > down, as from 2.7 till now it was not implemented. Clearing NETDEV_UP in > this case still cause it to be administratively down (just external > entities might not know due to the link staying up). Or do you suggest to > log it as a warning that the physical link up/down is not supported by the > PMD?
I think a warning would be welcome along with the state the device will remain in. Ian _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev