Skip to content

Commit 547d2db

Browse files
committed
Merge branch 'eth-bnxt-fix-several-bugs-in-the-bnxt-module'
Taehee Yoo says: ==================== eth: bnxt: fix several bugs in the bnxt module The first fixes setting incorrect skb->truesize. When xdp-mb prog returns XDP_PASS, skb is allocated and initialized. Currently, The truesize is calculated as BNXT_RX_PAGE_SIZE * sinfo->nr_frags, but sinfo->nr_frags is flushed by napi_build_skb(). So, it stores sinfo before calling napi_build_skb() and then use it for calculate truesize. The second fixes kernel panic in the bnxt_queue_mem_alloc(). The bnxt_queue_mem_alloc() accesses rx ring descriptor. rx ring descriptors are allocated when the interface is up and it's freed when the interface is down. So, if bnxt_queue_mem_alloc() is called when the interface is down, kernel panic occurs. This patch makes the bnxt_queue_mem_alloc() return -ENETDOWN if rx ring descriptors are not allocated. The third patch fixes kernel panic in the bnxt_queue_{start | stop}(). When a queue is restarted bnxt_queue_{start | stop}() are called. These functions set MRU to 0 to stop packet flow and then to set up the remaining things. MRU variable is a member of vnic_info[] the first vnic_info is for default and the second is for ntuple. The first vnic_info is always allocated when interface is up, but the second is allocated only when ntuple is enabled. (ethtool -K eth0 ntuple <on | off>). Currently, the bnxt_queue_{start | stop}() access vnic_info[BNXT_VNIC_NTUPLE] regardless of whether ntuple is enabled or not. So kernel panic occurs. This patch make the bnxt_queue_{start | stop}() use bp->nr_vnics instead of BNXT_VNIC_NTUPLE. The fourth patch fixes a warning due to checksum state. The bnxt_rx_pkt() checks whether skb->ip_summed is not CHECKSUM_NONE before updating ip_summed. if ip_summed is not CHECKSUM_NONE, it WARNS about it. However, the bnxt_xdp_build_skb() is called in XDP-MB-PASS path and it updates ip_summed earlier than bnxt_rx_pkt(). So, in the XDP-MB-PASS path, the bnxt_rx_pkt() always warns about checksum. Updating ip_summed at the bnxt_xdp_build_skb() is unnecessary and duplicate, so it is removed. The fifth patch fixes a kernel panic in the bnxt_get_queue_stats{rx | tx}(). The bnxt_get_queue_stats{rx | tx}() callback functions are called when a queue is resetting. These internally access rx and tx rings without null check, but rings are allocated and initialized when the interface is up. So, these functions are called when the interface is down, it occurs a kernel panic. The sixth patch fixes memory leak in queue reset logic. When a queue is resetting, tpa_info is allocated for the new queue and tpa_info for an old queue is not used anymore. So it should be freed, but not. The seventh patch makes net_devmem_unbind_dmabuf() ignore -ENETDOWN. When devmem socket is closed, net_devmem_unbind_dmabuf() is called to unbind/release resources. If interface is down, the driver returns -ENETDOWN. The -ENETDOWN return value is not an actual error, because the interface will release resources when the interface is down. So, net_devmem_unbind_dmabuf() needs to ignore -ENETDOWN. The last patch adds XDP testcases to tools/testing/selftests/drivers/net/ping.py. ==================== Link: https://patch.msgid.link/20250309134219.91670-1-ap420073@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents cfa693b + 75cc19c commit 547d2db

File tree

6 files changed

+220
-31
lines changed

6 files changed

+220
-31
lines changed

drivers/net/ethernet/broadcom/bnxt/bnxt.c

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2038,6 +2038,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
20382038
struct rx_cmp_ext *rxcmp1;
20392039
u32 tmp_raw_cons = *raw_cons;
20402040
u16 cons, prod, cp_cons = RING_CMP(tmp_raw_cons);
2041+
struct skb_shared_info *sinfo;
20412042
struct bnxt_sw_rx_bd *rx_buf;
20422043
unsigned int len;
20432044
u8 *data_ptr, agg_bufs, cmp_type;
@@ -2164,6 +2165,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
21642165
false);
21652166
if (!frag_len)
21662167
goto oom_next_rx;
2168+
21672169
}
21682170
xdp_active = true;
21692171
}
@@ -2173,6 +2175,12 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
21732175
rc = 1;
21742176
goto next_rx;
21752177
}
2178+
if (xdp_buff_has_frags(&xdp)) {
2179+
sinfo = xdp_get_shared_info_from_buff(&xdp);
2180+
agg_bufs = sinfo->nr_frags;
2181+
} else {
2182+
agg_bufs = 0;
2183+
}
21762184
}
21772185

21782186
if (len <= bp->rx_copybreak) {
@@ -2210,7 +2218,8 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
22102218
if (!skb)
22112219
goto oom_next_rx;
22122220
} else {
2213-
skb = bnxt_xdp_build_skb(bp, skb, agg_bufs, rxr->page_pool, &xdp, rxcmp1);
2221+
skb = bnxt_xdp_build_skb(bp, skb, agg_bufs,
2222+
rxr->page_pool, &xdp);
22142223
if (!skb) {
22152224
/* we should be able to free the old skb here */
22162225
bnxt_xdp_buff_frags_free(rxr, &xdp);
@@ -15375,6 +15384,9 @@ static void bnxt_get_queue_stats_rx(struct net_device *dev, int i,
1537515384
struct bnxt_cp_ring_info *cpr;
1537615385
u64 *sw;
1537715386

15387+
if (!bp->bnapi)
15388+
return;
15389+
1537815390
cpr = &bp->bnapi[i]->cp_ring;
1537915391
sw = cpr->stats.sw_stats;
1538015392

@@ -15398,6 +15410,9 @@ static void bnxt_get_queue_stats_tx(struct net_device *dev, int i,
1539815410
struct bnxt_napi *bnapi;
1539915411
u64 *sw;
1540015412

15413+
if (!bp->tx_ring)
15414+
return;
15415+
1540115416
bnapi = bp->tx_ring[bp->tx_ring_map[i]].bnapi;
1540215417
sw = bnapi->cp_ring.stats.sw_stats;
1540315418

@@ -15439,6 +15454,9 @@ static int bnxt_queue_mem_alloc(struct net_device *dev, void *qmem, int idx)
1543915454
struct bnxt_ring_struct *ring;
1544015455
int rc;
1544115456

15457+
if (!bp->rx_ring)
15458+
return -ENETDOWN;
15459+
1544215460
rxr = &bp->rx_ring[idx];
1544315461
clone = qmem;
1544415462
memcpy(clone, rxr, sizeof(*rxr));
@@ -15521,6 +15539,7 @@ static void bnxt_queue_mem_free(struct net_device *dev, void *qmem)
1552115539
struct bnxt_ring_struct *ring;
1552215540

1552315541
bnxt_free_one_rx_ring_skbs(bp, rxr);
15542+
bnxt_free_one_tpa_info(bp, rxr);
1552415543

1552515544
xdp_rxq_info_unreg(&rxr->xdp_rxq);
1552615545

@@ -15632,7 +15651,7 @@ static int bnxt_queue_start(struct net_device *dev, void *qmem, int idx)
1563215651
cpr = &rxr->bnapi->cp_ring;
1563315652
cpr->sw_stats->rx.rx_resets++;
1563415653

15635-
for (i = 0; i <= BNXT_VNIC_NTUPLE; i++) {
15654+
for (i = 0; i <= bp->nr_vnics; i++) {
1563615655
vnic = &bp->vnic_info[i];
1563715656

1563815657
rc = bnxt_hwrm_vnic_set_rss_p5(bp, vnic, true);
@@ -15660,7 +15679,7 @@ static int bnxt_queue_stop(struct net_device *dev, void *qmem, int idx)
1566015679
struct bnxt_vnic_info *vnic;
1566115680
int i;
1566215681

15663-
for (i = 0; i <= BNXT_VNIC_NTUPLE; i++) {
15682+
for (i = 0; i <= bp->nr_vnics; i++) {
1566415683
vnic = &bp->vnic_info[i];
1566515684
vnic->mru = 0;
1566615685
bnxt_hwrm_vnic_update(bp, vnic,

drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -460,23 +460,16 @@ int bnxt_xdp(struct net_device *dev, struct netdev_bpf *xdp)
460460

461461
struct sk_buff *
462462
bnxt_xdp_build_skb(struct bnxt *bp, struct sk_buff *skb, u8 num_frags,
463-
struct page_pool *pool, struct xdp_buff *xdp,
464-
struct rx_cmp_ext *rxcmp1)
463+
struct page_pool *pool, struct xdp_buff *xdp)
465464
{
466465
struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
467466

468467
if (!skb)
469468
return NULL;
470-
skb_checksum_none_assert(skb);
471-
if (RX_CMP_L4_CS_OK(rxcmp1)) {
472-
if (bp->dev->features & NETIF_F_RXCSUM) {
473-
skb->ip_summed = CHECKSUM_UNNECESSARY;
474-
skb->csum_level = RX_CMP_ENCAP(rxcmp1);
475-
}
476-
}
469+
477470
xdp_update_skb_shared_info(skb, num_frags,
478471
sinfo->xdp_frags_size,
479-
BNXT_RX_PAGE_SIZE * sinfo->nr_frags,
472+
BNXT_RX_PAGE_SIZE * num_frags,
480473
xdp_buff_is_frag_pfmemalloc(xdp));
481474
return skb;
482475
}

drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,5 @@ void bnxt_xdp_buff_frags_free(struct bnxt_rx_ring_info *rxr,
3333
struct xdp_buff *xdp);
3434
struct sk_buff *bnxt_xdp_build_skb(struct bnxt *bp, struct sk_buff *skb,
3535
u8 num_frags, struct page_pool *pool,
36-
struct xdp_buff *xdp,
37-
struct rx_cmp_ext *rxcmp1);
36+
struct xdp_buff *xdp);
3837
#endif

net/core/devmem.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
109109
struct netdev_rx_queue *rxq;
110110
unsigned long xa_idx;
111111
unsigned int rxq_idx;
112+
int err;
112113

113114
if (binding->list.next)
114115
list_del(&binding->list);
@@ -120,7 +121,8 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
120121

121122
rxq_idx = get_netdev_rx_queue_index(rxq);
122123

123-
WARN_ON(netdev_rx_queue_restart(binding->dev, rxq_idx));
124+
err = netdev_rx_queue_restart(binding->dev, rxq_idx);
125+
WARN_ON(err && err != -ENETDOWN);
124126
}
125127

126128
xa_erase(&net_devmem_dmabuf_bindings, binding->id);

tools/testing/selftests/drivers/net/ping.py

Lines changed: 185 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,219 @@
11
#!/usr/bin/env python3
22
# SPDX-License-Identifier: GPL-2.0
33

4+
import os
5+
import random, string, time
46
from lib.py import ksft_run, ksft_exit
5-
from lib.py import ksft_eq
6-
from lib.py import NetDrvEpEnv
7+
from lib.py import ksft_eq, KsftSkipEx, KsftFailEx
8+
from lib.py import EthtoolFamily, NetDrvEpEnv
79
from lib.py import bkg, cmd, wait_port_listen, rand_port
10+
from lib.py import ethtool, ip
811

12+
remote_ifname=""
13+
no_sleep=False
914

10-
def test_v4(cfg) -> None:
15+
def _test_v4(cfg) -> None:
1116
cfg.require_v4()
1217

1318
cmd(f"ping -c 1 -W0.5 {cfg.remote_v4}")
1419
cmd(f"ping -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
20+
cmd(f"ping -s 65000 -c 1 -W0.5 {cfg.remote_v4}")
21+
cmd(f"ping -s 65000 -c 1 -W0.5 {cfg.v4}", host=cfg.remote)
1522

16-
17-
def test_v6(cfg) -> None:
23+
def _test_v6(cfg) -> None:
1824
cfg.require_v6()
1925

20-
cmd(f"ping -c 1 -W0.5 {cfg.remote_v6}")
21-
cmd(f"ping -c 1 -W0.5 {cfg.v6}", host=cfg.remote)
22-
26+
cmd(f"ping -c 1 -W5 {cfg.remote_v6}")
27+
cmd(f"ping -c 1 -W5 {cfg.v6}", host=cfg.remote)
28+
cmd(f"ping -s 65000 -c 1 -W0.5 {cfg.remote_v6}")
29+
cmd(f"ping -s 65000 -c 1 -W0.5 {cfg.v6}", host=cfg.remote)
2330

24-
def test_tcp(cfg) -> None:
31+
def _test_tcp(cfg) -> None:
2532
cfg.require_cmd("socat", remote=True)
2633

2734
port = rand_port()
2835
listen_cmd = f"socat -{cfg.addr_ipver} -t 2 -u TCP-LISTEN:{port},reuseport STDOUT"
2936

37+
test_string = ''.join(random.choice(string.ascii_lowercase) for _ in range(65536))
3038
with bkg(listen_cmd, exit_wait=True) as nc:
3139
wait_port_listen(port)
3240

33-
cmd(f"echo ping | socat -t 2 -u STDIN TCP:{cfg.baddr}:{port}",
41+
cmd(f"echo {test_string} | socat -t 2 -u STDIN TCP:{cfg.baddr}:{port}",
3442
shell=True, host=cfg.remote)
35-
ksft_eq(nc.stdout.strip(), "ping")
43+
ksft_eq(nc.stdout.strip(), test_string)
3644

45+
test_string = ''.join(random.choice(string.ascii_lowercase) for _ in range(65536))
3746
with bkg(listen_cmd, host=cfg.remote, exit_wait=True) as nc:
3847
wait_port_listen(port, host=cfg.remote)
3948

40-
cmd(f"echo ping | socat -t 2 -u STDIN TCP:{cfg.remote_baddr}:{port}", shell=True)
41-
ksft_eq(nc.stdout.strip(), "ping")
42-
49+
cmd(f"echo {test_string} | socat -t 2 -u STDIN TCP:{cfg.remote_baddr}:{port}", shell=True)
50+
ksft_eq(nc.stdout.strip(), test_string)
51+
52+
def _set_offload_checksum(cfg, netnl, on) -> None:
53+
try:
54+
ethtool(f" -K {cfg.ifname} rx {on} tx {on} ")
55+
except:
56+
return
57+
58+
def _set_xdp_generic_sb_on(cfg) -> None:
59+
test_dir = os.path.dirname(os.path.realpath(__file__))
60+
prog = test_dir + "/../../net/lib/xdp_dummy.bpf.o"
61+
cmd(f"ip link set dev {remote_ifname} mtu 1500", shell=True, host=cfg.remote)
62+
cmd(f"ip link set dev {cfg.ifname} mtu 1500 xdpgeneric obj {prog} sec xdp", shell=True)
63+
64+
if no_sleep != True:
65+
time.sleep(10)
66+
67+
def _set_xdp_generic_mb_on(cfg) -> None:
68+
test_dir = os.path.dirname(os.path.realpath(__file__))
69+
prog = test_dir + "/../../net/lib/xdp_dummy.bpf.o"
70+
cmd(f"ip link set dev {remote_ifname} mtu 9000", shell=True, host=cfg.remote)
71+
ip("link set dev %s mtu 9000 xdpgeneric obj %s sec xdp.frags" % (cfg.ifname, prog))
72+
73+
if no_sleep != True:
74+
time.sleep(10)
75+
76+
def _set_xdp_native_sb_on(cfg) -> None:
77+
test_dir = os.path.dirname(os.path.realpath(__file__))
78+
prog = test_dir + "/../../net/lib/xdp_dummy.bpf.o"
79+
cmd(f"ip link set dev {remote_ifname} mtu 1500", shell=True, host=cfg.remote)
80+
cmd(f"ip -j link set dev {cfg.ifname} mtu 1500 xdp obj {prog} sec xdp", shell=True)
81+
xdp_info = ip("-d link show %s" % (cfg.ifname), json=True)[0]
82+
if xdp_info['xdp']['mode'] != 1:
83+
"""
84+
If the interface doesn't support native-mode, it falls back to generic mode.
85+
The mode value 1 is native and 2 is generic.
86+
So it raises an exception if mode is not 1(native mode).
87+
"""
88+
raise KsftSkipEx('device does not support native-XDP')
89+
90+
if no_sleep != True:
91+
time.sleep(10)
92+
93+
def _set_xdp_native_mb_on(cfg) -> None:
94+
test_dir = os.path.dirname(os.path.realpath(__file__))
95+
prog = test_dir + "/../../net/lib/xdp_dummy.bpf.o"
96+
cmd(f"ip link set dev {remote_ifname} mtu 9000", shell=True, host=cfg.remote)
97+
try:
98+
cmd(f"ip link set dev {cfg.ifname} mtu 9000 xdp obj {prog} sec xdp.frags", shell=True)
99+
except Exception as e:
100+
cmd(f"ip link set dev {remote_ifname} mtu 1500", shell=True, host=cfg.remote)
101+
raise KsftSkipEx('device does not support native-multi-buffer XDP')
102+
103+
if no_sleep != True:
104+
time.sleep(10)
105+
106+
def _set_xdp_offload_on(cfg) -> None:
107+
test_dir = os.path.dirname(os.path.realpath(__file__))
108+
prog = test_dir + "/../../net/lib/xdp_dummy.bpf.o"
109+
cmd(f"ip link set dev {cfg.ifname} mtu 1500", shell=True)
110+
try:
111+
cmd(f"ip link set dev {cfg.ifname} xdpoffload obj {prog} sec xdp", shell=True)
112+
except Exception as e:
113+
raise KsftSkipEx('device does not support offloaded XDP')
114+
cmd(f"ip link set dev {remote_ifname} mtu 1500", shell=True, host=cfg.remote)
115+
116+
if no_sleep != True:
117+
time.sleep(10)
118+
119+
def get_interface_info(cfg) -> None:
120+
global remote_ifname
121+
global no_sleep
122+
123+
remote_info = cmd(f"ip -4 -o addr show to {cfg.remote_v4} | awk '{{print $2}}'", shell=True, host=cfg.remote).stdout
124+
remote_ifname = remote_info.rstrip('\n')
125+
if remote_ifname == "":
126+
raise KsftFailEx('Can not get remote interface')
127+
local_info = ip("-d link show %s" % (cfg.ifname), json=True)[0]
128+
if 'parentbus' in local_info and local_info['parentbus'] == "netdevsim":
129+
no_sleep=True
130+
if 'linkinfo' in local_info and local_info['linkinfo']['info_kind'] == "veth":
131+
no_sleep=True
132+
133+
def set_interface_init(cfg) -> None:
134+
cmd(f"ip link set dev {cfg.ifname} mtu 1500", shell=True)
135+
cmd(f"ip link set dev {cfg.ifname} xdp off ", shell=True)
136+
cmd(f"ip link set dev {cfg.ifname} xdpgeneric off ", shell=True)
137+
cmd(f"ip link set dev {cfg.ifname} xdpoffload off", shell=True)
138+
cmd(f"ip link set dev {remote_ifname} mtu 1500", shell=True, host=cfg.remote)
139+
140+
def test_default(cfg, netnl) -> None:
141+
_set_offload_checksum(cfg, netnl, "off")
142+
_test_v4(cfg)
143+
_test_v6(cfg)
144+
_test_tcp(cfg)
145+
_set_offload_checksum(cfg, netnl, "on")
146+
_test_v4(cfg)
147+
_test_v6(cfg)
148+
_test_tcp(cfg)
149+
150+
def test_xdp_generic_sb(cfg, netnl) -> None:
151+
_set_xdp_generic_sb_on(cfg)
152+
_set_offload_checksum(cfg, netnl, "off")
153+
_test_v4(cfg)
154+
_test_v6(cfg)
155+
_test_tcp(cfg)
156+
_set_offload_checksum(cfg, netnl, "on")
157+
_test_v4(cfg)
158+
_test_v6(cfg)
159+
_test_tcp(cfg)
160+
ip("link set dev %s xdpgeneric off" % cfg.ifname)
161+
162+
def test_xdp_generic_mb(cfg, netnl) -> None:
163+
_set_xdp_generic_mb_on(cfg)
164+
_set_offload_checksum(cfg, netnl, "off")
165+
_test_v4(cfg)
166+
_test_v6(cfg)
167+
_test_tcp(cfg)
168+
_set_offload_checksum(cfg, netnl, "on")
169+
_test_v4(cfg)
170+
_test_v6(cfg)
171+
_test_tcp(cfg)
172+
ip("link set dev %s xdpgeneric off" % cfg.ifname)
173+
174+
def test_xdp_native_sb(cfg, netnl) -> None:
175+
_set_xdp_native_sb_on(cfg)
176+
_set_offload_checksum(cfg, netnl, "off")
177+
_test_v4(cfg)
178+
_test_v6(cfg)
179+
_test_tcp(cfg)
180+
_set_offload_checksum(cfg, netnl, "on")
181+
_test_v4(cfg)
182+
_test_v6(cfg)
183+
_test_tcp(cfg)
184+
ip("link set dev %s xdp off" % cfg.ifname)
185+
186+
def test_xdp_native_mb(cfg, netnl) -> None:
187+
_set_xdp_native_mb_on(cfg)
188+
_set_offload_checksum(cfg, netnl, "off")
189+
_test_v4(cfg)
190+
_test_v6(cfg)
191+
_test_tcp(cfg)
192+
_set_offload_checksum(cfg, netnl, "on")
193+
_test_v4(cfg)
194+
_test_v6(cfg)
195+
_test_tcp(cfg)
196+
ip("link set dev %s xdp off" % cfg.ifname)
197+
198+
def test_xdp_offload(cfg, netnl) -> None:
199+
_set_xdp_offload_on(cfg)
200+
_test_v4(cfg)
201+
_test_v6(cfg)
202+
_test_tcp(cfg)
203+
ip("link set dev %s xdpoffload off" % cfg.ifname)
43204

44205
def main() -> None:
45206
with NetDrvEpEnv(__file__) as cfg:
46-
ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg, ))
207+
get_interface_info(cfg)
208+
set_interface_init(cfg)
209+
ksft_run([test_default,
210+
test_xdp_generic_sb,
211+
test_xdp_generic_mb,
212+
test_xdp_native_sb,
213+
test_xdp_native_mb,
214+
test_xdp_offload],
215+
args=(cfg, EthtoolFamily()))
216+
set_interface_init(cfg)
47217
ksft_exit()
48218

49219

tools/testing/selftests/net/lib/xdp_dummy.bpf.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,10 @@ int xdp_dummy_prog(struct xdp_md *ctx)
1010
return XDP_PASS;
1111
}
1212

13+
SEC("xdp.frags")
14+
int xdp_dummy_prog_frags(struct xdp_md *ctx)
15+
{
16+
return XDP_PASS;
17+
}
18+
1319
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)