This is not a full review, just s suggestion. See inline. Best regards, Ilya Maximets.
On 09.01.2018 12:54, Yuanhan Liu wrote: > 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 <[email protected]> > Cc: Thomas Monjalon <[email protected]> > Cc: Kevin Traynor <[email protected]> > Signed-off-by: Yuanhan Liu <[email protected]> > --- > > v2: - added doc for the new devags syntax > - added error log for wrong mac format > --- > Documentation/howto/dpdk.rst | 13 +++++++ > lib/netdev-dpdk.c | 91 > ++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 89 insertions(+), 15 deletions(-) > > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst > index 2393c2f..4167f04 100644 > --- a/Documentation/howto/dpdk.rst > +++ b/Documentation/howto/dpdk.rst > @@ -48,6 +48,19 @@ 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, for the parsing is currently done > +in OVS-DPDK. DPDK v18.05 is supposed to support this new syntax, therefore, > +hotplug should work. > + > 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..95dbabd 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1202,30 +1202,91 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id) > return NULL; > } > > +static int > +netdev_dpdk_str_to_ether(const char *mac, struct ether_addr *ea) > +{ > + unsigned int bytes[6]; > + int i; > + > + if (sscanf(mac, "%x:%x:%x:%x:%x:%x", > + &bytes[0], &bytes[1], &bytes[2], > + &bytes[3], &bytes[4], &bytes[5]) != 6) { > + VLOG_ERR("invalid mac: %s", mac); > + return -1; > + } > + > + for (i = 0; i < 6; i++) { > + ea->addr_bytes[i] = bytes[i]; > + } > + > + return 0; > +} > + > +static dpdk_port_t > +netdev_dpdk_get_port_by_mac(const char *mac_str) > +{ > + int i; > + struct ether_addr mac; > + struct ether_addr port_mac; > + > + if (netdev_dpdk_str_to_ether(mac_str, &mac) != 0) { > + return DPDK_ETH_PORT_ID_INVALID; > + } > + > + for (i = 0; i < RTE_MAX_ETHPORTS; i++) { > + if (!rte_eth_dev_is_valid_port(i)) { > + continue; > + } > + > + rte_eth_macaddr_get(i, &port_mac); > + if (is_same_ether_addr(&mac, &port_mac)) { > + return i; > + } > + } > + > + return DPDK_ETH_PORT_ID_INVALID; > +} How about reusing of existing OVS functions and RTE_ETH_FOREACH_DEV for iteration ? Like this (not even compile tested): static dpdk_port_t netdev_dpdk_get_port_by_mac(const char *mac_str) { dpdk_port_t pid; struct eth_addr mac, port_mac; struct ether_addr dpdk_port_mac; if (!eth_addr_from_string(mac_str, &mac)) { return DPDK_ETH_PORT_ID_INVALID; } RTE_ETH_FOREACH_DEV (pid) { rte_eth_macaddr_get(pid, &dpdk_port_mac); memcpy(port_mac.ea, dpdk_port_mac.addr_bytes, ETH_ADDR_LEN); if (!eth_addr_equals(mac, port_mac)) { return pid; } } 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; > } > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
