> -----Original Message----- > From: Ferruh Yigit <ferruh.yi...@amd.com> > Sent: Friday, March 10, 2023 12:00 PM > To: Dumitrescu, Cristian <cristian.dumitre...@intel.com>; Stephen Hemminger > <step...@networkplumber.org>; Jangra, Yogesh <yogesh.jan...@intel.com>; > Singh, Aman Deep <aman.deep.si...@intel.com>; Zhang, Yuying > <yuying.zh...@intel.com> > Cc: dev@dpdk.org; R, Kamalakannan <kamalakanna...@intel.com>; Suresh > Narayane, Harshad <harshad.suresh.naray...@intel.com> > Subject: Re: [PATCH] app/testpmd: fix closing softnic port before ethdev ports > > On 3/9/2023 5:19 PM, Dumitrescu, Cristian wrote: > > > > > >> -----Original Message----- > >> From: Stephen Hemminger <step...@networkplumber.org> > >> Sent: Thursday, March 9, 2023 4:31 PM > >> To: Jangra, Yogesh <yogesh.jan...@intel.com> > >> Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitre...@intel.com>; R, > >> Kamalakannan <kamalakanna...@intel.com>; Suresh Narayane, Harshad > >> <harshad.suresh.naray...@intel.com> > >> Subject: Re: [PATCH] app/testpmd: fix closing softnic port before ethdev > ports > >> > >> On Thu, 9 Mar 2023 14:42:49 +0000 > >> Yogesh Jangra <yogesh.jan...@intel.com> wrote: > >> > >>> + /* > >>> + * SoftNIC runs on the sevice core, it uses the resources from > >>> + * the testpmd application. When we run quit command, the > >> testpmd > >>> + * application stops ethdev ports first, SoftNIC will try to > >>> + * access the port and sometimes that result in segmentation > >>> + * error. So first closing the SoftNIC port. > >>> + */ > >>> + RTE_ETH_FOREACH_DEV(pt_id) { > >>> + if (!strcmp(ports[pt_id].dev_info.driver_name, > >> "net_softnic")) { > >>> + stop_port(pt_id); > >>> + close_port(pt_id); > >>> + } > >>> + } > >>> + > >> > >> NAK > >> No driver specific hacks please. > >> > >> Instead fix the driver design or bug please. > > > > Hi Stephen, > > > > This is not a Soft NIC driver-specific hack, this is required for working > > around > some of the ethdev drivers that don't implement the stop() API correctly and > free up the device queues or some other internal resources on stop() instead > of > close(). > > > > Why not fix the misbehaving drivers, instead of working around for > softnic, as Stephen suggested? > > Is there a list of problematic drivers? >
Ferruh, I think this is not a reasonable request. We don't have the expertise to fix all drivers, not the hardware to test all drivers. > > The Soft NIC is a meta-device that sits on top of other "physical" ethdev > devices, so when the Soft NIC device continues to poll the queues of those > physical devices after their queues have been freed, the Soft NIC will get a > segfault. This fix is required to protect against this sort of incorrect > driver > behavior by simply stopping the Soft NIC devices first. > > > > We already have several driver specific branches in the test-pmd for e.g. > > LAG > or virtual devices; IMO this small change falls in the same category and it > should > get accepted. > > > > Please let us know if this makes sense to you? > > > > Regards, > > Cristian