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

[Thomas Monjalon]

22/10/2021 23:14, Bing Zhao:
> In the function "eth_dev_fp_ops_reset", a structure assignment
> operation is used to reset one queue's callback functions, etc., but
> it is not thread safe.
> 
> The structure assignment is not atomic, a lot of instructions will
> be generated. Right now, since not all the fields are needed, the
> fields in the "dummy_ops" which is not set explicitly will be 0s
> based on the specification and compiler behavior. In order to make
> "fpo" has the same content with "dummy_ops", some clearing to 0
> operation is needed.
> 
> By checking the object instructions (e.g. with GCC 4.8.5)
>    0x0000000000a58317 <+35>:	mov    %rsi,%rdi
>    0x0000000000a5831a <+38>:	mov    %rdx,%rcx
> => 0x0000000000a5831d <+41>:	rep stos %rax,%es:(%rdi)
>    0x0000000000a58320 <+44>:	mov    -0x38(%rsp),%rax
>    0x0000000000a58325 <+49>:	lea    -0xe0(%rip),%rdx
>         // # 0xa5824c <dummy_eth_rx_burst>
> 
> It shows that "rep stos" will clear the "fpo" structure before
> assigning new values.
> 
> In the other thread, if some data path Tx / Rx functions are still
> running, there is a risk to get 0 instead of the correct dummy
> content.
>   1. qd = p->rxq.data[queue_id]
>   2. (void **)&p->rxq.clbk[queue_id]
> "data" and "clbk" may be observed with NULL (0) in other threads.
> Even it is temporary, the accessing to a NULL pointer will cause a
> crash. Using "memcpy" could get rid of this.
> 
> Fixes: c87d435a4d79 ("ethdev: copy fast-path API into separate structure")
> Cc: konstantin.ananyev at intel.com
> 
> Signed-off-by: Bing Zhao <bingz at nvidia.com>
> ---
> --- 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));

That's not trivial.
Please add a comment to briefly explain that memcpy avoids zeroing of a simple assignment.