On 10.01.2019 16:42, Thomas Monjalon wrote:
> Hi,
>
> 10/01/2019 12:32, Ilya Maximets:
>> On 08.01.2019 12:31, Ophir Munk wrote:
>>> if (dev->attached) {
>>> + /* Remove the port eth device. */
>>> rte_eth_dev_close(dev->port_id);
>>> - rte_eth_dev_info_get(dev->port_id, &dev_info);
>>> - if (dev_info.device && !rte_dev_remove(dev_info.device)) {
>>> - VLOG_INFO("Device '%s' has been detached", dev->devargs);
>>
>> Please keep this log message for the successful case.
>>
>>> - } else {
>>> - VLOG_ERR("Device '%s' can not be detached", dev->devargs);
>>> + VLOG_INFO("Device '%s' has been removed", dev->devargs);
>>> + /* If this is the last port_id using this rte device
>>> + * remove this rte device and all its eth devices.
>>> + * FIXME: avoid direct access to DPDK internal array
>>> rte_eth_devices.
>>> + */
>>> + rte_dev = rte_eth_devices[dev->port_id].device;
>>> + if (netdev_dpdk_get_num_ports(rte_dev) == 1) {
>>
>> rte_eth_dev_close() sets the RTE_ETH_DEV_UNUSED state for PMDs with
>> RTE_ETH_DEV_CLOSE_REMOVE flag.
>> The function netdev_dpdk_get_num_ports() skips unused ports.
>> The result should not be equal to 1. We need the other way to detect
>> the siblings or close the port after calculating them.
>> In this case we'll probably do not need to have check for UNUSED state
>> inside the _get_num_ports().
>
> Maybe I don't understand correctly.
> Why do you want to count closed sibling ports?
Proposed version of 'netdev_dpdk_get_num_ports' iterates over
'dpdk_list' which contains only ports that currently opened by OVS.
So, the only port that could have UNUSED state in that list is the
port that we're just closed here. If we'll count number of siblings
before closing the port where should be no UNUSED ports in that list.
The issue I tried to raise here is that rte_eth_dev_close() changed the
state only for PMDs with RTE_ETH_DEV_CLOSE_REMOVE. So, the result
of 'netdev_dpdk_get_num_ports' will be different for different PMDs.
>
>>> + if (rte_dev_remove(rte_dev) < 0) {
>>> + VLOG_ERR("Device '%s' can not be detached", dev->devargs);
>>> + }
>>> }
>>> }
>
> [...]
>> Also, do we still need netdev_dpdk_get_port_by_mac() if we're using
>> devargs iterator ?
>
> The devargs iterator manages "mac=" property in DPDK 18.11.
Good.
>
> [...]
>>> - rte_eth_dev_close(port_id);
>>> + /* FIXME: avoid direct access to DPDK internal array rte_eth_devices.
>>> */
>>> + rte_dev = rte_eth_devices[port_id].device;
>>> + if (netdev_dpdk_get_num_ports(rte_dev)) {
>>
>> Probably same here.
>> DPDK does not set RTE_ETH_DEV_UNUSED state for PMDs without
>> RTE_ETH_DEV_CLOSE_REMOVE flag.
>
> If RTE_ETH_DEV_CLOSE_REMOVE is not set, you should call rte_dev_remove()
> just after rte_eth_dev_close().
And what if we call rte_dev_remove() after rte_eth_dev_close() for
PMD with RTE_ETH_DEV_CLOSE_REMOVE ?
>
>>> + response = xasprintf("Device '%s' is being shared with other "
>>> + "interfaces. Remove them before detaching.",
>>> + argv[1]);
>>> + goto error;
>>> + }
>>>
>>> - rte_eth_dev_info_get(port_id, &dev_info);
>>> - if (!dev_info.device || rte_dev_remove(dev_info.device)) {
>>> - response = xasprintf("Device '%s' can not be detached", argv[1]);
>>> + rte_eth_dev_close(port_id);
>>> + if (rte_dev_remove(rte_dev) < 0) {
>>> + response = xasprintf("Device '%s' can not be removed", argv[1]);
>
>
>
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
