[PATCH net 0/3] eth: bnxt: fix several bugs in the bnxt module

[PATCH net 0/3] eth: bnxt: fix several bugs in the bnxt module

[Taehee Yoo]

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 not correct at this moment.
So, it should use num_frags instead of sinfo->nr_frags.

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 fix 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.

Taehee Yoo (3):
  eth: bnxt: fix truesize for mb-xdp-pass case
  eth: bnxt: return fail if interface is down in bnxt_queue_mem_alloc()
  eth: bnxt: do not use BNXT_VNIC_NTUPLE unconditionally in queue
    restart logic

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 7 +++++--
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH net 1/3] eth: bnxt: fix truesize for mb-xdp-pass case

[Taehee Yoo]

When mb-xdp is set and return is XDP_PASS, packet is converted from
xdp_buff to sk_buff with xdp_update_skb_shared_info() in
bnxt_xdp_build_skb().
bnxt_xdp_build_skb() passes incorrect truesize argument to
xdp_update_skb_shared_info().
truesize is calculated as BNXT_RX_PAGE_SIZE * sinfo->nr_frags but
sinfo->nr_frags should not be used because sinfo->nr_frags is not yet
updated.
so it should use num_frags instead.

Splat looks like:
 ------------[ cut here ]------------
 WARNING: CPU: 2 PID: 0 at net/core/skbuff.c:6072 skb_try_coalesce+0x504/0x590
 Modules linked in: xt_nat xt_tcpudp veth af_packet xt_conntrack nft_chain_nat xt_MASQUERADE nf_conntrack_netlink xfrm_user xt_addrtype nft_coms
 CPU: 2 UID: 0 PID: 0 Comm: swapper/2 Not tainted 6.14.0-rc2+ #3
 RIP: 0010:skb_try_coalesce+0x504/0x590
 Code: 4b fd ff ff 49 8b 34 24 40 80 e6 40 0f 84 3d fd ff ff 49 8b 74 24 48 40 f6 c6 01 0f 84 2e fd ff ff 48 8d 4e ff e9 25 fd ff ff <0f> 0b e99
 RSP: 0018:ffffb62c4120caa8 EFLAGS: 00010287
 RAX: 0000000000000003 RBX: ffffb62c4120cb14 RCX: 0000000000000ec0
 RDX: 0000000000001000 RSI: ffffa06e5d7dc000 RDI: 0000000000000003
 RBP: ffffa06e5d7ddec0 R08: ffffa06e6120a800 R09: ffffa06e7a119900
 R10: 0000000000002310 R11: ffffa06e5d7dcec0 R12: ffffe4360575f740
 R13: ffffe43600000000 R14: 0000000000000002 R15: 0000000000000002
 FS:  0000000000000000(0000) GS:ffffa0755f700000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007f147b76b0f8 CR3: 00000001615d4000 CR4: 00000000007506f0
 PKRU: 55555554
 Call Trace:
  <IRQ>
  ? __warn+0x84/0x130
  ? skb_try_coalesce+0x504/0x590
  ? report_bug+0x18a/0x1a0
  ? handle_bug+0x53/0x90
  ? exc_invalid_op+0x14/0x70
  ? asm_exc_invalid_op+0x16/0x20
  ? skb_try_coalesce+0x504/0x590
  inet_frag_reasm_finish+0x11f/0x2e0
  ip_defrag+0x37a/0x900
  ip_local_deliver+0x51/0x120
  ip_sublist_rcv_finish+0x64/0x70
  ip_sublist_rcv+0x179/0x210
  ip_list_rcv+0xf9/0x130

How to reproduce:
<Node A>
ip link set $interface1 xdp obj xdp_pass.o
ip link set $interface1 mtu 9000 up
ip a a 10.0.0.1/24 dev $interface1
<Node B>
ip link set $interfac2 mtu 9000 up
ip a a 10.0.0.2/24 dev $interface2
ping 10.0.0.1 -s 65000

Fixes: 1dc4c557bfed ("bnxt: adding bnxt_xdp_build_skb to build skb from multibuffer xdp_buff")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
index e6c64e4bd66c..e9b49cb5b735 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
@@ -476,7 +476,7 @@ bnxt_xdp_build_skb(struct bnxt *bp, struct sk_buff *skb, u8 num_frags,
 	}
 	xdp_update_skb_shared_info(skb, num_frags,
 				   sinfo->xdp_frags_size,
-				   BNXT_RX_PAGE_SIZE * sinfo->nr_frags,
+				   BNXT_RX_PAGE_SIZE * num_frags,
 				   xdp_buff_is_frag_pfmemalloc(xdp));
 	return skb;
 }
-- 
2.34.1

[PATCH net 2/3] eth: bnxt: return fail if interface is down in bnxt_queue_mem_alloc()

[Taehee Yoo]

The bnxt_queue_mem_alloc() is called to allocate new queue memory when
a queue is restarted.
It internally accesses rx buffer descriptor corresponding to the index.
The rx buffer descriptor is allocated and set when the interface is up
and it's freed when the interface is down.
So, if queue is restarted if interface is down, kernel panic occurs.

Splat looks like:
 BUG: unable to handle page fault for address: 000000000000b240
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0
 Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
 CPU: 3 UID: 0 PID: 1563 Comm: ncdevmem2 Not tainted 6.14.0-rc2+ #9 844ddba6e7c459cafd0bf4db9a3198e
 Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021
 RIP: 0010:bnxt_queue_mem_alloc+0x3f/0x4e0 [bnxt_en]
 Code: 41 54 4d 89 c4 4d 69 c0 c0 05 00 00 55 48 89 f5 53 48 89 fb 4c 8d b5 40 05 00 00 48 83 ec 15
 RSP: 0018:ffff9dcc83fef9e8 EFLAGS: 00010202
 RAX: ffffffffc0457720 RBX: ffff934ed8d40000 RCX: 0000000000000000
 RDX: 000000000000001f RSI: ffff934ea508f800 RDI: ffff934ea508f808
 RBP: ffff934ea508f800 R08: 000000000000b240 R09: ffff934e84f4b000
 R10: ffff9dcc83fefa30 R11: ffff934e84f4b000 R12: 000000000000001f
 R13: ffff934ed8d40ac0 R14: ffff934ea508fd40 R15: ffff934e84f4b000
 FS:  00007fa73888c740(0000) GS:ffff93559f780000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 000000000000b240 CR3: 0000000145a2e000 CR4: 00000000007506f0
 PKRU: 55555554
 Call Trace:
  <TASK>
  ? __die+0x20/0x70
  ? page_fault_oops+0x15a/0x460
  ? exc_page_fault+0x6e/0x180
  ? asm_exc_page_fault+0x22/0x30
  ? __pfx_bnxt_queue_mem_alloc+0x10/0x10 [bnxt_en 7f85e76f4d724ba07471d7e39d9e773aea6597b7]
  ? bnxt_queue_mem_alloc+0x3f/0x4e0 [bnxt_en 7f85e76f4d724ba07471d7e39d9e773aea6597b7]
  netdev_rx_queue_restart+0xc5/0x240
  net_devmem_bind_dmabuf_to_queue+0xf8/0x200
  netdev_nl_bind_rx_doit+0x3a7/0x450
  genl_family_rcv_msg_doit+0xd9/0x130
  genl_rcv_msg+0x184/0x2b0
  ? __pfx_netdev_nl_bind_rx_doit+0x10/0x10
  ? __pfx_genl_rcv_msg+0x10/0x10
  netlink_rcv_skb+0x54/0x100
  genl_rcv+0x24/0x40
...

Fixes: 2d694c27d32e ("bnxt_en: implement netdev_queue_mgmt_ops")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 7b8b5b39c7bb..1f7042248ccc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -15439,6 +15439,9 @@ static int bnxt_queue_mem_alloc(struct net_device *dev, void *qmem, int idx)
 	struct bnxt_ring_struct *ring;
 	int rc;
 
+	if (!bp->rx_ring)
+		return -ENETDOWN;
+
 	rxr = &bp->rx_ring[idx];
 	clone = qmem;
 	memcpy(clone, rxr, sizeof(*rxr));
-- 
2.34.1

[PATCH net 3/3] eth: bnxt: do not use BNXT_VNIC_NTUPLE unconditionally in queue restart logic

[Taehee Yoo]

When a queue is restarted, it sets MRU to 0 for stopping packet flow.
MRU variable is a member of vnic_info[], the first vnic_info is default
and the second is ntuple.
Only when ntuple is enabled(ethtool -K eth0 ntuple on), vnic_info for
ntuple is allocated in init logic.
The bp->nr_vnics indicates how many vnic_info are allocated.
However bnxt_queue_{start | stop}() accesses vnic_info[BNXT_VNIC_NTUPLE]
regardless of ntuple state.

Fixes: b9d2956e869c ("bnxt_en: stop packet flow during bnxt_queue_stop/start")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 1f7042248ccc..29849bfeed14 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -15635,7 +15635,7 @@ static int bnxt_queue_start(struct net_device *dev, void *qmem, int idx)
 	cpr = &rxr->bnapi->cp_ring;
 	cpr->sw_stats->rx.rx_resets++;
 
-	for (i = 0; i <= BNXT_VNIC_NTUPLE; i++) {
+	for (i = 0; i <= bp->nr_vnics; i++) {
 		vnic = &bp->vnic_info[i];
 
 		rc = bnxt_hwrm_vnic_set_rss_p5(bp, vnic, true);
@@ -15663,7 +15663,7 @@ static int bnxt_queue_stop(struct net_device *dev, void *qmem, int idx)
 	struct bnxt_vnic_info *vnic;
 	int i;
 
-	for (i = 0; i <= BNXT_VNIC_NTUPLE; i++) {
+	for (i = 0; i <= bp->nr_vnics; i++) {
 		vnic = &bp->vnic_info[i];
 		vnic->mru = 0;
 		bnxt_hwrm_vnic_update(bp, vnic,
-- 
2.34.1

Re: [PATCH net 2/3] eth: bnxt: return fail if interface is down in bnxt_queue_mem_alloc()

[Somnath Kotur]

[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]

On Wed, Feb 26, 2025 at 11:49 AM Taehee Yoo <ap420073@gmail.com> wrote:
>
> The bnxt_queue_mem_alloc() is called to allocate new queue memory when
> a queue is restarted.
> It internally accesses rx buffer descriptor corresponding to the index.
> The rx buffer descriptor is allocated and set when the interface is up
> and it's freed when the interface is down.
> So, if queue is restarted if interface is down, kernel panic occurs.
>
> Splat looks like:
>  BUG: unable to handle page fault for address: 000000000000b240
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 0 P4D 0
>  Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
>  CPU: 3 UID: 0 PID: 1563 Comm: ncdevmem2 Not tainted 6.14.0-rc2+ #9 844ddba6e7c459cafd0bf4db9a3198e
>  Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021
>  RIP: 0010:bnxt_queue_mem_alloc+0x3f/0x4e0 [bnxt_en]
>  Code: 41 54 4d 89 c4 4d 69 c0 c0 05 00 00 55 48 89 f5 53 48 89 fb 4c 8d b5 40 05 00 00 48 83 ec 15
>  RSP: 0018:ffff9dcc83fef9e8 EFLAGS: 00010202
>  RAX: ffffffffc0457720 RBX: ffff934ed8d40000 RCX: 0000000000000000
>  RDX: 000000000000001f RSI: ffff934ea508f800 RDI: ffff934ea508f808
>  RBP: ffff934ea508f800 R08: 000000000000b240 R09: ffff934e84f4b000
>  R10: ffff9dcc83fefa30 R11: ffff934e84f4b000 R12: 000000000000001f
>  R13: ffff934ed8d40ac0 R14: ffff934ea508fd40 R15: ffff934e84f4b000
>  FS:  00007fa73888c740(0000) GS:ffff93559f780000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 000000000000b240 CR3: 0000000145a2e000 CR4: 00000000007506f0
>  PKRU: 55555554
>  Call Trace:
>   <TASK>
>   ? __die+0x20/0x70
>   ? page_fault_oops+0x15a/0x460
>   ? exc_page_fault+0x6e/0x180
>   ? asm_exc_page_fault+0x22/0x30
>   ? __pfx_bnxt_queue_mem_alloc+0x10/0x10 [bnxt_en 7f85e76f4d724ba07471d7e39d9e773aea6597b7]
>   ? bnxt_queue_mem_alloc+0x3f/0x4e0 [bnxt_en 7f85e76f4d724ba07471d7e39d9e773aea6597b7]
>   netdev_rx_queue_restart+0xc5/0x240
>   net_devmem_bind_dmabuf_to_queue+0xf8/0x200
>   netdev_nl_bind_rx_doit+0x3a7/0x450
>   genl_family_rcv_msg_doit+0xd9/0x130
>   genl_rcv_msg+0x184/0x2b0
>   ? __pfx_netdev_nl_bind_rx_doit+0x10/0x10
>   ? __pfx_genl_rcv_msg+0x10/0x10
>   netlink_rcv_skb+0x54/0x100
>   genl_rcv+0x24/0x40
> ...
>
> Fixes: 2d694c27d32e ("bnxt_en: implement netdev_queue_mgmt_ops")
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 7b8b5b39c7bb..1f7042248ccc 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -15439,6 +15439,9 @@ static int bnxt_queue_mem_alloc(struct net_device *dev, void *qmem, int idx)
>         struct bnxt_ring_struct *ring;
>         int rc;
>
> +       if (!bp->rx_ring)
> +               return -ENETDOWN;
> +
>         rxr = &bp->rx_ring[idx];
>         clone = qmem;
>         memcpy(clone, rxr, sizeof(*rxr));
> --
> 2.34.1
>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH net 3/3] eth: bnxt: do not use BNXT_VNIC_NTUPLE unconditionally in queue restart logic

[Somnath Kotur]

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

On Wed, Feb 26, 2025 at 11:49 AM Taehee Yoo <ap420073@gmail.com> wrote:
>
> When a queue is restarted, it sets MRU to 0 for stopping packet flow.
> MRU variable is a member of vnic_info[], the first vnic_info is default
> and the second is ntuple.
> Only when ntuple is enabled(ethtool -K eth0 ntuple on), vnic_info for
> ntuple is allocated in init logic.
> The bp->nr_vnics indicates how many vnic_info are allocated.
> However bnxt_queue_{start | stop}() accesses vnic_info[BNXT_VNIC_NTUPLE]
> regardless of ntuple state.
>
> Fixes: b9d2956e869c ("bnxt_en: stop packet flow during bnxt_queue_stop/start")
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 1f7042248ccc..29849bfeed14 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -15635,7 +15635,7 @@ static int bnxt_queue_start(struct net_device *dev, void *qmem, int idx)
>         cpr = &rxr->bnapi->cp_ring;
>         cpr->sw_stats->rx.rx_resets++;
>
> -       for (i = 0; i <= BNXT_VNIC_NTUPLE; i++) {
> +       for (i = 0; i <= bp->nr_vnics; i++) {
>                 vnic = &bp->vnic_info[i];
>
>                 rc = bnxt_hwrm_vnic_set_rss_p5(bp, vnic, true);
> @@ -15663,7 +15663,7 @@ static int bnxt_queue_stop(struct net_device *dev, void *qmem, int idx)
>         struct bnxt_vnic_info *vnic;
>         int i;
>
> -       for (i = 0; i <= BNXT_VNIC_NTUPLE; i++) {
> +       for (i = 0; i <= bp->nr_vnics; i++) {
>                 vnic = &bp->vnic_info[i];
>                 vnic->mru = 0;
>                 bnxt_hwrm_vnic_update(bp, vnic,
> --
> 2.34.1
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH net 1/3] eth: bnxt: fix truesize for mb-xdp-pass case

[Somnath Kotur]

[-- Attachment #1: Type: text/plain, Size: 3493 bytes --]

On Wed, Feb 26, 2025 at 11:48 AM Taehee Yoo <ap420073@gmail.com> wrote:
>
> When mb-xdp is set and return is XDP_PASS, packet is converted from
> xdp_buff to sk_buff with xdp_update_skb_shared_info() in
> bnxt_xdp_build_skb().
> bnxt_xdp_build_skb() passes incorrect truesize argument to
> xdp_update_skb_shared_info().
> truesize is calculated as BNXT_RX_PAGE_SIZE * sinfo->nr_frags but
> sinfo->nr_frags should not be used because sinfo->nr_frags is not yet
> updated.
> so it should use num_frags instead.
>
> Splat looks like:
>  ------------[ cut here ]------------
>  WARNING: CPU: 2 PID: 0 at net/core/skbuff.c:6072 skb_try_coalesce+0x504/0x590
>  Modules linked in: xt_nat xt_tcpudp veth af_packet xt_conntrack nft_chain_nat xt_MASQUERADE nf_conntrack_netlink xfrm_user xt_addrtype nft_coms
>  CPU: 2 UID: 0 PID: 0 Comm: swapper/2 Not tainted 6.14.0-rc2+ #3
>  RIP: 0010:skb_try_coalesce+0x504/0x590
>  Code: 4b fd ff ff 49 8b 34 24 40 80 e6 40 0f 84 3d fd ff ff 49 8b 74 24 48 40 f6 c6 01 0f 84 2e fd ff ff 48 8d 4e ff e9 25 fd ff ff <0f> 0b e99
>  RSP: 0018:ffffb62c4120caa8 EFLAGS: 00010287
>  RAX: 0000000000000003 RBX: ffffb62c4120cb14 RCX: 0000000000000ec0
>  RDX: 0000000000001000 RSI: ffffa06e5d7dc000 RDI: 0000000000000003
>  RBP: ffffa06e5d7ddec0 R08: ffffa06e6120a800 R09: ffffa06e7a119900
>  R10: 0000000000002310 R11: ffffa06e5d7dcec0 R12: ffffe4360575f740
>  R13: ffffe43600000000 R14: 0000000000000002 R15: 0000000000000002
>  FS:  0000000000000000(0000) GS:ffffa0755f700000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00007f147b76b0f8 CR3: 00000001615d4000 CR4: 00000000007506f0
>  PKRU: 55555554
>  Call Trace:
>   <IRQ>
>   ? __warn+0x84/0x130
>   ? skb_try_coalesce+0x504/0x590
>   ? report_bug+0x18a/0x1a0
>   ? handle_bug+0x53/0x90
>   ? exc_invalid_op+0x14/0x70
>   ? asm_exc_invalid_op+0x16/0x20
>   ? skb_try_coalesce+0x504/0x590
>   inet_frag_reasm_finish+0x11f/0x2e0
>   ip_defrag+0x37a/0x900
>   ip_local_deliver+0x51/0x120
>   ip_sublist_rcv_finish+0x64/0x70
>   ip_sublist_rcv+0x179/0x210
>   ip_list_rcv+0xf9/0x130
>
> How to reproduce:
> <Node A>
> ip link set $interface1 xdp obj xdp_pass.o
> ip link set $interface1 mtu 9000 up
> ip a a 10.0.0.1/24 dev $interface1
> <Node B>
> ip link set $interfac2 mtu 9000 up
> ip a a 10.0.0.2/24 dev $interface2
> ping 10.0.0.1 -s 65000
>
> Fixes: 1dc4c557bfed ("bnxt: adding bnxt_xdp_build_skb to build skb from multibuffer xdp_buff")
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
> index e6c64e4bd66c..e9b49cb5b735 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
> @@ -476,7 +476,7 @@ bnxt_xdp_build_skb(struct bnxt *bp, struct sk_buff *skb, u8 num_frags,
>         }
>         xdp_update_skb_shared_info(skb, num_frags,
>                                    sinfo->xdp_frags_size,
> -                                  BNXT_RX_PAGE_SIZE * sinfo->nr_frags,
> +                                  BNXT_RX_PAGE_SIZE * num_frags,
>                                    xdp_buff_is_frag_pfmemalloc(xdp));
>         return skb;
>  }
> --
> 2.34.1
>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH net 1/3] eth: bnxt: fix truesize for mb-xdp-pass case

[Jakub Kicinski]

On Wed, 26 Feb 2025 06:18:35 +0000 Taehee Yoo wrote:
> When mb-xdp is set and return is XDP_PASS, packet is converted from
> xdp_buff to sk_buff with xdp_update_skb_shared_info() in
> bnxt_xdp_build_skb().
> bnxt_xdp_build_skb() passes incorrect truesize argument to
> xdp_update_skb_shared_info().
> truesize is calculated as BNXT_RX_PAGE_SIZE * sinfo->nr_frags but
> sinfo->nr_frags should not be used because sinfo->nr_frags is not yet
> updated.

"not yet updated" sounds misleading the problem is that calling
build_skb() wipes the shared info back to 0, but it was initialized.

> so it should use num_frags instead.

num_frags may be stale, tho. The program can trim the skb effectively
discarding the fragments. Maybe we should fix that also..

Could you follow up and switch to xdp_build_skb_from_buff() in net-next?
It does all the right things already.

> How to reproduce:
> <Node A>
> ip link set $interface1 xdp obj xdp_pass.o
> ip link set $interface1 mtu 9000 up
> ip a a 10.0.0.1/24 dev $interface1
> <Node B>
> ip link set $interfac2 mtu 9000 up
> ip a a 10.0.0.2/24 dev $interface2
> ping 10.0.0.1 -s 65000

Would you be willing to turn this into a selftest?
The xdp program I added for HDS recently is a PASS so we can reuse it.
29b036be1b0bfcf
-- 
pw-bot: cr

Re: [PATCH net 2/3] eth: bnxt: return fail if interface is down in bnxt_queue_mem_alloc()

[Jakub Kicinski]

On Wed, 26 Feb 2025 06:18:36 +0000 Taehee Yoo wrote:
> The bnxt_queue_mem_alloc() is called to allocate new queue memory when
> a queue is restarted.
> It internally accesses rx buffer descriptor corresponding to the index.
> The rx buffer descriptor is allocated and set when the interface is up
> and it's freed when the interface is down.
> So, if queue is restarted if interface is down, kernel panic occurs.

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH net 1/3] eth: bnxt: fix truesize for mb-xdp-pass case

[Taehee Yoo]

On Thu, Feb 27, 2025 at 11:07 AM Jakub Kicinski <kuba@kernel.org> wrote:
>

Hi Jakub,
Thanks a lot for your review!

> On Wed, 26 Feb 2025 06:18:35 +0000 Taehee Yoo wrote:
> > When mb-xdp is set and return is XDP_PASS, packet is converted from
> > xdp_buff to sk_buff with xdp_update_skb_shared_info() in
> > bnxt_xdp_build_skb().
> > bnxt_xdp_build_skb() passes incorrect truesize argument to
> > xdp_update_skb_shared_info().
> > truesize is calculated as BNXT_RX_PAGE_SIZE * sinfo->nr_frags but
> > sinfo->nr_frags should not be used because sinfo->nr_frags is not yet
> > updated.
>
> "not yet updated" sounds misleading the problem is that calling
> build_skb() wipes the shared info back to 0, but it was initialized.

I checked again after this review, and you're right.
sinfo->nr_frags was wiped in the napi_build_skb() in the
bnxt_rx_multi_page_skb(), this is called before bnxt_xdp_build_skb().
So, this patch is wrong.
Thanks a lot!

>
> > so it should use num_frags instead.
>
> num_frags may be stale, tho. The program can trim the skb effectively
> discarding the fragments. Maybe we should fix that also..

I didn't think it through, Thanks!

>
> Could you follow up and switch to xdp_build_skb_from_buff() in net-next?
> It does all the right things already.

Okay, I will try to switch to xdp_build_skb_from_buff() in net-next.

>
> > How to reproduce:
> > <Node A>
> > ip link set $interface1 xdp obj xdp_pass.o
> > ip link set $interface1 mtu 9000 up
> > ip a a 10.0.0.1/24 dev $interface1
> > <Node B>
> > ip link set $interfac2 mtu 9000 up
> > ip a a 10.0.0.2/24 dev $interface2
> > ping 10.0.0.1 -s 65000
>
> Would you be willing to turn this into a selftest?
> The xdp program I added for HDS recently is a PASS so we can reuse it.
> 29b036be1b0bfcf

Of course, I will add selftest case for this.

Thanks a lot!
Taehee Yoo

> --
> pw-bot: cr