[PATCH] net/mlx5: fix metadata endianness in modify field action

[Slava Ovsiienko]

Hi, Ferruh

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit at intel.com>
> Sent: Tuesday, December 7, 2021 15:45
> To: Slava Ovsiienko <viacheslavo at nvidia.com>; dev at dpdk.org
> Cc: Matan Azrad <matan at nvidia.com>; Raslan Darawsheh
> <rasland at nvidia.com>; stable at dpdk.org
> Subject: Re: [PATCH] net/mlx5: fix metadata endianness in modify field action
> 
> On 11/29/2021 12:32 PM, Viacheslav Ovsiienko wrote:
> > As modify field action immediate source parameter the metadata should
> > follow the CPU endianness (according to SET_META action structure
> > format), and mlx5 PMD wrongly handled the immediate parameter
> metadata
> > buffer as big-endian, resulting in wrong metadata set action with
> > incorrect endianness.
> >
> > Fixes: 40c8fb1fd3b3 ("net/mlx5: update modify field action")
> > Cc: stable at dpdk.org
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>
> > ---
> >   drivers/net/mlx5/mlx5_flow_dv.c | 23 +++++++++++------------
> >   1 file changed, 11 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > b/drivers/net/mlx5/mlx5_flow_dv.c index 4834c752d9..1c6cae8779 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -1465,7 +1465,7 @@ static void
> >   mlx5_flow_field_id_to_modify_info
> >   		(const struct rte_flow_action_modify_data *data,
> >   		 struct field_modify_info *info, uint32_t *mask,
> > -		 uint32_t width, uint32_t *shift, struct rte_eth_dev *dev,
> > +		 uint32_t width, struct rte_eth_dev *dev,
> 
> Hi Viacheslav,
> 
> Is removing (unused) 'shift' variable related to the problem mentioned in the
> commit log?
Related indirectly to metadata, but not directly to the issue. 
"shift" is unused leftover after changing immediate value format.
And this patch just provides collateral cleanup. Do you think we should
separate into dedicated cleanup patch? Or mention this cleanup in commit
message?

With best regards,
Slava



> 
> Only below "meta = rte_cpu_to_be_32(meta);" change block seems fixing the
> issue and rest is cleanup, if that is the case can you please split the patch?