RE: [PATCH v4 net-next 05/14] net: enetc: add support for the "Add" operation to VLAN filter table

Wei Fang
2026-04-08T07:12:30Z

> > 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
>
> [ ... ]
>
> > @@ -652,5 +655,52 @@ int ntmp_fdbt_search_port_entry(struct ntmp_user
> *user, int port,
> > +int ntmp_vft_add_entry(struct ntmp_user *user, u16 vid,
> > + const struct vft_cfge_data *cfge)
> > +{
> > + struct ntmp_dma_buf data = {
> > + .dev = user->dev,
> > + .size = sizeof(struct vft_req_ua),
> > + };
> > + struct vft_req_ua *req;
> > + union netc_cbd cbd;
> > + u32 len;
> > + int err;
> > +
> > + err = ntmp_alloc_data_mem(&data, (void **)&req);
> > + if (err)
> > + return err;
> > +
> > + /* Request data */
> > + ntmp_fill_crd(&req->crd, user->tbl.vft_ver, 0,
> > + NTMP_GEN_UA_CFGEU);
> > + req->ak.exact.vid = cpu_to_le16(vid);
> > + req->cfge = *cfge;
> > +
> > + /* Request header */
> > + len = NTMP_LEN(data.size, NTMP_STATUS_RESP_LEN);
> > + ntmp_fill_request_hdr(&cbd, data.dma, len, NTMP_VFT_ID,
> > + NTMP_CMD_ADD, NTMP_AM_EXACT_KEY);
> > +
> > + err = netc_xmit_ntmp_cmd(user, &cbd);
> > + if (err)
> > + dev_err(user->dev,
> > + "Failed to add %s entry, vid: %u, err: %pe\n",
> > + ntmp_table_name(NTMP_VFT_ID), vid, ERR_PTR(err));
> > +
> > + ntmp_free_data_mem(&data);
>
> Does this code safely handle hardware timeouts?
>
> If netc_xmit_ntmp_cmd() times out and returns an error, the pending
> transaction is not explicitly aborted before ntmp_free_data_mem()
> unconditionally frees the DMA buffer.
>
> Could the hardware eventually process the pending command and perform a
> DMA write of the response to the physical address of the freed buffer?
>
> This might lead to silent memory corruption if that memory has already
> been reallocated to another subsystem. Would it be safer to explicitly
> abort the pending DMA transaction, reset the device on timeout, or
> intentionally delay freeing the buffer until it is proven safe?

This is indeed a potential issue, I will add a patch to fix it.