From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3A933A04FD; Wed, 28 Dec 2022 17:13:53 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D1AB540A7F; Wed, 28 Dec 2022 17:13:52 +0100 (CET) Received: from mail-qk1-f178.google.com (mail-qk1-f178.google.com [209.85.222.178]) by mails.dpdk.org (Postfix) with ESMTP id 3965940698 for ; Wed, 28 Dec 2022 17:13:52 +0100 (CET) Received: by mail-qk1-f178.google.com with SMTP id pe2so7804406qkn.1 for ; Wed, 28 Dec 2022 08:13:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf.com; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=8Wv8Ix3pguovoJ2jrpX6OeB3PRh296gFkNN4YvR2u50=; b=QNTjQw3hEaqujG1PVqrZZhjTQ0XEoyUJVKEFPxpGXFTCxPED28STt0HsfQkfuSWLnu IoZp/YPOjgF9a0sPhswqNOAwRo77PyURStnjF7agT+XN4++VFKHP58epiuNr6KRQSOTT h3PhpE/UY137H8s/fDOzQOxHu7m0MRJaGNBvU6/14O/lOM9Z9A013tZVFXhN2WO0Jud/ 0heZcAbo9+hyxCZDwbMQfs8Z5MEdnwk1q26N2lg6KfrUHlVKe77q/OwqvhcZFQc1gGHJ 8vJe9+5q/GsmcUlmoj26WpN7oU7mcHmOHdfXtgnCUCIUuM01RAfLpZ2aULdgB3vJlILI mnEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=8Wv8Ix3pguovoJ2jrpX6OeB3PRh296gFkNN4YvR2u50=; b=VbO96s5WRkcL1uWU8jxdZpr+4GsYGLOo+Z4tDOF4z6vWiJ5I6ZUYAzcmFenyGDi2VR HLmPo1fpZRNJfyN75J1a+dqSE2X8TRZUfzYp6nI9NENFKnfdI/ZmlWcsvajGztVJEVpz FpdrL6ptGkCUmVYVo8ZA53ZdMKZ4u5amBovyC8amfZH4T6rtWrV1ZadeQQ+t9qxuAuLt Vpyg873Smcy0VMLmC/pRjvJviD7TCbOwMY7djFAXO9u3bRiY0NjffunNW7aV5vM7jq2S WAFDCUlH8OLJo7fTWgE79EDQSLB9pv78r26zY7oSXVPkjukCQ7u0y66+yYdbZWPd3tCM wgsA== X-Gm-Message-State: AFqh2krP0SkQqbjJb0OLQ1lqb3BeM157QyPNQ3NN523F9sVXlYghK9Kd QrhaXsu6n9LgcDuYnJymuTR0IalDtvmh3caoyh920Q== X-Google-Smtp-Source: AMrXdXtFaE0cx7ljn4OKxWwBIDlHeJyHTPL049DSO6zU1Tk06peIMMaxCyQUZws7vN6A/D3hvu5tcoVs3GGOvjkw9ak= X-Received: by 2002:a05:620a:1286:b0:702:4f9a:ff11 with SMTP id w6-20020a05620a128600b007024f9aff11mr1293850qki.446.1672244031525; Wed, 28 Dec 2022 08:13:51 -0800 (PST) MIME-Version: 1.0 References: <20221202153432.131023-1-mb@smartsharesystems.com> <20221228151019.101309-1-mb@smartsharesystems.com> <20221228151019.101309-2-mb@smartsharesystems.com> In-Reply-To: <20221228151019.101309-2-mb@smartsharesystems.com> From: =?UTF-8?Q?Stanis=C5=82aw_Kardach?= Date: Wed, 28 Dec 2022 17:13:39 +0100 Message-ID: Subject: Re: [PATCH v5 2/4] net/bnx2x: fix warnings about rte_memcpy lengths To: =?UTF-8?Q?Morten_Br=C3=B8rup?= Cc: dev , Tyler Retzlaff , rmody@marvell.com, timothy.mcdaniel@intel.com, matan@nvidia.com, viacheslavo@nvidia.com, Ruifeng Wang , Min Zhou , David Christensen , Bruce Richardson , Konstantin Ananyev Content-Type: multipart/alternative; boundary="0000000000007a2dba05f0e5a734" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --0000000000007a2dba05f0e5a734 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Dec 28, 2022, 16:10 Morten Br=C3=B8rup w= rote: > 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? > 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. > > Bugzilla ID: 1146 > > Signed-off-by: Morten Br=C3=B8rup > > v5: > * No changes. > v4: > * Type casting did not fix the warnings, so use RTE_PTR_ADD instead. > v3: > * First patch in series. > --- > drivers/net/bnx2x/bnx2x_stats.c | 11 +++++++---- > drivers/net/bnx2x/bnx2x_vfpf.c | 2 +- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/bnx2x/bnx2x_stats.c > b/drivers/net/bnx2x/bnx2x_stats.c > index c07b01510a..1c44504663 100644 > --- a/drivers/net/bnx2x/bnx2x_stats.c > +++ b/drivers/net/bnx2x/bnx2x_stats.c > @@ -819,8 +819,9 @@ bnx2x_hw_stats_update(struct bnx2x_softc *sc) > > rte_memcpy(old, new, sizeof(struct nig_stats)); > > - rte_memcpy(&(estats->rx_stat_ifhcinbadoctets_hi), > &(pstats->mac_stx[1]), > - sizeof(struct mac_stx)); > + rte_memcpy(RTE_PTR_ADD(estats, > + offsetof(struct bnx2x_eth_stats, rx_stat_ifhcinbadoctets_hi)), > + &(pstats->mac_stx[1]), sizeof(struct mac_stx)); > estats->brb_drop_hi =3D pstats->brb_drop_hi; > estats->brb_drop_lo =3D pstats->brb_drop_lo; > > @@ -1492,9 +1493,11 @@ bnx2x_stats_init(struct bnx2x_softc *sc) > REG_RD(sc, NIG_REG_STAT0_BRB_TRUNCATE + port*0x38); > if (!CHIP_IS_E3(sc)) { > REG_RD_DMAE(sc, NIG_REG_STAT0_EGRESS_MAC_PKT0 + port*0x50= , > - > &(sc->port.old_nig_stats.egress_mac_pkt0_lo), 2); > + RTE_PTR_ADD(&(sc->port.old_nig_stats), > + offsetof(struct nig_stats, > egress_mac_pkt0_lo)), 2); > REG_RD_DMAE(sc, NIG_REG_STAT0_EGRESS_MAC_PKT1 + port*0x50= , > - > &(sc->port.old_nig_stats.egress_mac_pkt1_lo), 2); > + RTE_PTR_ADD(&(sc->port.old_nig_stats), > + offsetof(struct nig_stats, > egress_mac_pkt1_lo)), 2); > } > > /* function stats */ > diff --git a/drivers/net/bnx2x/bnx2x_vfpf.c > b/drivers/net/bnx2x/bnx2x_vfpf.c > index 63953c2979..87631c76ca 100644 > --- a/drivers/net/bnx2x/bnx2x_vfpf.c > +++ b/drivers/net/bnx2x/bnx2x_vfpf.c > @@ -54,7 +54,7 @@ bnx2x_check_bull(struct bnx2x_softc *sc) > if (valid_bitmap & (1 << MAC_ADDR_VALID) && memcmp(bull->mac, > sc->old_bulletin.mac, ETH_ALEN)) > rte_memcpy(&sc->link_params.mac_addr, bull->mac, ETH_ALEN= ); > if (valid_bitmap & (1 << VLAN_VALID)) > - rte_memcpy(&bull->vlan, &sc->old_bulletin.vlan, > RTE_VLAN_HLEN); > + rte_memcpy(&bull->vlan, &sc->old_bulletin.vlan, > sizeof(bull->vlan)); > > sc->old_bulletin =3D *bull; > > -- > 2.17.1 > > --0000000000007a2dba05f0e5a734 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

On Wed, Dec 28, 2022, 16:10 Morten Br=C3=B8rup <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 r= te_memcpy for a 2 byte / half-word copy? Shouldn't assignment with cast= s be enough?
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.

Bugzilla ID: 1146

Signed-off-by: Morten Br=C3=B8rup <mb@smartsharesystems.com>= ;

v5:
* No changes.
v4:
* Type casting did not fix the warnings, so use RTE_PTR_ADD instead.
v3:
* First patch in series.
---
=C2=A0drivers/net/bnx2x/bnx2x_stats.c | 11 +++++++----
=C2=A0drivers/net/bnx2x/bnx2x_vfpf.c=C2=A0 |=C2=A0 2 +-
=C2=A02 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_stats.c b/drivers/net/bnx2x/bnx2x_stat= s.c
index c07b01510a..1c44504663 100644
--- a/drivers/net/bnx2x/bnx2x_stats.c
+++ b/drivers/net/bnx2x/bnx2x_stats.c
@@ -819,8 +819,9 @@ bnx2x_hw_stats_update(struct bnx2x_softc *sc)

=C2=A0 =C2=A0 =C2=A0rte_memcpy(old, new, sizeof(struct nig_stats));

-=C2=A0 =C2=A0 rte_memcpy(&(estats->rx_stat_ifhcinbadoctets_hi), &am= p;(pstats->mac_stx[1]),
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeof(struct mac_stx));
+=C2=A0 =C2=A0 rte_memcpy(RTE_PTR_ADD(estats,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offsetof(struct bnx2x_eth_stats, rx_sta= t_ifhcinbadoctets_hi)),
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &(pstats->mac_stx[1]), sizeof(st= ruct mac_stx));
=C2=A0 =C2=A0 =C2=A0estats->brb_drop_hi =3D pstats->brb_drop_hi;
=C2=A0 =C2=A0 =C2=A0estats->brb_drop_lo =3D pstats->brb_drop_lo;

@@ -1492,9 +1493,11 @@ bnx2x_stats_init(struct bnx2x_softc *sc)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 REG_RD(sc, NIG_REG_= STAT0_BRB_TRUNCATE + port*0x38);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!CHIP_IS_E3(sc)) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 REG_RD_DMAE(sc, NIG= _REG_STAT0_EGRESS_MAC_PKT0 + port*0x50,
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&(sc->port.old_nig_stats.egres= s_mac_pkt0_lo), 2);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0RTE_PTR_ADD(&(sc->port.old_nig= _stats),
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0offsetof(struct nig_stats, egress_mac= _pkt0_lo)), 2);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 REG_RD_DMAE(sc, NIG= _REG_STAT0_EGRESS_MAC_PKT1 + port*0x50,
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&(sc->port.old_nig_stats.egres= s_mac_pkt1_lo), 2);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0RTE_PTR_ADD(&(sc->port.old_nig= _stats),
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0offsetof(struct nig_stats, egress_mac= _pkt1_lo)), 2);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }

=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* function stats */
diff --git a/drivers/net/bnx2x/bnx2x_vfpf.c b/drivers/net/bnx2x/bnx2x_vfpf.= c
index 63953c2979..87631c76ca 100644
--- a/drivers/net/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/bnx2x/bnx2x_vfpf.c
@@ -54,7 +54,7 @@ bnx2x_check_bull(struct bnx2x_softc *sc)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (valid_bitmap & (1 << MAC_ADDR_VAL= ID) && memcmp(bull->mac, sc->old_bulletin.mac, ETH_ALEN))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 rte_memcpy(&sc-= >link_params.mac_addr, bull->mac, ETH_ALEN);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (valid_bitmap & (1 << VLAN_VALID))=
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rte_memcpy(&bul= l->vlan, &sc->old_bulletin.vlan, RTE_VLAN_HLEN);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rte_memcpy(&bul= l->vlan, &sc->old_bulletin.vlan, sizeof(bull->vlan));

=C2=A0 =C2=A0 =C2=A0 =C2=A0 sc->old_bulletin =3D *bull;

--
2.17.1

--0000000000007a2dba05f0e5a734--