| Message ID | 1515553529-5857-1-git-send-email-yliu@fridaylinux.org |
|---|---|
| State | Accepted |
| Delegated to: | Ian Stokes |
| Headers | show |
| Series | [ovs-dev,v3] netdev-dpdk: fix port addition for ports sharing same PCI id | expand |
> -----Original Message----- > From: Yuanhan Liu [mailto:yliu@fridaylinux.org] > Sent: Wednesday, January 10, 2018 3:05 AM > To: dev@openvswitch.org > Cc: Stokes, Ian <ian.stokes@intel.com>; Shahaf Shuler > <shahafs@mellanox.com>; Yuanhan Liu <yliu@fridaylinux.org>; Loftus, Ciara > <ciara.loftus@intel.com>; Thomas Monjalon <thomas@monjalon.net>; Kevin > Traynor <ktraynor@redhat.com> > Subject: [PATCH v3] netdev-dpdk: fix port addition for ports sharing same > PCI id > > 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. > > To achieve that, this patch uses a new syntax that will be adapted and > implemented in future DPDK release (likely, v18.05): > http://dpdk.org/ml/archives/dev/2017-December/084234.html > > And since it's the DPDK duty to parse the (complete and full) syntax and > this patch is more likely to serve as an intermediate workaround, here I > take a simpler and shorter syntax from it (note it's allowed to have only > one category being provided): > class=eth,mac=00:11:22:33:44:55:66 > > Also, old compatibility is kept. Users can still go on with using the PCI > id to add a port (if that's enough for them). Meaning, this patch will not > break anything. > > This patch is basically based on the one from Ciara: > https://mail.openvswitch.org/pipermail/ovs-dev/2017- > October/339496.html > > Cc: Loftus Ciara <ciara.loftus@intel.com> > Cc: Thomas Monjalon <thomas@monjalon.net> > Cc: Kevin Traynor <ktraynor@redhat.com> > Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org> Thanks for the work on this Yuanhan, it's a valuable feature/bug fix. At this point I think this aligns to the DPDK model? If so, and if there are no other comments on this patchset then I plan to merge this to DPDK merge over the weekend. Thanks Ian > --- > > v3: - reuse eth helper functions from OVS, suggested by Ilya Maximets > > v2: - added doc for the new devags syntax > - added error log for wrong mac format > --- > Documentation/howto/dpdk.rst | 12 ++++++++ > lib/netdev-dpdk.c | 70 ++++++++++++++++++++++++++++++++++----- > ----- > 2 files changed, 67 insertions(+), 15 deletions(-) > > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst > index 2393c2f..e7cb7dd 100644 > --- a/Documentation/howto/dpdk.rst > +++ b/Documentation/howto/dpdk.rst > @@ -48,6 +48,18 @@ number of dpdk devices found in the log file:: > $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \ > options:dpdk-devargs=0000:01:00.1 > > +Some NICs (i.e. Mellanox ConnectX-3) have only one PCI address > +associated with multiple ports. Using a PCI device like above won't > +work. Instead, below usage is suggested:: > + > + $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \ > + options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01" > + $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \ > + options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02" > + > +Note: such syntax won't support hotplug. The hotplug is supposed to > +work with future DPDK release, v18.05. > + > 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 ``ps`` commands:: > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 364f545..c4f0f5e > 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1203,29 +1203,69 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id) > } > > static dpdk_port_t > +netdev_dpdk_get_port_by_mac(const char *mac_str) { > + dpdk_port_t port_id; > + struct eth_addr mac, port_mac; > + > + if (!eth_addr_from_string(mac_str, &mac)) { > + VLOG_ERR("invalid mac: %s", mac_str); > + return DPDK_ETH_PORT_ID_INVALID; > + } > + > + RTE_ETH_FOREACH_DEV (port_id) { > + struct ether_addr ea; > + > + rte_eth_macaddr_get(port_id, &ea); > + memcpy(port_mac.ea, ea.addr_bytes, ETH_ADDR_LEN); > + if (eth_addr_equals(mac, port_mac)) { > + return port_id; > + } > + } > + > + return DPDK_ETH_PORT_ID_INVALID; > +} > + > +/* > + * Normally, a PCI id is enough for identifying a specific DPDK port. > + * However, for some NICs having multiple ports sharing the same PCI > + * id, using PCI id won't work then. > + * > + * To fix that, here one more method is introduced: "class=eth,mac=$MAC". > + * > + * Note that the compatibility is fully kept: user can still use the > + * PCI id for adding ports (when it's enough for them). > + */ > +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 *name; > dpdk_port_t new_port_id = DPDK_ETH_PORT_ID_INVALID; > > - if (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); > + if (strncmp(devargs, "class=eth,mac=", 14) == 0) { > + new_port_id = netdev_dpdk_get_port_by_mac(&devargs[14]); > + } else { > + name = xmemdup0(devargs, strcspn(devargs, ",")); > + if (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); > } > > - free(name); > return new_port_id; > } > > -- > 2.7.4
On Fri, Jan 19, 2018 at 11:03:52AM +0000, Stokes, Ian wrote: > > -----Original Message----- > > From: Yuanhan Liu [mailto:yliu@fridaylinux.org] > > Sent: Wednesday, January 10, 2018 3:05 AM > > To: dev@openvswitch.org > > Cc: Stokes, Ian <ian.stokes@intel.com>; Shahaf Shuler > > <shahafs@mellanox.com>; Yuanhan Liu <yliu@fridaylinux.org>; Loftus, Ciara > > <ciara.loftus@intel.com>; Thomas Monjalon <thomas@monjalon.net>; Kevin > > Traynor <ktraynor@redhat.com> > > Subject: [PATCH v3] netdev-dpdk: fix port addition for ports sharing same > > PCI id > > > > 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. > > > > To achieve that, this patch uses a new syntax that will be adapted and > > implemented in future DPDK release (likely, v18.05): > > http://dpdk.org/ml/archives/dev/2017-December/084234.html > > > > And since it's the DPDK duty to parse the (complete and full) syntax and > > this patch is more likely to serve as an intermediate workaround, here I > > take a simpler and shorter syntax from it (note it's allowed to have only > > one category being provided): > > class=eth,mac=00:11:22:33:44:55:66 > > > > Also, old compatibility is kept. Users can still go on with using the PCI > > id to add a port (if that's enough for them). Meaning, this patch will not > > break anything. > > > > This patch is basically based on the one from Ciara: > > https://mail.openvswitch.org/pipermail/ovs-dev/2017- > > October/339496.html > > > > Cc: Loftus Ciara <ciara.loftus@intel.com> > > Cc: Thomas Monjalon <thomas@monjalon.net> > > Cc: Kevin Traynor <ktraynor@redhat.com> > > Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org> > > Thanks for the work on this Yuanhan, it's a valuable feature/bug fix. > > At this point I think this aligns to the DPDK model? Yes, I think so. > > If so, and if there are no other comments on this patchset then I plan to merge this to DPDK merge over the weekend. > Thank you for taking care of it. --yliu
> -----Original Message----- > From: Yuanhan Liu [mailto:yliu@fridaylinux.org] > Sent: Friday, January 19, 2018 2:48 PM > To: Stokes, Ian <ian.stokes@intel.com> > Cc: dev@openvswitch.org; Shahaf Shuler <shahafs@mellanox.com>; Loftus, > Ciara <ciara.loftus@intel.com>; Thomas Monjalon <thomas@monjalon.net>; > Kevin Traynor <ktraynor@redhat.com>; Ilya Maximets > (i.maximets@samsung.com) <i.maximets@samsung.com>; Olga Shern > <olgas@mellanox.com> > Subject: Re: [PATCH v3] netdev-dpdk: fix port addition for ports sharing > same PCI id > > On Fri, Jan 19, 2018 at 11:03:52AM +0000, Stokes, Ian wrote: > > > -----Original Message----- > > > From: Yuanhan Liu [mailto:yliu@fridaylinux.org] > > > Sent: Wednesday, January 10, 2018 3:05 AM > > > To: dev@openvswitch.org > > > Cc: Stokes, Ian <ian.stokes@intel.com>; Shahaf Shuler > > > <shahafs@mellanox.com>; Yuanhan Liu <yliu@fridaylinux.org>; Loftus, > > > Ciara <ciara.loftus@intel.com>; Thomas Monjalon > > > <thomas@monjalon.net>; Kevin Traynor <ktraynor@redhat.com> > > > Subject: [PATCH v3] netdev-dpdk: fix port addition for ports sharing > > > same PCI id > > > > > > 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. > > > > > > To achieve that, this patch uses a new syntax that will be adapted > > > and implemented in future DPDK release (likely, v18.05): > > > http://dpdk.org/ml/archives/dev/2017-December/084234.html > > > > > > And since it's the DPDK duty to parse the (complete and full) syntax > > > and this patch is more likely to serve as an intermediate > > > workaround, here I take a simpler and shorter syntax from it (note > > > it's allowed to have only one category being provided): > > > class=eth,mac=00:11:22:33:44:55:66 > > > > > > Also, old compatibility is kept. Users can still go on with using > > > the PCI id to add a port (if that's enough for them). Meaning, this > > > patch will not break anything. > > > > > > This patch is basically based on the one from Ciara: > > > https://mail.openvswitch.org/pipermail/ovs-dev/2017- > > > October/339496.html > > > > > > Cc: Loftus Ciara <ciara.loftus@intel.com> > > > Cc: Thomas Monjalon <thomas@monjalon.net> > > > Cc: Kevin Traynor <ktraynor@redhat.com> > > > Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org> > > > > Thanks for the work on this Yuanhan, it's a valuable feature/bug fix. > > > > At this point I think this aligns to the DPDK model? > > Yes, I think so. > > > > > If so, and if there are no other comments on this patchset then I plan > to merge this to DPDK merge over the weekend. > > > > Thank you for taking care of it. Thanks for all the work on this, I've pushed this to DPDK_MERGE and will also push it to DPDK_MERGE_2_9 for it to be part of the 2.9 release. Ian > > --yliu
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst index 2393c2f..e7cb7dd 100644 --- a/Documentation/howto/dpdk.rst +++ b/Documentation/howto/dpdk.rst @@ -48,6 +48,18 @@ number of dpdk devices found in the log file:: $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \ options:dpdk-devargs=0000:01:00.1 +Some NICs (i.e. Mellanox ConnectX-3) have only one PCI address associated +with multiple ports. Using a PCI device like above won't work. Instead, below +usage is suggested:: + + $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \ + options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01" + $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \ + options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02" + +Note: such syntax won't support hotplug. The hotplug is supposed to work with +future DPDK release, v18.05. + 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 ``ps`` commands:: diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 364f545..c4f0f5e 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -1203,29 +1203,69 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id) } static dpdk_port_t +netdev_dpdk_get_port_by_mac(const char *mac_str) +{ + dpdk_port_t port_id; + struct eth_addr mac, port_mac; + + if (!eth_addr_from_string(mac_str, &mac)) { + VLOG_ERR("invalid mac: %s", mac_str); + return DPDK_ETH_PORT_ID_INVALID; + } + + RTE_ETH_FOREACH_DEV (port_id) { + struct ether_addr ea; + + rte_eth_macaddr_get(port_id, &ea); + memcpy(port_mac.ea, ea.addr_bytes, ETH_ADDR_LEN); + if (eth_addr_equals(mac, port_mac)) { + return port_id; + } + } + + return DPDK_ETH_PORT_ID_INVALID; +} + +/* + * Normally, a PCI id is enough for identifying a specific DPDK port. + * However, for some NICs having multiple ports sharing the same PCI + * id, using PCI id won't work then. + * + * To fix that, here one more method is introduced: "class=eth,mac=$MAC". + * + * Note that the compatibility is fully kept: user can still use the + * PCI id for adding ports (when it's enough for them). + */ +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 *name; dpdk_port_t new_port_id = DPDK_ETH_PORT_ID_INVALID; - if (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); + if (strncmp(devargs, "class=eth,mac=", 14) == 0) { + new_port_id = netdev_dpdk_get_port_by_mac(&devargs[14]); + } else { + name = xmemdup0(devargs, strcspn(devargs, ",")); + if (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); } - free(name); return new_port_id; }
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. To achieve that, this patch uses a new syntax that will be adapted and implemented in future DPDK release (likely, v18.05): http://dpdk.org/ml/archives/dev/2017-December/084234.html And since it's the DPDK duty to parse the (complete and full) syntax and this patch is more likely to serve as an intermediate workaround, here I take a simpler and shorter syntax from it (note it's allowed to have only one category being provided): class=eth,mac=00:11:22:33:44:55:66 Also, old compatibility is kept. Users can still go on with using the PCI id to add a port (if that's enough for them). Meaning, this patch will not break anything. This patch is basically based on the one from Ciara: https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/339496.html Cc: Loftus Ciara <ciara.loftus@intel.com> Cc: Thomas Monjalon <thomas@monjalon.net> Cc: Kevin Traynor <ktraynor@redhat.com> Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org> --- v3: - reuse eth helper functions from OVS, suggested by Ilya Maximets v2: - added doc for the new devags syntax - added error log for wrong mac format --- Documentation/howto/dpdk.rst | 12 ++++++++ lib/netdev-dpdk.c | 70 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 67 insertions(+), 15 deletions(-)