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?
Maybe looking at it, we could ignore any value returned, just making it
a best effort attempt?
For example testing with a i40e_vf, calling ovs-appctl
netdev-dpdk/set-admin-state <port> down will result with a success
message "ok" after issuing the command. Listing the interface via
ovs-vsctl after the call shows that the status of the link and admin
state have indeed been set to down even though
rte_eth_dev_set_link_down() returned ENOTSUP and the link is not down.
For sure this is a corner case but upon inspection in DPDK 17.11 at
least there are more drivers that do not have implementations for
rte_eth_dev_set_link_up() and rte_eth_dev_set_link_down() than there
are drivers which have it implemented a function for it.
Ian
+ }
+ }
+
if (dev->flags & NETDEV_PROMISC) {
rte_eth_promiscuous_enable(dev->port_id);
}
+ if (!(dev->flags & NETDEV_UP)) {
+ err = rte_eth_dev_set_link_down(dev->port_id);
+ if (err < 0 && err != -ENOTSUP) {
+ return -err;
+ }
+ }
+
netdev_change_seq_changed(&dev->up);
} else {
/* If DPDK_DEV_VHOST device's NETDEV_UP flag was changed
and
vhost is
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
