On 8 Nov 2018, at 15:34, Ilya Maximets wrote:
On 08.11.2018 16:58, Eelco Chaudron 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 <[email protected]>
---
* V2:
- Only call link state change function if the upstate changed
- Restore original flags on error
- Log a message when the NIC does not support the link state API
lib/netdev-dpdk.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 7e0a593..382e287 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2942,6 +2942,25 @@ netdev_dpdk_update_flags__(struct netdev_dpdk
*dev,
}
if (dev->type == DPDK_DEV_ETH) {
+
+ if ((dev->flags ^ *old_flagsp) & NETDEV_UP) {
+ int err;
+
+ if (dev->flags & NETDEV_UP) {
+ err = rte_eth_dev_set_link_up(dev->port_id);
+ } else {
+ err = rte_eth_dev_set_link_down(dev->port_id);
+ }
+ if (err == -ENOTSUP) {
+ VLOG_WARN("Interface %s does not support link state
"
+ "configuration, physical link will always
be up.",
This message is not fully correct. ENOTSUP means that we can't change
the state
from the OVS side, but it could be changed physically by disconnecting
the cable
or setting down the other side. IMHO, need to rephrase somehow. Maybe
just drop
the second part about physical link?
I see the confusion here… I’ll drop the second part in the next
re-spin.
Also, as Ian said, ENOTSUP is a common case, maybe we can reduce log
level to INFO?
Maybe even DBG to save some space in logs.
This message should not be so frequent, and I think the actual number of
PMDs is not that big (mainly the virtual ones). But I’m fine changing
it to INFO or DBG, Ian?
+ dev->up.name);
+ } else if (err < 0) {
+ dev->flags = *old_flagsp;
+ return -err;
+ }
+ }
+
if (dev->flags & NETDEV_PROMISC) {
rte_eth_promiscuous_enable(dev->port_id);
}
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
