RE: [EXTERNAL] [PATCH] net/netvsc: Fix on race condition of multiple commands

[Long Li <longli@microsoft.com>]

> When multiple processes issue command requests(like: device info get and
> link-status) at same-time, then we could see the command request failures,
> due to race-condition of common function execution.

Hi Madhuker,

I'm not sure if we should use a lock in the driver for this. It's not clear in DPDK documents but in general the calls to query device status are not thread safe.

Is it possible that the application uses a lock to sync calling to this?

Thanks,
Long

> 
> So, to avoid such race-condition introduced a new spin-lock to protect the
> code of critical section.
> 
> Fixes: 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")
> Signed-off-by: Madhuker Mythri <madhukar.mythri@gmail.com>
> ---
>  drivers/net/netvsc/hn_ethdev.c | 1 +
>  drivers/net/netvsc/hn_rndis.c  | 2 ++
>  drivers/net/netvsc/hn_var.h    | 1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
> index 6584819f4f..b525e287fa 100644
> --- a/drivers/net/netvsc/hn_ethdev.c
> +++ b/drivers/net/netvsc/hn_ethdev.c
> @@ -1310,6 +1310,7 @@ eth_hn_dev_init(struct rte_eth_dev *eth_dev)
>         PMD_INIT_FUNC_TRACE();
> 
>         rte_spinlock_init(&hv->hotadd_lock);
> +       rte_spinlock_init(&hv->cmd_lock);
>         LIST_INIT(&hv->hotadd_list);
> 
>         vmbus = container_of(device, struct rte_vmbus_device, device); diff --git
> a/drivers/net/netvsc/hn_rndis.c b/drivers/net/netvsc/hn_rndis.c index
> 7c54eebcef..8a0716df89 100644
> --- a/drivers/net/netvsc/hn_rndis.c
> +++ b/drivers/net/netvsc/hn_rndis.c
> @@ -500,8 +500,10 @@ hn_rndis_query(struct hn_data *hv, uint32_t oid,
>         /* Input data immediately follows RNDIS query. */
>         memcpy(req + 1, idata, idlen);
> 
> +       rte_spinlock_lock(&hv->cmd_lock);
>         error = hn_rndis_execute(hv, rid, req, reqlen,
>                                  comp, comp_len, RNDIS_QUERY_CMPLT);
> +       rte_spinlock_unlock(&hv->cmd_lock);
> 
>         if (error)
>                 goto done;
> diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h index
> 17c1d5d07b..66ed186c0a 100644
> --- a/drivers/net/netvsc/hn_var.h
> +++ b/drivers/net/netvsc/hn_var.h
> @@ -179,6 +179,7 @@ struct hn_data {
>         struct vmbus_channel *channels[HN_MAX_CHANNELS];
> 
>         rte_spinlock_t  hotadd_lock;
> +       rte_spinlock_t  cmd_lock;
>         LIST_HEAD(hotadd_list, hv_hotadd_context) hotadd_list;  };
> 
> --
> 2.50.1 (Apple Git-155)