Skip to content

Commit e6cc7ac

Browse files
author
Paolo Abeni
committed
Merge branch 'eth-fbnic-fix-xdp_tx-and-xdp-vs-qstats'
Jakub Kicinski says: ==================== eth: fbnic: fix XDP_TX and XDP vs qstats Fix XDP_TX hangs and adjust the XDP statistics to match the definition of qstats. The three problems are somewhat distinct. XDP_TX hangs is a simple coding bug (patch 1). The accounting of XDP packets is all over the place. Fix it to obey qstat rules (packets seen by XDP always counted as Rx packets). Patch 2 fixes the basic accounting, patch 3 touches up saving the stats when rings are freed. Patch 6 corrects reporting of alloc_fail stats which prevented the pp_alloc_fail test from passing. Patches 4, 5, 7, 8, 9 add or fix related test cases. v2: - [patch 2] remove now unnecessary byte adjustment - [patch 8] use seen_fails more v1: https://lore.kernel.org/20251003233025.1157158-1-kuba@kernel.org Testing on fbnic below: $ ./tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py TAP version 13 1..1 fbnic-err: bad MMIO read address 0x80074 fbnic-err: bad MMIO read address 0x80074 # Seen: pkts:20605 fails:40 (pass thrs:12) # ethtool -G change retval: success ok 1 pp_alloc_fail.test_pp_alloc # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0 $ ./tools/testing/selftests/drivers/net/xdp.py TAP version 13 1..13 ok 1 xdp.test_xdp_native_pass_sb ok 2 xdp.test_xdp_native_pass_mb ok 3 xdp.test_xdp_native_drop_sb ok 4 xdp.test_xdp_native_drop_mb ok 5 xdp.test_xdp_native_tx_sb ok 6 xdp.test_xdp_native_tx_mb # Failed run: pkt_sz 2048, offset 1. Last successful run: pkt_sz 1024, offset 256. Reason: Adjustment failed ok 7 xdp.test_xdp_native_adjst_tail_grow_data ok 8 xdp.test_xdp_native_adjst_tail_shrnk_data # Failed run: pkt_sz 512, offset -256. Last successful run: pkt_sz 512, offset -128. Reason: Adjustment failed ok 9 xdp.test_xdp_native_adjst_head_grow_data # Failed run: pkt_sz (2048) > HDS threshold (1536) and offset 64 > 48 ok 10 xdp.test_xdp_native_adjst_head_shrnk_data ok 11 xdp.test_xdp_native_qstats_pass ok 12 xdp.test_xdp_native_qstats_drop ok 13 xdp.test_xdp_native_qstats_tx # Totals: pass:13 fail:0 xfail:0 xpass:0 skip:0 error:0 ==================== Link: https://patch.msgid.link/20251007232653.2099376-1-kuba@kernel.org Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2 parents 2854378 + 5d683e5 commit e6cc7ac

File tree

9 files changed

+209
-49
lines changed

9 files changed

+209
-49
lines changed

drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,13 @@ static void fbnic_aggregate_vector_counters(struct fbnic_net *fbn,
185185

186186
for (i = 0; i < nv->txt_count; i++) {
187187
fbnic_aggregate_ring_tx_counters(fbn, &nv->qt[i].sub0);
188-
fbnic_aggregate_ring_tx_counters(fbn, &nv->qt[i].sub1);
188+
fbnic_aggregate_ring_xdp_counters(fbn, &nv->qt[i].sub1);
189189
fbnic_aggregate_ring_tx_counters(fbn, &nv->qt[i].cmpl);
190190
}
191191

192192
for (j = 0; j < nv->rxt_count; j++, i++) {
193-
fbnic_aggregate_ring_rx_counters(fbn, &nv->qt[i].sub0);
194-
fbnic_aggregate_ring_rx_counters(fbn, &nv->qt[i].sub1);
193+
fbnic_aggregate_ring_bdq_counters(fbn, &nv->qt[i].sub0);
194+
fbnic_aggregate_ring_bdq_counters(fbn, &nv->qt[i].sub1);
195195
fbnic_aggregate_ring_rx_counters(fbn, &nv->qt[i].cmpl);
196196
}
197197
}

drivers/net/ethernet/meta/fbnic/fbnic_mac.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,16 @@ static void fbnic_mac_init_axi(struct fbnic_dev *fbd)
8383

8484
static void fbnic_mac_init_qm(struct fbnic_dev *fbd)
8585
{
86+
u64 default_meta = FIELD_PREP(FBNIC_TWD_L2_HLEN_MASK, ETH_HLEN) |
87+
FBNIC_TWD_FLAG_REQ_COMPLETION;
8688
u32 clock_freq;
8789

90+
/* Configure default TWQ Metadata descriptor */
91+
wr32(fbd, FBNIC_QM_TWQ_DEFAULT_META_L,
92+
lower_32_bits(default_meta));
93+
wr32(fbd, FBNIC_QM_TWQ_DEFAULT_META_H,
94+
upper_32_bits(default_meta));
95+
8896
/* Configure TSO behavior */
8997
wr32(fbd, FBNIC_QM_TQS_CTL0,
9098
FIELD_PREP(FBNIC_QM_TQS_CTL0_LSO_TS_MASK,

drivers/net/ethernet/meta/fbnic/fbnic_netdev.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,17 +543,21 @@ static const struct net_device_ops fbnic_netdev_ops = {
543543
static void fbnic_get_queue_stats_rx(struct net_device *dev, int idx,
544544
struct netdev_queue_stats_rx *rx)
545545
{
546+
u64 bytes, packets, alloc_fail, alloc_fail_bdq;
546547
struct fbnic_net *fbn = netdev_priv(dev);
547548
struct fbnic_ring *rxr = fbn->rx[idx];
548549
struct fbnic_dev *fbd = fbn->fbd;
549550
struct fbnic_queue_stats *stats;
550-
u64 bytes, packets, alloc_fail;
551551
u64 csum_complete, csum_none;
552+
struct fbnic_q_triad *qt;
552553
unsigned int start;
553554

554555
if (!rxr)
555556
return;
556557

558+
/* fbn->rx points to completion queues */
559+
qt = container_of(rxr, struct fbnic_q_triad, cmpl);
560+
557561
stats = &rxr->stats;
558562
do {
559563
start = u64_stats_fetch_begin(&stats->syncp);
@@ -564,6 +568,20 @@ static void fbnic_get_queue_stats_rx(struct net_device *dev, int idx,
564568
csum_none = stats->rx.csum_none;
565569
} while (u64_stats_fetch_retry(&stats->syncp, start));
566570

571+
stats = &qt->sub0.stats;
572+
do {
573+
start = u64_stats_fetch_begin(&stats->syncp);
574+
alloc_fail_bdq = stats->bdq.alloc_failed;
575+
} while (u64_stats_fetch_retry(&stats->syncp, start));
576+
alloc_fail += alloc_fail_bdq;
577+
578+
stats = &qt->sub1.stats;
579+
do {
580+
start = u64_stats_fetch_begin(&stats->syncp);
581+
alloc_fail_bdq = stats->bdq.alloc_failed;
582+
} while (u64_stats_fetch_retry(&stats->syncp, start));
583+
alloc_fail += alloc_fail_bdq;
584+
567585
rx->bytes = bytes;
568586
rx->packets = packets;
569587
rx->alloc_fail = alloc_fail;
@@ -641,7 +659,8 @@ static void fbnic_get_base_stats(struct net_device *dev,
641659

642660
rx->bytes = fbn->rx_stats.bytes;
643661
rx->packets = fbn->rx_stats.packets;
644-
rx->alloc_fail = fbn->rx_stats.rx.alloc_failed;
662+
rx->alloc_fail = fbn->rx_stats.rx.alloc_failed +
663+
fbn->bdq_stats.bdq.alloc_failed;
645664
rx->csum_complete = fbn->rx_stats.rx.csum_complete;
646665
rx->csum_none = fbn->rx_stats.rx.csum_none;
647666
}

drivers/net/ethernet/meta/fbnic/fbnic_netdev.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ struct fbnic_net {
6868
/* Storage for stats after ring destruction */
6969
struct fbnic_queue_stats tx_stats;
7070
struct fbnic_queue_stats rx_stats;
71+
struct fbnic_queue_stats bdq_stats;
7172
u64 link_down_events;
7273

7374
/* Time stamping filter config */

drivers/net/ethernet/meta/fbnic/fbnic_txrx.c

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,7 @@ static void fbnic_fill_bdq(struct fbnic_ring *bdq)
904904
netmem = page_pool_dev_alloc_netmems(bdq->page_pool);
905905
if (!netmem) {
906906
u64_stats_update_begin(&bdq->stats.syncp);
907-
bdq->stats.rx.alloc_failed++;
907+
bdq->stats.bdq.alloc_failed++;
908908
u64_stats_update_end(&bdq->stats.syncp);
909909

910910
break;
@@ -1242,6 +1242,7 @@ static int fbnic_clean_rcq(struct fbnic_napi_vector *nv,
12421242
/* Walk the completion queue collecting the heads reported by NIC */
12431243
while (likely(packets < budget)) {
12441244
struct sk_buff *skb = ERR_PTR(-EINVAL);
1245+
u32 pkt_bytes;
12451246
u64 rcd;
12461247

12471248
if ((*raw_rcd & cpu_to_le64(FBNIC_RCD_DONE)) == done)
@@ -1272,37 +1273,38 @@ static int fbnic_clean_rcq(struct fbnic_napi_vector *nv,
12721273
/* We currently ignore the action table index */
12731274
break;
12741275
case FBNIC_RCD_TYPE_META:
1275-
if (unlikely(pkt->add_frag_failed))
1276-
skb = NULL;
1277-
else if (likely(!fbnic_rcd_metadata_err(rcd)))
1276+
if (likely(!fbnic_rcd_metadata_err(rcd) &&
1277+
!pkt->add_frag_failed)) {
1278+
pkt_bytes = xdp_get_buff_len(&pkt->buff);
12781279
skb = fbnic_run_xdp(nv, pkt);
1280+
}
12791281

12801282
/* Populate skb and invalidate XDP */
12811283
if (!IS_ERR_OR_NULL(skb)) {
12821284
fbnic_populate_skb_fields(nv, rcd, skb, qt,
12831285
&csum_complete,
12841286
&csum_none);
1285-
1286-
packets++;
1287-
bytes += skb->len;
1288-
12891287
napi_gro_receive(&nv->napi, skb);
12901288
} else if (skb == ERR_PTR(-FBNIC_XDP_TX)) {
12911289
pkt_tail = nv->qt[0].sub1.tail;
1292-
bytes += xdp_get_buff_len(&pkt->buff);
1290+
} else if (PTR_ERR(skb) == -FBNIC_XDP_CONSUME) {
1291+
fbnic_put_pkt_buff(qt, pkt, 1);
12931292
} else {
1294-
if (!skb) {
1293+
if (!skb)
12951294
alloc_failed++;
1296-
dropped++;
1297-
} else if (skb == ERR_PTR(-FBNIC_XDP_LEN_ERR)) {
1295+
1296+
if (skb == ERR_PTR(-FBNIC_XDP_LEN_ERR))
12981297
length_errors++;
1299-
} else {
1298+
else
13001299
dropped++;
1301-
}
13021300

13031301
fbnic_put_pkt_buff(qt, pkt, 1);
1302+
goto next_dont_count;
13041303
}
13051304

1305+
packets++;
1306+
bytes += pkt_bytes;
1307+
next_dont_count:
13061308
pkt->buff.data_hard_start = NULL;
13071309

13081310
break;
@@ -1319,8 +1321,6 @@ static int fbnic_clean_rcq(struct fbnic_napi_vector *nv,
13191321
u64_stats_update_begin(&rcq->stats.syncp);
13201322
rcq->stats.packets += packets;
13211323
rcq->stats.bytes += bytes;
1322-
/* Re-add ethernet header length (removed in fbnic_build_skb) */
1323-
rcq->stats.bytes += ETH_HLEN * packets;
13241324
rcq->stats.dropped += dropped;
13251325
rcq->stats.rx.alloc_failed += alloc_failed;
13261326
rcq->stats.rx.csum_complete += csum_complete;
@@ -1414,6 +1414,17 @@ void fbnic_aggregate_ring_rx_counters(struct fbnic_net *fbn,
14141414
BUILD_BUG_ON(sizeof(fbn->rx_stats.rx) / 8 != 4);
14151415
}
14161416

1417+
void fbnic_aggregate_ring_bdq_counters(struct fbnic_net *fbn,
1418+
struct fbnic_ring *bdq)
1419+
{
1420+
struct fbnic_queue_stats *stats = &bdq->stats;
1421+
1422+
/* Capture stats from queues before dissasociating them */
1423+
fbn->bdq_stats.bdq.alloc_failed += stats->bdq.alloc_failed;
1424+
/* Remember to add new stats here */
1425+
BUILD_BUG_ON(sizeof(fbn->rx_stats.bdq) / 8 != 1);
1426+
}
1427+
14171428
void fbnic_aggregate_ring_tx_counters(struct fbnic_net *fbn,
14181429
struct fbnic_ring *txr)
14191430
{
@@ -1433,18 +1444,16 @@ void fbnic_aggregate_ring_tx_counters(struct fbnic_net *fbn,
14331444
BUILD_BUG_ON(sizeof(fbn->tx_stats.twq) / 8 != 6);
14341445
}
14351446

1436-
static void fbnic_aggregate_ring_xdp_counters(struct fbnic_net *fbn,
1437-
struct fbnic_ring *xdpr)
1447+
void fbnic_aggregate_ring_xdp_counters(struct fbnic_net *fbn,
1448+
struct fbnic_ring *xdpr)
14381449
{
14391450
struct fbnic_queue_stats *stats = &xdpr->stats;
14401451

14411452
if (!(xdpr->flags & FBNIC_RING_F_STATS))
14421453
return;
14431454

14441455
/* Capture stats from queues before dissasociating them */
1445-
fbn->rx_stats.bytes += stats->bytes;
1446-
fbn->rx_stats.packets += stats->packets;
1447-
fbn->rx_stats.dropped += stats->dropped;
1456+
fbn->tx_stats.dropped += stats->dropped;
14481457
fbn->tx_stats.bytes += stats->bytes;
14491458
fbn->tx_stats.packets += stats->packets;
14501459
}
@@ -1488,6 +1497,15 @@ static void fbnic_remove_rx_ring(struct fbnic_net *fbn,
14881497
fbn->rx[rxr->q_idx] = NULL;
14891498
}
14901499

1500+
static void fbnic_remove_bdq_ring(struct fbnic_net *fbn,
1501+
struct fbnic_ring *bdq)
1502+
{
1503+
if (!(bdq->flags & FBNIC_RING_F_STATS))
1504+
return;
1505+
1506+
fbnic_aggregate_ring_bdq_counters(fbn, bdq);
1507+
}
1508+
14911509
static void fbnic_free_qt_page_pools(struct fbnic_q_triad *qt)
14921510
{
14931511
page_pool_destroy(qt->sub0.page_pool);
@@ -1507,8 +1525,8 @@ static void fbnic_free_napi_vector(struct fbnic_net *fbn,
15071525
}
15081526

15091527
for (j = 0; j < nv->rxt_count; j++, i++) {
1510-
fbnic_remove_rx_ring(fbn, &nv->qt[i].sub0);
1511-
fbnic_remove_rx_ring(fbn, &nv->qt[i].sub1);
1528+
fbnic_remove_bdq_ring(fbn, &nv->qt[i].sub0);
1529+
fbnic_remove_bdq_ring(fbn, &nv->qt[i].sub1);
15121530
fbnic_remove_rx_ring(fbn, &nv->qt[i].cmpl);
15131531
}
15141532

@@ -1707,11 +1725,13 @@ static int fbnic_alloc_napi_vector(struct fbnic_dev *fbd, struct fbnic_net *fbn,
17071725
while (rxt_count) {
17081726
/* Configure header queue */
17091727
db = &uc_addr[FBNIC_QUEUE(rxq_idx) + FBNIC_QUEUE_BDQ_HPQ_TAIL];
1710-
fbnic_ring_init(&qt->sub0, db, 0, FBNIC_RING_F_CTX);
1728+
fbnic_ring_init(&qt->sub0, db, 0,
1729+
FBNIC_RING_F_CTX | FBNIC_RING_F_STATS);
17111730

17121731
/* Configure payload queue */
17131732
db = &uc_addr[FBNIC_QUEUE(rxq_idx) + FBNIC_QUEUE_BDQ_PPQ_TAIL];
1714-
fbnic_ring_init(&qt->sub1, db, 0, FBNIC_RING_F_CTX);
1733+
fbnic_ring_init(&qt->sub1, db, 0,
1734+
FBNIC_RING_F_CTX | FBNIC_RING_F_STATS);
17151735

17161736
/* Configure Rx completion queue */
17171737
db = &uc_addr[FBNIC_QUEUE(rxq_idx) + FBNIC_QUEUE_RCQ_HEAD];
@@ -2830,8 +2850,8 @@ static int fbnic_queue_start(struct net_device *dev, void *qmem, int idx)
28302850
real = container_of(fbn->rx[idx], struct fbnic_q_triad, cmpl);
28312851
nv = fbn->napi[idx % fbn->num_napi];
28322852

2833-
fbnic_aggregate_ring_rx_counters(fbn, &real->sub0);
2834-
fbnic_aggregate_ring_rx_counters(fbn, &real->sub1);
2853+
fbnic_aggregate_ring_bdq_counters(fbn, &real->sub0);
2854+
fbnic_aggregate_ring_bdq_counters(fbn, &real->sub1);
28352855
fbnic_aggregate_ring_rx_counters(fbn, &real->cmpl);
28362856

28372857
memcpy(real, qmem, sizeof(*real));

drivers/net/ethernet/meta/fbnic/fbnic_txrx.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ struct fbnic_queue_stats {
9292
u64 csum_none;
9393
u64 length_errors;
9494
} rx;
95+
struct {
96+
u64 alloc_failed;
97+
} bdq;
9598
};
9699
u64 dropped;
97100
struct u64_stats_sync syncp;
@@ -165,8 +168,12 @@ fbnic_features_check(struct sk_buff *skb, struct net_device *dev,
165168

166169
void fbnic_aggregate_ring_rx_counters(struct fbnic_net *fbn,
167170
struct fbnic_ring *rxr);
171+
void fbnic_aggregate_ring_bdq_counters(struct fbnic_net *fbn,
172+
struct fbnic_ring *rxr);
168173
void fbnic_aggregate_ring_tx_counters(struct fbnic_net *fbn,
169174
struct fbnic_ring *txr);
175+
void fbnic_aggregate_ring_xdp_counters(struct fbnic_net *fbn,
176+
struct fbnic_ring *xdpr);
170177

171178
int fbnic_alloc_napi_vectors(struct fbnic_net *fbn);
172179
void fbnic_free_napi_vectors(struct fbnic_net *fbn);

tools/testing/selftests/drivers/net/hw/config

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
CONFIG_FAIL_FUNCTION=y
2+
CONFIG_FAULT_INJECTION=y
3+
CONFIG_FAULT_INJECTION_DEBUG_FS=y
4+
CONFIG_FUNCTION_ERROR_INJECTION=y
15
CONFIG_IO_URING=y
26
CONFIG_IPV6=y
37
CONFIG_IPV6_GRE=y

0 commit comments

Comments
 (0)