RE: [PATCH v9] net/bnx2x: fix warnings about rte_memcpy lengths

["Morten Brørup" <mb@smartsharesystems.com>]

+To: Raslan, regarding MLX5 patch

> From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> Sent: Tuesday, 27 February 2024 12.01
> 
> On Mon, Feb 26, 2024 at 8:17 PM Morten Brørup <mb@smartsharesystems.com>
> wrote:
> >
> > > From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> > > Sent: Monday, 26 February 2024 09.34
> > >
> > > On Fri, Feb 23, 2024 at 7:30 PM Morten Brørup <mb@smartsharesystems.com>
> > > wrote:
> > > >
> > > > Bugfix: The vlan in the bulletin does not contain a VLAN header, only
> > > the
> > > > VLAN ID, so only copy 2 byte, not 4. The target structure has padding
> > > > after the field, so copying 2 byte too many is effectively harmless.
> > > > There is no need to backport this patch.
> > > >
> > > > Use RTE_PTR_ADD where copying arrays to the offset of a first field in
> > > a
> > > > structure holding multiple fields, to avoid compiler warnings with
> > > > decorated rte_memcpy.
> > > >
> > > > Bugzilla ID: 1146
> > > >
> > > > Fixes: 540a211084a7695a1c7bc43068934c140d6989be ("bnx2x: driver core")
> > > > Cc: stephen@networkplumber.org
> > > > Cc: rmody@marvell.com
> > > > Cc: shshaikh@marvell.com
> > > > Cc: palok@marvell.com
> > > >
> > > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > > > Acked-by: Devendra Singh Rawat <dsinghrawat@marvell.com>
> > > > ---
> > > > v9:
> > > > * Fix checkpatch warning about spaces.
> > >
> > > Fixed the following issues[1] and updated the git commit as follows
> > > and applied to dpdk-next-net-mrvl/for-main. Thanks
> >
> > Thank you, Jerin.
> >
> > [...]
> >
> > > Is it candidate for Cc: stable@dpdk.org backport?
> >
> > No, I don't think so:
> > 1. The extra 2 byte copy is effectively harmless due to padding, as
> mentioned in the commit message.
> > 2. The decorated rte_memcpy (if work on that patch series is ever resumed)
> is an improvement, not a bug fix, and will not be backported. So the memcpy
> parts of this patch are irrelevant for the stable versions.
> 
> 
> Shall remove Fixes: tag then?. Since the patch has a Fixes tag, I
> thought good to merge to stable as it is fixing.

Although the patch formally fixes a bug, the bug is harmless, so I don't think it is worth the effort backporting.
I don't know the policy for Fixes: tags in such cases. However you proceed with it is perfectly fine with me.

> 
> Also, could you comment on @Stephen Hemminger latest comments, Should
> I wait for new version? or new changes can go as separate patches.

I'm not providing a new version of this patch.

This patch was part of a series, where its rte_memcpy changes were required for the primary patch in the series [1]. The purpose of the primary patch was to tighten rte_memcpy's parameter requirements by adding access-mode attributes.

[1]: https://patchwork.dpdk.org/project/dpdk/patch/20230116130724.50277-4-mb@smartsharesystems.com/

It was not really my intention to fix other things in the drivers I submitted patches for. That was only a positive side effect. ;-)

@Raslan: Also, the patch for the mlx5 driver [2] has seen no progress for a year, so I'm going to abandon it. I will resume it if I start working on the decorated rte_memcpy again. Should I change it's state to Not Applicable or something else?

[2]: https://patchwork.dpdk.org/project/dpdk/patch/20230116130724.50277-3-mb@smartsharesystems.com/