[ovs-dev] [PATCH RFC] netdev-dpdk: Allow specification of index for PCI devices

[Stokes, Ian]

Hi Ciara, thanks for working on this patch. A few comments inline.

> Some NICs have only one PCI address associated with multiple ports. This
> patch extends the dpdk-devargs option's format to cater for such devices.
> Whereas before only one of N ports associated with one PCI address could
> be added, now N can.
> 
> This patch allows for the following use of the dpdk-devargs option:
> 
> ovs-vsctl set Interface myport options:dpdk-devargs=0000:06:00.0,X
> 
> Where X is an unsigned integer representing one of multiple ports
> associated with the PCI address provided.
> 
> This patch has not been tested.

Have you considered the hotplug usecase?

I think attaching in this case will be ok but OVS provides a mechanism to detach via appctl as well using the pci address (see command below.

$ ovs-appctl netdev-dpdk/detach 0000:01:00.0

I think this will have to be modified also to take into account the index of the port otherwise we hit the same situation again.

It's probably something that will need to be tested with this patch going forward.

Ian
> 
> Signed-off-by: Ciara Loftus <ciara.loftus at intel.com>
> ---
>  lib/netdev-dpdk.c | 79 ++++++++++++++++++++++++++++++++++++++++++++------
> -----
>  1 file changed, 63 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index c60f46f..c3562a3
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1214,30 +1214,77 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id)
> }
> 
>  static dpdk_port_t
> +process_devargs_index(char *name, int idx) {
> +    int i = 0;
> +    struct rte_eth_dev_info dev_info;
> +    char pci_addr[PCI_PRI_STR_SIZE];
> +    dpdk_port_t offset_port_id = DPDK_ETH_PORT_ID_INVALID;
> +
> +    for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
> +        if (!rte_eth_dev_is_valid_port(i)) {
> +            continue;
> +        }
> +        rte_eth_dev_info_get(i, &dev_info);
> +        rte_pci_device_name(&dev_info.pci_dev->addr, pci_addr,
> +                            sizeof(pci_addr));
> +        if (!strcmp(pci_addr, name)) {
> +            offset_port_id = i + idx;
> +            if (rte_eth_dev_is_valid_port(offset_port_id)) {
> +                rte_eth_dev_info_get(offset_port_id, &dev_info);
> +                rte_pci_device_name(&dev_info.pci_dev->addr, pci_addr,
> +                                    sizeof(pci_addr));
> +                if (!strcmp(pci_addr, name)) {
> +                    return offset_port_id;
> +                } else {
> +                    break;
> +                }
> +            } else {
> +                break;
> +            }
> +        }
> +    }
> +
> +    VLOG_INFO("Invalid PCI offset %i for device %s", idx, name);
> +    return DPDK_ETH_PORT_ID_INVALID;
> +}
> +
> +static dpdk_port_t
>  netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
>                              const char *devargs, char **errp)  {
> -    /* Get the name up to the first comma. */
> -    char *name = xmemdup0(devargs, strcspn(devargs, ","));
> +    char *devargs_copy = xmemdup0((devargs), strlen(devargs));
> +    char *name, *idx_str;
> +    unsigned idx;
>      dpdk_port_t new_port_id = DPDK_ETH_PORT_ID_INVALID;
> 
> -    if (!rte_eth_dev_count()
> -            || rte_eth_dev_get_port_by_name(name, &new_port_id)
> -            || !rte_eth_dev_is_valid_port(new_port_id)) {
> -        /* Device not found in DPDK, attempt to attach it */
> -        if (!rte_eth_dev_attach(devargs, &new_port_id)) {
> -            /* Attach successful */
> -            dev->attached = true;
> -            VLOG_INFO("Device '%s' attached to DPDK", devargs);
> -        } else {
> -            /* Attach unsuccessful */
> -            new_port_id = DPDK_ETH_PORT_ID_INVALID;
> -            VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK",
> -                          devargs);
> +    name = strtok_r(devargs_copy, ",", &devargs_copy);
> +    idx_str = strtok_r(devargs_copy, ",", &devargs_copy);
> +
> +    if (idx_str) {
> +        idx = atoi(idx_str);
> +        new_port_id = process_devargs_index(name, idx);
> +    } else {
> +        if (!rte_eth_dev_count()
> +                || rte_eth_dev_get_port_by_name(name, &new_port_id)
> +                || !rte_eth_dev_is_valid_port(new_port_id)) {
> +            /* Device not found in DPDK, attempt to attach it */
> +            if (!rte_eth_dev_attach(devargs, &new_port_id)) {
> +                /* Attach successful */
> +                dev->attached = true;
> +                VLOG_INFO("Device '%s' attached to DPDK", devargs);
> +            } else {
> +                /* Attach unsuccessful */
> +                new_port_id = DPDK_ETH_PORT_ID_INVALID;
> +            }
>          }
>      }
> 
> -    free(name);
> +    if (new_port_id == DPDK_ETH_PORT_ID_INVALID) {
> +        VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK",
> +                      devargs);
> +    }
> +
>      return new_port_id;
>  }
> 
> --
> 2.7.5
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev