From: Subbaraya Sundeep <sbhatta@marvell.com>
To: Simon Horman <horms@kernel.org>
Cc: <andrew+netdev@lunn.ch>, <davem@davemloft.net>,
<edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
<gakula@marvell.com>, <hkelam@marvell.com>,
<bbhushan2@marvell.com>, <jerinj@marvell.com>,
<lcherian@marvell.com>, <sgoutham@marvell.com>,
<netdev@vger.kernel.org>, Kees Cook <kees@kernel.org>,
<linux-hardening@vger.kernel.org>
Subject: Re: [net-next PATCH 01/11] octeontx2-af: Simplify context writing and reading to hardware
Date: Tue, 15 Jul 2025 16:46:47 +0000 [thread overview]
Message-ID: <aHaF9yM4FC0OICpt@2bdc17c5cd25> (raw)
In-Reply-To: <20250714104557.GG721198@horms.kernel.org>
On 2025-07-14 at 10:45:57, Simon Horman (horms@kernel.org) wrote:
> + Kees, linux-hardening
>
> On Sun, Jul 13, 2025 at 09:00:59PM +0530, Subbaraya Sundeep wrote:
> > Simplify NIX context reading and writing by using hardware
> > maximum context size instead of using individual sizes of
> > each context type.
> >
> > Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> > ---
> > .../ethernet/marvell/octeontx2/af/rvu_nix.c | 46 ++++++++++---------
> > 1 file changed, 24 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > index bdf4d852c15d..48d44911b663 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > @@ -17,6 +17,8 @@
> > #include "lmac_common.h"
> > #include "rvu_npc_hash.h"
> >
> > +#define NIX_MAX_CTX_SIZE 128
> > +
> > static void nix_free_tx_vtag_entries(struct rvu *rvu, u16 pcifunc);
> > static int rvu_nix_get_bpid(struct rvu *rvu, struct nix_bp_cfg_req *req,
> > int type, int chan_id);
> > @@ -1149,36 +1151,36 @@ static int rvu_nix_blk_aq_enq_inst(struct rvu *rvu, struct nix_hw *nix_hw,
> > case NIX_AQ_INSTOP_WRITE:
> > if (req->ctype == NIX_AQ_CTYPE_RQ)
> > memcpy(mask, &req->rq_mask,
> > - sizeof(struct nix_rq_ctx_s));
> > + NIX_MAX_CTX_SIZE);
> > else if (req->ctype == NIX_AQ_CTYPE_SQ)
> > memcpy(mask, &req->sq_mask,
> > - sizeof(struct nix_sq_ctx_s));
> > + NIX_MAX_CTX_SIZE);
> > else if (req->ctype == NIX_AQ_CTYPE_CQ)
> > memcpy(mask, &req->cq_mask,
> > - sizeof(struct nix_cq_ctx_s));
> > + NIX_MAX_CTX_SIZE);
> > else if (req->ctype == NIX_AQ_CTYPE_RSS)
> > memcpy(mask, &req->rss_mask,
> > - sizeof(struct nix_rsse_s));
> > + NIX_MAX_CTX_SIZE);
> > else if (req->ctype == NIX_AQ_CTYPE_MCE)
> > memcpy(mask, &req->mce_mask,
> > - sizeof(struct nix_rx_mce_s));
> > + NIX_MAX_CTX_SIZE);
> > else if (req->ctype == NIX_AQ_CTYPE_BANDPROF)
> > memcpy(mask, &req->prof_mask,
> > - sizeof(struct nix_bandprof_s));
> > + NIX_MAX_CTX_SIZE);
> > fallthrough;
>
> Hi Subbaraya,
>
> Unfortunately this patch adds string fortification warnings
> because, e.g. the size of req->rss_mask is less than 128 bytes.
>
> GCC 15.1.0 flags this as follows:
>
> In function 'fortify_memcpy_chk',
> inlined from 'rvu_nix_blk_aq_enq_inst' at drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c:1159:4:
> ./include/linux/fortify-string.h:580:4: warning: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()?
> __read_overflow2_field(q_size_field, size);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> There may there is nicer way to do this. And it's entirely possible I've
> muddled up the combination of structures and unions here. But I wonder if
> an approach like this can reach your goals wile keeping the string
> fortification checker happy.
Hi Simon,
Thanks for reviewing the patch. The mask and context structures are
accessed by hardware, instead of typecasting to new context structures for new silicon
I used fixed size of 128 which is hardware maximum context size. Say CQ context
is 32 bytes and if I fill valid 32 bytes + 96 invalid/junk bytes then Hardware does not
care about the invalid bytes and it uses only 32 bytes since CTYPE is set as CQ when
submitting the instruction to hardware. To keep the string fortification
happy will padd the structures which are lesser than 128 to 128. I will submit
v2 with those changes.
Sundeep
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 0bc0dc79868b..0aa1e823cbd3 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -985,14 +985,17 @@ struct nix_aq_enq_req {
> struct nix_rx_mce_s mce;
> struct nix_bandprof_s prof;
> };
> - union {
> - struct nix_rq_ctx_s rq_mask;
> - struct nix_sq_ctx_s sq_mask;
> - struct nix_cq_ctx_s cq_mask;
> - struct nix_rsse_s rss_mask;
> - struct nix_rx_mce_s mce_mask;
> - struct nix_bandprof_s prof_mask;
> - };
> + struct_group(
> + mask,
> + union {
> + struct nix_rq_ctx_s rq_mask;
> + struct nix_sq_ctx_s sq_mask;
> + struct nix_cq_ctx_s cq_mask;
> + struct nix_rsse_s rss_mask;
> + struct nix_rx_mce_s mce_mask;
> + struct nix_bandprof_s prof_mask;
> + };
> + );
> };
>
> struct nix_aq_enq_rsp {
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> index bdf4d852c15d..4089933d5a0b 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> @@ -1147,24 +1147,7 @@ static int rvu_nix_blk_aq_enq_inst(struct rvu *rvu, struct nix_hw *nix_hw,
>
> switch (req->op) {
> case NIX_AQ_INSTOP_WRITE:
> - if (req->ctype == NIX_AQ_CTYPE_RQ)
> - memcpy(mask, &req->rq_mask,
> - sizeof(struct nix_rq_ctx_s));
> - else if (req->ctype == NIX_AQ_CTYPE_SQ)
> - memcpy(mask, &req->sq_mask,
> - sizeof(struct nix_sq_ctx_s));
> - else if (req->ctype == NIX_AQ_CTYPE_CQ)
> - memcpy(mask, &req->cq_mask,
> - sizeof(struct nix_cq_ctx_s));
> - else if (req->ctype == NIX_AQ_CTYPE_RSS)
> - memcpy(mask, &req->rss_mask,
> - sizeof(struct nix_rsse_s));
> - else if (req->ctype == NIX_AQ_CTYPE_MCE)
> - memcpy(mask, &req->mce_mask,
> - sizeof(struct nix_rx_mce_s));
> - else if (req->ctype == NIX_AQ_CTYPE_BANDPROF)
> - memcpy(mask, &req->prof_mask,
> - sizeof(struct nix_bandprof_s));
> + memcpy(mask, &req->mask, sizeof(req->mask));
> fallthrough;
> case NIX_AQ_INSTOP_INIT:
> if (req->ctype == NIX_AQ_CTYPE_RQ)
>
> ...
>
> --
> pw-bot: changes-requested
next prev parent reply other threads:[~2025-07-15 16:47 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-13 15:30 [net-next PATCH 00/11] Add CN20K NIX and NPA contexts Subbaraya Sundeep
2025-07-13 15:30 ` [net-next PATCH 01/11] octeontx2-af: Simplify context writing and reading to hardware Subbaraya Sundeep
2025-07-14 10:45 ` Simon Horman
2025-07-15 16:46 ` Subbaraya Sundeep [this message]
2025-07-13 15:31 ` [net-next PATCH 02/11] octeontx2-af: Add cn20k NIX block contexts Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 03/11] octeontx2-af: Extend debugfs support for cn20k NIX Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 04/11] octeontx2-af: Add cn20k NPA block contexts Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 05/11] octeontx2-af: Extend debugfs support for cn20k NPA Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 06/11] octeontx2-af: Skip NDC operations for cn20k Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 07/11] octeontx2-pf: Initialize cn20k specific aura and pool contexts Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 08/11] octeontx2-pf: Initialize new NIX SQ context for cn20k Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 09/11] octeontx2-af: Accommodate more bandwidth profiles " Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 10/11] octeontx2-af: Display new bandwidth profiles too in debugfs Subbaraya Sundeep
2025-07-13 15:31 ` [net-next PATCH 11/11] octeontx2-pf: Use new bandwidth profiles in receive queue Subbaraya Sundeep
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aHaF9yM4FC0OICpt@2bdc17c5cd25 \
--to=sbhatta@marvell.com \
--cc=andrew+netdev@lunn.ch \
--cc=bbhushan2@marvell.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gakula@marvell.com \
--cc=hkelam@marvell.com \
--cc=horms@kernel.org \
--cc=jerinj@marvell.com \
--cc=kees@kernel.org \
--cc=kuba@kernel.org \
--cc=lcherian@marvell.com \
--cc=linux-hardening@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sgoutham@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).