[ovs-dev] [PATCH v3] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event

Ilya Maximets i.maximets at samsung.com
Thu Sep 12 10:24:18 UTC 2019


On 12.09.2019 13:19, Ilya Maximets wrote:
> On 12.09.2019 13:07, Eelco Chaudron wrote:
>>
>>
>> On 12 Sep 2019, at 10:39, Ilya Maximets wrote:
>>
>>> On 11.09.2019 16:20, Eelco Chaudron wrote:
>>>> Currently, OVS does not register and therefore not handle the
>>>> interface reset event from the DPDK framework. This would cause a
>>>> problem in cases where a VF is used as an interface, and its
>>>> configuration changes.
>>>>
>>>> As an example in the following scenario the MAC change is not
>>>> detected/acted upon until OVS is restarted without the patch applied:
>>>>
>>>>   $ echo 1 > /sys/bus/pci/devices/0000:05:00.1/sriov_numvfs
>>>>   $ ovs-vsctl add-port ovs_pvp_br0 dpdk0 -- \
>>>>             set Interface dpdk0 type=dpdk -- \
>>>>             set Interface dpdk0 options:dpdk-devargs=0000:05:0a.0
>>>>
>>>>   $ ip link set p5p2 vf 0 mac 52:54:00:92:d3:33
>>>>
>>>> Signed-off-by: Eelco Chaudron <echaudro at redhat.com>
>>>> ---
>>>
>>> <snip>
>>>
>>>> @@ -4180,13 +4223,19 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>>>>          && dev->rxq_size == dev->requested_rxq_size
>>>>          && dev->txq_size == dev->requested_txq_size
>>>>          && dev->socket_id == dev->requested_socket_id
>>>> -        && dev->started) {
>>>> +        && dev->started && !dev->reset_needed) {
>>>>          /* Reconfiguration is unnecessary */
>>>>
>>>>          goto out;
>>>>      }
>>>>
>>>> -    rte_eth_dev_stop(dev->port_id);
>>>> +    if (dev->reset_needed) {
>>>> +        rte_eth_dev_reset(dev->port_id);
>>>
>>> Thinking more about the change and looking at flow control configuration,
>>> it seems that on reset we'll lost configurations done in set_config().
>>> Device reset should return it to initial state, i.e. all the default settings,
>>> but set_config() will not be called after that.
>>> I know, that VFs commonly doesn't support flow control, but if we'll add like
>>> real configuration of MAC address, it will be lost too.
>>>
>>> What do you think?
>>
>> Doesn’t the full bridge run sequence take care of this?
>>
>> So in callback we do netdev_request_reconfigure() which triggers the following sequence…
>>
>> bridge_run__()
>>  ...
>>    dpif_netdev_run()
>>      ...
>>        reconfigure_datapath()
>>          ...
>>            netdev_dpdk_reconfigure()
>>
>> But than also it will call in the next run
>>
>> bridge_run()
>>  ...
>>    bridge_delete_or_reconfigure_ports()
>>      ...
>>         netdev_set_config()
>>           netdev_dpdk_set_config()
>>
>>
>> Or do I miss something?
> 
> dpif_netdev_run() is called from ofproto_type_run() which is called
> unconditionally from the bridge_run().
> But bridge_delete_or_reconfigure_ports() only called from bridge_reconfigure(),
> which is protected by the following condition:
> 
>     if (ovsdb_idl_get_seqno(idl) != idl_seqno ||
>         if_notifier_changed(ifnotifier)) {
> 
> i.e. if there will be no DB updates or there will be no netlink notifications,
> set_config() will not be called. I understand that in your scenario there
> might be netlink notification for interface changes since PF is controlled
> by the kernel, but it might be not the case if PF attached as a DPDK port
> to OVS or some other application.

Hmm. Basically, dpdk_eth_event_callback() is an analogue of the
if_notifier, but for DPDK application.

Maybe we can add it as another trigger for above 'if' condition?

> 
>>
>>>
>>> BTW, there is a discussion about flow control configuration:
>>> https://patchwork.ozlabs.org/patch/1159689/
>>>
>>>> +        dev->reset_needed = false;
>>>> +    } else {
>>>> +        rte_eth_dev_stop(dev->port_id);
>>>> +    }
>>>> +
>>>>      dev->started = false;
>>>>
>>>>      err = netdev_dpdk_mempool_configure(dev);
>>
>>
> 
> 


More information about the dev mailing list