[ovs-dev] [PATCH v5 2/3] netdev-dpdk: Arbitrary 'dpdk' port naming
Daniele Di Proietto
diproiettod at ovn.org
Fri Jan 6 04:15:05 UTC 2017
If new_port_id == -1 I think it's better to return failure from
set_config(), rather than return failure from a later reconfigure(),
so I squashed the following incremental:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e66ff2711..038f79b80 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1231,12 +1231,13 @@ netdev_dpdk_set_config(struct netdev *netdev,
const struct smap *args)
* is valid */
if (!(dev->devargs && !strcmp(dev->devargs, new_devargs)
&& rte_eth_dev_is_valid_port(dev->port_id))) {
- err = EINVAL;
int new_port_id = netdev_dpdk_process_devargs(new_devargs);
- if (new_port_id == dev->port_id) {
+ if (!rte_eth_dev_is_valid_port(new_port_id)) {
+ err = EINVAL;
+ } else 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)) {
+ } else {
struct netdev_dpdk *dup_dev;
dup_dev = netdev_dpdk_lookup_by_port_id(new_port_id);
if (dup_dev) {
and applied to master, thanks!
2017-01-05 2:42 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
> 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>
> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
> Co-authored-by: Kevin Traynor <ktraynor at redhat.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
> * Removed unnecessary requires_mutex from devargs processing fn
> * Fix case where port is re-attached after detach
> * Add note to documentation that devices won't work until devargs set.
> * Set netdev type and dpdk-devargs in one command in docs to avoid
> errors.
> * Change port names in documentation to emphasise arbitrary-ness.
> * Add Kevin's incremental & sign-off/co-authored-by
> * Check if devargs string has changed before processing it as suggested
> by Daniele.
> * Print error if attach fails
>
> Documentation/howto/dpdk.rst | 7 +-
> Documentation/intro/install/dpdk.rst | 9 +-
> NEWS | 5 +
> lib/netdev-dpdk.c | 181 ++++++++++++++++++++++++-----------
> vswitchd/vswitch.xml | 8 ++
> 5 files changed, 148 insertions(+), 62 deletions(-)
>
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index 13da548..1ff672c 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -312,14 +312,13 @@ In order to attach a port, it has to be bound to DPDK using the
>
> Then it can be attached to OVS::
>
> - $ ovs-appctl netdev-dpdk/attach 0000:01:00.0
> -
> -At this point, the user can create a dpdk port using the ``add-port`` command.
> + $ ovs-vsctl add-port br0 dpdkx -- set Interface dpdkx 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::
>
> - $ ovs-appctl netdev-dpdk/detach dpdk0
> + $ ovs-appctl netdev-dpdk/detach dpdkx
>
> This feature is not supported with VFIO and does not work with some NICs.
> For more information please refer to the `DPDK Port Hotplug Framework
> diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
> index 10ceabb..fff0a1a 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -252,8 +252,13 @@ ports. For example, to create a userspace bridge named ``br0`` and add two
> ``dpdk`` ports to it, run::
>
> $ ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev
> - $ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
> - $ ovs-vsctl add-port br0 dpdk1 -- set Interface dpdk1 type=dpdk
> + $ ovs-vsctl add-port br0 myportnameone -- set Interface myportnameone \
> + type=dpdk options:dpdk-devargs=0000:06:00.0
> + $ ovs-vsctl add-port br0 myportnametwo -- set Interface myportnametwo \
> + type=dpdk options:dpdk-devargs=0000:06:00.1
> +
> +DPDK devices will not be available for use until a valid dpdk-devargs is
> +specified.
>
> Refer to ovs-vsctl(8) and :doc:`/howto/dpdk` for more details.
>
> diff --git a/NEWS b/NEWS
> index 8f8c2f7..d66d402 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -48,6 +48,11 @@ Post-v2.6.0
> * Support for DPDK v16.11.
> * Support for rx checksum offload. Refer DPDK HOWTO for details.
> * 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 b716b57..e66ff27 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -356,6 +356,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);
>
> @@ -846,7 +849,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());
> @@ -888,9 +891,11 @@ netdev_dpdk_init(struct netdev *netdev, unsigned int port_no,
> /* Initilize the hardware offload flags to 0 */
> dev->hw_ol_features = 0;
> 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 {
> @@ -986,17 +991,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;
> }
> @@ -1010,6 +1008,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));
>
> @@ -1117,6 +1116,49 @@ 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)
> +{
> + 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_count()
> + || rte_eth_dev_get_port_by_name(devargs, &new_port_id)
> + || !rte_eth_dev_is_valid_port(new_port_id)) {
> + /* 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 */
> + VLOG_INFO("Error attaching device "PCI_PRI_FMT" to DPDK",
> + addr.domain, addr.bus, addr.devid, addr.function);
> + return -1;
> + }
> + }
> + }
> +
> + return new_port_id;
> +}
> +
> static void
> dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
> OVS_REQUIRES(dev->mutex)
> @@ -1159,7 +1201,10 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args)
> };
> bool rx_chksm_ofld;
> bool temp_flag;
> + const char *new_devargs;
> + int err = 0;
>
> + ovs_mutex_lock(&dpdk_mutex);
> ovs_mutex_lock(&dev->mutex);
>
> dpdk_set_rxq_config(dev, args);
> @@ -1171,6 +1216,52 @@ 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 */
> + if (new_devargs && new_devargs[0]) {
> + /* Don't process dpdk-devargs if value is unchanged and port id
> + * is valid */
> + if (!(dev->devargs && !strcmp(dev->devargs, new_devargs)
> + && rte_eth_dev_is_valid_port(dev->port_id))) {
> + err = EINVAL;
> + int 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;
> + }
> + }
> + }
> + } else {
> + /* dpdk-devargs unspecified - can't configure device */
> + err = EINVAL;
> + }
> +
> + 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);
> @@ -1191,9 +1282,12 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args)
> dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
> dpdk_eth_checksum_offload_configure(dev);
> }
> +
> +out:
> ovs_mutex_unlock(&dev->mutex);
> + ovs_mutex_unlock(&dpdk_mutex);
>
> - return 0;
> + return err;
> }
>
> static int
> @@ -2358,57 +2452,35 @@ 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_count() || 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;
> }
>
> @@ -2416,11 +2488,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);
> @@ -2708,11 +2780,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 e73f184..c34c295 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