> -----Original Message-----
> From: Singh, Aman Deep <aman.deep.si...@intel.com>
> Sent: Friday, March 10, 2023 9:09 AM
> To: Stephen Hemminger <step...@networkplumber.org>; Dumitrescu, Cristian
> <cristian.dumitre...@intel.com>
> Cc: Jangra, Yogesh <yogesh.jan...@intel.com>; 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/10/2023 1:52 AM, Stephen Hemminger wrote:
> > On Thu, 9 Mar 2023 17:19:59 +0000
> > "Dumitrescu, Cristian" <cristian.dumitre...@intel.com> 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().
> >>
> >> 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
> > If the application has to do this then something is wrong with the
> > architecture.
> >
> >
> > You aren't propagating state changes through in a safe manner.
> > Other applications will have same issue.
> >
> > If LAG or virtual devices have similar problems then a generic solution is
> needed.
>
> At exit, there is call to stop_packet_forwarding(). Shouldn't that stop
> SoftNic
> from polling of the queues ?
Hi Aman,
Unfortunately not. Calling the stop_packet_forwarding() will not stop the core
running the Soft NIC, as Soft NIC is running on a service core, and the service
cores are not handled by this function.
Moreover, a Soft NIC device is not taking over the entire service core; the
same service core can also run other services e.g. Soft NIC for device A,
telemetry, stats, Soft NIC for device B, etc). Therefore, stopping the service
core associated with a Soft NIC device will also incorrectly result in stopping
all the other services running on the same service core.
Stopping the service for a Soft NIC device is done by the stop() function,
which is exactly what we are trying to do with this short patch.
Regards,
Cristian
