Re: [dpdk-dev] [ovs-dev] [PATCH] netdev-dpdk: defer MTU set after interface start

[Jan Scheurich <jan.scheurich@ericsson.com>]

> > The issue only arises with the qede PMD and 67fe6d635193
> > ("netdev-dpdk: use rte_eth_dev_set_mtu.")
>
> I had some more time to look at this today but this patch will break OVS DPDK for existing supported DPDK ports during testing.
>
> I tested with an XL710 and the MTU will fail to apply, the device must be stopped before configuration changes can be applied such as
> MTU. See log message below
>
> 2017-11-28T17:13:50Z|00086|dpdk|ERR|i40e_dev_mtu_set(): port 0 must be stopped before configuration
> 2017-11-28T17:13:50Z|00087|netdev_dpdk|ERR|Interface dpdk0 MTU (1500) setup error: Device or resource busy
>
> Did you come across it in your testing? I guess you didn’t see this for QEDE pmd. In my case the DPDK devices will simply fail to add to the
> bridge.
>
> As is, the patch would not be acceptable as its breaking existing functionality. It would have to be reworked to configure for device that
> cannot reconfigure when busy.
>
> Thanks
> Ian

I fully support the original decision to switch to using rte_dev_set_mtu() in OVS. The prior approach setting max_rx_pkt_len in rte_eth_dev_configure() was non-portable as that field has no well-defined semantics and its relation to the MTU size is different for almost every PMD.

I had a look at the qede PMD implementation of rte_dev_set_mtu() in DPDK 17.05 and 17.11. It assumes that the device must be started because qede_set_mtu() unconditionally stops the device before and restarts it after changing the MTU value. Given that other PMDs like i40e don’t accept it after start, there is no possible way OVS can use rte_dev_set_mtu() that will work with all drivers.

I think it is a weakness of the rte_ethdev API that it does not specify clearly when API functions like rte_dev_set_mtu() may be called. From the documentation of error -EBUSY one can guess that the intention was to optionally support it when the device is started. Implicitly one could conclude that it MUST be supported when the device stopped. That is logical and also what most PMDs do.

I would say the qede PMD should be fixed. It should accept the rte_dev_set_mtu() at any time between rte_eth_dev_configure() and rte_eth_dev_start() (and optionally also after start).

BR, Jan