* [RFC PATCH net-next 0/2] net: ti: icssg: Multi-flow RX and ethtool RX flow classification
@ 2026-11-03 13:44 Meghana Malladi
2026-11-03 13:44 ` [RFC PATCH net-next 1/2] icssg: Add multi-flow RX support with per-flow NAPI and classification Meghana Malladi
2026-11-03 13:44 ` [RFC PATCH net-next 2/2] icssg: Add ethtool RX flow classification support Meghana Malladi
0 siblings, 2 replies; 5+ messages in thread
From: Meghana Malladi @ 2026-11-03 13:44 UTC (permalink / raw)
To: haokexin, philippe.schenker, devnexen, horms, jacob.e.keller,
m-malladi, sdf, john.fastabend, hawk, daniel, ast, pabeni, kuba,
edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, danishanwar
This series adds multi-flow RX support to the ICSSG ethernet driver and
exposes it via ethtool RX flow classification (rxnfc).
Patch (1/2): restructures the driver to support up to 8 independent RX flows
per EMAC interface. Each flow gets its own NAPI instance, IRQ, hrtimer,
page pool, and XDP/AF_XDP context while sharing a single UDMA channel and
descriptor pool. AF_XDP zero-copy is updated to track per-flow pools via a
bitmap, and per-flow page pool recycling is fixed by storing the flow_id in
prueth_swdata.
Patch (2/2): adds ethtool ETHTOOL_SRXCLSRLINS/SRXCLSRLDEL/SRXCLSRLALL support.
Rules match on EtherType (ETHER_FLOW) and steer matched frames to a
user-specified RX queue via ICSSG FT3 hardware content filters and the
16-entry classifier table. The classifier N result maps directly to UDMA
flow rx_flow_id_base+N, so queue steering takes effect in hardware without
per-packet CPU involvement.
Tested on AM64x EVM (SR2), both eth1 (PRU0/slice0) and eth2
(PRU1/slice1) with 8 RX flows. Verified via /proc/interrupts IRQ deltas:
ethtool -N eth2 flow-type ether proto 0x0800 action 2 → flow2 +123 frames
ethtool -N eth2 flow-type ether proto 0x0806 action 3 → flow3 +26 frames
default flow0 leakage: +2
Meghana Malladi (2):
icssg: Add multi-flow RX support with per-flow NAPI and classification
icssg: Add ethtool RX flow classification support
.../net/ethernet/ti/icssg/icssg_classifier.c | 340 +++++++++++++++++
drivers/net/ethernet/ti/icssg/icssg_common.c | 359 ++++++++++--------
drivers/net/ethernet/ti/icssg/icssg_config.h | 7 +-
drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 65 +++-
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 281 +++++++-------
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 93 +++--
.../net/ethernet/ti/icssg/icssg_prueth_sr1.c | 4 +-
7 files changed, 831 insertions(+), 318 deletions(-)
base-commit: 7b6ab4dbc156e2f96419715130b3d5738a306c93
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH net-next 1/2] icssg: Add multi-flow RX support with per-flow NAPI and classification
2026-11-03 13:44 [RFC PATCH net-next 0/2] net: ti: icssg: Multi-flow RX and ethtool RX flow classification Meghana Malladi
@ 2026-11-03 13:44 ` Meghana Malladi
2026-11-04 13:46 ` sashiko-bot
2026-11-03 13:44 ` [RFC PATCH net-next 2/2] icssg: Add ethtool RX flow classification support Meghana Malladi
1 sibling, 1 reply; 5+ messages in thread
From: Meghana Malladi @ 2026-11-03 13:44 UTC (permalink / raw)
To: haokexin, philippe.schenker, devnexen, horms, jacob.e.keller,
m-malladi, sdf, john.fastabend, hawk, daniel, ast, pabeni, kuba,
edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, danishanwar
Extend ICSSG driver to support up to 8 independent RX flows per EMAC
(PRUETH_MAX_RX_FLOWS 1→8), enabling ethtool queue steering added in the
next commit.
Data structures:
- Introduce prueth_rx_flow for per-flow state: NAPI, hrtimer, page pool,
XDP rxq, AF_XDP pool, IRQ. All flows share one UDMA channel and
descriptor pool (PRUETH_MAX_RX_DESC × flow_count).
- Restructure prueth_rx_chn: strip per-flow fields, add flows[] array.
- Add flow_id to prueth_swdata so recycled pages return to the correct
per-flow page pool (previously always used flows[0]).
- Replace xsk_qid int with xsk_queues bitmap for multi-pool AF_XDP.
Channel lifecycle:
- prueth_create_rxq / prueth_destroy_rxq operate on a single flow.
- prueth_create_rxqs / prueth_destroy_rxqs iterate all active flows and
enable the shared UDMA channel only after all flows are ready.
- prueth_ndev_add_rx_napi / prueth_ndev_del_rx_napi register per-flow
NAPI, hrtimer, and IRQ with teardown on partial failure.
ethtool: report rx_flow_num active queues; ICSSG_MAX_RFLOWS as max.
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
drivers/net/ethernet/ti/icssg/icssg_common.c | 359 ++++++++++--------
drivers/net/ethernet/ti/icssg/icssg_config.h | 7 +-
drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 17 +-
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 256 ++++++-------
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 61 ++-
.../net/ethernet/ti/icssg/icssg_prueth_sr1.c | 4 +-
6 files changed, 386 insertions(+), 318 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 4a7d1a6f470b1..99f4353b46966 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -39,16 +39,10 @@
NETIF_MSG_HW | \
NETIF_MSG_WOL)
-#define prueth_napi_to_emac(napi) container_of(napi, struct prueth_emac, napi_rx)
-void prueth_cleanup_rx_chns(struct prueth_emac *emac,
- struct prueth_rx_chn *rx_chn,
- int max_rflows)
+void prueth_cleanup_rx_chns(struct prueth_emac *emac)
{
- if (rx_chn->pg_pool) {
- page_pool_destroy(rx_chn->pg_pool);
- rx_chn->pg_pool = NULL;
- }
+ struct prueth_rx_chn *rx_chn = &emac->rx_chns;
if (rx_chn->desc_pool)
k3_cppi_desc_pool_destroy(rx_chn->desc_pool);
@@ -79,6 +73,20 @@ void prueth_cleanup_tx_chns(struct prueth_emac *emac)
}
EXPORT_SYMBOL_GPL(prueth_cleanup_tx_chns);
+void prueth_ndev_del_rx_napi(struct prueth_emac *emac, int num)
+{
+ int i;
+
+ for (i = 0; i < num; i++) {
+ struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[i];
+
+ if (rx_flow->irq)
+ free_irq(rx_flow->irq, &rx_flow->napi_rx);
+ netif_napi_del(&rx_flow->napi_rx);
+ }
+}
+EXPORT_SYMBOL_GPL(prueth_ndev_del_rx_napi);
+
void prueth_ndev_del_tx_napi(struct prueth_emac *emac, int num)
{
int i;
@@ -311,6 +319,19 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
return num_tx;
}
+static enum hrtimer_restart emac_rx_timer_callback(struct hrtimer *timer)
+{
+ struct prueth_rx_flow *rx_flow =
+ container_of(timer, struct prueth_rx_flow, rx_hrtimer);
+
+ if (rx_flow->irq_disabled) {
+ /* re-enable the RX IRQ */
+ rx_flow->irq_disabled = false;
+ enable_irq(rx_flow->irq);
+ }
+ return HRTIMER_NORESTART;
+}
+
static enum hrtimer_restart emac_tx_timer_callback(struct hrtimer *timer)
{
struct prueth_tx_chn *tx_chns =
@@ -363,6 +384,35 @@ static irqreturn_t prueth_tx_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
+int prueth_ndev_add_rx_napi(struct prueth_emac *emac)
+{
+ struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth *prueth = emac->prueth;
+ struct prueth_rx_flow *flow;
+ int i, ret;
+
+ for (i = 0; i < emac->rx_flow_num; i++) {
+ flow = &rx_chn->flows[i];
+
+ netif_napi_add(emac->ndev, &flow->napi_rx, icssg_napi_rx_poll);
+ hrtimer_setup(&flow->rx_hrtimer, &emac_rx_timer_callback, CLOCK_MONOTONIC,
+ HRTIMER_MODE_REL_PINNED);
+ ret = request_irq(emac->rx_chns.flows[i].irq, prueth_rx_irq,
+ IRQF_TRIGGER_HIGH, flow->name, &flow->napi_rx);
+ if (ret) {
+ netif_napi_del(&flow->napi_rx);
+ dev_err(prueth->dev, "unable to request RX IRQ\n");
+ goto fail;
+ }
+ }
+
+ return 0;
+fail:
+ prueth_ndev_del_rx_napi(emac, i);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(prueth_ndev_add_rx_napi);
+
int prueth_ndev_add_tx_napi(struct prueth_emac *emac)
{
struct prueth *prueth = emac->prueth;
@@ -475,7 +525,8 @@ EXPORT_SYMBOL_GPL(prueth_init_tx_chns);
static struct page_pool *prueth_create_page_pool(struct prueth_emac *emac,
struct device *dma_dev,
- int size)
+ int size,
+ struct prueth_rx_flow *rx_flow)
{
struct page_pool_params pp_params = { 0 };
struct page_pool *pool;
@@ -486,7 +537,7 @@ static struct page_pool *prueth_create_page_pool(struct prueth_emac *emac,
pp_params.nid = dev_to_node(emac->prueth->dev);
pp_params.dma_dir = DMA_BIDIRECTIONAL;
pp_params.dev = dma_dev;
- pp_params.napi = &emac->napi_rx;
+ pp_params.napi = &rx_flow->napi_rx;
pp_params.max_len = PAGE_SIZE;
pool = page_pool_create(&pp_params);
@@ -496,38 +547,30 @@ static struct page_pool *prueth_create_page_pool(struct prueth_emac *emac,
return pool;
}
-int prueth_init_rx_chns(struct prueth_emac *emac,
- struct prueth_rx_chn *rx_chn,
- char *name, u32 max_rflows,
- u32 max_desc_num)
+int prueth_init_rx_chns(struct prueth_emac *emac)
{
+ struct prueth_rx_chn *rx_chn = &emac->rx_chns;
struct k3_udma_glue_rx_channel_cfg rx_cfg;
struct device *dev = emac->prueth->dev;
struct net_device *ndev = emac->ndev;
+ struct prueth_rx_flow *flow;
u32 fdqring_id, hdesc_size;
- struct page_pool *pool;
int i, ret = 0, slice;
- int flow_id_base;
+ char udma_name[32];
slice = prueth_emac_slice(emac);
if (slice < 0)
return slice;
- /* To differentiate channels for SLICE0 vs SLICE1 */
- snprintf(rx_chn->name, sizeof(rx_chn->name), "%s%d", name, slice);
-
hdesc_size = cppi5_hdesc_calc_size(true, PRUETH_NAV_PS_DATA_SIZE,
PRUETH_NAV_SW_DATA_SIZE);
memset(&rx_cfg, 0, sizeof(rx_cfg));
rx_cfg.swdata_size = PRUETH_NAV_SW_DATA_SIZE;
- rx_cfg.flow_id_num = max_rflows;
+ rx_cfg.flow_id_num = emac->rx_flow_num;
rx_cfg.flow_id_base = -1; /* udmax will auto select flow id base */
- /* init all flows */
- rx_chn->dev = dev;
- rx_chn->descs_num = max_desc_num;
-
- rx_chn->rx_chn = k3_udma_glue_request_rx_chn(dev, rx_chn->name,
+ snprintf(udma_name, sizeof(udma_name), "%s%d", "rx", slice);
+ rx_chn->rx_chn = k3_udma_glue_request_rx_chn(dev, udma_name,
&rx_cfg);
if (IS_ERR(rx_chn->rx_chn)) {
ret = PTR_ERR(rx_chn->rx_chn);
@@ -536,11 +579,14 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
goto fail;
}
+ /* Initialize the RX channel (shared by all flows) */
+ rx_chn->emac = emac;
+ rx_chn->descs_num = PRUETH_MAX_RX_DESC * rx_cfg.flow_id_num;
rx_chn->dma_dev = k3_udma_glue_rx_get_dma_device(rx_chn->rx_chn);
rx_chn->desc_pool = k3_cppi_desc_pool_create_name(rx_chn->dma_dev,
rx_chn->descs_num,
hdesc_size,
- rx_chn->name);
+ "rx_desc_pool");
if (IS_ERR(rx_chn->desc_pool)) {
ret = PTR_ERR(rx_chn->desc_pool);
rx_chn->desc_pool = NULL;
@@ -548,22 +594,14 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
goto fail;
}
- pool = prueth_create_page_pool(emac, rx_chn->dma_dev, rx_chn->descs_num);
- if (IS_ERR(pool)) {
- ret = PTR_ERR(pool);
- goto fail;
+ for (i = 0; i < rx_cfg.flow_id_num; i++) {
+ flow = &rx_chn->flows[i];
+ flow->pg_pool = NULL;
+ flow->xsk_pool = NULL;
}
- rx_chn->pg_pool = pool;
-
- flow_id_base = k3_udma_glue_rx_get_flow_id_base(rx_chn->rx_chn);
- if (emac->is_sr1 && !strcmp(name, "rxmgm")) {
- emac->rx_mgm_flow_id_base = flow_id_base;
- netdev_dbg(ndev, "mgm flow id base = %d\n", flow_id_base);
- } else {
- emac->rx_flow_id_base = flow_id_base;
- netdev_dbg(ndev, "flow id base = %d\n", flow_id_base);
- }
+ emac->rx_flow_id_base = k3_udma_glue_rx_get_flow_id_base(rx_chn->rx_chn);
+ netdev_dbg(ndev, "flow id base = %d\n", emac->rx_flow_id_base);
fdqring_id = K3_RINGACC_RING_ID_ANY;
for (i = 0; i < rx_cfg.flow_id_num; i++) {
@@ -584,9 +622,14 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
K3_UDMA_GLUE_SRC_TAG_LO_USE_REMOTE_SRC_TAG,
};
+ flow = &rx_chn->flows[i];
+ flow->emac = emac;
+ flow->id = i;
+ flow->irq = -EINVAL;
+
rx_flow_cfg.ring_rxfdq0_id = fdqring_id;
- rx_flow_cfg.rx_cfg.size = max_desc_num;
- rx_flow_cfg.rxfdq_cfg.size = max_desc_num;
+ rx_flow_cfg.rx_cfg.size = PRUETH_MAX_RX_DESC;
+ rx_flow_cfg.rxfdq_cfg.size = PRUETH_MAX_RX_DESC;
rx_flow_cfg.rxfdq_cfg.mode = emac->prueth->pdata.fdqring_mode;
ret = k3_udma_glue_rx_flow_init(rx_chn->rx_chn,
@@ -599,25 +642,29 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
if (!i)
fdqring_id = k3_udma_glue_rx_flow_get_fdq_id(rx_chn->rx_chn,
i);
- ret = k3_udma_glue_rx_get_irq(rx_chn->rx_chn, i);
- if (ret < 0) {
+ flow->irq = k3_udma_glue_rx_get_irq(rx_chn->rx_chn, i);
+ if (flow->irq <= 0) {
+ ret = flow->irq;
netdev_err(ndev, "Failed to get rx dma irq");
goto fail;
}
- rx_chn->irq[i] = ret;
+
+ snprintf(flow->name, sizeof(flow->name),
+ "%s-rx%d-%d", dev_name(dev), slice, i);
}
return 0;
fail:
- prueth_cleanup_rx_chns(emac, rx_chn, max_rflows);
+ prueth_cleanup_rx_chns(emac);
return ret;
}
EXPORT_SYMBOL_GPL(prueth_init_rx_chns);
int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
struct prueth_rx_chn *rx_chn,
- struct page *page, u32 buf_len)
+ struct page *page, u32 buf_len,
+ int flow_id)
{
struct net_device *ndev = emac->ndev;
struct cppi5_host_desc_t *desc_rx;
@@ -641,8 +688,9 @@ int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
swdata = cppi5_hdesc_get_swdata(desc_rx);
swdata->type = PRUETH_SWDATA_PAGE;
swdata->data.page = page;
+ swdata->flow_id = flow_id;
- return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, PRUETH_RX_FLOW_DATA,
+ return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, flow_id,
desc_rx, desc_dma);
}
EXPORT_SYMBOL_GPL(prueth_dma_rx_push_mapped);
@@ -863,6 +911,7 @@ static int prueth_dma_rx_push_mapped_zc(struct prueth_emac *emac,
struct xdp_buff *xdp)
{
struct net_device *ndev = emac->ndev;
+ struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[PRUETH_RX_FLOW_DATA];
struct cppi5_host_desc_t *desc_rx;
struct prueth_swdata *swdata;
dma_addr_t desc_dma;
@@ -880,7 +929,7 @@ static int prueth_dma_rx_push_mapped_zc(struct prueth_emac *emac,
cppi5_hdesc_init(desc_rx, CPPI5_INFO0_HDESC_EPIB_PRESENT,
PRUETH_NAV_PS_DATA_SIZE);
k3_udma_glue_rx_dma_to_cppi5_addr(rx_chn->rx_chn, &buf_dma);
- buf_len = xsk_pool_get_rx_frame_size(rx_chn->xsk_pool);
+ buf_len = xsk_pool_get_rx_frame_size(rx_flow->xsk_pool);
cppi5_hdesc_attach_buf(desc_rx, buf_dma, buf_len, buf_dma, buf_len);
swdata = cppi5_hdesc_get_swdata(desc_rx);
swdata->type = PRUETH_SWDATA_XSK;
@@ -890,18 +939,18 @@ static int prueth_dma_rx_push_mapped_zc(struct prueth_emac *emac,
desc_rx, desc_dma);
}
-static int prueth_rx_alloc_zc(struct prueth_emac *emac, int budget)
+static int prueth_rx_alloc_zc(struct prueth_emac *emac, int flow_id, int budget)
{
- struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[flow_id];
struct xdp_buff *xdp;
int i, ret;
for (i = 0; i < budget; i++) {
- xdp = xsk_buff_alloc(rx_chn->xsk_pool);
+ xdp = xsk_buff_alloc(rx_flow->xsk_pool);
if (!xdp)
break;
- ret = prueth_dma_rx_push_mapped_zc(emac, rx_chn, xdp);
+ ret = prueth_dma_rx_push_mapped_zc(emac, &emac->rx_chns, xdp);
if (ret) {
netdev_err(emac->ndev, "rx alloc: failed to map descriptors to xdp buff\n");
xsk_buff_free(xdp);
@@ -912,14 +961,14 @@ static int prueth_rx_alloc_zc(struct prueth_emac *emac, int budget)
return i;
}
-static void emac_dispatch_skb_zc(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *psdata)
+static void emac_dispatch_skb_zc(struct prueth_emac *emac, struct xdp_buff *xdp, u32 *psdata, int flow_id)
{
unsigned int headroom = xdp->data - xdp->data_hard_start;
unsigned int pkt_len = xdp->data_end - xdp->data;
struct net_device *ndev = emac->ndev;
struct sk_buff *skb;
- skb = napi_alloc_skb(&emac->napi_rx, xdp->data_end - xdp->data_hard_start);
+ skb = napi_alloc_skb(&emac->rx_chns.flows[flow_id].napi_rx, xdp->data_end - xdp->data_hard_start);
if (unlikely(!skb)) {
ndev->stats.rx_dropped++;
return;
@@ -938,7 +987,7 @@ static void emac_dispatch_skb_zc(struct prueth_emac *emac, struct xdp_buff *xdp,
skb->offload_fwd_mark = emac->offload_fwd_mark;
skb->protocol = eth_type_trans(skb, ndev);
- napi_gro_receive(&emac->napi_rx, skb);
+ napi_gro_receive(&emac->rx_chns.flows[flow_id].napi_rx, skb);
ndev->stats.rx_bytes += pkt_len;
ndev->stats.rx_packets++;
}
@@ -947,6 +996,7 @@ static int emac_rx_packet_zc(struct prueth_emac *emac, u32 flow_id,
int budget)
{
struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[flow_id];
u32 buf_dma_len, pkt_len, port_id = 0;
struct net_device *ndev = emac->ndev;
struct cppi5_host_desc_t *desc_rx;
@@ -997,7 +1047,7 @@ static int emac_rx_packet_zc(struct prueth_emac *emac, u32 flow_id,
switch (ret) {
case ICSSG_XDP_PASS:
/* prepare skb and send to n/w stack */
- emac_dispatch_skb_zc(emac, xdp, psdata);
+ emac_dispatch_skb_zc(emac, xdp, psdata, flow_id);
xsk_buff_free(xdp);
break;
case ICSSG_XDP_CONSUMED:
@@ -1010,7 +1060,7 @@ static int emac_rx_packet_zc(struct prueth_emac *emac, u32 flow_id,
}
} else {
/* prepare skb and send to n/w stack */
- emac_dispatch_skb_zc(emac, xdp, psdata);
+ emac_dispatch_skb_zc(emac, xdp, psdata, flow_id);
xsk_buff_free(xdp);
}
k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
@@ -1022,13 +1072,13 @@ static int emac_rx_packet_zc(struct prueth_emac *emac, u32 flow_id,
avail_desc = k3_cppi_desc_pool_avail(rx_chn->desc_pool);
alloc_budget = min_t(int, budget, avail_desc);
- ret = prueth_rx_alloc_zc(emac, alloc_budget);
+ ret = prueth_rx_alloc_zc(emac, flow_id, alloc_budget);
- if (xsk_uses_need_wakeup(rx_chn->xsk_pool)) {
+ if (xsk_uses_need_wakeup(rx_flow->xsk_pool)) {
if (ret < alloc_budget)
- xsk_set_rx_need_wakeup(rx_chn->xsk_pool);
+ xsk_set_rx_need_wakeup(rx_flow->xsk_pool);
else
- xsk_clear_rx_need_wakeup(rx_chn->xsk_pool);
+ xsk_clear_rx_need_wakeup(rx_flow->xsk_pool);
}
return count;
@@ -1037,6 +1087,7 @@ static int emac_rx_packet_zc(struct prueth_emac *emac, u32 flow_id,
static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, u32 *xdp_state)
{
struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[flow_id];
u32 buf_dma_len, pkt_len, port_id = 0;
struct net_device *ndev = emac->ndev;
struct cppi5_host_desc_t *desc_rx;
@@ -1051,7 +1102,7 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, u32 *xdp_state)
void *pa;
*xdp_state = 0;
- pool = rx_chn->pg_pool;
+ pool = rx_flow->pg_pool;
ret = k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, flow_id, &desc_dma);
if (ret) {
if (ret != -ENODATA)
@@ -1093,7 +1144,7 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, u32 *xdp_state)
pa = page_address(page);
if (prueth_xdp_is_enabled(emac)) {
- xdp_init_buff(&xdp, PAGE_SIZE, &rx_chn->xdp_rxq);
+ xdp_init_buff(&xdp, PAGE_SIZE, &rx_flow->xdp_rxq);
xdp_prepare_buff(&xdp, pa, PRUETH_HEADROOM, pkt_len, false);
*xdp_state = emac_run_xdp(emac, &xdp, &pkt_len);
@@ -1132,15 +1183,15 @@ static int emac_rx_packet(struct prueth_emac *emac, u32 flow_id, u32 *xdp_state)
skb->protocol = eth_type_trans(skb, ndev);
skb_mark_for_recycle(skb);
- napi_gro_receive(&emac->napi_rx, skb);
+ napi_gro_receive(&emac->rx_chns.flows[flow_id].napi_rx, skb);
ndev->stats.rx_bytes += pkt_len;
ndev->stats.rx_packets++;
requeue:
k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
/* queue another RX DMA */
- ret = prueth_dma_rx_push_mapped(emac, &emac->rx_chns, new_page,
- PRUETH_MAX_PKT_SIZE);
+ ret = prueth_dma_rx_push_mapped(emac, rx_chn, new_page,
+ PRUETH_MAX_PKT_SIZE, flow_id);
if (WARN_ON(ret < 0)) {
page_pool_recycle_direct(pool, new_page);
ndev->stats.rx_errors++;
@@ -1155,19 +1206,22 @@ void prueth_rx_cleanup(void *data, dma_addr_t desc_dma)
struct prueth_rx_chn *rx_chn = data;
struct cppi5_host_desc_t *desc_rx;
struct prueth_swdata *swdata;
- struct page_pool *pool;
+ struct page_pool *pool = NULL;
struct xdp_buff *xdp;
struct page *page;
- pool = rx_chn->pg_pool;
desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
swdata = cppi5_hdesc_get_swdata(desc_rx);
- if (rx_chn->xsk_pool) {
+
+ if (swdata->type == PRUETH_SWDATA_XSK) {
xdp = swdata->data.xdp;
xsk_buff_free(xdp);
- } else {
- page = swdata->data.page;
- page_pool_recycle_direct(pool, page);
+ } else if (swdata->type == PRUETH_SWDATA_PAGE) {
+ pool = rx_chn->flows[swdata->flow_id].pg_pool;
+ if (pool) {
+ page = swdata->data.page;
+ page_pool_recycle_direct(pool, page);
+ }
}
k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
@@ -1411,11 +1465,12 @@ EXPORT_SYMBOL_GPL(prueth_tx_cleanup);
irqreturn_t prueth_rx_irq(int irq, void *dev_id)
{
- struct prueth_emac *emac = dev_id;
+ struct napi_struct *napi_rx = dev_id;
+ struct prueth_rx_flow *rx_flow = container_of(napi_rx, struct prueth_rx_flow, napi_rx);
- emac->rx_chns.irq_disabled = true;
+ rx_flow->irq_disabled = true;
disable_irq_nosync(irq);
- napi_schedule(&emac->napi_rx);
+ napi_schedule(napi_rx);
return IRQ_HANDLED;
}
@@ -1436,50 +1491,38 @@ EXPORT_SYMBOL_GPL(prueth_cleanup_tx_ts);
int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget)
{
- struct prueth_emac *emac = prueth_napi_to_emac(napi_rx);
- int rx_flow = emac->is_sr1 ?
- PRUETH_RX_FLOW_DATA_SR1 : PRUETH_RX_FLOW_DATA;
- int flow = emac->is_sr1 ?
- PRUETH_MAX_RX_FLOWS_SR1 : PRUETH_MAX_RX_FLOWS;
- struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *rx_flow = prueth_napi_to_rx_chn(napi_rx);
+ struct prueth_emac *emac = rx_flow->emac;
int xdp_state_or = 0;
int num_rx = 0;
- int cur_budget;
u32 xdp_state;
int ret;
- while (flow--) {
- if (rx_chn->xsk_pool) {
- num_rx = emac_rx_packet_zc(emac, flow, budget);
- } else {
- cur_budget = budget - num_rx;
-
- while (cur_budget--) {
- ret = emac_rx_packet(emac, flow, &xdp_state);
- xdp_state_or |= xdp_state;
- if (ret)
- break;
- num_rx++;
- }
+ if (rx_flow->xsk_pool) {
+ num_rx = emac_rx_packet_zc(emac, rx_flow->id, budget);
+ } else {
+ while (budget - num_rx > 0) {
+ ret = emac_rx_packet(emac, rx_flow->id, &xdp_state);
+ xdp_state_or |= xdp_state;
+ if (ret)
+ break;
+ num_rx++;
}
-
- if (num_rx >= budget)
- break;
}
if (xdp_state_or & ICSSG_XDP_REDIR)
xdp_do_flush();
if (num_rx < budget && napi_complete_done(napi_rx, num_rx)) {
- if (unlikely(emac->rx_pace_timeout_ns)) {
- hrtimer_start(&emac->rx_hrtimer,
- ns_to_ktime(emac->rx_pace_timeout_ns),
+ if (unlikely(rx_flow->rx_pace_timeout_ns)) {
+ hrtimer_start(&rx_flow->rx_hrtimer,
+ ns_to_ktime(rx_flow->rx_pace_timeout_ns),
HRTIMER_MODE_REL_PINNED);
} else {
- if (emac->rx_chns.irq_disabled) {
+ if (rx_flow->irq_disabled) {
/* re-enable the RX IRQ */
- emac->rx_chns.irq_disabled = false;
- enable_irq(emac->rx_chns.irq[rx_flow]);
+ rx_flow->irq_disabled = false;
+ enable_irq(rx_flow->irq);
}
}
}
@@ -1488,59 +1531,77 @@ int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget)
}
EXPORT_SYMBOL_GPL(icssg_napi_rx_poll);
-int prueth_prepare_rx_chan(struct prueth_emac *emac,
- struct prueth_rx_chn *chn,
- int buf_size)
+int prueth_prepare_rx_flow(struct prueth_emac *emac,
+ int flow)
{
+ int buf_size = PRUETH_MAX_PKT_SIZE;
+ struct prueth_rx_flow *rx_flow;
+ struct prueth_rx_chn *rx_chn;
+ struct page_pool *pool;
struct page *page;
int desc_avail;
int i, ret;
- desc_avail = k3_cppi_desc_pool_avail(chn->desc_pool);
- if (desc_avail < chn->descs_num)
- netdev_warn(emac->ndev,
- "not enough RX descriptors available %d < %d\n",
- desc_avail, chn->descs_num);
+ rx_chn = &emac->rx_chns;
+ rx_flow = &emac->rx_chns.flows[flow];
- if (chn->xsk_pool) {
- /* get pages from xsk_pool and push to RX ring
- * queue as much as possible
+ if (rx_flow->xsk_pool) {
+ /* XSK zero-copy: buffers come from the XSK fill ring, no page
+ * pool needed. Push as many XSK buffers as descriptors allow;
+ * the fill ring will be replenished by user-space.
*/
- ret = prueth_rx_alloc_zc(emac, desc_avail);
- if (!ret)
- goto recycle_alloc_pg;
- } else {
- for (i = 0; i < desc_avail; i++) {
- /* NOTE: we're not using memory efficiently here.
- * 1 full page (4KB?) used here instead of
- * PRUETH_MAX_PKT_SIZE (~1.5KB?)
- */
- page = page_pool_dev_alloc_pages(chn->pg_pool);
- if (!page) {
- netdev_err(emac->ndev, "couldn't allocate rx page\n");
- ret = -ENOMEM;
- goto recycle_alloc_pg;
- }
+ desc_avail = k3_cppi_desc_pool_avail(rx_chn->desc_pool);
+ if (desc_avail < PRUETH_MAX_RX_DESC)
+ netdev_warn(emac->ndev,
+ "not enough RX descriptors available %d < %d\n",
+ desc_avail, PRUETH_MAX_RX_DESC);
- ret = prueth_dma_rx_push_mapped(emac, chn, page, buf_size);
- if (ret < 0) {
- netdev_err(emac->ndev,
- "cannot submit page for rx chan %s ret %d\n",
- chn->name, ret);
- page_pool_recycle_direct(chn->pg_pool, page);
- goto recycle_alloc_pg;
- }
+ prueth_rx_alloc_zc(emac, flow, desc_avail);
+ return 0;
+ }
+
+ pool = prueth_create_page_pool(emac, rx_chn->dma_dev, rx_chn->descs_num, rx_flow);
+ if (IS_ERR(pool))
+ return PTR_ERR(pool);
+
+ rx_flow->pg_pool = pool;
+
+ desc_avail = k3_cppi_desc_pool_avail(rx_chn->desc_pool);
+ if (desc_avail < PRUETH_MAX_RX_DESC)
+ netdev_warn(emac->ndev,
+ "not enough RX descriptors available %d < %d\n",
+ desc_avail, PRUETH_MAX_RX_DESC);
+
+ for (i = 0; i < PRUETH_MAX_RX_DESC; ++i) {
+ page = page_pool_dev_alloc_pages(pool);
+ if (!page) {
+ netdev_err(emac->ndev, "couldn't allocate rx page\n");
+ ret = -ENOMEM;
+ goto err_free_pushed;
+ }
+
+ ret = prueth_dma_rx_push_mapped(emac, rx_chn, page, buf_size, flow);
+ if (ret < 0) {
+ netdev_err(emac->ndev,
+ "cannot submit skb for rx chan %s ret %d\n",
+ rx_flow->name, ret);
+ page_pool_recycle_direct(pool, page);
+ goto err_free_pushed;
}
}
return 0;
-recycle_alloc_pg:
- prueth_reset_rx_chan(&emac->rx_chns, PRUETH_MAX_RX_FLOWS, false);
-
+err_free_pushed:
+ /* i pages were successfully pushed to the ring. Reset the channel
+ * to drain them back and recycle via prueth_rx_cleanup before the
+ * caller destroys the page pool.
+ */
+ k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, flow, rx_chn,
+ prueth_rx_cleanup);
return ret;
}
-EXPORT_SYMBOL_GPL(prueth_prepare_rx_chan);
+EXPORT_SYMBOL_GPL(prueth_prepare_rx_flow);
void prueth_reset_tx_chan(struct prueth_emac *emac, int ch_num,
bool free_skb)
@@ -1557,16 +1618,20 @@ void prueth_reset_tx_chan(struct prueth_emac *emac, int ch_num,
}
EXPORT_SYMBOL_GPL(prueth_reset_tx_chan);
-void prueth_reset_rx_chan(struct prueth_rx_chn *chn,
- int num_flows, bool disable)
+void prueth_reset_rx_chan(struct prueth_emac *emac, bool disable)
{
int i;
+ int num_flows = emac->rx_flow_num;
+ struct prueth_rx_chn *chn;
+
+ chn = &emac->rx_chns;
for (i = 0; i < num_flows; i++)
k3_udma_glue_reset_rx_chn(chn->rx_chn, i, chn,
prueth_rx_cleanup);
+
if (disable)
- k3_udma_glue_disable_rx_chn(chn->rx_chn);
+ k3_udma_glue_disable_rx_chn(emac->rx_chns.rx_chn);
}
EXPORT_SYMBOL_GPL(prueth_reset_rx_chan);
@@ -1752,8 +1817,6 @@ void prueth_netdev_exit(struct prueth *prueth,
if (of_phy_is_fixed_link(emac->phy_node))
of_phy_deregister_fixed_link(emac->phy_node);
- netif_napi_del(&emac->napi_rx);
-
pruss_release_mem_region(prueth->pruss, &emac->dram);
free_netdev(emac->ndev);
prueth->emac[mac] = NULL;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
index 60d69744ffae2..25f5863d27028 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
@@ -23,7 +23,12 @@ struct icssg_flow_cfg {
#define PRUETH_NAV_SW_DATA_SIZE 16 /* SW related data size */
#define PRUETH_MAX_TX_DESC 512
#define PRUETH_MAX_RX_DESC 512
-#define PRUETH_MAX_RX_FLOWS 1 /* excluding default flow */
+
+/* There are 4 Tx DMA channels, but the highest priority is CH3 (thread 3)
+ * and lower three are lower priority channels or threads.
+ */
+#define PRUETH_MAX_TX_QUEUES 4
+#define PRUETH_MAX_RX_FLOWS 8
#define PRUETH_RX_FLOW_DATA 0
/* Defines for forwarding path buffer pools:
diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
index b715af21d23ac..5c2936ecd595c 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
@@ -140,13 +140,14 @@ static int emac_set_channels(struct net_device *ndev,
{
struct prueth_emac *emac = netdev_priv(ndev);
- /* Check if interface is up. Can change the num queues when
- * the interface is down.
+ /* Check if interface is up. Can't change the num queues when
+ * the interface is up.
*/
if (netif_running(emac->ndev))
return -EBUSY;
emac->tx_ch_num = ch->tx_count;
+ emac->rx_flow_num = ch->rx_count;
if (emac->is_sr1)
emac->tx_ch_num++;
@@ -159,7 +160,7 @@ static void emac_get_channels(struct net_device *ndev,
{
struct prueth_emac *emac = netdev_priv(ndev);
- ch->max_rx = 1;
+ ch->max_rx = ICSSG_MAX_RFLOWS;
ch->max_tx = PRUETH_MAX_TX_QUEUES;
/* Disable multiple TX channels due to timeouts
@@ -167,7 +168,7 @@ static void emac_get_channels(struct net_device *ndev,
if (emac->is_sr1)
ch->max_tx = 1;
- ch->rx_count = 1;
+ ch->rx_count = emac->rx_flow_num;
ch->tx_count = emac->tx_ch_num;
if (emac->is_sr1)
@@ -213,11 +214,13 @@ static int emac_get_coalesce(struct net_device *ndev,
struct netlink_ext_ack *extack)
{
struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *flow = &rx_chn->flows[0];
struct prueth_tx_chn *tx_chn;
tx_chn = &emac->tx_chns[0];
- coal->rx_coalesce_usecs = emac->rx_pace_timeout_ns / 1000;
+ coal->rx_coalesce_usecs = flow->rx_pace_timeout_ns / 1000;
coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout_ns / 1000;
return 0;
@@ -245,6 +248,8 @@ static int emac_set_coalesce(struct net_device *ndev,
struct netlink_ext_ack *extack)
{
struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *flow = &rx_chn->flows[0];
struct prueth *prueth = emac->prueth;
struct prueth_tx_chn *tx_chn;
@@ -264,7 +269,7 @@ static int emac_set_coalesce(struct net_device *ndev,
coal->tx_coalesce_usecs = ICSSG_MIN_COALESCE_USECS;
}
- emac->rx_pace_timeout_ns = coal->rx_coalesce_usecs * 1000;
+ flow->rx_pace_timeout_ns = coal->rx_coalesce_usecs * 1000;
tx_chn->tx_pace_timeout_ns = coal->tx_coalesce_usecs * 1000;
return 0;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 591be5c8056b4..8338402597b21 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -403,20 +403,6 @@ static void emac_adjust_link(struct net_device *ndev)
}
}
-static enum hrtimer_restart emac_rx_timer_callback(struct hrtimer *timer)
-{
- struct prueth_emac *emac =
- container_of(timer, struct prueth_emac, rx_hrtimer);
- int rx_flow = PRUETH_RX_FLOW_DATA;
-
- if (emac->rx_chns.irq_disabled) {
- /* re-enable the RX IRQ */
- emac->rx_chns.irq_disabled = false;
- enable_irq(emac->rx_chns.irq[rx_flow]);
- }
- return HRTIMER_NORESTART;
-}
-
static int emac_phy_connect(struct prueth_emac *emac)
{
struct prueth *prueth = emac->prueth;
@@ -587,43 +573,6 @@ const struct icss_iep_clockops prueth_iep_clockops = {
.perout_enable = prueth_perout_enable,
};
-static void prueth_destroy_xdp_rxqs(struct prueth_emac *emac)
-{
- struct xdp_rxq_info *rxq = &emac->rx_chns.xdp_rxq;
-
- if (xdp_rxq_info_is_reg(rxq))
- xdp_rxq_info_unreg(rxq);
-}
-
-static int prueth_create_xdp_rxqs(struct prueth_emac *emac)
-{
- struct xdp_rxq_info *rxq = &emac->rx_chns.xdp_rxq;
- struct page_pool *pool = emac->rx_chns.pg_pool;
- struct prueth_rx_chn *rx_chn = &emac->rx_chns;
- int ret;
-
- ret = xdp_rxq_info_reg(rxq, emac->ndev, 0, emac->napi_rx.napi_id);
- if (ret)
- return ret;
-
- if (rx_chn->xsk_pool) {
- ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_XSK_BUFF_POOL, NULL);
- if (ret)
- goto xdp_unreg;
- xsk_pool_set_rxq_info(rx_chn->xsk_pool, rxq);
- } else {
- ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_PAGE_POOL, pool);
- if (ret)
- goto xdp_unreg;
- }
-
- return 0;
-
-xdp_unreg:
- prueth_destroy_xdp_rxqs(emac);
- return ret;
-}
-
static int icssg_prueth_add_mcast(struct net_device *ndev, const u8 *addr)
{
struct net_device *real_dev;
@@ -883,14 +832,14 @@ static int icssg_update_vlan_mcast(struct net_device *vdev, int vid,
static void prueth_set_xsk_pool(struct prueth_emac *emac, u16 queue_id)
{
struct prueth_tx_chn *tx_chn = &emac->tx_chns[queue_id];
- struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[PRUETH_RX_FLOW_DATA];
- if (emac->xsk_qid != queue_id) {
- rx_chn->xsk_pool = NULL;
- tx_chn->xsk_pool = NULL;
- } else {
- rx_chn->xsk_pool = xsk_get_pool_from_qid(emac->ndev, queue_id);
+ if (test_bit(queue_id, emac->xsk_queues)) {
+ rx_flow->xsk_pool = xsk_get_pool_from_qid(emac->ndev, queue_id);
tx_chn->xsk_pool = xsk_get_pool_from_qid(emac->ndev, queue_id);
+ } else {
+ rx_flow->xsk_pool = NULL;
+ tx_chn->xsk_pool = NULL;
}
}
@@ -921,7 +870,27 @@ static void prueth_destroy_txq(struct prueth_emac *emac)
}
}
-static void prueth_destroy_rxq(struct prueth_emac *emac)
+static void prueth_destroy_rxq(struct prueth_emac *emac, int qid)
+{
+ struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *flow = &rx_chn->flows[qid];
+
+ /* reset napi and hrtimer, and free all pending buffers */
+ napi_disable(&flow->napi_rx);
+ hrtimer_cancel(&flow->rx_hrtimer);
+ k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, flow->id, rx_chn,
+ prueth_rx_cleanup);
+
+ if (xdp_rxq_info_is_reg(&flow->xdp_rxq))
+ xdp_rxq_info_unreg(&flow->xdp_rxq);
+
+ if (flow->pg_pool) {
+ page_pool_destroy(flow->pg_pool);
+ flow->pg_pool = NULL;
+ }
+}
+
+static void prueth_destroy_rxqs(struct prueth_emac *emac)
{
int i, ret;
@@ -936,9 +905,13 @@ static void prueth_destroy_rxq(struct prueth_emac *emac)
* RX NAPI Handler. To avoid timing out when waiting for the TDCM to
* be popped, schedule the RX NAPI handler to run immediately.
*/
- if (!napi_if_scheduled_mark_missed(&emac->napi_rx)) {
- if (napi_schedule_prep(&emac->napi_rx))
- __napi_schedule(&emac->napi_rx);
+ for (int i = 0; i < emac->rx_flow_num; i++) {
+ if (test_bit(i, emac->xsk_queues)) {
+ if (!napi_if_scheduled_mark_missed(&emac->rx_chns.flows[i].napi_rx)) {
+ if (napi_schedule_prep(&emac->rx_chns.flows[i].napi_rx))
+ __napi_schedule(&emac->rx_chns.flows[i].napi_rx);
+ }
+ }
}
ret = wait_for_completion_timeout(&emac->tdown_complete,
@@ -946,15 +919,9 @@ static void prueth_destroy_rxq(struct prueth_emac *emac)
if (!ret)
netdev_err(emac->ndev, "rx teardown timeout\n");
- for (i = 0; i < PRUETH_MAX_RX_FLOWS; i++) {
- napi_disable(&emac->napi_rx);
- hrtimer_cancel(&emac->rx_hrtimer);
- k3_udma_glue_reset_rx_chn(emac->rx_chns.rx_chn, i,
- &emac->rx_chns,
- prueth_rx_cleanup);
- }
+ for (i = 0; i < emac->rx_flow_num; i++)
+ prueth_destroy_rxq(emac, i);
- prueth_destroy_xdp_rxqs(emac);
k3_udma_glue_disable_rx_chn(emac->rx_chns.rx_chn);
}
@@ -978,27 +945,62 @@ static int prueth_create_txq(struct prueth_emac *emac)
return ret;
}
-static int prueth_create_rxq(struct prueth_emac *emac)
+static int prueth_create_rxq(struct prueth_emac *emac, int id)
{
+ struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *flow = &rx_chn->flows[id];
+ struct xdp_rxq_info *rxq = &flow->xdp_rxq;
int ret;
- ret = prueth_prepare_rx_chan(emac, &emac->rx_chns, PRUETH_MAX_PKT_SIZE);
+ ret = prueth_prepare_rx_flow(emac, id);
if (ret)
- return ret;
+ goto err;
- ret = k3_udma_glue_enable_rx_chn(emac->rx_chns.rx_chn);
+ ret = xdp_rxq_info_reg(rxq, emac->ndev, 0, flow->napi_rx.napi_id);
if (ret)
- goto reset_rx_chn;
+ goto err;
- ret = prueth_create_xdp_rxqs(emac);
- if (ret)
- goto reset_rx_chn;
+ if (flow->xsk_pool) {
+ ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_XSK_BUFF_POOL, NULL);
+ if (ret)
+ goto err;
+ xsk_pool_set_rxq_info(flow->xsk_pool, rxq);
+ } else {
+ ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_PAGE_POOL, flow->pg_pool);
+ if (ret)
+ goto err;
+ }
+
+ napi_enable(&flow->napi_rx);
+ return 0;
+
+err:
+ prueth_destroy_rxq(emac, id);
+ return ret;
+}
+
+static int prueth_create_rxqs(struct prueth_emac *emac)
+{
+ int i, ret;
- napi_enable(&emac->napi_rx);
+ for (i = 0; i < emac->rx_flow_num; i++) {
+ ret = prueth_create_rxq(emac, i);
+ if (ret) {
+ netdev_err(emac->ndev, "failed to create rxq for flow %d: %d\n",
+ i, ret);
+ goto err;
+ }
+ }
+
+ /* All flows share the same DMA channel */
+ ret = k3_udma_glue_enable_rx_chn(emac->rx_chns.rx_chn);
+ if (ret)
+ goto err;
return 0;
-reset_rx_chn:
- prueth_reset_rx_chan(&emac->rx_chns, PRUETH_MAX_RX_FLOWS, false);
+err:
+ for (--i; i >= 0; i--)
+ prueth_destroy_rxq(emac, i);
return ret;
}
@@ -1018,8 +1020,6 @@ static int emac_ndo_open(struct net_device *ndev)
struct prueth *prueth = emac->prueth;
int slice = prueth_emac_slice(emac);
struct device *dev = prueth->dev;
- int max_rx_flows;
- int rx_flow;
/* set h/w MAC as user might have re-configured */
ether_addr_copy(emac->mac_addr, ndev->dev_addr);
@@ -1034,7 +1034,6 @@ static int emac_ndo_open(struct net_device *ndev)
return ret;
}
- emac->xsk_qid = -EINVAL;
init_completion(&emac->cmd_complete);
ret = prueth_init_tx_chns(emac);
if (ret) {
@@ -1042,9 +1041,7 @@ static int emac_ndo_open(struct net_device *ndev)
return ret;
}
- max_rx_flows = PRUETH_MAX_RX_FLOWS;
- ret = prueth_init_rx_chns(emac, &emac->rx_chns, "rx",
- max_rx_flows, PRUETH_MAX_RX_DESC);
+ ret = prueth_init_rx_chns(emac);
if (ret) {
dev_err(dev, "failed to init rx channel: %d\n", ret);
goto cleanup_tx;
@@ -1054,14 +1051,9 @@ static int emac_ndo_open(struct net_device *ndev)
if (ret)
goto cleanup_rx;
- /* we use only the highest priority flow for now i.e. @irq[3] */
- rx_flow = PRUETH_RX_FLOW_DATA;
- ret = request_irq(emac->rx_chns.irq[rx_flow], prueth_rx_irq,
- IRQF_TRIGGER_HIGH, dev_name(dev), emac);
- if (ret) {
- dev_err(dev, "unable to request RX IRQ\n");
+ ret = prueth_ndev_add_rx_napi(emac);
+ if (ret)
goto cleanup_napi;
- }
if (!prueth->emacs_initialized) {
ret = prueth_emac_common_start(prueth);
@@ -1087,7 +1079,7 @@ static int emac_ndo_open(struct net_device *ndev)
goto stop;
/* Prepare RX */
- ret = prueth_create_rxq(emac);
+ ret = prueth_create_rxqs(emac);
if (ret)
goto free_tx_ts_irq;
@@ -1105,18 +1097,18 @@ static int emac_ndo_open(struct net_device *ndev)
return 0;
destroy_rxq:
- prueth_destroy_rxq(emac);
+ prueth_destroy_rxqs(emac);
free_tx_ts_irq:
free_irq(emac->tx_ts_irq, emac);
stop:
if (!prueth->emacs_initialized)
prueth_emac_common_stop(prueth);
free_rx_irq:
- free_irq(emac->rx_chns.irq[rx_flow], emac);
+ prueth_ndev_del_rx_napi(emac, emac->rx_flow_num);
cleanup_napi:
prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
cleanup_rx:
- prueth_cleanup_rx_chns(emac, &emac->rx_chns, max_rx_flows);
+ prueth_cleanup_rx_chns(emac);
cleanup_tx:
prueth_cleanup_tx_chns(emac);
@@ -1149,7 +1141,7 @@ static int emac_ndo_stop(struct net_device *ndev)
__dev_mc_unsync(ndev, icssg_prueth_del_mcast);
prueth_destroy_txq(emac);
- prueth_destroy_rxq(emac);
+ prueth_destroy_rxqs(emac);
cancel_work_sync(&emac->rx_mode_work);
@@ -1162,10 +1154,10 @@ static int emac_ndo_stop(struct net_device *ndev)
free_irq(emac->tx_ts_irq, emac);
- free_irq(emac->rx_chns.irq[PRUETH_RX_FLOW_DATA], emac);
+ prueth_ndev_del_rx_napi(emac, emac->rx_flow_num);
prueth_ndev_del_tx_napi(emac, emac->tx_ch_num);
- prueth_cleanup_rx_chns(emac, &emac->rx_chns, PRUETH_MAX_RX_FLOWS);
+ prueth_cleanup_rx_chns(emac);
prueth_cleanup_tx_chns(emac);
prueth->emacs_initialized--;
@@ -1467,7 +1459,6 @@ static int emac_xdp_setup(struct prueth_emac *emac, struct netdev_bpf *bpf)
static int prueth_xsk_pool_enable(struct prueth_emac *emac,
struct xsk_buff_pool *pool, u16 queue_id)
{
- struct prueth_rx_chn *rx_chn = &emac->rx_chns;
u32 frame_size;
int ret;
@@ -1481,7 +1472,7 @@ static int prueth_xsk_pool_enable(struct prueth_emac *emac,
if (frame_size < PRUETH_MAX_PKT_SIZE)
return -EOPNOTSUPP;
- ret = xsk_pool_dma_map(pool, rx_chn->dma_dev, PRUETH_RX_DMA_ATTR);
+ ret = xsk_pool_dma_map(pool, emac->rx_chns.dma_dev, PRUETH_RX_DMA_ATTR);
if (ret) {
netdev_err(emac->ndev, "Failed to map XSK pool: %d\n", ret);
return ret;
@@ -1492,21 +1483,21 @@ static int prueth_xsk_pool_enable(struct prueth_emac *emac,
ret = icssg_set_port_state(emac, ICSSG_EMAC_PORT_DISABLE);
if (ret)
return ret;
- prueth_destroy_rxq(emac);
+ prueth_destroy_rxq(emac, queue_id);
}
- emac->xsk_qid = queue_id;
+ set_bit(queue_id, emac->xsk_queues);
prueth_set_xsk_pool(emac, queue_id);
if (netif_running(emac->ndev)) {
- ret = prueth_create_rxq(emac);
+ ret = prueth_create_rxq(emac, queue_id);
if (ret) {
netdev_err(emac->ndev, "Failed to create RX queue: %d\n", ret);
return ret;
}
ret = icssg_set_port_state(emac, ICSSG_EMAC_PORT_FORWARD);
if (ret) {
- prueth_destroy_rxq(emac);
+ prueth_destroy_rxq(emac, queue_id);
return ret;
}
ret = prueth_xsk_wakeup(emac->ndev, queue_id, XDP_WAKEUP_RX);
@@ -1528,7 +1519,7 @@ static int prueth_xsk_pool_disable(struct prueth_emac *emac, u16 queue_id)
return -EINVAL;
}
- if (emac->xsk_qid != queue_id) {
+ if (!test_bit(queue_id, emac->xsk_queues)) {
netdev_err(emac->ndev, "XSK queue ID %d not registered\n", queue_id);
return -EINVAL;
}
@@ -1544,27 +1535,28 @@ static int prueth_xsk_pool_disable(struct prueth_emac *emac, u16 queue_id)
ret = icssg_set_port_state(emac, ICSSG_EMAC_PORT_DISABLE);
if (ret)
return ret;
- prueth_destroy_rxq(emac);
+ prueth_destroy_rxq(emac, queue_id);
}
xsk_pool_dma_unmap(pool, PRUETH_RX_DMA_ATTR);
- emac->xsk_qid = -EINVAL;
+ clear_bit(queue_id, emac->xsk_queues);
prueth_set_xsk_pool(emac, queue_id);
if (netif_running(emac->ndev)) {
- ret = prueth_create_rxq(emac);
- if (ret) {
- netdev_err(emac->ndev, "Failed to create RX queue: %d\n", ret);
- return ret;
- }
- ret = icssg_set_port_state(emac, ICSSG_EMAC_PORT_FORWARD);
- if (ret) {
- prueth_destroy_rxq(emac);
- return ret;
- }
+ ret = prueth_create_rxq(emac, queue_id);
+ if (ret)
+ netdev_err(emac->ndev, "Failed to recreate RX queue after XSK disable: %d\n",
+ ret);
+
+ /* Always re-enable port forwarding so the interface is not left
+ * permanently disabled if prueth_create_rxq() failed due to a
+ * transient descriptor shortage (inflight pages not yet returned
+ * by the network stack).
+ */
+ icssg_set_port_state(emac, ICSSG_EMAC_PORT_FORWARD);
}
- return 0;
+ return ret;
}
/**
@@ -1594,9 +1586,9 @@ int prueth_xsk_wakeup(struct net_device *ndev, u32 qid, u32 flags)
{
struct prueth_emac *emac = netdev_priv(ndev);
struct prueth_tx_chn *tx_chn = &emac->tx_chns[qid];
- struct prueth_rx_chn *rx_chn = &emac->rx_chns;
+ struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[PRUETH_RX_FLOW_DATA];
- if (emac->xsk_qid != qid) {
+ if (!test_bit(qid, emac->xsk_queues)) {
netdev_err(ndev, "XSK queue %d not registered\n", qid);
return -EINVAL;
}
@@ -1611,7 +1603,7 @@ int prueth_xsk_wakeup(struct net_device *ndev, u32 qid, u32 flags)
return -EINVAL;
}
- if (!rx_chn->xsk_pool) {
+ if (!rx_flow->xsk_pool) {
netdev_err(ndev, "XSK pool not registered for RX queue %d\n", qid);
return -EINVAL;
}
@@ -1624,9 +1616,9 @@ int prueth_xsk_wakeup(struct net_device *ndev, u32 qid, u32 flags)
}
if (flags & XDP_WAKEUP_RX) {
- if (!napi_if_scheduled_mark_missed(&emac->napi_rx)) {
- if (likely(napi_schedule_prep(&emac->napi_rx)))
- __napi_schedule(&emac->napi_rx);
+ if (!napi_if_scheduled_mark_missed(&rx_flow->napi_rx)) {
+ if (likely(napi_schedule_prep(&rx_flow->napi_rx)))
+ __napi_schedule(&rx_flow->napi_rx);
}
}
@@ -1697,6 +1689,14 @@ static int prueth_netdev_init(struct prueth *prueth,
}
emac->tx_ch_num = 1;
+ emac->rx_flow_num = 1;
+ emac->xsk_queues = devm_bitmap_zalloc(prueth->dev,
+ PRUETH_MAX_RX_FLOWS, GFP_KERNEL);
+ if (!emac->xsk_queues) {
+ dev_err(prueth->dev, "failed to allocate xsk_queues bitmap\n");
+ ret = -ENOMEM;
+ goto free_ndev;
+ }
irq_name = "tx_ts0";
if (emac->port_id == PRUETH_PORT_MII1)
@@ -1783,11 +1783,7 @@ static int prueth_netdev_init(struct prueth *prueth,
NETDEV_XDP_ACT_NDO_XMIT |
NETDEV_XDP_ACT_XSK_ZEROCOPY);
- netif_napi_add(ndev, &emac->napi_rx, icssg_napi_rx_poll);
- hrtimer_setup(&emac->rx_hrtimer, &emac_rx_timer_callback, CLOCK_MONOTONIC,
- HRTIMER_MODE_REL_PINNED);
prueth->emac[mac] = emac;
-
return 0;
free:
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index df93d15c5b786..509b9cbda58cf 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -132,18 +132,27 @@ struct prueth_tx_chn {
bool irq_disabled;
};
+struct prueth_rx_flow {
+ u32 id;
+ struct prueth_emac *emac;
+ struct napi_struct napi_rx;
+ unsigned int irq;
+ bool irq_disabled;
+ struct hrtimer rx_hrtimer;
+ unsigned long rx_pace_timeout_ns;
+ struct page_pool *pg_pool;
+ struct xdp_rxq_info xdp_rxq;
+ struct xsk_buff_pool *xsk_pool;
+ char name[32];
+};
+
struct prueth_rx_chn {
- struct device *dev;
struct device *dma_dev;
struct k3_cppi_desc_pool *desc_pool;
struct k3_udma_glue_rx_channel *rx_chn;
+ struct prueth_emac *emac;
u32 descs_num;
- unsigned int irq[ICSSG_MAX_RFLOWS]; /* separate irq per flow */
- char name[32];
- struct page_pool *pg_pool;
- struct xdp_rxq_info xdp_rxq;
- struct xsk_buff_pool *xsk_pool;
- bool irq_disabled;
+ struct prueth_rx_flow flows[PRUETH_MAX_RX_FLOWS];
};
enum prueth_swdata_type {
@@ -162,6 +171,7 @@ enum prueth_tx_buff_type {
struct prueth_swdata {
enum prueth_swdata_type type;
+ u32 flow_id;
union prueth_data {
struct sk_buff *skb;
struct page *page;
@@ -171,11 +181,6 @@ struct prueth_swdata {
} data;
};
-/* There are 4 Tx DMA channels, but the highest priority is CH3 (thread 3)
- * and lower three are lower priority channels or threads.
- */
-#define PRUETH_MAX_TX_QUEUES 4
-
#define PRUETH_MAX_TX_TS_REQUESTS 50 /* Max simultaneous TX_TS requests */
/* XDP BPF state */
@@ -193,7 +198,6 @@ struct prueth_emac {
struct prueth *prueth;
struct net_device *ndev;
u8 mac_addr[6];
- struct napi_struct napi_rx;
u32 msg_enable;
int link;
@@ -216,6 +220,7 @@ struct prueth_emac {
struct prueth_rx_chn rx_chns;
int rx_flow_id_base;
int tx_ch_num;
+ int rx_flow_num;
/* SR1.0 Management channel */
struct prueth_rx_chn rx_mgm_chn;
@@ -246,14 +251,10 @@ struct prueth_emac {
u64 stats[ICSSG_NUM_MIIG_STATS];
u64 pa_stats[ICSSG_NUM_PA_STATS];
- /* RX IRQ Coalescing Related */
- struct hrtimer rx_hrtimer;
- unsigned long rx_pace_timeout_ns;
-
struct netdev_hw_addr_list vlan_mcast_list[MAX_VLAN_ID];
struct bpf_prog *xdp_prog;
struct xdp_attachment_info xdpi;
- int xsk_qid;
+ unsigned long *xsk_queues;
};
/* The buf includes headroom compatible with both skb and xdpf */
@@ -455,30 +456,30 @@ void icssg_set_pvid(struct prueth *prueth, u8 vid, u8 port);
int emac_fdb_flow_id_updated(struct prueth_emac *emac);
#define prueth_napi_to_tx_chn(pnapi) \
container_of(pnapi, struct prueth_tx_chn, napi_tx)
+#define prueth_napi_to_rx_chn(pnapi) \
+ container_of(pnapi, struct prueth_rx_flow, napi_rx)
void icssg_stats_work_handler(struct work_struct *work);
void emac_update_hardware_stats(struct prueth_emac *emac);
int emac_get_stat_by_name(struct prueth_emac *emac, char *stat_name);
/* Common functions */
-void prueth_cleanup_rx_chns(struct prueth_emac *emac,
- struct prueth_rx_chn *rx_chn,
- int max_rflows);
+void prueth_cleanup_rx_chns(struct prueth_emac *emac);
void prueth_cleanup_tx_chns(struct prueth_emac *emac);
void prueth_ndev_del_tx_napi(struct prueth_emac *emac, int num);
+void prueth_ndev_del_rx_napi(struct prueth_emac *emac, int num);
void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
struct cppi5_host_desc_t *desc);
int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
int budget, bool *tdown);
int prueth_ndev_add_tx_napi(struct prueth_emac *emac);
+int prueth_ndev_add_rx_napi(struct prueth_emac *emac);
int prueth_init_tx_chns(struct prueth_emac *emac);
-int prueth_init_rx_chns(struct prueth_emac *emac,
- struct prueth_rx_chn *rx_chn,
- char *name, u32 max_rflows,
- u32 max_desc_num);
+int prueth_init_rx_chns(struct prueth_emac *emac);
int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
struct prueth_rx_chn *rx_chn,
- struct page *page, u32 buf_len);
+ struct page *page, u32 buf_len,
+ int flow_id);
unsigned int prueth_rxbuf_total_len(unsigned int len);
void emac_rx_timestamp(struct prueth_emac *emac,
struct sk_buff *skb, u32 *psdata);
@@ -486,13 +487,11 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
irqreturn_t prueth_rx_irq(int irq, void *dev_id);
void prueth_cleanup_tx_ts(struct prueth_emac *emac);
int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget);
-int prueth_prepare_rx_chan(struct prueth_emac *emac,
- struct prueth_rx_chn *chn,
- int buf_size);
+int prueth_prepare_rx_flow(struct prueth_emac *emac,
+ int flow);
void prueth_reset_tx_chan(struct prueth_emac *emac, int ch_num,
bool free_skb);
-void prueth_reset_rx_chan(struct prueth_rx_chn *chn,
- int num_flows, bool disable);
+void prueth_reset_rx_chan(struct prueth_emac *emac, bool disable);
void icssg_ndo_tx_timeout(struct net_device *ndev, unsigned int txqueue);
int icssg_ndo_get_ts_config(struct net_device *ndev,
struct kernel_hwtstamp_config *config);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
index b8115ca47082f..582d582cba89f 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
@@ -570,11 +570,11 @@ static int emac_ndo_open(struct net_device *ndev)
icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
/* Prepare RX */
- ret = prueth_prepare_rx_chan(emac, &emac->rx_chns, PRUETH_MAX_PKT_SIZE);
+ ret = prueth_prepare_rx_flow(emac, &emac->rx_chns, PRUETH_MAX_PKT_SIZE);
if (ret)
goto stop;
- ret = prueth_prepare_rx_chan(emac, &emac->rx_mgm_chn, 64);
+ ret = prueth_prepare_rx_flow(emac, &emac->rx_mgm_chn, 64);
if (ret)
goto reset_rx_chn;
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC PATCH net-next 2/2] icssg: Add ethtool RX flow classification support
2026-11-03 13:44 [RFC PATCH net-next 0/2] net: ti: icssg: Multi-flow RX and ethtool RX flow classification Meghana Malladi
2026-11-03 13:44 ` [RFC PATCH net-next 1/2] icssg: Add multi-flow RX support with per-flow NAPI and classification Meghana Malladi
@ 2026-11-03 13:44 ` Meghana Malladi
2026-11-04 13:46 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: Meghana Malladi @ 2026-11-03 13:44 UTC (permalink / raw)
To: haokexin, philippe.schenker, devnexen, horms, jacob.e.keller,
m-malladi, sdf, john.fastabend, hawk, daniel, ast, pabeni, kuba,
edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, danishanwar
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="yes", Size: 17997 bytes --]
Enable dynamic RX flow classification via ethtool set_rxnfc/get_rxnfc.
Users can install/remove filter rules to steer packets to specific RX flows
based on EtherType patterns via FT3 hardware filters.
- Add RXNFC rule tracking structures and lifecycle management
- Implement ethtool get_rxnfc/set_rxnfc callbacks
- FT3 slot allocation and hardware programming for pattern matching
- RX classifier OR table updates based on active rules
- Max 15 rules per EMAC, using FT3 slots 0-13,15 (slot 14 reserved)
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
.../net/ethernet/ti/icssg/icssg_classifier.c | 340 ++++++++++++++++++
drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 48 +++
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 27 +-
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 32 ++
4 files changed, 446 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_classifier.c b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
index 87a8577093dbb..f11539611df75 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_classifier.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
@@ -105,6 +105,14 @@ enum ft1_cfg_type {
((BIT(slot) << RX_CLASS_FT_FT1_MATCH_SHIFT) & \
RX_CLASS_FT_FT1_MATCH_MASK)
+#define RX_CLASS_FT_FT3_MATCH(slot) \
+ ((BIT(slot) << RX_CLASS_FT_FT3_MATCH_SHIFT) & \
+ RX_CLASS_FT_FT3_MATCH_MASK)
+
+/* FT3 Configuration registers and modes */
+#define FT3_CFG_REGISTER_EQUAL (1U)
+#define FT3_CFG_REGISTER_VLAN_SKIP BIT(2)
+
/* HSR/PRP protocol frame filtering */
#define HSR_PTP_ETHERTYPE_OFFSET 0x12
#define PRP_PTP_ETHERTYPE_OFFSET 0x0C
@@ -532,3 +540,335 @@ void icssg_ft3_hsr_configurations(struct regmap *miig_rt, int slice,
}
}
EXPORT_SYMBOL_GPL(icssg_ft3_hsr_configurations);
+
+/* RX Flow Classification (ethtool) rule management */
+
+/* Allocate free FT3 slot, avoiding reserved slot 14 */
+static int allocate_ft3_slot(struct prueth_emac *emac)
+{
+ u16 used = emac->rxnfc.ft3_slots_used | ICSSG_FT3_RESERVED_SLOTS;
+ int slot;
+
+ for (slot = 0; slot < ICSSG_NUM_FT3_SLOTS; slot++) {
+ if (!(used & BIT(slot))) {
+ emac->rxnfc.ft3_slots_used |= BIT(slot);
+ return slot;
+ }
+ }
+ return -ENOSPC;
+}
+
+/* Free allocated FT3 slot */
+static void free_ft3_slot(struct prueth_emac *emac, int slot)
+{
+ emac->rxnfc.ft3_slots_used &= ~BIT(slot);
+}
+
+/* Program FT3 filter slot with ethertype pattern */
+static void icssg_ft3_program_filter(struct regmap *miig_rt, int slice,
+ int slot, struct icssg_rxnfc_rule *rule)
+{
+ const struct ethtool_rx_flow_spec *fs = &rule->flow_spec;
+ u16 ether_type;
+
+ /* Extract EtherType from flow spec (network byte order) */
+ ether_type = fs->h_u.ether_spec.h_proto;
+
+ /* Deactivate filter before programming */
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_CFG), 0);
+
+ /* EtherType is at byte 12 in standard Ethernet frames */
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_START),
+ PRP_PTP_ETHERTYPE_OFFSET);
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_START_AUTO), 0);
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_START_OFFSET), 0);
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_JUMP_OFFSET), 0);
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_LEN), 0);
+
+ /* FT3 compares a 32-bit LE word at FT3_START. EtherType occupies the
+ * lower 16 bits of that word. FT3_T_MASK=0xFFFF0000 masks the upper
+ * 16 bits (payload bytes), so only the lower 16 bits are compared.
+ *
+ * h_proto is __be16 (network byte order). On LE the raw integer value
+ * equals the lower 16 bits of the LE 32-bit word at the EtherType
+ * offset in the frame — e.g. IPv4 0x0800 BE → h_proto = 0x0008 on LE,
+ * which is exactly what FT3 sees in the lower 16 bits of the word at
+ * byte 12. No conversion needed; using be16_to_cpu() here would invert
+ * the byte order and break the match.
+ * Reference: FT3_PTP_PATTERN = 0xF788 = htons(0x88F7) = raw h_proto value.
+ */
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_T),
+ (__force u32)ether_type);
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_T_MASK), FT3_ETHERTYPE_MASK);
+
+ /* Enable filter: register-equal mode */
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_CFG),
+ FT3_CFG_REGISTER_EQUAL);
+}
+
+/* Disable FT3 filter slot */
+static void icssg_ft3_disable_filter(struct regmap *miig_rt, int slice, int slot)
+{
+ regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_CFG), 0);
+}
+
+/* Update RX classifiers for all active FT3 rules.
+ * Classifier N maps to RX flow N (firmware routes classifier-N hits to
+ * UDMA flow rx_flow_id_base + N). Build a per-classifier OR table so
+ * that each target queue gets its own classifier entry.
+ */
+static void icssg_rxnfc_classifier_update(struct prueth_emac *emac)
+{
+ struct prueth *prueth = emac->prueth;
+ struct regmap *miig_rt = prueth->miig_rt;
+ int slice = prueth_emac_slice(emac);
+ u32 or_table[ICSSG_NUM_CLASSIFIERS] = { };
+ struct icssg_rxnfc_rule *rule;
+ int n;
+
+ if (slice < 0)
+ return;
+
+ /* Build per-classifier OR table: classifier index = target queue index */
+ list_for_each_entry(rule, &emac->rxnfc.rules, list) {
+ int cls = rule->queue_index;
+
+ if (cls < ICSSG_NUM_CLASSIFIERS)
+ or_table[cls] |= RX_CLASS_FT_FT3_MATCH(rule->ft3_slot);
+ }
+
+ /* Write OR tables for user-rule classifiers; skip classifier 0 (default flow).
+ * Use RX_CLASS_N_REG with RX_CLASS_OR_EN — NOT RX_CLASS_OR_REG which uses
+ * rx_class_or_base (= rx_class_base + 4), causing an off-by-4 that lands on
+ * AND_EN of classifier n+1 instead of OR_EN of classifier n.
+ */
+ for (n = 1; n < ICSSG_NUM_CLASSIFIERS; n++) {
+ regmap_write(miig_rt,
+ RX_CLASS_N_REG(slice, n, RX_CLASS_OR_EN),
+ or_table[n]);
+ }
+}
+
+/* Validate rule parameters */
+static int validate_rule(struct prueth_emac *emac,
+ struct ethtool_rx_flow_spec *fs)
+{
+ /* Only ETHER_FLOW supported for Phase 1 */
+ if (fs->flow_type != ETHER_FLOW)
+ return -EOPNOTSUPP;
+
+ /* Check RX queue is valid */
+ if (fs->ring_cookie >= emac->rx_flow_num)
+ return -EINVAL;
+
+ return 0;
+}
+
+/* Check if identical rule already exists */
+static bool rule_exists(struct prueth_emac *emac,
+ struct ethtool_rx_flow_spec *fs)
+{
+ struct icssg_rxnfc_rule *rule;
+
+ list_for_each_entry(rule, &emac->rxnfc.rules, list) {
+ if (rule->flow_spec.flow_type == fs->flow_type &&
+ rule->flow_spec.h_u.ether_spec.h_proto ==
+ fs->h_u.ether_spec.h_proto) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/* Initialize RX flow classification */
+void icssg_rxnfc_rule_init(struct prueth_emac *emac)
+{
+ INIT_LIST_HEAD(&emac->rxnfc.rules);
+ mutex_init(&emac->rxnfc.rxnfc_lock);
+ emac->rxnfc.rxnfc_rule_count = 0;
+ emac->rxnfc.ft3_slots_used = ICSSG_FT3_RESERVED_SLOTS; /* Mark slot 14 reserved */
+}
+EXPORT_SYMBOL_GPL(icssg_rxnfc_rule_init);
+
+/* Cleanup RX flow classification */
+void icssg_rxnfc_rule_cleanup(struct prueth_emac *emac)
+{
+ struct prueth *prueth = emac->prueth;
+ struct regmap *miig_rt = prueth->miig_rt;
+ int slice = prueth_emac_slice(emac);
+ struct icssg_rxnfc_rule *rule, *tmp;
+ int n;
+
+ if (slice < 0)
+ return;
+
+ mutex_lock(&emac->rxnfc.rxnfc_lock);
+
+ /* Disable all active FT3 filters */
+ list_for_each_entry_safe(rule, tmp, &emac->rxnfc.rules, list) {
+ icssg_ft3_disable_filter(miig_rt, slice, rule->ft3_slot);
+ free_ft3_slot(emac, rule->ft3_slot);
+ list_del(&rule->list);
+ kfree(rule);
+ }
+
+ emac->rxnfc.rxnfc_rule_count = 0;
+ emac->rxnfc.ft3_slots_used = ICSSG_FT3_RESERVED_SLOTS;
+
+ /* Clear all user-rule classifier OR tables (classifier 0 = default, skip) */
+ for (n = 1; n < ICSSG_NUM_CLASSIFIERS; n++)
+ regmap_write(miig_rt,
+ RX_CLASS_N_REG(slice, n, RX_CLASS_OR_EN), 0);
+
+ mutex_unlock(&emac->rxnfc.rxnfc_lock);
+}
+EXPORT_SYMBOL_GPL(icssg_rxnfc_rule_cleanup);
+
+/* Add a new RX flow classification rule */
+int icssg_rxnfc_rule_add(struct prueth_emac *emac, struct ethtool_rxnfc *info)
+{
+ struct ethtool_rx_flow_spec *fs = &info->fs;
+ struct prueth *prueth = emac->prueth;
+ struct regmap *miig_rt = prueth->miig_rt;
+ int slice = prueth_emac_slice(emac);
+ struct icssg_rxnfc_rule *rule;
+ int ft3_slot;
+ int ret;
+
+ if (slice < 0)
+ return -EINVAL;
+
+ /* Validate rule parameters */
+ ret = validate_rule(emac, fs);
+ if (ret)
+ return ret;
+
+ mutex_lock(&emac->rxnfc.rxnfc_lock);
+
+ /* Check max rules */
+ if (emac->rxnfc.rxnfc_rule_count >= ICSSG_RXNFC_MAX_RULES) {
+ ret = -ENOSPC;
+ goto unlock;
+ }
+
+ /* Check for duplicate */
+ if (rule_exists(emac, fs)) {
+ ret = -EEXIST;
+ goto unlock;
+ }
+
+ /* Allocate FT3 slot */
+ ft3_slot = allocate_ft3_slot(emac);
+ if (ft3_slot < 0) {
+ ret = -ENOSPC;
+ goto unlock;
+ }
+
+ /* Auto-assign location from FT3 slot if RX_CLS_LOC_ANY requested */
+ if (fs->location == RX_CLS_LOC_ANY)
+ fs->location = ft3_slot;
+
+ /* Allocate rule structure */
+ rule = kzalloc(sizeof(*rule), GFP_KERNEL);
+ if (!rule) {
+ ret = -ENOMEM;
+ free_ft3_slot(emac, ft3_slot);
+ goto unlock;
+ }
+
+ /* Program FT3 hardware */
+ rule->flow_spec = *fs;
+ rule->ft3_slot = ft3_slot;
+ rule->queue_index = fs->ring_cookie;
+ rule->location = fs->location;
+
+ icssg_ft3_program_filter(miig_rt, slice, ft3_slot, rule);
+
+ /* Add to rule list */
+ list_add(&rule->list, &emac->rxnfc.rules);
+ emac->rxnfc.rxnfc_rule_count++;
+
+ /* Update classifier */
+ icssg_rxnfc_classifier_update(emac);
+
+ ret = 0;
+
+unlock:
+ mutex_unlock(&emac->rxnfc.rxnfc_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(icssg_rxnfc_rule_add);
+
+/* Delete a RX flow classification rule */
+int icssg_rxnfc_rule_del(struct prueth_emac *emac, u32 location)
+{
+ struct prueth *prueth = emac->prueth;
+ struct regmap *miig_rt = prueth->miig_rt;
+ int slice = prueth_emac_slice(emac);
+ struct icssg_rxnfc_rule *rule, *tmp;
+ int ret = -ENOENT;
+
+ if (slice < 0)
+ return -EINVAL;
+
+ mutex_lock(&emac->rxnfc.rxnfc_lock);
+
+ list_for_each_entry_safe(rule, tmp, &emac->rxnfc.rules, list) {
+ if (rule->location == location) {
+ icssg_ft3_disable_filter(miig_rt, slice, rule->ft3_slot);
+ free_ft3_slot(emac, rule->ft3_slot);
+ list_del(&rule->list);
+ kfree(rule);
+ emac->rxnfc.rxnfc_rule_count--;
+ icssg_rxnfc_classifier_update(emac);
+ ret = 0;
+ break;
+ }
+ }
+
+ mutex_unlock(&emac->rxnfc.rxnfc_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(icssg_rxnfc_rule_del);
+
+/* Get single RX flow classification rule */
+int icssg_rxnfc_rule_get(struct prueth_emac *emac, u32 location,
+ struct ethtool_rx_flow_spec *fs)
+{
+ struct icssg_rxnfc_rule *rule;
+ int ret = -ENOENT;
+
+ mutex_lock(&emac->rxnfc.rxnfc_lock);
+
+ list_for_each_entry(rule, &emac->rxnfc.rules, list) {
+ if (rule->location == location) {
+ *fs = rule->flow_spec;
+ ret = 0;
+ break;
+ }
+ }
+
+ mutex_unlock(&emac->rxnfc.rxnfc_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(icssg_rxnfc_rule_get);
+
+/* Get all RX flow classification rules */
+int icssg_rxnfc_rule_get_all(struct prueth_emac *emac, u32 *rule_locs,
+ u32 *rule_cnt)
+{
+ struct icssg_rxnfc_rule *rule;
+ u32 count = 0;
+
+ mutex_lock(&emac->rxnfc.rxnfc_lock);
+
+ list_for_each_entry(rule, &emac->rxnfc.rules, list) {
+ rule_locs[count++] = rule->location;
+ }
+
+ *rule_cnt = count;
+
+ mutex_unlock(&emac->rxnfc.rxnfc_lock);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(icssg_rxnfc_rule_get_all);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
index 5c2936ecd595c..9f0a1116e530c 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
@@ -299,6 +299,52 @@ static int emac_set_per_queue_coalesce(struct net_device *ndev, u32 queue,
return 0;
}
+static int emac_get_rxnfc(struct net_device *ndev, struct ethtool_rxnfc *info,
+ u32 *rule_locs)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ int ret = 0;
+
+ switch (info->cmd) {
+ case ETHTOOL_GRXRINGS:
+ info->data = emac->rx_flow_num;
+ break;
+ case ETHTOOL_GRXCLSRLCNT:
+ info->rule_cnt = emac->rxnfc.rxnfc_rule_count;
+ info->data = ICSSG_RXNFC_MAX_RULES | RX_CLS_LOC_SPECIAL;
+ break;
+ case ETHTOOL_GRXCLSRULE:
+ ret = icssg_rxnfc_rule_get(emac, info->fs.location, &info->fs);
+ break;
+ case ETHTOOL_GRXCLSRLALL: {
+ u32 rule_cnt = 0;
+
+ ret = icssg_rxnfc_rule_get_all(emac, rule_locs, &rule_cnt);
+ if (!ret)
+ info->rule_cnt = rule_cnt;
+ break;
+ }
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return ret;
+}
+
+static int emac_set_rxnfc(struct net_device *ndev, struct ethtool_rxnfc *info)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+
+ switch (info->cmd) {
+ case ETHTOOL_SRXCLSRLINS:
+ return icssg_rxnfc_rule_add(emac, info);
+ case ETHTOOL_SRXCLSRLDEL:
+ return icssg_rxnfc_rule_del(emac, info->fs.location);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
const struct ethtool_ops icssg_ethtool_ops = {
.get_drvinfo = emac_get_drvinfo,
.get_msglevel = emac_get_msglevel,
@@ -322,5 +368,7 @@ const struct ethtool_ops icssg_ethtool_ops = {
.set_eee = emac_set_eee,
.nway_reset = emac_nway_reset,
.get_rmon_stats = emac_get_rmon_stats,
+ .get_rxnfc = emac_get_rxnfc,
+ .set_rxnfc = emac_set_rxnfc,
};
EXPORT_SYMBOL_GPL(icssg_ethtool_ops);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 8338402597b21..f234198d4cc42 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -874,10 +874,29 @@ static void prueth_destroy_rxq(struct prueth_emac *emac, int qid)
{
struct prueth_rx_chn *rx_chn = &emac->rx_chns;
struct prueth_rx_flow *flow = &rx_chn->flows[qid];
+ dma_addr_t desc_dma;
- /* reset napi and hrtimer, and free all pending buffers */
napi_disable(&flow->napi_rx);
hrtimer_cancel(&flow->rx_hrtimer);
+
+ /* Drain the UDMA completion ring for this flow before resetting
+ * the free ring. k3_udma_glue_reset_rx_chn() only drains the
+ * free ring (descriptors pushed as empty receive buffers). Any
+ * frames that the hardware already DMA-completed but NAPI hadn't
+ * popped yet remain in the completion ring with their pages still
+ * counted as "inflight" in the page pool. Draining them here
+ * ensures page_pool_destroy() finds zero inflight pages and
+ * completes immediately instead of stalling for 60+ seconds.
+ *
+ * The caller must have stopped new frames from arriving (e.g. via
+ * icssg_set_port_state(ICSSG_EMAC_PORT_DISABLE)) before calling
+ * prueth_destroy_rxq() so this loop terminates.
+ */
+ while (!k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, qid, &desc_dma)) {
+ if (!cppi5_desc_is_tdcm(desc_dma))
+ prueth_rx_cleanup(rx_chn, desc_dma);
+ }
+
k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, flow->id, rx_chn,
prueth_rx_cleanup);
@@ -1160,6 +1179,9 @@ static int emac_ndo_stop(struct net_device *ndev)
prueth_cleanup_rx_chns(emac);
prueth_cleanup_tx_chns(emac);
+ /* Cleanup RX flow classification rules */
+ icssg_rxnfc_rule_cleanup(emac);
+
prueth->emacs_initialized--;
return 0;
@@ -1676,6 +1698,9 @@ static int prueth_netdev_init(struct prueth *prueth,
emac->ndev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
INIT_WORK(&emac->rx_mode_work, emac_ndo_set_rx_mode_work);
+ /* Initialize RX flow classification */
+ icssg_rxnfc_rule_init(emac);
+
INIT_DELAYED_WORK(&emac->stats_work, icssg_stats_work_handler);
ret = pruss_request_mem_region(prueth->pruss,
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 509b9cbda58cf..7a316184aee32 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -11,6 +11,7 @@
#include <linux/bpf.h>
#include <linux/bpf_trace.h>
#include <linux/etherdevice.h>
+#include <linux/ethtool.h>
#include <linux/genalloc.h>
#include <linux/if_vlan.h>
#include <linux/if_hsr.h>
@@ -192,6 +193,19 @@ struct prueth_swdata {
/* Minimum coalesce time in usecs for both Tx and Rx */
#define ICSSG_MIN_COALESCE_USECS 20
+/* RX flow classification (ethtool) */
+#define ICSSG_RXNFC_MAX_RULES 15 /* Max user-installable rules */
+#define ICSSG_FT3_RESERVED_SLOTS BIT(14) /* Slot 14 reserved for HSR/PRP */
+
+/* RX flow classification rule structure */
+struct icssg_rxnfc_rule {
+ struct list_head list;
+ struct ethtool_rx_flow_spec flow_spec; /* User-provided filter */
+ u16 ft3_slot; /* Allocated FT3 slot (0-15, excluding 14) */
+ int location; /* Rule location (user-provided) */
+ u32 queue_index; /* RX flow to route matches to */
+};
+
/* data for each emac port */
struct prueth_emac {
bool is_sr1;
@@ -255,6 +269,14 @@ struct prueth_emac {
struct bpf_prog *xdp_prog;
struct xdp_attachment_info xdpi;
unsigned long *xsk_queues;
+
+ /* RX flow classification via ethtool */
+ struct {
+ struct list_head rules; /* List of active RXNFC rules */
+ struct mutex rxnfc_lock; /* Protect rule list */
+ unsigned int rxnfc_rule_count; /* Number of active rules */
+ u16 ft3_slots_used; /* Bitmask of allocated FT3 slots */
+ } rxnfc;
};
/* The buf includes headroom compatible with both skb and xdpf */
@@ -399,6 +421,16 @@ static inline int prueth_emac_slice(struct prueth_emac *emac)
}
}
+/* RX flow classification functions */
+void icssg_rxnfc_rule_init(struct prueth_emac *emac);
+void icssg_rxnfc_rule_cleanup(struct prueth_emac *emac);
+int icssg_rxnfc_rule_add(struct prueth_emac *emac, struct ethtool_rxnfc *info);
+int icssg_rxnfc_rule_del(struct prueth_emac *emac, u32 location);
+int icssg_rxnfc_rule_get(struct prueth_emac *emac, u32 location,
+ struct ethtool_rx_flow_spec *fs);
+int icssg_rxnfc_rule_get_all(struct prueth_emac *emac, u32 *rule_locs,
+ u32 *rule_cnt);
+
extern const struct ethtool_ops icssg_ethtool_ops;
extern const struct dev_pm_ops prueth_dev_pm_ops;
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH net-next 1/2] icssg: Add multi-flow RX support with per-flow NAPI and classification
2026-11-03 13:44 ` [RFC PATCH net-next 1/2] icssg: Add multi-flow RX support with per-flow NAPI and classification Meghana Malladi
@ 2026-11-04 13:46 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-04 13:46 UTC (permalink / raw)
To: Meghana Malladi; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 12 potential issue(s) to consider:
- [Critical] net: ti: icssg: hardcoded flow 0 and queue 0 in multi-flow XSK and XDP
- [Critical] net: ti: icssg: missing rx_flow_num bounds check in XSK pool and wakeup
- [High] net: ti: icssg: undersized shared RX FDQ ring in prueth_init_rx_chns()
- [High] net: ti: icssg: shared RX FDQ page leaks in prueth_destroy_rxq() teardown
- [High] net: ti: icssg: build breakage in icssg_prueth_sr1.c from RX API changes
- [High] net: ti: icssg: hang in prueth_destroy_rxq() on prueth_create_rxq() error
- [Medium] net: ti: icssg: unsigned irq check misses errors in prueth_init_rx_chns()
- [Medium] net: ti: icssg: emac_set_coalesce() only sets RX coalescing on flow 0
- [Medium] net: ti: icssg: prueth_destroy_rxqs() skips NAPI wakeup for non-XSK flows
- [Medium] net: ti: icssg: RX teardown and IRQ pacing bugs in prueth_destroy_rxqs()
- [Medium] net: ti: icssg: PRUSS DRAM leak in prueth_netdev_init() on bitmap alloc error
- [Medium] net: ti: icssg: allocate PRUETH_MAX_RX_FLOWS rx queues and set real_num_rx_queues
--
commit a2ae6a161ac2d7097a317321767fd99ea3cc193c
Author: Meghana Malladi <m-malladi@ti.com>
icssg: Add multi-flow RX support with per-flow NAPI and classification
This commit extends the ICSSG driver to support up to 8 RX flows per EMAC,
introducing per-flow NAPI, hrtimer, page pool, XDP rxq, AF_XDP pool, and
IRQ management while sharing a single UDMA RX channel and descriptor pool.
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 4a7d1a6f470b1..99f4353b46966 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
[ ... ]
> @@ -584,40 +622,49 @@ int prueth_init_rx_chns(struct prueth_emac *emac,
> K3_UDMA_GLUE_SRC_TAG_LO_USE_REMOTE_SRC_TAG,
> };
>
> + flow = &rx_chn->flows[i];
> + flow->emac = emac;
> + flow->id = i;
> + flow->irq = -EINVAL;
> +
> rx_flow_cfg.ring_rxfdq0_id = fdqring_id;
> - rx_flow_cfg.rx_cfg.size = max_desc_num;
> - rx_flow_cfg.rxfdq_cfg.size = max_desc_num;
> + rx_flow_cfg.rx_cfg.size = PRUETH_MAX_RX_DESC;
> + rx_flow_cfg.rxfdq_cfg.size = PRUETH_MAX_RX_DESC;
[Severity: High]
Does rx_flow_cfg.rxfdq_cfg.size need to be scaled to rx_chn->descs_num
(PRUETH_MAX_RX_DESC * rx_cfg.flow_id_num) here?
In prueth_init_rx_chns(), flow 0 allocates a shared FDQ ring with
K3_RINGACC_RING_SHARED and subsequent flows pass rx_flow_cfg.ring_rxfdq0_id
= fdqring_id. On UDMAP platforms such as AM654, k3_udma_glue_cfg_rx_flow()
shares flow 0's ringrxfdq across all flows, and k3_ringacc_ring_cfg() only
configures the ring size on the first user.
When emac->rx_flow_num > 1, prueth_prepare_rx_flow(emac, 0) pushes
PRUETH_MAX_RX_DESC (512) descriptors into the 512-entry shared FDQ ring,
completely filling it, so prueth_prepare_rx_flow(emac, 1) immediately fails
with -ENOMEM in k3_ringacc_ring_push().
> rx_flow_cfg.rxfdq_cfg.mode = emac->prueth->pdata.fdqring_mode;
>
> ret = k3_udma_glue_rx_flow_init(rx_chn->rx_chn,
> i, &rx_flow_cfg);
> if (ret) {
> netdev_err(ndev, "Failed to init rx flow%d %d\n",
> i, ret);
> goto fail;
> }
> if (!i)
> fdqring_id = k3_udma_glue_rx_flow_get_fdq_id(rx_chn->rx_chn,
> i);
> - ret = k3_udma_glue_rx_get_irq(rx_chn->rx_chn, i);
> - if (ret < 0) {
> + flow->irq = k3_udma_glue_rx_get_irq(rx_chn->rx_chn, i);
> + if (flow->irq <= 0) {
> + ret = flow->irq;
> netdev_err(ndev, "Failed to get rx dma irq");
> goto fail;
> }
[Severity: Medium]
In struct prueth_rx_flow, irq is declared as unsigned int. When
k3_udma_glue_rx_get_irq() returns a negative error code, assigning it
directly to flow->irq wraps it to a large positive value, causing
flow->irq <= 0 to evaluate to false and bypassing the error check.
Furthermore, if flow->irq is 0, ret = flow->irq sets ret to 0 and jumps to
fail:, which cleans up the RX channel and returns 0 (success) to
emac_ndo_open(). Should the return value of k3_udma_glue_rx_get_irq() be
checked in a signed int variable before assigning to flow->irq?
[ ... ]
> @@ -641,8 +688,9 @@ int prueth_dma_rx_push_mapped(struct prueth_emac *emac,
> swdata = cppi5_hdesc_get_swdata(desc_rx);
> swdata->type = PRUETH_SWDATA_PAGE;
> swdata->data.page = page;
> + swdata->flow_id = flow_id;
>
> - return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, PRUETH_RX_FLOW_DATA,
> + return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, flow_id,
> desc_rx, desc_dma);
> }
[Severity: High]
If k3_udma_glue_push_rx_chn() fails in prueth_dma_rx_push_mapped() (or in
prueth_dma_rx_push_mapped_zc()), does this leak desc_rx from
rx_chn->desc_pool because k3_cppi_desc_pool_free() is not called before
returning?
[ ... ]
> @@ -863,31 +911,32 @@ static int prueth_dma_rx_push_mapped_zc(struct prueth_emac *emac,
> struct xdp_buff *xdp)
> {
> struct net_device *ndev = emac->ndev;
> + struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[PRUETH_RX_FLOW_DATA];
> struct cppi5_host_desc_t *desc_rx;
> struct prueth_swdata *swdata;
> dma_addr_t desc_dma;
> dma_addr_t buf_dma;
> int buf_len;
>
> buf_dma = xsk_buff_xdp_get_dma(xdp);
> desc_rx = k3_cppi_desc_pool_alloc(rx_chn->desc_pool);
> if (!desc_rx) {
> netdev_err(ndev, "rx push: failed to allocate descriptor\n");
> return -ENOMEM;
> }
> desc_dma = k3_cppi_desc_pool_virt2dma(rx_chn->desc_pool, desc_rx);
>
> cppi5_hdesc_init(desc_rx, CPPI5_INFO0_HDESC_EPIB_PRESENT,
> PRUETH_NAV_PS_DATA_SIZE);
> k3_udma_glue_rx_dma_to_cppi5_addr(rx_chn->rx_chn, &buf_dma);
> - buf_len = xsk_pool_get_rx_frame_size(rx_chn->xsk_pool);
> + buf_len = xsk_pool_get_rx_frame_size(rx_flow->xsk_pool);
> cppi5_hdesc_attach_buf(desc_rx, buf_dma, buf_len, buf_dma, buf_len);
> swdata = cppi5_hdesc_get_swdata(desc_rx);
> swdata->type = PRUETH_SWDATA_XSK;
> swdata->data.xdp = xdp;
>
> return k3_udma_glue_push_rx_chn(rx_chn->rx_chn, PRUETH_RX_FLOW_DATA,
> desc_rx, desc_dma);
> }
>
> -static int prueth_rx_alloc_zc(struct prueth_emac *emac, int budget)
> +static int prueth_rx_alloc_zc(struct prueth_emac *emac, int flow_id, int budget)
> {
> - struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> + struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[flow_id];
> struct xdp_buff *xdp;
> int i, ret;
>
> for (i = 0; i < budget; i++) {
> - xdp = xsk_buff_alloc(rx_chn->xsk_pool);
> + xdp = xsk_buff_alloc(rx_flow->xsk_pool);
> if (!xdp)
> break;
>
> - ret = prueth_dma_rx_push_mapped_zc(emac, rx_chn, xdp);
> + ret = prueth_dma_rx_push_mapped_zc(emac, &emac->rx_chns, xdp);
[Severity: Critical]
In prueth_rx_alloc_zc(), xdp buffers are allocated from
flows[flow_id].xsk_pool, but prueth_dma_rx_push_mapped_zc() does not take a
flow_id parameter and instead hardcodes PRUETH_RX_FLOW_DATA (0) for both
rx_flow->xsk_pool and k3_udma_glue_push_rx_chn().
If an XSK pool is active on flow_id > 0 while flows[0].xsk_pool is NULL,
won't xsk_pool_get_rx_frame_size(rx_flow->xsk_pool) dereference a NULL
pointer, or otherwise push flow_id's XSK buffers into flow 0's ring?
[ ... ]
> @@ -1155,19 +1206,22 @@ void prueth_rx_cleanup(void *data, dma_addr_t desc_dma)
> struct prueth_rx_chn *rx_chn = data;
> struct cppi5_host_desc_t *desc_rx;
> struct prueth_swdata *swdata;
> - struct page_pool *pool;
> + struct page_pool *pool = NULL;
> struct xdp_buff *xdp;
> struct page *page;
>
> - pool = rx_chn->pg_pool;
> desc_rx = k3_cppi_desc_pool_dma2virt(rx_chn->desc_pool, desc_dma);
> swdata = cppi5_hdesc_get_swdata(desc_rx);
> - if (rx_chn->xsk_pool) {
> +
> + if (swdata->type == PRUETH_SWDATA_XSK) {
> xdp = swdata->data.xdp;
> xsk_buff_free(xdp);
> - } else {
> - page = swdata->data.page;
> - page_pool_recycle_direct(pool, page);
> + } else if (swdata->type == PRUETH_SWDATA_PAGE) {
> + pool = rx_chn->flows[swdata->flow_id].pg_pool;
> + if (pool) {
> + page = swdata->data.page;
> + page_pool_recycle_direct(pool, page);
> + }
> }
>
> k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
[ ... ]
> @@ -1488,59 +1531,77 @@ int icssg_napi_rx_poll(struct napi_struct *napi_rx, int budget)
> }
> EXPORT_SYMBOL_GPL(icssg_napi_rx_poll);
>
> -int prueth_prepare_rx_chan(struct prueth_emac *emac,
> - struct prueth_rx_chn *chn,
> - int buf_size)
> +int prueth_prepare_rx_flow(struct prueth_emac *emac,
> + int flow)
> {
> + int buf_size = PRUETH_MAX_PKT_SIZE;
> + struct prueth_rx_flow *rx_flow;
> + struct prueth_rx_chn *rx_chn;
> + struct page_pool *pool;
> struct page *page;
> int desc_avail;
> int i, ret;
>
> - desc_avail = k3_cppi_desc_pool_avail(chn->desc_pool);
> - if (desc_avail < chn->descs_num)
> - netdev_warn(emac->ndev,
> - "not enough RX descriptors available %d < %d\n",
> - desc_avail, chn->descs_num);
> + rx_chn = &emac->rx_chns;
> + rx_flow = &emac->rx_chns.flows[flow];
>
> - if (chn->xsk_pool) {
> - /* get pages from xsk_pool and push to RX ring
> - * queue as much as possible
> + if (rx_flow->xsk_pool) {
> + /* XSK zero-copy: buffers come from the XSK fill ring, no page
> + * pool needed. Push as many XSK buffers as descriptors allow;
> + * the fill ring will be replenished by user-space.
> */
> - ret = prueth_rx_alloc_zc(emac, desc_avail);
> - if (!ret)
> - goto recycle_alloc_pg;
> - } else {
> - for (i = 0; i < desc_avail; i++) {
> - /* NOTE: we're not using memory efficiently here.
> - * 1 full page (4KB?) used here instead of
> - * PRUETH_MAX_PKT_SIZE (~1.5KB?)
> - */
> - page = page_pool_dev_alloc_pages(chn->pg_pool);
> - if (!page) {
> - netdev_err(emac->ndev, "couldn't allocate rx page\n");
> - ret = -ENOMEM;
> - goto recycle_alloc_pg;
> - }
> + desc_avail = k3_cppi_desc_pool_avail(rx_chn->desc_pool);
> + if (desc_avail < PRUETH_MAX_RX_DESC)
> + netdev_warn(emac->ndev,
> + "not enough RX descriptors available %d < %d\n",
> + desc_avail, PRUETH_MAX_RX_DESC);
>
> - ret = prueth_dma_rx_push_mapped(emac, chn, page, buf_size);
> - if (ret < 0) {
> - netdev_err(emac->ndev,
> - "cannot submit page for rx chan %s ret %d\n",
> - chn->name, ret);
> - page_pool_recycle_direct(chn->pg_pool, page);
> - goto recycle_alloc_pg;
> - }
> + prueth_rx_alloc_zc(emac, flow, desc_avail);
> + return 0;
> + }
[Severity: High]
In the rx_flow->xsk_pool branch of prueth_prepare_rx_flow(), desc_avail is
the total number of available descriptors in the shared rx_chn->desc_pool
(up to PRUETH_MAX_RX_DESC * emac->rx_flow_num). Should the budget passed to
prueth_rx_alloc_zc() be capped at min(desc_avail, PRUETH_MAX_RX_DESC) so a
single XSK flow does not consume descriptors needed by subsequent flows?
> +
> + pool = prueth_create_page_pool(emac, rx_chn->dma_dev, rx_chn->descs_num, rx_flow);
> + if (IS_ERR(pool))
> + return PTR_ERR(pool);
> +
> + rx_flow->pg_pool = pool;
> +
> + desc_avail = k3_cppi_desc_pool_avail(rx_chn->desc_pool);
> + if (desc_avail < PRUETH_MAX_RX_DESC)
> + netdev_warn(emac->ndev,
> + "not enough RX descriptors available %d < %d\n",
> + desc_avail, PRUETH_MAX_RX_DESC);
> +
> + for (i = 0; i < PRUETH_MAX_RX_DESC; ++i) {
> + page = page_pool_dev_alloc_pages(pool);
> + if (!page) {
> + netdev_err(emac->ndev, "couldn't allocate rx page\n");
> + ret = -ENOMEM;
> + goto err_free_pushed;
> }
> +
> + ret = prueth_dma_rx_push_mapped(emac, rx_chn, page, buf_size, flow);
> + if (ret < 0) {
> + netdev_err(emac->ndev,
> + "cannot submit skb for rx chan %s ret %d\n",
> + rx_flow->name, ret);
> + page_pool_recycle_direct(pool, page);
> + goto err_free_pushed;
> + }
> }
>
> return 0;
>
> -recycle_alloc_pg:
> - prueth_reset_rx_chan(&emac->rx_chns, PRUETH_MAX_RX_FLOWS, false);
> -
> +err_free_pushed:
> + /* i pages were successfully pushed to the ring. Reset the channel
> + * to drain them back and recycle via prueth_rx_cleanup before the
> + * caller destroys the page pool.
> + */
> + k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, flow, rx_chn,
> + prueth_rx_cleanup);
> return ret;
> }
[Severity: High]
How does per-flow reset interact with the shared FDQ ring on UDMAP
platforms?
In k3_udma_glue_reset_rx_chn():
if (rx_chn->single_fdq && flow_num)
goto do_reset;
Because all flows share flow 0's FDQ (rx_chn->single_fdq is true on UDMAP),
k3_udma_glue_reset_rx_chn() only drains ringrxfdq when flow_num == 0 and
skips it completely when flow_num > 0:
1. Here at err_free_pushed:, if flow > 0 fails partway through pushing
pages, k3_udma_glue_reset_rx_chn(..., flow, ...) skips ringrxfdq and
does not drain the pushed pages before the caller destroys
rx_flow->pg_pool.
2. In prueth_create_rxqs(), the error unwind loop runs backwards:
for (--i; i >= 0; i--)
prueth_destroy_rxq(emac, i);
For i > 0, prueth_destroy_rxq(emac, i) skips draining ringrxfdq, calls
page_pool_destroy(flow->pg_pool), and sets flow->pg_pool = NULL. When
i == 0 finally runs k3_udma_glue_reset_rx_chn(..., 0, ...),
prueth_rx_cleanup() sees rx_chn->flows[swdata->flow_id].pg_pool == NULL
for all descriptors with swdata->flow_id > 0 and leaks those pages.
3. In prueth_xsk_pool_enable() and prueth_xsk_pool_disable(), calling
prueth_destroy_rxq(emac, queue_id) on a single queue while the shared RX
channel is enabled either drains and resets the shared FDQ for all flows
(if queue_id == 0) or fails to drain queue_id's descriptors from the
shared FDQ before destroying its page pool or unmapping its XSK pool
(if queue_id > 0).
4. During normal multi-flow RX on a shared FDQ ring, UDMAP pops descriptors
from the shared FDQ in FIFO order regardless of which flow's page_pool
allocated the page, whereas emac_rx_packet() syncs and recycles pages
using rx_flow->pg_pool rather than rx_chn->flows[swdata->flow_id].pg_pool,
and concurrent per-flow NAPI instances call k3_ringacc_ring_push() on
the same shared struct k3_ring without serialization.
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> index b715af21d23ac..5c2936ecd595c 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
[ ... ]
> @@ -245,6 +248,8 @@ static int emac_set_coalesce(struct net_device *ndev,
> struct netlink_ext_ack *extack)
> {
> struct prueth_emac *emac = netdev_priv(ndev);
> + struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> + struct prueth_rx_flow *flow = &rx_chn->flows[0];
> struct prueth *prueth = emac->prueth;
> struct prueth_tx_chn *tx_chn;
>
> @@ -264,7 +269,7 @@ static int emac_set_coalesce(struct net_device *ndev,
> coal->tx_coalesce_usecs = ICSSG_MIN_COALESCE_USECS;
> }
>
> - emac->rx_pace_timeout_ns = coal->rx_coalesce_usecs * 1000;
> + flow->rx_pace_timeout_ns = coal->rx_coalesce_usecs * 1000;
> tx_chn->tx_pace_timeout_ns = coal->tx_coalesce_usecs * 1000;
>
> return 0;
[Severity: Medium]
Now that rx_pace_timeout_ns is stored per flow in struct prueth_rx_flow and
checked by icssg_napi_rx_poll() on each flow, emac_set_coalesce() only
updates rx_chn->flows[0].rx_pace_timeout_ns (and
emac_set_per_queue_coalesce() only handles TX queues). Does this leave RX
flows 1..7 with rx_pace_timeout_ns == 0 when RX coalescing is configured?
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 591be5c8056b4..8338402597b21 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
[ ... ]
> @@ -883,14 +832,14 @@ static int icssg_update_vlan_mcast(struct net_device *vdev, int vid,
> static void prueth_set_xsk_pool(struct prueth_emac *emac, u16 queue_id)
> {
> struct prueth_tx_chn *tx_chn = &emac->tx_chns[queue_id];
> - struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> + struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[PRUETH_RX_FLOW_DATA];
>
> - if (emac->xsk_qid != queue_id) {
> - rx_chn->xsk_pool = NULL;
> - tx_chn->xsk_pool = NULL;
> - } else {
> - rx_chn->xsk_pool = xsk_get_pool_from_qid(emac->ndev, queue_id);
> + if (test_bit(queue_id, emac->xsk_queues)) {
> + rx_flow->xsk_pool = xsk_get_pool_from_qid(emac->ndev, queue_id);
> tx_chn->xsk_pool = xsk_get_pool_from_qid(emac->ndev, queue_id);
> + } else {
> + rx_flow->xsk_pool = NULL;
> + tx_chn->xsk_pool = NULL;
> }
> }
[Severity: Critical]
In prueth_set_xsk_pool(), rx_flow is indexed with PRUETH_RX_FLOW_DATA (0)
instead of queue_id. When enabling or disabling an XSK pool on queue_id > 0,
won't this overwrite or clear flows[0].xsk_pool while flow 0 is running and
leave flows[queue_id].xsk_pool untouched?
Also, in prueth_init_rx_chns(), flow->xsk_pool is set to NULL for all flows
during emac_ndo_open() without calling prueth_set_xsk_pool() to restore
pools for queues set in emac->xsk_queues.
[ ... ]
> @@ -921,40 +870,58 @@ static void prueth_destroy_txq(struct prueth_emac *emac)
> }
> }
>
> -static void prueth_destroy_rxq(struct prueth_emac *emac)
> +static void prueth_destroy_rxq(struct prueth_emac *emac, int qid)
> +{
> + struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> + struct prueth_rx_flow *flow = &rx_chn->flows[qid];
> +
> + /* reset napi and hrtimer, and free all pending buffers */
> + napi_disable(&flow->napi_rx);
> + hrtimer_cancel(&flow->rx_hrtimer);
[Severity: Medium]
If flow->rx_hrtimer is pending when prueth_destroy_rxq() cancels it,
flow->irq_disabled is true and flow->irq is currently disabled at the IRQ
chip waiting for emac_rx_timer_callback() to re-enable it.
When prueth_destroy_rxq() is called from prueth_xsk_pool_enable() or
prueth_xsk_pool_disable() on a running interface, cancelling the timer
without re-enabling flow->irq and clearing flow->irq_disabled leaves the
flow's RX IRQ permanently disabled (and across emac_ndo_stop() /
emac_ndo_open(), flow->irq_disabled remains true because
prueth_init_rx_chns() does not reset it). Should prueth_destroy_rxq() check
flow->irq_disabled and call enable_irq(flow->irq) after hrtimer_cancel()?
> + k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, flow->id, rx_chn,
> + prueth_rx_cleanup);
> +
> + if (xdp_rxq_info_is_reg(&flow->xdp_rxq))
> + xdp_rxq_info_unreg(&flow->xdp_rxq);
> +
> + if (flow->pg_pool) {
> + page_pool_destroy(flow->pg_pool);
> + flow->pg_pool = NULL;
> + }
> +}
> +
> +static void prueth_destroy_rxqs(struct prueth_emac *emac)
> {
> int i, ret;
>
> /* tear down and disable UDMA channels */
> reinit_completion(&emac->tdown_complete);
> k3_udma_glue_tdown_rx_chn(emac->rx_chns.rx_chn, true);
>
> /* When RX DMA Channel Teardown is initiated, it will result in an
> * interrupt and a Teardown Completion Marker (TDCM) is queued into
> * the RX Completion queue. Acknowledging the interrupt involves
> * popping the TDCM descriptor from the RX Completion queue via the
> * RX NAPI Handler. To avoid timing out when waiting for the TDCM to
> * be popped, schedule the RX NAPI handler to run immediately.
> */
> - if (!napi_if_scheduled_mark_missed(&emac->napi_rx)) {
> - if (napi_schedule_prep(&emac->napi_rx))
> - __napi_schedule(&emac->napi_rx);
> + for (int i = 0; i < emac->rx_flow_num; i++) {
> + if (test_bit(i, emac->xsk_queues)) {
> + if (!napi_if_scheduled_mark_missed(&emac->rx_chns.flows[i].napi_rx)) {
> + if (napi_schedule_prep(&emac->rx_chns.flows[i].napi_rx))
> + __napi_schedule(&emac->rx_chns.flows[i].napi_rx);
> + }
> + }
> }
[Severity: Medium]
Why is NAPI scheduling during RX teardown restricted to flows with
test_bit(i, emac->xsk_queues) set (also shadowing the outer int i)?
If XSK is not enabled on a flow and its RX IRQ is currently disabled by
rx_hrtimer coalescing when prueth_destroy_rxqs() initiates teardown, NAPI
will not be scheduled immediately to pop the TDCM descriptor, causing
wait_for_completion_timeout(&emac->tdown_complete, msecs_to_jiffies(1000))
to time out.
[ ... ]
> @@ -978,27 +945,62 @@ static int prueth_create_txq(struct prueth_emac *emac)
> return ret;
> }
>
> -static int prueth_create_rxq(struct prueth_emac *emac)
> +static int prueth_create_rxq(struct prueth_emac *emac, int id)
> {
> + struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> + struct prueth_rx_flow *flow = &rx_chn->flows[id];
> + struct xdp_rxq_info *rxq = &flow->xdp_rxq;
> int ret;
>
> - ret = prueth_prepare_rx_chan(emac, &emac->rx_chns, PRUETH_MAX_PKT_SIZE);
> + ret = prueth_prepare_rx_flow(emac, id);
> if (ret)
> - return ret;
> + goto err;
>
> - ret = k3_udma_glue_enable_rx_chn(emac->rx_chns.rx_chn);
> + ret = xdp_rxq_info_reg(rxq, emac->ndev, 0, flow->napi_rx.napi_id);
[Severity: Critical]
Should the third argument to xdp_rxq_info_reg() be id instead of hardcoded
0 so each flow's xdp_rxq_info is registered with its own queue index? With
queue_index hardcoded to 0, XDP_REDIRECT to AF_XDP sockets bound to queues
1..7 will fail queue index validation in xsk_rcv_check().
> if (ret)
> - goto reset_rx_chn;
> + goto err;
>
> - ret = prueth_create_xdp_rxqs(emac);
> - if (ret)
> - goto reset_rx_chn;
> + if (flow->xsk_pool) {
> + ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_XSK_BUFF_POOL, NULL);
> + if (ret)
> + goto err;
> + xsk_pool_set_rxq_info(flow->xsk_pool, rxq);
> + } else {
> + ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_PAGE_POOL, flow->pg_pool);
> + if (ret)
> + goto err;
> + }
> +
> + napi_enable(&flow->napi_rx);
> + return 0;
> +
> +err:
> + prueth_destroy_rxq(emac, id);
> + return ret;
> +}
[Severity: High]
If prueth_prepare_rx_flow(), xdp_rxq_info_reg(), or
xdp_rxq_info_reg_mem_model() fails in prueth_create_rxq(), execution jumps
to err: and calls prueth_destroy_rxq(emac, id) before
napi_enable(&flow->napi_rx) has been called.
Because netif_napi_add() (or a prior napi_disable()) leaves
NAPIF_STATE_SCHED set until napi_enable() is called, won't
napi_disable(&flow->napi_rx) in prueth_destroy_rxq() hang in an infinite
loop waiting for NAPIF_STATE_SCHED to clear?
[ ... ]
> @@ -1018,24 +1020,21 @@ static int emac_ndo_open(struct net_device *ndev)
> struct prueth *prueth = emac->prueth;
> int slice = prueth_emac_slice(emac);
> struct device *dev = prueth->dev;
> - int max_rx_flows;
> - int rx_flow;
>
> /* set h/w MAC as user might have re-configured */
> ether_addr_copy(emac->mac_addr, ndev->dev_addr);
>
> icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
> icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr);
>
> /* Notify the stack of the actual queue counts. */
> ret = netif_set_real_num_tx_queues(ndev, num_data_chn);
> if (ret) {
> dev_err(dev, "cannot set real number of tx queues\n");
> return ret;
> }
[Severity: Medium]
In prueth_netdev_init(), alloc_etherdev_mq(sizeof(*emac), num_tx_chn) is
called with num_tx_chn = PRUETH_MAX_TX_QUEUES (4), which only allocates 4
RX queues in ndev->_rx (ndev->num_rx_queues = 4, ndev->real_num_rx_queues =
4), and emac_ndo_open() never calls
netif_set_real_num_rx_queues(ndev, emac->rx_flow_num).
When emac->rx_flow_num is set to 5..8 via ethtool, queues 4..7 have no
struct netdev_rx_queue entries in ndev->_rx (breaking AF_XDP, RPS, sysfs,
and netlink queue operations for queues >= 4), and when emac->rx_flow_num
is 1..3, the stack still sees 4 active RX queues. Should
prueth_netdev_init() use alloc_etherdev_mqs(..., PRUETH_MAX_TX_QUEUES,
PRUETH_MAX_RX_FLOWS) and emac_ndo_open() call
netif_set_real_num_rx_queues(ndev, emac->rx_flow_num)?
[ ... ]
> @@ -1467,7 +1459,6 @@ static int emac_xdp_setup(struct prueth_emac *emac, struct netdev_bpf *bpf)
> static int prueth_xsk_pool_enable(struct prueth_emac *emac,
> struct xsk_buff_pool *pool, u16 queue_id)
> {
> - struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> u32 frame_size;
> int ret;
>
> @@ -1481,7 +1472,7 @@ static int prueth_xsk_pool_enable(struct prueth_emac *emac,
> if (frame_size < PRUETH_MAX_PKT_SIZE)
> return -EOPNOTSUPP;
>
> - ret = xsk_pool_dma_map(pool, rx_chn->dma_dev, PRUETH_RX_DMA_ATTR);
> + ret = xsk_pool_dma_map(pool, emac->rx_chns.dma_dev, PRUETH_RX_DMA_ATTR);
> if (ret) {
> netdev_err(emac->ndev, "Failed to map XSK pool: %d\n", ret);
> return ret;
> @@ -1492,21 +1483,21 @@ static int prueth_xsk_pool_enable(struct prueth_emac *emac,
> ret = icssg_set_port_state(emac, ICSSG_EMAC_PORT_DISABLE);
> if (ret)
> return ret;
> - prueth_destroy_rxq(emac);
> + prueth_destroy_rxq(emac, queue_id);
> }
>
> - emac->xsk_qid = queue_id;
> + set_bit(queue_id, emac->xsk_queues);
> prueth_set_xsk_pool(emac, queue_id);
>
> if (netif_running(emac->ndev)) {
> - ret = prueth_create_rxq(emac);
> + ret = prueth_create_rxq(emac, queue_id);
> if (ret) {
> netdev_err(emac->ndev, "Failed to create RX queue: %d\n", ret);
> return ret;
> }
> ret = icssg_set_port_state(emac, ICSSG_EMAC_PORT_FORWARD);
> if (ret) {
> - prueth_destroy_rxq(emac);
> + prueth_destroy_rxq(emac, queue_id);
> return ret;
> }
> ret = prueth_xsk_wakeup(emac->ndev, queue_id, XDP_WAKEUP_RX);
[Severity: Critical]
In prueth_xsk_pool_enable() and prueth_xsk_pool_disable(), queue_id is
checked against PRUETH_MAX_RX_FLOWS and emac->tx_ch_num, but not against
emac->rx_flow_num:
if (queue_id >= PRUETH_MAX_RX_FLOWS ||
queue_id >= emac->tx_ch_num)
If emac->tx_ch_num > emac->rx_flow_num (e.g. tx_ch_num == 4 and
rx_flow_num == 1) and an AF_XDP pool is bound to queue_id == 1,
prueth_destroy_rxq(emac, 1) and prueth_create_rxq(emac, 1) will operate on
an uninitialized flows[1] (hanging in napi_disable() on an uninitialized
napi_struct and indexing past rx_chn->rx_chn->flows in
k3_udma_glue_reset_rx_chn()).
Also, if prueth_xsk_pool_enable() fails after xsk_pool_dma_map() or
set_bit(queue_id, emac->xsk_queues), does it need to unmap the pool and
clear the bit and xsk_pool pointers on the error path?
[ ... ]
> @@ -1594,9 +1586,9 @@ int prueth_xsk_wakeup(struct net_device *ndev, u32 qid, u32 flags)
> {
> struct prueth_emac *emac = netdev_priv(ndev);
> struct prueth_tx_chn *tx_chn = &emac->tx_chns[qid];
> - struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> + struct prueth_rx_flow *rx_flow = &emac->rx_chns.flows[PRUETH_RX_FLOW_DATA];
>
> - if (emac->xsk_qid != qid) {
> + if (!test_bit(qid, emac->xsk_queues)) {
> netdev_err(ndev, "XSK queue %d not registered\n", qid);
> return -EINVAL;
> }
> @@ -1611,7 +1603,7 @@ int prueth_xsk_wakeup(struct net_device *ndev, u32 qid, u32 flags)
> return -EINVAL;
> }
>
> - if (!rx_chn->xsk_pool) {
> + if (!rx_flow->xsk_pool) {
> netdev_err(ndev, "XSK pool not registered for RX queue %d\n", qid);
> return -EINVAL;
> }
> @@ -1624,9 +1616,9 @@ int prueth_xsk_wakeup(struct net_device *ndev, u32 qid, u32 flags)
> }
>
> if (flags & XDP_WAKEUP_RX) {
> - if (!napi_if_scheduled_mark_missed(&emac->napi_rx)) {
> - if (likely(napi_schedule_prep(&emac->napi_rx)))
> - __napi_schedule(&emac->napi_rx);
> + if (!napi_if_scheduled_mark_missed(&rx_flow->napi_rx)) {
> + if (likely(napi_schedule_prep(&rx_flow->napi_rx)))
> + __napi_schedule(&rx_flow->napi_rx);
> }
> }
[Severity: Critical]
In prueth_xsk_wakeup():
1. test_bit(qid, emac->xsk_queues) is called before checking whether
qid >= PRUETH_MAX_RX_FLOWS, which can read out of bounds of the
emac->xsk_queues bitmap if qid is out of range (and the bounds check
itself also misses qid >= emac->rx_flow_num).
2. rx_flow is hardcoded to &emac->rx_chns.flows[PRUETH_RX_FLOW_DATA]
instead of &emac->rx_chns.flows[qid], so XDP_WAKEUP_RX always checks and
schedules flow 0's NAPI instead of flow qid's NAPI.
> @@ -1697,6 +1689,14 @@ static int prueth_netdev_init(struct prueth *prueth,
> }
>
> emac->tx_ch_num = 1;
> + emac->rx_flow_num = 1;
> + emac->xsk_queues = devm_bitmap_zalloc(prueth->dev,
> + PRUETH_MAX_RX_FLOWS, GFP_KERNEL);
> + if (!emac->xsk_queues) {
> + dev_err(prueth->dev, "failed to allocate xsk_queues bitmap\n");
> + ret = -ENOMEM;
> + goto free_ndev;
> + }
[Severity: Medium]
If devm_bitmap_zalloc() fails here in prueth_netdev_init(), jumping to
free_ndev skips pruss_release_mem_region(prueth->pruss, &emac->dram) at the
free: label. Should this jump to free instead so emac->dram is not leaked?
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> index b8115ca47082f..582d582cba89f 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> @@ -570,11 +570,11 @@ static int emac_ndo_open(struct net_device *ndev)
> icssg_mii_update_mtu(prueth->mii_rt, slice, ndev->max_mtu);
>
> /* Prepare RX */
> - ret = prueth_prepare_rx_chan(emac, &emac->rx_chns, PRUETH_MAX_PKT_SIZE);
> + ret = prueth_prepare_rx_flow(emac, &emac->rx_chns, PRUETH_MAX_PKT_SIZE);
> if (ret)
> goto stop;
>
> - ret = prueth_prepare_rx_chan(emac, &emac->rx_mgm_chn, 64);
> + ret = prueth_prepare_rx_flow(emac, &emac->rx_mgm_chn, 64);
> if (ret)
> goto reset_rx_chn;
[Severity: High]
Does drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c still build when
CONFIG_TI_ICSSG_PRUETH_SR1 is enabled?
Here in emac_ndo_open(), prueth_prepare_rx_flow() is called with 3
arguments (passing struct prueth_rx_chn * as the second argument), whereas
its prototype in icssg_prueth.h is now:
int prueth_prepare_rx_flow(struct prueth_emac *emac, int flow);
Elsewhere in icssg_prueth_sr1.c:
- prueth_process_rx_mgm() still accesses the removed rx_chn->pg_pool member
and calls the 5-argument prueth_dma_rx_push_mapped() with 4 arguments.
- emac_ndo_open(), emac_ndo_stop(), and prueth_netdev_init() still access
the removed emac->napi_rx, emac->rx_chns.irq, and emac->rx_mgm_chn.irq
fields, and call prueth_init_rx_chns(), prueth_reset_rx_chan(), and
prueth_cleanup_rx_chns() with their old argument lists.
- emac_ndo_open() passes emac as dev_id to request_irq(..., prueth_rx_irq,
..., emac), whereas prueth_rx_irq() now expects &rx_flow->napi_rx as
dev_id.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103134501.4123194-1-m-malladi@ti.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH net-next 2/2] icssg: Add ethtool RX flow classification support
2026-11-03 13:44 ` [RFC PATCH net-next 2/2] icssg: Add ethtool RX flow classification support Meghana Malladi
@ 2026-11-04 13:46 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-04 13:46 UTC (permalink / raw)
To: Meghana Malladi; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 13 potential issue(s) to consider:
- [Critical] net: ti: icssg: heap overflow and NULL deref in icssg_rxnfc_rule_get_all()
- [High] net: ti: icssg: icssg_rxnfc_classifier_update() clobbers HSR classifiers
- [High] icssg: uninitialized rxnfc lock and list on SR1.0 via icssg_ethtool_ops
- [High] icssg: RXNFC rules wiped on open/restart and deleted across suspend
- [High] icssg: icssg_rxnfc_classifier_update() clobbers HSR/PRP classifiers
- [High] icssg: premature page_pool_destroy() and FDQ race in prueth_destroy_rxq()
- [High] icssg: napi_disable() hang on prueth_create_rxq() error and FDQ overflow
- [Medium] net: ti: icssg: missing location validation in icssg_rxnfc_rule_add()
- [Medium] net: ti: icssg: unvalidated ether_spec masks and queue 0 in validate_rule()
- [Medium] net: ti: icssg: unreachable ETHTOOL_GRXRINGS in emac_get_rxnfc()
- [Medium] icssg: memory leak of rxnfc rules in prueth_netdev_exit()
- [Low] net: ti: icssg: sparse endianness mismatch in icssg_ft3_program_filter()
- [Low] icssg: undocumented UDMA RX completion drain in prueth_destroy_rxq()
--
commit 21ba5b8fca0b052692d50b8d741061eff437183c
Author: Meghana Malladi <m-malladi@ti.com>
icssg: Add ethtool RX flow classification support
This commit adds ethtool get_rxnfc and set_rxnfc support to the ICSSG
driver to steer packets to RX flows using FT3 hardware filters and RX
classifier OR tables, and drains the UDMA RX completion ring during RX
queue teardown.
> icssg: Add ethtool RX flow classification support
>
> Enable dynamic RX flow classification via ethtool set_rxnfc/get_rxnfc.
> Users can install/remove filter rules to steer packets to specific RX flows
> based on EtherType patterns via FT3 hardware filters.
>
> - Add RXNFC rule tracking structures and lifecycle management
> - Implement ethtool get_rxnfc/set_rxnfc callbacks
> - FT3 slot allocation and hardware programming for pattern matching
> - RX classifier OR table updates based on active rules
> - Max 15 rules per EMAC, using FT3 slots 0-13,15 (slot 14 reserved)
>
> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
[Severity: Low]
This isn't a bug, but prueth_destroy_rxq() in
drivers/net/ethernet/ti/icssg/icssg_prueth.c also adds a loop to drain the
UDMA RX completion ring before calling k3_udma_glue_reset_rx_chn() so that
page_pool_destroy() does not stall waiting for inflight pages. Should that
change be documented in the commit message or split into a separate patch?
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_classifier.c b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
> index 87a8577093dbb..f11539611df75 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_classifier.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_classifier.c
[ ... ]
> +/* Program FT3 filter slot with ethertype pattern */
> +static void icssg_ft3_program_filter(struct regmap *miig_rt, int slice,
> + int slot, struct icssg_rxnfc_rule *rule)
> +{
> + const struct ethtool_rx_flow_spec *fs = &rule->flow_spec;
> + u16 ether_type;
> +
> + /* Extract EtherType from flow spec (network byte order) */
> + ether_type = fs->h_u.ether_spec.h_proto;
[Severity: Low]
Does assigning fs->h_u.ether_spec.h_proto (which is __be16) directly to u16
ether_type trigger a sparse endianness warning when building
drivers/net/ethernet/ti/icssg/icssg_classifier.c with C=1, while the
(__force u32) cast further down in icssg_ft3_program_filter() is applied to
what is already a plain u16?
> +
> + /* Deactivate filter before programming */
> + regmap_write(miig_rt, FT3_N_REG(slice, slot, FT3_CFG), 0);
[ ... ]
> +static void icssg_rxnfc_classifier_update(struct prueth_emac *emac)
> +{
> + struct prueth *prueth = emac->prueth;
> + struct regmap *miig_rt = prueth->miig_rt;
> + int slice = prueth_emac_slice(emac);
> + u32 or_table[ICSSG_NUM_CLASSIFIERS] = { };
> + struct icssg_rxnfc_rule *rule;
> + int n;
> +
> + if (slice < 0)
> + return;
> +
> + /* Build per-classifier OR table: classifier index = target queue index */
> + list_for_each_entry(rule, &emac->rxnfc.rules, list) {
> + int cls = rule->queue_index;
> +
> + if (cls < ICSSG_NUM_CLASSIFIERS)
> + or_table[cls] |= RX_CLASS_FT_FT3_MATCH(rule->ft3_slot);
> + }
> +
> + /* Write OR tables for user-rule classifiers; skip classifier 0 (default flow).
> + * Use RX_CLASS_N_REG with RX_CLASS_OR_EN — NOT RX_CLASS_OR_REG which uses
> + * rx_class_or_base (= rx_class_base + 4), causing an off-by-4 that lands on
> + * AND_EN of classifier n+1 instead of OR_EN of classifier n.
> + */
> + for (n = 1; n < ICSSG_NUM_CLASSIFIERS; n++) {
> + regmap_write(miig_rt,
> + RX_CLASS_N_REG(slice, n, RX_CLASS_OR_EN),
> + or_table[n]);
> + }
> +}
[Severity: High]
Won't looping up to ICSSG_NUM_CLASSIFIERS (16) in
icssg_rxnfc_classifier_update() overwrite the HSR/PRP classifiers with 0?
In icssg_ft3_hsr_configurations(), HSR/PRP offload configures FT3_PTP_SLOT
(slot 14) and programs CLASSIFIER_PTP_DUP (classifier 10) and
CLASSIFIER_HSR_TAG (classifier 11):
drivers/net/ethernet/ti/icssg/icssg_classifier.c:icssg_ft3_hsr_configurations() {
...
regmap_write(miig_rt, RX_CLASS_OR_REG(slice, CLASSIFIER_PTP_DUP, RX_CLASS_AND_EN),
RX_CLASS_OR_DUP_PTP);
...
if (prueth->hsr_prp_version != PRP_V1) {
...
regmap_write(miig_rt, RX_CLASS_OR_REG(slice, CLASSIFIER_HSR_TAG, RX_CLASS_AND_EN),
RX_CLASS_OR_HSR_TAG);
...
}
Although FT3 slot 14 is reserved via ICSSG_FT3_RESERVED_SLOTS, user RX
queues are bounded by emac->rx_flow_num <= ICSSG_MAX_RFLOWS (8, classifiers
0..7), so or_table[8..15] are always 0. When a rule is added or deleted via
emac_set_rxnfc() -> icssg_rxnfc_rule_add() / icssg_rxnfc_rule_del() ->
icssg_rxnfc_classifier_update(), writing or_table[n] for n = 1..15 zeroes
RX_CLASS_OR_EN on classifiers 10 and 11.
In addition, when prueth_emac_common_start() runs during emac_ndo_open() or
prueth_emac_restart(), it calls icssg_class_default() ->
icssg_class_disable(), which zeroes RX_CLASS_OR_EN across all classifiers
without calling icssg_rxnfc_classifier_update() to restore active rules in
emac->rxnfc.rules.
> +
> +/* Validate rule parameters */
> +static int validate_rule(struct prueth_emac *emac,
> + struct ethtool_rx_flow_spec *fs)
> +{
> + /* Only ETHER_FLOW supported for Phase 1 */
> + if (fs->flow_type != ETHER_FLOW)
> + return -EOPNOTSUPP;
> +
> + /* Check RX queue is valid */
> + if (fs->ring_cookie >= emac->rx_flow_num)
> + return -EINVAL;
> +
> + return 0;
> +}
[Severity: Medium]
What happens in validate_rule() when userspace passes an ETHER_FLOW rule
with MAC address masks or a partial EtherType mask in fs->m_u.ether_spec,
flow_type extension flags (FLOW_EXT or FLOW_MAC_EXT), or fs->ring_cookie ==
0?
In validate_rule(), fs->m_u.ether_spec.h_dest, fs->m_u.ether_spec.h_source,
and fs->m_u.ether_spec.h_proto are not checked before
icssg_ft3_program_filter() programs FT3_T with fs->h_u.ether_spec.h_proto
and FT3_T_MASK with FT3_ETHERTYPE_MASK, so MAC filter requests are silently
accepted and programmed as EtherType matches (matching 0x0000 if h_proto was
not set).
Also, validate_rule() allows fs->ring_cookie == 0 (which is the only valid
queue under the default emac->rx_flow_num = 1 in prueth_netdev_init()). A
rule with ring_cookie == 0 allocates an FT3 slot and sets or_table[0] in
icssg_rxnfc_classifier_update(), but the register write loop starts at
n = 1 and never writes or_table[0] to hardware.
[ ... ]
> +/* Cleanup RX flow classification */
> +void icssg_rxnfc_rule_cleanup(struct prueth_emac *emac)
> +{
> + struct prueth *prueth = emac->prueth;
> + struct regmap *miig_rt = prueth->miig_rt;
> + int slice = prueth_emac_slice(emac);
> + struct icssg_rxnfc_rule *rule, *tmp;
> + int n;
> +
> + if (slice < 0)
> + return;
> +
> + mutex_lock(&emac->rxnfc.rxnfc_lock);
> +
> + /* Disable all active FT3 filters */
> + list_for_each_entry_safe(rule, tmp, &emac->rxnfc.rules, list) {
> + icssg_ft3_disable_filter(miig_rt, slice, rule->ft3_slot);
> + free_ft3_slot(emac, rule->ft3_slot);
> + list_del(&rule->list);
> + kfree(rule);
> + }
> +
> + emac->rxnfc.rxnfc_rule_count = 0;
> + emac->rxnfc.ft3_slots_used = ICSSG_FT3_RESERVED_SLOTS;
> +
> + /* Clear all user-rule classifier OR tables (classifier 0 = default, skip) */
> + for (n = 1; n < ICSSG_NUM_CLASSIFIERS; n++)
> + regmap_write(miig_rt,
> + RX_CLASS_N_REG(slice, n, RX_CLASS_OR_EN), 0);
> +
> + mutex_unlock(&emac->rxnfc.rxnfc_lock);
> +}
> +EXPORT_SYMBOL_GPL(icssg_rxnfc_rule_cleanup);
[Severity: High]
Similar to icssg_rxnfc_classifier_update(), doesn't the loop in
icssg_rxnfc_rule_cleanup() clear RX_CLASS_N_REG(slice, n, RX_CLASS_OR_EN)
for n = 1..15 (ICSSG_NUM_CLASSIFIERS - 1) instead of stopping at
ICSSG_MAX_RFLOWS - 1 (7)?
Because offs[slice].rx_class_or_base is offs[slice].rx_class_base + 4
(RX_CLASS_OR_EN), RX_CLASS_N_REG(slice, n, RX_CLASS_OR_EN) is the same
register as RX_CLASS_OR_REG(slice, n, RX_CLASS_AND_EN) programmed by
icssg_ft3_hsr_configurations() for CLASSIFIER_PTP_DUP (10) and
CLASSIFIER_HSR_TAG (11), so calling icssg_rxnfc_rule_cleanup() clobbers both
HSR/PRP classifiers.
> +
> +/* Add a new RX flow classification rule */
> +int icssg_rxnfc_rule_add(struct prueth_emac *emac, struct ethtool_rxnfc *info)
> +{
[ ... ]
> + /* Check for duplicate */
> + if (rule_exists(emac, fs)) {
> + ret = -EEXIST;
> + goto unlock;
> + }
> +
> + /* Allocate FT3 slot */
> + ft3_slot = allocate_ft3_slot(emac);
> + if (ft3_slot < 0) {
> + ret = -ENOSPC;
> + goto unlock;
> + }
> +
> + /* Auto-assign location from FT3 slot if RX_CLS_LOC_ANY requested */
> + if (fs->location == RX_CLS_LOC_ANY)
> + fs->location = ft3_slot;
> +
> + /* Allocate rule structure */
> + rule = kzalloc(sizeof(*rule), GFP_KERNEL);
> + if (!rule) {
> + ret = -ENOMEM;
> + free_ft3_slot(emac, ft3_slot);
> + goto unlock;
> + }
> +
> + /* Program FT3 hardware */
> + rule->flow_spec = *fs;
> + rule->ft3_slot = ft3_slot;
> + rule->queue_index = fs->ring_cookie;
> + rule->location = fs->location;
[Severity: Medium]
How are explicit or special fs->location values validated in
icssg_rxnfc_rule_add()?
In emac_get_rxnfc(), ETHTOOL_GRXCLSRLCNT sets info->data to
ICSSG_RXNFC_MAX_RULES | RX_CLS_LOC_SPECIAL (advertising 15 locations,
0..14, plus special location specifiers). However:
1. Neither validate_rule() nor icssg_rxnfc_rule_add() validates fs->location
when fs->location != RX_CLS_LOC_ANY, so RX_CLS_LOC_FIRST,
RX_CLS_LOC_LAST, reserved slot 14, or values >= ICSSG_RXNFC_MAX_RULES are
stored directly in rule->location.
2. When fs->location == RX_CLS_LOC_ANY, allocate_ft3_slot() skips reserved
slot 14 and returns ft3_slot = 15 for the 15th rule, assigning
fs->location = 15 (>= ICSSG_RXNFC_MAX_RULES).
3. Because allocate_ft3_slot() picks the lowest free FT3 slot independently
of an explicit fs->location and rule_exists() only checks flow_type and
h_proto, adding a rule with explicit location 1 (getting ft3_slot = 0,
rule->location = 1) followed by a rule with RX_CLS_LOC_ANY (getting
ft3_slot = 1, rule->location = 1) or another rule with location 1 creates
duplicate rule->location entries in emac->rxnfc.rules. Since
icssg_rxnfc_rule_get() and icssg_rxnfc_rule_del() stop at the first match
from list_add(), the older rule is shadowed and also hidden from
ethtool_get_max_rxnfc_channel() when reducing channel count.
[ ... ]
> +/* Get all RX flow classification rules */
> +int icssg_rxnfc_rule_get_all(struct prueth_emac *emac, u32 *rule_locs,
> + u32 *rule_cnt)
> +{
> + struct icssg_rxnfc_rule *rule;
> + u32 count = 0;
> +
> + mutex_lock(&emac->rxnfc.rxnfc_lock);
> +
> + list_for_each_entry(rule, &emac->rxnfc.rules, list) {
> + rule_locs[count++] = rule->location;
> + }
> +
> + *rule_cnt = count;
> +
> + mutex_unlock(&emac->rxnfc.rxnfc_lock);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icssg_rxnfc_rule_get_all);
[Severity: Critical]
Can icssg_rxnfc_rule_get_all() dereference a NULL rule_locs pointer or
overflow the rule_locs[] heap buffer when called from emac_get_rxnfc() for
ETHTOOL_GRXCLSRLALL?
In net/ethtool/ioctl.c:ethtool_get_rxnfc(), when info.cmd ==
ETHTOOL_GRXCLSRLALL, the kernel allocates rule_buf based on the
user-supplied info.rule_cnt (or leaves rule_buf as NULL if info.rule_cnt ==
0):
net/ethtool/ioctl.c:ethtool_get_rxnfc() {
...
if (info.cmd == ETHTOOL_GRXCLSRLALL) {
if (info.rule_cnt > 0) {
if (info.rule_cnt <= KMALLOC_MAX_SIZE / sizeof(u32))
rule_buf = kcalloc(info.rule_cnt, sizeof(u32),
GFP_USER);
if (!rule_buf)
return -ENOMEM;
}
}
...
ret = ops->get_rxnfc(dev, &info, rule_buf);
}
In emac_get_rxnfc(), the ETHTOOL_GRXCLSRLALL case ignores the caller's
info->rule_cnt capacity, passes a local rule_cnt = 0 to
icssg_rxnfc_rule_get_all(), and does not set info->data to the rule table
size (ICSSG_RXNFC_MAX_RULES). Inside icssg_rxnfc_rule_get_all(), the loop
writes rule_locs[count++] = rule->location for every active rule without
checking if rule_locs is NULL or if count has reached the caller's
info->rule_cnt capacity (where it should return -EMSGSIZE).
If rules are installed and userspace passes info.rule_cnt == 0,
rule_locs[count++] dereferences NULL. If userspace passes 0 < info.rule_cnt
< emac->rxnfc.rxnfc_rule_count, rule_locs[count++] writes past the end of
the kcalloc() buffer and updates info->rule_cnt to the larger count, causing
ethtool_rxnfc_copy_to_user() to read out-of-bounds kernel heap memory back
to userspace.
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> index 5c2936ecd595c..9f0a1116e530c 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> @@ -299,6 +299,52 @@ static int emac_set_per_queue_coalesce(struct net_device *ndev, u32 queue,
> return 0;
> }
>
> +static int emac_get_rxnfc(struct net_device *ndev, struct ethtool_rxnfc *info,
> + u32 *rule_locs)
> +{
> + struct prueth_emac *emac = netdev_priv(ndev);
> + int ret = 0;
> +
> + switch (info->cmd) {
> + case ETHTOOL_GRXRINGS:
> + info->data = emac->rx_flow_num;
> + break;
[Severity: Medium]
Is the ETHTOOL_GRXRINGS case in emac_get_rxnfc() reachable?
In net/ethtool/ioctl.c:ethtool_get_rxrings(), ETHTOOL_GRXRINGS is handled by
calling ethtool_get_rx_ring_count(), which checks ops->get_rx_ring_count in
struct ethtool_ops and returns -EOPNOTSUPP when it is NULL, rather than
calling ops->get_rxnfc(). Should icssg_ethtool_ops implement
.get_rx_ring_count instead?
[ ... ]
> const struct ethtool_ops icssg_ethtool_ops = {
[ ... ]
> .nway_reset = emac_nway_reset,
> .get_rmon_stats = emac_get_rmon_stats,
> + .get_rxnfc = emac_get_rxnfc,
> + .set_rxnfc = emac_set_rxnfc,
> };
> EXPORT_SYMBOL_GPL(icssg_ethtool_ops);
[Severity: High]
What happens when emac_get_rxnfc() or emac_set_rxnfc() (or
ethtool_set_channels() -> ethtool_check_max_channel() ->
ethtool_get_max_rxnfc_channel()) is called on an SR1.0 device?
In drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c:prueth_netdev_init(),
SR1.0 net_devices also use icssg_ethtool_ops:
drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c:prueth_netdev_init() {
...
ndev->netdev_ops = &emac_netdev_ops;
ndev->ethtool_ops = &icssg_ethtool_ops;
...
}
However, icssg_rxnfc_rule_init(emac) is only called in
drivers/net/ethernet/ti/icssg/icssg_prueth.c:prueth_netdev_init() and not in
icssg_prueth_sr1.c, nor do emac_get_rxnfc() / emac_set_rxnfc() guard against
emac->is_sr1. On SR1.0 devices, emac->rxnfc.rxnfc_lock is uninitialized and
emac->rxnfc.rules has NULL next/prev pointers, so any RXNFC call will lock
an uninitialized mutex and dereference a NULL list_head. (Note also that
drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c:emac_ndo_open() and related
RX channel calls in this series still use the old struct prueth_rx_chn
fields and helper signatures, breaking the build when
CONFIG_TI_ICSSG_PRUETH_SR1 is enabled.)
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 8338402597b21..f234198d4cc42 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -874,10 +874,29 @@ static void prueth_destroy_rxq(struct prueth_emac *emac, int qid)
> {
> struct prueth_rx_chn *rx_chn = &emac->rx_chns;
> struct prueth_rx_flow *flow = &rx_chn->flows[qid];
> + dma_addr_t desc_dma;
>
> - /* reset napi and hrtimer, and free all pending buffers */
> napi_disable(&flow->napi_rx);
> hrtimer_cancel(&flow->rx_hrtimer);
> +
> + /* Drain the UDMA completion ring for this flow before resetting
> + * the free ring. k3_udma_glue_reset_rx_chn() only drains the
> + * free ring (descriptors pushed as empty receive buffers). Any
> + * frames that the hardware already DMA-completed but NAPI hadn't
> + * popped yet remain in the completion ring with their pages still
> + * counted as "inflight" in the page pool. Draining them here
> + * ensures page_pool_destroy() finds zero inflight pages and
> + * completes immediately instead of stalling for 60+ seconds.
> + *
> + * The caller must have stopped new frames from arriving (e.g. via
> + * icssg_set_port_state(ICSSG_EMAC_PORT_DISABLE)) before calling
> + * prueth_destroy_rxq() so this loop terminates.
> + */
> + while (!k3_udma_glue_pop_rx_chn(rx_chn->rx_chn, qid, &desc_dma)) {
> + if (!cppi5_desc_is_tdcm(desc_dma))
> + prueth_rx_cleanup(rx_chn, desc_dma);
> + }
> +
> k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, flow->id, rx_chn,
> prueth_rx_cleanup);
>
[Severity: High]
When multiple RX flows are active (emac->rx_flow_num > 1), doesn't calling
prueth_destroy_rxq(emac, i) sequentially in prueth_destroy_rxqs() destroy
flows[0].pg_pool before higher flows' completion rings are drained?
In prueth_init_rx_chns(), all RX flows share flow 0's Free Descriptor Queue
(ringrxfdq), so descriptors pushed by flow 0 (with swdata->flow_id == 0 and
pages from flows[0].pg_pool) can be popped by UDMA when packets are steered
by FT3 rules to flow qid > 0 and placed into flows[qid]'s completion ring.
When prueth_destroy_rxqs() runs:
1. On qid == 0, prueth_destroy_rxq(emac, 0) disables only flows[0].napi_rx
while flows[1..rx_flow_num-1].napi_rx are still enabled and can push
requeued descriptors into the shared ringrxfdq concurrently with
k3_udma_glue_reset_rx_chn(rx_chn->rx_chn, 0, ...).
2. prueth_destroy_rxq(emac, 0) drains only flow 0's completion ring and
immediately calls page_pool_destroy(flows[0].pg_pool) and sets
flows[0].pg_pool = NULL while descriptors with swdata->flow_id == 0 are
still sitting in flows[1..rx_flow_num-1]'s completion rings.
3. When prueth_destroy_rxq(emac, qid) subsequently drains flows[qid] for
qid >= 1 and calls prueth_rx_cleanup():
drivers/net/ethernet/ti/icssg/icssg_common.c:prueth_rx_cleanup() {
...
} else if (swdata->type == PRUETH_SWDATA_PAGE) {
pool = rx_chn->flows[swdata->flow_id].pg_pool;
if (pool) {
page = swdata->data.page;
page_pool_recycle_direct(pool, page);
}
}
}
rx_chn->flows[0].pg_pool is already NULL, so page_pool_recycle_direct()
is skipped and the inflight pages from flows[0].pg_pool are never
recycled, causing page_pool_destroy() to stall and leaking the pages.
[Severity: High]
Can prueth_create_rxq() also hang in napi_disable() when its error path
calls prueth_destroy_rxq()?
In prueth_create_rxq(), if prueth_prepare_rx_flow(), xdp_rxq_info_reg(), or
xdp_rxq_info_reg_mem_model() fails, execution jumps to err before
napi_enable(&flow->napi_rx) is called:
drivers/net/ethernet/ti/icssg/icssg_prueth.c:prueth_create_rxq() {
...
ret = prueth_prepare_rx_flow(emac, id);
if (ret)
goto err;
...
napi_enable(&flow->napi_rx);
return 0;
err:
prueth_destroy_rxq(emac, id);
return ret;
}
Because netif_napi_add() sets NAPIF_STATE_SCHED and napi_enable() was not
called to clear it, calling napi_disable(&flow->napi_rx) at the start of
prueth_destroy_rxq() loops forever waiting for NAPIF_STATE_SCHED to clear.
Furthermore, in prueth_init_rx_chns(), the shared ringrxfdq is sized to
PRUETH_MAX_RX_DESC (512) while each flow pushes 512 descriptors in
prueth_prepare_rx_flow(), so opening an interface with emac->rx_flow_num > 1
deterministically overflows ringrxfdq on flow 1 and hits this hang.
> @@ -1160,6 +1179,9 @@ static int emac_ndo_stop(struct net_device *ndev)
> prueth_cleanup_rx_chns(emac);
> prueth_cleanup_tx_chns(emac);
>
> + /* Cleanup RX flow classification rules */
> + icssg_rxnfc_rule_cleanup(emac);
> +
> prueth->emacs_initialized--;
>
> return 0;
[Severity: High]
Does calling icssg_rxnfc_rule_cleanup(emac) in emac_ndo_stop() cause rules
to be lost across system suspend/resume, while rules added before
emac_ndo_open() or before prueth_emac_restart() lose their hardware
classifier programming?
Specifically:
1. Changing RX channel count in emac_set_channels() requires the interface
to be down (!netif_running(emac->ndev)), and emac_set_rxnfc() allows
adding rules while down. When emac_ndo_open() or prueth_emac_restart()
subsequently calls prueth_emac_common_start() -> icssg_class_default() ->
icssg_class_disable(), all RX_CLASS_OR_EN registers on both slices are
reset to 0 without re-applying emac->rxnfc.rules via
icssg_rxnfc_classifier_update().
2. During suspend, prueth_suspend() calls ndev->netdev_ops->ndo_stop(ndev)
-> emac_ndo_stop() -> icssg_rxnfc_rule_cleanup(emac), which frees and
deletes all user-configured RXNFC rules so they are gone upon
prueth_resume().
> @@ -1676,6 +1698,9 @@ static int prueth_netdev_init(struct prueth *prueth,
> emac->ndev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
> INIT_WORK(&emac->rx_mode_work, emac_ndo_set_rx_mode_work);
>
> + /* Initialize RX flow classification */
> + icssg_rxnfc_rule_init(emac);
> +
> INIT_DELAYED_WORK(&emac->stats_work, icssg_stats_work_handler);
>
> ret = pruss_request_mem_region(prueth->pruss,
[Severity: Medium]
Can struct icssg_rxnfc_rule entries in emac->rxnfc.rules be leaked when the
driver is removed in prueth_netdev_exit()?
Because icssg_rxnfc_rule_add() allocates struct icssg_rxnfc_rule via
kzalloc() even when the interface is down, and icssg_rxnfc_rule_cleanup() is
only called in emac_ndo_stop() (and not in
drivers/net/ethernet/ti/icssg/icssg_common.c:prueth_netdev_exit()), any
rules added while the interface is down are never freed if the device is
unregistered and freed without being opened and stopped first.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103134501.4123194-1-m-malladi@ti.com?part=2
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-04 13:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 13:44 [RFC PATCH net-next 0/2] net: ti: icssg: Multi-flow RX and ethtool RX flow classification Meghana Malladi
2026-11-03 13:44 ` [RFC PATCH net-next 1/2] icssg: Add multi-flow RX support with per-flow NAPI and classification Meghana Malladi
2026-11-04 13:46 ` sashiko-bot
2026-11-03 13:44 ` [RFC PATCH net-next 2/2] icssg: Add ethtool RX flow classification support Meghana Malladi
2026-11-04 13:46 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox