[ovs-dev] [PATCH v2 2/3] netdev-dpdk: Arbitrary 'dpdk' port naming
Daniele Di Proietto
diproiettod at ovn.org
Thu Dec 15 02:32:05 UTC 2016
Thanks for the new version, I'm still testing this.
One comment inline
2016-12-14 9:06 GMT-08:00 Ciara Loftus <ciara.loftus at intel.com>:
> '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 <ciara.loftus at intel.com>
> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
> Co-authored-by: Daniele Di Proietto <diproiettod at vmware.com>
> ---
> 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
> + /* 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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list