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

[Loftus, Ciara]

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

Thanks for your review Ian.

> 
> > 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.

It should be easy to reuse the process_devargs_index function for this case. I'll include this in a v2.

Thanks,
Ciara

> 
> 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