Re: [PATCH v6 net-next 06/15] net: enetc: add support for the "Update" operation to buffer pool table

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The BPT update command incorrectly sets the `BPT_UA_BPSEU` flag without providing the required BPSE payload in the DMA buffer.
--

commit 04028be08f5e1d61341410aae72c568ebdb38953
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: add support for the "Update" operation to buffer pool table

This commit adds the ntmp_bpt_update_entry helper to support updating
the specified entry of the buffer pool table.

> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp_private.h b/drivers/net/ethernet/freescale/enetc/ntmp_private.h
> --- a/drivers/net/ethernet/freescale/enetc/ntmp_private.h
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp_private.h

[ ... ]

> @@ -175,4 +175,10 @@ struct vft_req_ua {
>  	struct vft_cfge_data cfge;
>  };
>  
> +/* Buffer Pool Table Request Data Buffer Format of Update action */
> +struct bpt_req_update {
> +	struct ntmp_req_by_eid rbe;
> +	struct bpt_cfge_data cfge;
> +};
> +

[ ... ]

> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp.c b/drivers/net/ethernet/freescale/enetc/ntmp.c
> --- a/drivers/net/ethernet/freescale/enetc/ntmp.c
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp.c

[ ... ]

> @@ -749,5 +755,38 @@ int ntmp_vft_add_entry(struct ntmp_user *user, u16 vid,
>  }
>  EXPORT_SYMBOL_GPL(ntmp_vft_add_entry);
>  
> +int ntmp_bpt_update_entry(struct ntmp_user *user, u32 entry_id,
> +			  const struct bpt_cfge_data *cfge)
> +{
> +	struct bpt_req_update *req;
> +	struct netc_swcbd swcbd;
> +	struct netc_cbdr *cbdr;
> +	union netc_cbd cbd;
> +	int err;
> +
> +	swcbd.size = sizeof(*req);
> +	err = ntmp_alloc_data_mem(user->dev, &swcbd, (void **)&req);
> +	if (err)
> +		return err;
> +
> +	ntmp_fill_crd_eid(&req->rbe, user->tbl.bpt_ver, 0,
> +			  NTMP_GEN_UA_CFGEU | BPT_UA_BPSEU, entry_id);

Will this command fail due to a missing payload for the Buffer Pool Status
Entry?

The update action is instructed to process both CFGE and BPSE data by setting
NTMP_GEN_UA_CFGEU | BPT_UA_BPSEU, but the allocated struct bpt_req_update
shown above does not contain a struct bpt_bpse_data field.

Because the provided swcbd.size is only sizeof(*req), does this cause the
hardware to either reject the command with a length mismatch error, or
unintentionally read past the end of the provided buffer?

> +	req->cfge = *cfge;
> +	ntmp_fill_request_hdr(&cbd, swcbd.dma, NTMP_LEN(swcbd.size, 0),
> +			      NTMP_BPT_ID, NTMP_CMD_UPDATE, NTMP_AM_ENTRY_ID);
> +
> +	ntmp_select_and_lock_cbdr(user, &cbdr);
> +	err = netc_xmit_ntmp_cmd(cbdr, &cbd, &swcbd);
> +	if (err)
> +		dev_err(user->dev,
> +			"Failed to update %s entry 0x%x, err: %pe\n",
> +			ntmp_table_name(NTMP_BPT_ID), entry_id, ERR_PTR(err));
> +
> +	ntmp_unlock_cbdr(cbdr);
> +
> +	return err;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509102954.4116624-1-wei.fang@nxp.com?part=6