Re: [PATCH net-next v4 2/3] net: ti: icssm-prueth: Add priority based RX IRQ handlers
From: Simon Horman <horms@kernel.org>
Date: 2026-09-25 16:39:52
Also in:
linux-arm-kernel, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev/#/patchset/20260921134343.200426-4-parvathi@couthit.com?part=2 --- Subject: net: ti: icssm-prueth: Add priority based RX IRQ handlers This commit adds support for priority-based interrupt handling for the STP/RSTP Switch, HSR, and PRP protocols to address port dominance. It records an IEP hardware timestamp into the receive buffer to guarantee frames are delivered in wire-arrival order. The patch also converts statistics counters to atomic64_t and adds a firmware revision field to support different timestamp wraparound behaviors.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c b/drivers/net/ethernet/ti/icssm/icssm_prueth.c index 6c8888d9d66a6..4a37935b8d352 100644 --- a/drivers/net/ethernet/ti/icssm/icssm_prueth.c +++ b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1071,11 +1083,25 @@ static int icssm_emac_ndo_open(struct net_device *ndev) goto iep_exit; } - ret = icssm_emac_request_irqs(emac); - if (ret) - goto rproc_shutdown; + if (PRUETH_IS_EMAC(prueth)) { + napi_enable(&emac->napi); + } else { + if (!prueth->emac_configured && + (PRUETH_IS_SWITCH(prueth) || prueth_is_lre(prueth))) { + napi_enable(&prueth->napi_hpq); + napi_enable(&prueth->napi_lpq);
[Severity: High] Does this code risk a NULL pointer dereference? If MII0 is missing (e.g., absent from the device tree), the shared NAPI structures napi_hpq and napi_lpq are not initialized in icssm_prueth_netdev_init(). If the MII1 interface is later placed into SWITCH mode and brought up, it appears icssm_emac_ndo_open() will unconditionally call napi_enable() on these zero-initialized structures, causing a NULL pointer dereference on n->dev.
+ } + }
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1780,9 +1820,25 @@ static int icssm_prueth_netdev_init(struct prueth *prueth, netif_napi_add(ndev, &emac->napi, icssm_emac_napi_poll); + if ((prueth->support_lre || fw_data->support_switch) && + emac->port_id == PRUETH_PORT_MII0) { + netif_napi_add(ndev, &prueth->napi_hpq, + icssm_prueth_common_napi_poll_hpq); + netif_napi_add(ndev, &prueth->napi_lpq, + icssm_prueth_common_napi_poll_lpq); + } + hrtimer_setup(&emac->tx_hrtimer, &icssm_emac_tx_timer_callback, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED); + if ((prueth->support_lre || fw_data->support_switch) && + emac->port_id == PRUETH_PORT_MII0) { + prueth->hp->ndev = ndev; + prueth->hp->priority = 0; + prueth->lp->ndev = ndev; + prueth->lp->priority = 1; + } + return 0;
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth_common.c b/drivers/net/ethernet/ti/icssm/icssm_prueth_common.c new file mode 100644 index 0000000000000..9b7b876240b5f --- /dev/null +++ b/drivers/net/ethernet/ti/icssm/icssm_prueth_common.c
[ ... ]
+static irqreturn_t icssm_prueth_common_emac_rx_hardirq(int irq, void *dev_id)
+{
+ struct prueth_ndev_priority *ndev_prio;
+ struct prueth_emac *emac;
+ struct net_device *ndev;
+ struct prueth *prueth;
+
+ ndev_prio = (struct prueth_ndev_priority *)dev_id;
+ ndev = ndev_prio->ndev;[Severity: High] Similarly, can this IRQ handler dereference an uninitialized pointer? If MII0 is missing and these priority structures were skipped during initialization, it looks like the registered IRQ handler will dereference the zero-initialized ndev_prio->ndev here when an interrupt occurs.
+ emac = netdev_priv(ndev); + prueth = emac->prueth;
[Severity: Medium]
This is a pre-existing issue, but can the transmission path read out of bounds?
In drivers/net/ethernet/ti/icssm/icssm_prueth.c:icssm_prueth_get_tx_queue_id():
err = vlan_get_tag(skb, &vlan_tci);
icssm_emac_ndo_start_xmit() calls icssm_prueth_get_tx_queue_id() before
validating the length of the transmitted skb or padding it. Because the device
lacks NETIF_F_HW_VLAN_CTAG_TX, vlan_get_tag() falls back to __vlan_get_tag(),
which reads the VLAN TCI at offset 14. If a raw socket transmits a packet
smaller than 18 bytes (e.g., exactly 14 bytes) with h_vlan_proto ==
ETH_P_8021Q, this reads unallocated memory past the buffer.