Thank you Billy for the review. Please find below my reply. Regards _Sugesh
> -----Original Message----- > From: O Mahony, Billy > Sent: Wednesday, December 6, 2017 5:31 PM > To: Chandran, Sugesh <sugesh.chand...@intel.com>; d...@openvswitch.org; > b...@ovn.org > Subject: RE: [ovs-dev] [PATCH 2/2] Adding configuration option to whitelist > DPDK physical ports. > > Hi Sugesh, > > This is definitely a very useful feature. I'm looking forward to running trex > on the > same DUT as my ovs-dpdk. > > However I'd suggest adding an sscanf or some such to verify that the domain is > also specified for each whitelist member. And either add the default of > '0000' or > complain loudly if the domain is absent. [Sugesh] Will throw an error in that case then . > > Currently (without this patch) you must specify the domain when adding ports: > Vsctl add-port ... options:dpdk-devargs=0000:05:00.0 Or else an error such > as > 'Cannot find unplugged device (05:00.0)' is reported. > > And with the patch if you include the domain in the other_config (e.g. > other_config:dpdk-whitelist-pci-ids="0000:05:00.0") everything works just as > before. > > However with the patch if you add the whitelist *without* a domain e.g. > ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-whitelist-pci- > ids="05:00.0" > > There is no immediate error. However later when doing add-port if you include > the domain (current required practice) you will get an error. If you omit the > domain all is well. [Sugesh] It looks to me, the dpdk-devargs need the PCI id with the '0000'. But to bind and PCI scan its not necessary. So to keep it consistent, I would add check for PCI-ID in whitelist config too, and throw error incase pci-id are mentioned wrong(means without '0000'. Does it looks OK to you? > > It's a little bit strange as regardless of domain or no domain in the > other_config > the PCI probe always reports the NIC as expected: > 2017-12-06T16:55:27Z|00013|dpdk|INFO|EAL: PCI device 0000:05:00.0 on > NUMA socket -1 > 2017-12-06T16:55:27Z|00014|dpdk|WARN|EAL: Invalid NUMA socket, > default to 0 > 2017-12-06T16:55:27Z|00015|dpdk|INFO|EAL: probe driver: 8086:1572 > net_i40e > > I'll be using the other patch in this series "isolate rte-mempool allocation" > over > the next few days so I'll review that in due course. > > Thanks, > Billy. > > > -----Original Message----- > > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > > boun...@openvswitch.org] On Behalf Of Sugesh Chandran > > Sent: Friday, November 10, 2017 1:29 AM > > To: d...@openvswitch.org; b...@ovn.org > > Subject: [ovs-dev] [PATCH 2/2] Adding configuration option to > > whitelist DPDK physical ports. > > > > Adding a OVS configuration option to whitelist DPDK physical ports. By > > default running multiple instances of DPDK on a single platform cannot > > use physical ports at the same time even though they are distinct. > > > > The eal init scans all the ports that are bound to DPDK and initialize > > the drivers accordingly. This happens for every DPDK process init. > > On a multi instance deployment usecase, it causes issues for using > > physical NIC ports. > > Consider a two DPDK process that are running on a single platform, the > > second DPDK primary process will try to initialize the drivers for all > > the physical ports even though it may be used in first DPDK process. > > > > To avoid this situation user can whitelist the ports for each DPDK > > application. > > Whitelisting of ports/PCI-ID in a DPDK process will limit the eal-init > > only on those ports. > > > > To whitelist two physical ports "0000:06:00.0" and "0000:06:00.1", the > > configuration option in OVS would be > > ovs-vsctl set Open_vSwitch . other_config:dpdk-whitelist-pci- > > ids="0000:06:00.0,0000:06:00.1" > > > > To update the whitelist ports, OVS daemon has to be restarted. > > > > Signed-off-by: Sugesh Chandran <sugesh.chand...@intel.com> > > --- > > lib/dpdk.c | 29 +++++++++++++++++++++++++++++ > > vswitchd/vswitch.xml | 21 +++++++++++++++++++++ > > 2 files changed, 50 insertions(+) > > > > diff --git a/lib/dpdk.c b/lib/dpdk.c > > index 9d187c7..0f11977 100644 > > --- a/lib/dpdk.c > > +++ b/lib/dpdk.c > > @@ -323,6 +323,34 @@ dpdk_isolate_rte_mem_config(const struct smap > > *ovs_other_config, } > > > > static void > > +dpdk_whitelist_pci_ids(const struct smap *ovs_other_config, char ***argv, > > + int *argc) > > +{ > > + const char *pci_ids; > > + char *pci_dev; > > + int len; > > + int i; > > + pci_ids = smap_get(ovs_other_config, "dpdk-whitelist-pci-ids"); > > + if (!pci_ids) { > > + return; > > + } > > + len = strlen(pci_ids); > > + do { > > + i = strcspn(pci_ids, ","); > > + pci_dev = xmemdup0(pci_ids, i); > > + if (!strlen(pci_dev)) { > > + break; > > + } > > + *argv = grow_argv(argv, *argc, 2); > > + (*argv)[(*argc)++] = xstrdup("-w"); > > + (*argv)[(*argc)++] = pci_dev; > > + i++; > > + pci_ids += i; > > + len -= i; > > + } while (pci_ids && len > 0); > > +} > > + > > +static void > > dpdk_init__(const struct smap *ovs_other_config) { > > char **argv = NULL, **argv_to_release = NULL; @@ -409,6 +437,7 @@ > > dpdk_init__(const struct smap *ovs_other_config) > > } > > > > dpdk_isolate_rte_mem_config(ovs_other_config, &argv, &argc); > > + dpdk_whitelist_pci_ids(ovs_other_config, &argv, &argc); > > argv = grow_argv(&argv, argc, 1); > > argv[argc] = NULL; > > > > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index > > 7462b30..0b64b25 100644 > > --- a/vswitchd/vswitch.xml > > +++ b/vswitchd/vswitch.xml > > @@ -442,6 +442,27 @@ > > </p> > > </column> > > > > + <column name="other_config" key="dpdk-whitelist-pci-ids"> > > + <p> > > + Specifies list of pci-ids separated by , for whitelisting > > available > > + physical NIC ports in OVS. The option valid only when DPDK is > > enabled > > + in OVS and the ports are already bound to DPDK userspace driver. > > + </p> > > + <p> > > + By default, all the DPDK bound ports are initialized at the time > > of > > + vswitchd start. Whitelisting a list of pci-ids is used to limit > > the > > + initialization only to specific ports. Changing this value > > requires > > + restarting the daemon. > > + </p> > > + <p> > > + This option allow user to run multiple instance of DPDK including > > + vswitchd simultaneously by exclusively allocating the physical NIC > > + ports between them. Its impossible to use the NIC ports in > > + multiple primary DPDK processes without whitelisting them > > + even the application is using distinct ports. > > + </p> > > + </column> > > + > > </group> > > > > <group title="Status"> > > -- > > 2.7.4 > > > > _______________________________________________ > > dev mailing list > > d...@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev