> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Wednesday, 28 December 2022 18.03
>
> On Wed, 28 Dec 2022 17:38:56 +0100
> Morten Brørup <m...@smartsharesystems.com> wrote:
>
> > From: Stanisław Kardach [mailto:k...@semihalf.com]
> > Sent: Wednesday, 28 December 2022 17.14
> > > On Wed, Dec 28, 2022, 16:10 Morten Brørup
> <m...@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;
