Re: [PATCH net-next 1/9] net: enetc: add interfaces to manage FDB entries

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ntmp_fdbt_update_activity_element() fails to filter for dynamic FDB entries, causing it to incorrectly match and update the activity elements of all entries (including static ones).
--

commit 871b9a0ec7e03bf7ca6a2b118e3fd29a9b0a249e
Author: Wei Fang <wei.fang@nxp.com>
Subject: net: enetc: add interfaces to manage FDB entries

This commit adds four interfaces to manage entries in the FDB table. It
includes functions for deleting entries by key element, updating activity
elements of dynamic entries, deleting aging entries, and deleting port
dynamic entries.

> 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
[ ... ]
> +/**
> + * ntmp_fdbt_update_activity_element - update the aging time of all the
> + * dynamic entries in the FDB table.
[ ... ]
> +int ntmp_fdbt_update_activity_element(struct ntmp_user *user)
> +{
> +	struct fdbt_req_ua *req;
> +	struct netc_swcbd swcbd;
> +	struct netc_cbdr *cbdr;
> +	union netc_cbd cbd;
> +	u32 len;
> +	int err;
> +
> +	swcbd.size = sizeof(*req);
> +	err = ntmp_alloc_data_mem(user->dev, &swcbd, (void **)&req);
> +	if (err)
> +		return err;
> +
> +	/* Request data */
> +	ntmp_fill_crd(&req->crd, user->tbl.fdbt_ver, 0, FDBT_UA_ACTEU);
> +	req->ak.search.resume_eid = cpu_to_le32(NTMP_NULL_ENTRY_ID);
> +

[Severity: Medium]
Does this function accidentally update static FDB entries as well as dynamic
ones?

The function's description states it should update the aging time of all the
dynamic entries. However, since ntmp_alloc_data_mem() uses
dma_alloc_coherent(), the request structure is zero-initialized.

Because the search key configuration match fields (req->ak.search.cfge_mc
and req->ak.search.cfge.cfg) are left as 0, this appears to instruct the
hardware to match all FDB entries rather than restricting the operation to
dynamic entries.

For comparison, ntmp_fdbt_delete_aging_entries() explicitly sets these
fields to enforce the dynamic entry filter:

ntmp_fdbt_delete_aging_entries() {
    ...
    req->ak.search.cfge.cfg = cpu_to_le32(cfg);
    req->ak.search.cfge_mc = FDBT_CFGE_MC_DYNAMIC;
    ...
}

Could similar filter initialization be missing here?

> +	/* Request header */
> +	len = NTMP_LEN(swcbd.size, NTMP_STATUS_RESP_LEN);
> +	/* For activity update, the access method must be search */
> +	ntmp_fill_request_hdr(&cbd, swcbd.dma, len, NTMP_FDBT_ID,
> +			      NTMP_CMD_UPDATE, NTMP_AM_SEARCH);
[ ... ]

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