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?
> 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
