* Re: [net-next PATCH 01/11] octeontx2-af: Simplify context writing and reading to hardware
2025-07-14 10:45 ` [net-next PATCH 01/11] octeontx2-af: Simplify context writing and reading to hardware Simon Horman
@ 2025-07-15 16:46 ` Subbaraya Sundeep
0 siblings, 0 replies; 2+ messages in thread
From: Subbaraya Sundeep @ 2025-07-15 16:46 UTC (permalink / raw)
To: Simon Horman
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, gakula, hkelam,
bbhushan2, jerinj, lcherian, sgoutham, netdev, Kees Cook,
linux-hardening
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
^ permalink raw reply [flat|nested] 2+ messages in thread