[dpdk-dev] [PATCH 2/2] ethdev: fix the race condition for fp ops reset

[Bing Zhao]

Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Sunday, October 24, 2021 12:13 AM
> To: Bing Zhao <bingz at nvidia.com>
> Cc: NBU-Contact-Thomas Monjalon <thomas at monjalon.net>;
> ferruh.yigit at intel.com; andrew.rybchenko at oktetlabs.ru; dev at dpdk.org;
> konstantin.ananyev at intel.com
> Subject: Re: [dpdk-dev] [PATCH 2/2] ethdev: fix the race condition
> for fp ops reset
> 
> External email: Use caution opening links or attachments
> 
> 
> On Sat, 23 Oct 2021 00:14:07 +0300
> Bing Zhao <bingz at nvidia.com> wrote:
> 
> > diff --git a/lib/ethdev/ethdev_private.c
> b/lib/ethdev/ethdev_private.c
> > index fbc3df91ad..cda9a6e228 100644
> > --- a/lib/ethdev/ethdev_private.c
> > +++ b/lib/ethdev/ethdev_private.c
> > @@ -206,7 +206,7 @@ eth_dev_fp_ops_reset(struct rte_eth_fp_ops
> *fpo)
> >               .txq = {.data = dummy_data, .clbk = dummy_data,},
> >       };
> >
> > -     *fpo = dummy_ops;
> > +     rte_memcpy(fpo, &dummy_ops, sizeof(struct rte_eth_fp_ops));
> 
> memcpy is not thread safe either.

Sorry for my commit log and it may not be that clear. The reason is the thread-safe, it is because the structure assignment and initialization. Then the pointer will be set to 0 and then set to the new value again.
I am not quite sure if it is OK so use part of the old values and part of the new values in the same function (rx/tx burst). However, changing a pointer to NULL (0) is risk and would cause a crash of the whole application. Using memcpy will keep the old value still value and change is into the new value immediately w/o setting it to 0 fist.

But YES again, the application should avoid such case.

BR. Bing