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

[Aaron Conole <aconole@redhat.com>]

"Stokes, Ian" <ian.stokes@intel.com> writes:

>> > 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
>
> [IS] Thanks for doing this follow up Jan, it’s really valuable in my opinion.
>
> It’s definitely something that should be raised to the DPDK community
> as conceivably if there is no set process to follow we could see
> varying behavior from PMD device to device. I’m happy to take this and
> raise it to the DPDK community.

Agreed that the understanding of when/how this setter can be called is
confusing.  I contacted Harish Patil (who works on that driver for the
DPDK side) about this, and think that there's a followup to this thread
coming.

> Thanks
> Ian
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev