On 8 Nov 2018, at 16:09, Stokes, Ian wrote:
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?
Info would be fine with me, I'd just like it to be apparent to a user
when reading the logs. We do something similar for when
rte_eth_dev_set_mtu() is not supported, although that is a rarer
situation and in that case a warning is ok.
I did base the LOG level and message on the MTU one. Will send out a V3
as Info.
+ 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
