On 9 Nov 2018, at 13:07, Stokes, Ian wrote:
>> On 9 Nov 2018, at 12:16, Ilya Maximets wrote:
>>
>>> On 09.11.2018 11:20, 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]>
>>>>
>>>> ---
>>>> * V3:
>>>> - Update log message and changed the level to INFO
>>>>
>>>> * 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 | 18 ++++++++++++++++++
>>>> 1 file changed, 18 insertions(+)
>>>
>>> Looks OK in general. One small comment inline.
>>>
>>>
>>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>>>> 7e0a593..bfab857 100644
>>>> --- a/lib/netdev-dpdk.c
>>>> +++ b/lib/netdev-dpdk.c
>>>> @@ -2942,6 +2942,24 @@ 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_INFO("Interface %s does not support link state
>>>> "
>>>> + "configuration", dev->up.name);
>>>
>>> I think, we prefer 'netdev_get_name(&dev->up)' for a new code.
>>
>> Thanks Ilya, I’ll sent out a v4 ;) Ian anything else before I sent it?
>>
>
> LGTM, will be part of today's pull request.
Thanks I’ve sent out a V4, can you pull this for older versions also?
>
> Thanks
> Ian
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
