> > Thanks for the new version, I'm still testing this. Thanks Daniele. I plan to do some more thorough testing myself.
> > One comment inline > > 2016-12-14 9:06 GMT-08:00 Ciara Loftus <[email protected]>: > > 'dpdk' ports no longer have naming restrictions. Now, instead of > > specifying the dpdk port ID as part of the name, the PCI address of the > > device must be specified via the 'dpdk-devargs' option. eg. > > > > ovs-vsctl add-port br0 my-port > > ovs-vsctl set Interface my-port type=dpdk > > ovs-vsctl set Interface my-port options:dpdk-devargs=0000:06:00.3 > > > > The user must no longer hotplug attach DPDK ports by issuing the > > specific ovs-appctl netdev-dpdk/attach command. The hotplug is now > > automatically invoked when a valid PCI address is set in the > > dpdk-devargs. The format for ovs-appctl netdev-dpdk/detach command > > has changed in that the user now must specify the relevant PCI address > > as input instead of the port name. > > > > Signed-off-by: Ciara Loftus <[email protected]> > > Signed-off-by: Daniele Di Proietto <[email protected]> > > Co-authored-by: Daniele Di Proietto <[email protected]> > > --- > > Changelog: > > * Keep port-detach appctl function - use PCI as input arg > > * Add requires_mutex to devargs processing functions > > * use reconfigure infrastructure for devargs changes > > * process devargs even if valid portid ie. device already configured > > * report err if dpdk-devargs is not specified > > * Add Daniele's incremental patch & Sign-off + Co-author tags > > * Update details of detach command to reflect PCI is needed instead of > > port name > > * Update NEWS to mention that the new naming scheme is not backwards > > compatible > > * Use existing DPDk functions to get port ID from PCI address/devname > > * Merged process_devargs and process_pdevargs functions > > > > Documentation/intro/install/dpdk-advanced.rst | 5 +- > > Documentation/intro/install/dpdk.rst | 14 ++- > > NEWS | 5 + > > lib/netdev-dpdk.c | 169 > > +++++++++++++++++--------- > > vswitchd/vswitch.xml | 8 ++ > > 5 files changed, 136 insertions(+), 65 deletions(-) > > > > diff --git a/Documentation/intro/install/dpdk-advanced.rst > b/Documentation/intro/install/dpdk-advanced.rst > > index 43acfc6..260ed59 100644 > > --- a/Documentation/intro/install/dpdk-advanced.rst > > +++ b/Documentation/intro/install/dpdk-advanced.rst > > @@ -944,9 +944,8 @@ dpdk_nic_bind.py script: > > > > Then it can be attached to OVS: > > > > - $ ovs-appctl netdev-dpdk/attach 0000:01:00.0 > > - > > -At this point, the user can create a ovs port using the add-port command. > > + $ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ > > + options:dpdk-devargs=0000:01:00.0 > > > > It is also possible to detach a port from ovs, the user has to remove the > > port using the del-port command, then it can be detached using: > > diff --git a/Documentation/intro/install/dpdk.rst > b/Documentation/intro/install/dpdk.rst > > index 7724c8a..40b5b67 100644 > > --- a/Documentation/intro/install/dpdk.rst > > +++ b/Documentation/intro/install/dpdk.rst > > @@ -245,12 +245,14 @@ Bridges should be created with a > ``datapath_type=netdev``:: > > > > $ ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev > > > > -Now you can add DPDK devices. OVS expects DPDK device names to start > with > > -``dpdk`` and end with a portid. ovs-vswitchd should print the number of > dpdk > > -devices found in the log file:: > > - > > - $ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk > > - $ ovs-vsctl add-port br0 dpdk1 -- set Interface dpdk1 type=dpdk > > +Now you can add dpdk devices. The PCI address of the device needs to be > > +set using the 'dpdk-devargs' option. DPDK will print the PCI addresses of > > +eligible devices found during initialisation. > > + > > + $ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ > > + options:dpdk-devargs=0000:06:00.0 > > + $ ovs-vsctl add-port br0 dpdk1 -- set Interface dpdk1 type=dpdk \ > > + options:dpdk-devargs=0000:06:00.1 > > > > After the DPDK ports get added to switch, a polling thread continuously > polls > > DPDK devices and consumes 100% of the core, as can be checked from > 'top' and > > diff --git a/NEWS b/NEWS > > index b596cf3..8c7f38e 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -42,6 +42,11 @@ Post-v2.6.0 > > which set the number of rx and tx descriptors to use for the given > > port. > > * Support for DPDK v16.11. > > * Port Hotplug is now supported. > > + * DPDK physical ports can now have arbitrary names. The PCI address > of > > + the device must be set using the 'dpdk-devargs' option. > > Compatibility > > + with the old dpdk<portid> naming scheme is broken, and as such a > > + device will not be available for use until a valid dpdk-devargs is > > + specified. > > - Fedora packaging: > > * A package upgrade does not automatically restart OVS service. > > - ovs-vswitchd/ovs-vsctl: > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > index 0d57227..7d74d2f 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -352,6 +352,9 @@ struct netdev_dpdk { > > /* Identifier used to distinguish vhost devices from each other. */ > > char vhost_id[PATH_MAX]; > > > > + /* Device arguments for dpdk ports */ > > + char *devargs; > > + > > /* In dpdk_list. */ > > struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); > > > > @@ -813,7 +816,7 @@ netdev_dpdk_init(struct netdev *netdev, unsigned > int port_no, > > /* If the 'sid' is negative, it means that the kernel fails > > * to obtain the pci numa info. In that situation, always > > * use 'SOCKET0'. */ > > - if (type == DPDK_DEV_ETH) { > > + if (type == DPDK_DEV_ETH && rte_eth_dev_is_valid_port(dev- > >port_id)) { > > sid = rte_eth_dev_socket_id(port_no); > > } else { > > sid = rte_lcore_to_socket_id(rte_get_master_lcore()); > > @@ -852,9 +855,11 @@ netdev_dpdk_init(struct netdev *netdev, unsigned > int port_no, > > /* Initialize the flow control to NULL */ > > memset(&dev->fc_conf, 0, sizeof dev->fc_conf); > > if (type == DPDK_DEV_ETH) { > > - err = dpdk_eth_dev_init(dev); > > - if (err) { > > - goto unlock; > > + if (rte_eth_dev_is_valid_port(dev->port_id)) { > > + err = dpdk_eth_dev_init(dev); > > + if (err) { > > + goto unlock; > > + } > > } > > dev->tx_q = netdev_dpdk_alloc_txq(netdev->n_txq); > > } else { > > @@ -950,17 +955,10 @@ netdev_dpdk_vhost_client_construct(struct > netdev *netdev) > > static int > > netdev_dpdk_construct(struct netdev *netdev) > > { > > - unsigned int port_no; > > int err; > > > > - /* Names always start with "dpdk" */ > > - err = dpdk_dev_parse_name(netdev->name, "dpdk", &port_no); > > - if (err) { > > - return err; > > - } > > - > > ovs_mutex_lock(&dpdk_mutex); > > - err = netdev_dpdk_init(netdev, port_no, DPDK_DEV_ETH); > > + err = netdev_dpdk_init(netdev, -1, DPDK_DEV_ETH); > > ovs_mutex_unlock(&dpdk_mutex); > > return err; > > } > > @@ -974,6 +972,7 @@ netdev_dpdk_destruct(struct netdev *netdev) > > ovs_mutex_lock(&dev->mutex); > > > > rte_eth_dev_stop(dev->port_id); > > + free(dev->devargs); > > free(ovsrcu_get_protected(struct ingress_policer *, > > &dev->ingress_policer)); > > > > @@ -1078,6 +1077,46 @@ netdev_dpdk_get_config(const struct netdev > *netdev, struct smap *args) > > return 0; > > } > > > > +static struct netdev_dpdk * > > +netdev_dpdk_lookup_by_port_id(int port_id) > > + OVS_REQUIRES(dpdk_mutex) > > +{ > > + struct netdev_dpdk *dev; > > + > > + LIST_FOR_EACH (dev, list_node, &dpdk_list) { > > + if (dev->port_id == port_id) { > > + return dev; > > + } > > + } > > + > > + return NULL; > > +} > > + > > +static int > > +netdev_dpdk_process_devargs(const char *devargs) > > + OVS_REQUIRES(dev->mutex) > > The above thread safety annotation is not necessary anymore. > > If you compile with clang you can easily catch all these errors. > > > +{ > > + struct rte_pci_addr addr; > > + uint8_t new_port_id = UINT8_MAX; > > + > > + if (!eal_parse_pci_DomBDF(devargs, &addr)) { > > + /* Valid PCI address format detected - configure physical device */ > > + if (rte_eth_dev_get_port_by_name(devargs, &new_port_id)) { > > I tried to detach a device and then add it again. > > It didn't work because rte_eth_dev_get_port_by_name() returns the old > port id even if it is detached. > > I have to add || !rte_eth_dev_is_valid_port(new_port_id) to the > condition to make it work Ok - thanks for both catches. I've sent a v3 with these fixes included. Thanks, Ciara > > > + /* PCI device not found in DPDK, attempt to attach it */ > > + if (!rte_eth_dev_attach(devargs, &new_port_id)) { > > + /* Attach successful */ > > + VLOG_INFO("Device "PCI_PRI_FMT" has been attached to DPDK", > > + addr.domain, addr.bus, addr.devid, > > addr.function); > > + } else { > > + /* Attach unsuccessful */ > > + return -1; > > + } > > + } > > + } > > + > > + return new_port_id; > > +} > > + > > static void > > dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args) > > OVS_REQUIRES(dev->mutex) > > @@ -1118,7 +1157,10 @@ netdev_dpdk_set_config(struct netdev *netdev, > const struct smap *args) > > {RTE_FC_NONE, RTE_FC_TX_PAUSE}, > > {RTE_FC_RX_PAUSE, RTE_FC_FULL } > > }; > > + const char *new_devargs; > > + int err = 0; > > > > + ovs_mutex_lock(&dpdk_mutex); > > ovs_mutex_lock(&dev->mutex); > > > > dpdk_set_rxq_config(dev, args); > > @@ -1130,6 +1172,45 @@ netdev_dpdk_set_config(struct netdev *netdev, > const struct smap *args) > > NIC_PORT_DEFAULT_TXQ_SIZE, > > &dev->requested_txq_size); > > > > + new_devargs = smap_get(args, "dpdk-devargs"); > > + > > + if (dev->devargs && strcmp(new_devargs, dev->devargs)) { > > + /* The user requested a new device. If we return error, the caller > > + * will delete this netdev and try to recreate it. */ > > + err = EAGAIN; > > + goto out; > > + } > > + > > + /* dpdk-devargs is required for device configuration */ > > + err = EINVAL; > > + if (new_devargs && new_devargs[0]) { > > + int new_port_id; > > + > > + new_port_id = netdev_dpdk_process_devargs(new_devargs); > > + if (new_port_id == dev->port_id) { > > + /* Already configured, do not reconfigure again */ > > + err = 0; > > + } else if (rte_eth_dev_is_valid_port(new_port_id)) { > > + struct netdev_dpdk *dup_dev; > > + dup_dev = netdev_dpdk_lookup_by_port_id(new_port_id); > > + if (dup_dev) { > > + VLOG_WARN("'%s' is trying to use device '%s' which is > > already " > > + "in use by '%s'.", netdev_get_name(netdev), > > + new_devargs, netdev_get_name(&dup_dev->up)); > > + err = EADDRINUSE; > > + } else { > > + dev->devargs = xstrdup(new_devargs); > > + dev->port_id = new_port_id; > > + netdev_request_reconfigure(&dev->up); > > + err = 0; > > + } > > + } > > + } > > + > > + if (err) { > > + goto out; > > + } > > + > > rx_fc_en = smap_get_bool(args, "rx-flow-ctrl", false); > > tx_fc_en = smap_get_bool(args, "tx-flow-ctrl", false); > > autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); > > @@ -1141,9 +1222,11 @@ netdev_dpdk_set_config(struct netdev *netdev, > const struct smap *args) > > dpdk_eth_flow_ctrl_setup(dev); > > } > > > > +out: > > ovs_mutex_unlock(&dev->mutex); > > + ovs_mutex_unlock(&dpdk_mutex); > > > > - return 0; > > + return err; > > } > > > > static int > > @@ -2308,57 +2391,34 @@ netdev_dpdk_set_admin_state(struct > unixctl_conn *conn, int argc, > > } > > > > static void > > -netdev_dpdk_attach(struct unixctl_conn *conn, int argc OVS_UNUSED, > > - const char *argv[], void *aux OVS_UNUSED) > > -{ > > - int ret; > > - char *response; > > - uint8_t port_id; > > - > > - ovs_mutex_lock(&dpdk_mutex); > > - > > - ret = rte_eth_dev_attach(argv[1], &port_id); > > - if (ret < 0) { > > - response = xasprintf("Error attaching device '%s'", argv[1]); > > - ovs_mutex_unlock(&dpdk_mutex); > > - unixctl_command_reply_error(conn, response); > > - free(response); > > - return; > > - } > > - > > - response = xasprintf("Device '%s' has been attached as 'dpdk%d'", > > - argv[1], port_id); > > - > > - ovs_mutex_unlock(&dpdk_mutex); > > - unixctl_command_reply(conn, response); > > - free(response); > > -} > > - > > -static void > > netdev_dpdk_detach(struct unixctl_conn *conn, int argc OVS_UNUSED, > > const char *argv[], void *aux OVS_UNUSED) > > { > > int ret; > > char *response; > > - unsigned int parsed_port; > > uint8_t port_id; > > char devname[RTE_ETH_NAME_MAX_LEN]; > > + struct rte_pci_addr addr; > > + struct netdev_dpdk *dev; > > > > ovs_mutex_lock(&dpdk_mutex); > > > > - ret = dpdk_dev_parse_name(argv[1], "dpdk", &parsed_port); > > - if (ret) { > > - response = xasprintf("'%s' is not a valid port", argv[1]); > > + if (eal_parse_pci_DomBDF(argv[1], &addr)) { > > + response = xasprintf("Invalid PCI address '%s'. Cannot detach.", > > + argv[1]); > > goto error; > > } > > > > - port_id = parsed_port; > > + if (rte_eth_dev_get_port_by_name(argv[1], &port_id)) { > > + response = xasprintf("Device '%s' not found in DPDK", argv[1]); > > + goto error; > > + } > > > > - struct netdev *netdev = netdev_from_name(argv[1]); > > - if (netdev) { > > - netdev_close(netdev); > > - response = xasprintf("Port '%s' is being used. Remove it before" > > - "detaching", argv[1]); > > + dev = netdev_dpdk_lookup_by_port_id(port_id); > > + if (dev) { > > + response = xasprintf("Device '%s' is being used by interface '%s'. > > " > > + "Remove it before detaching", > > + argv[1], netdev_get_name(&dev->up)); > > goto error; > > } > > > > @@ -2366,11 +2426,11 @@ netdev_dpdk_detach(struct unixctl_conn > *conn, int argc OVS_UNUSED, > > > > ret = rte_eth_dev_detach(port_id, devname); > > if (ret < 0) { > > - response = xasprintf("Port '%s' can not be detached", argv[1]); > > + response = xasprintf("Device '%s' can not be detached", argv[1]); > > goto error; > > } > > > > - response = xasprintf("Port '%s' has been detached", argv[1]); > > + response = xasprintf("Device '%s' has been detached", argv[1]); > > > > ovs_mutex_unlock(&dpdk_mutex); > > unixctl_command_reply(conn, response); > > @@ -2658,11 +2718,8 @@ netdev_dpdk_class_init(void) > > unixctl_command_register("netdev-dpdk/set-admin-state", > > "[netdev] up|down", 1, 2, > > netdev_dpdk_set_admin_state, NULL); > > - unixctl_command_register("netdev-dpdk/attach", > > - "pci address of device", 1, 1, > > - netdev_dpdk_attach, NULL); > > unixctl_command_register("netdev-dpdk/detach", > > - "port", 1, 1, > > + "pci address of device", 1, 1, > > netdev_dpdk_detach, NULL); > > > > ovsthread_once_done(&once); > > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > > index b4af5a5..23ab469 100644 > > --- a/vswitchd/vswitch.xml > > +++ b/vswitchd/vswitch.xml > > @@ -2303,6 +2303,14 @@ > > </p> > > </column> > > > > + <column name="options" key="dpdk-devargs" > > + type='{"type": "string"}'> > > + <p> > > + Specifies the PCI address of a physical dpdk device. > > + Only supported by 'dpdk' devices. > > + </p> > > + </column> > > + > > <column name="other_config" key="pmd-rxq-affinity"> > > <p>Specifies mapping of RX queues of this interface to CPU > > cores.</p> > > <p>Value should be set in the following form:</p> > > -- > > 2.4.3 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
