RE: [PATCH v5 2/4] net/bnx2x: fix warnings about rte_memcpy lengths

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

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, 28 December 2022 18.03
> 
> On Wed, 28 Dec 2022 17:38:56 +0100
> Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> > From: Stanisław Kardach [mailto:kda@semihalf.com]
> > Sent: Wednesday, 28 December 2022 17.14
> > > On Wed, Dec 28, 2022, 16:10 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.
> > > It is a small nitpick but why use rte_memcpy for a 2 byte / half-
> word copy? Shouldn't assignment with casts be enough?
> >
> > Absolutely. It would also have prevented the bug to begin with.
> > But in order to keep the changes minimal, I kept the rte_memcpy().
> 
> For small fixed values compiler can optimize memcpy into one
> instruction.
> Not so with current rte_memcpy

Good point, Stephen.

I took another look at it, and the two byte rte_memcpy() is only used in a slow path function, so no need to optimize further.

If the maintainers disagree, and want to optimize anyway, here's a proposal:

/* check the mac address and VLAN and allocate memory if valid */
if (valid_bitmap & (1 << MAC_ADDR_VALID) && !rte_is_same_ether_addr(
		(const struct rte_ether_addr *)&bull->mac,
		(const struct rte_ether_addr *)&sc->old_bulletin.mac))
	rte_ether_addr_copy((const struct rte_ether_addr *)&bull->mac,
			(struct rte_ether_addr *)&sc->link_params.mac_addr);
if (valid_bitmap & (1 << VLAN_VALID))
	bull->vlan = sc->old_bulletin.vlan;