* [PATCH net-next v8 1/2] net: pppoe: implement GRO/GSO support
From: Qingfang Deng @ 2026-05-01 3:50 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, David Ahern, Ido Schimmel,
Qingfang Deng, Guillaume Nault, Kees Cook, Felix Fietkau,
Eric Woudstra, Willem de Bruijn, Kuniyuki Iwashima,
Richard Gobert, netdev, linux-kernel
Cc: linux-ppp
From: Felix Fietkau <nbd@nbd.name>
Only handles packets where the pppoe header length field matches the exact
packet length. Significantly improves rx throughput.
When running NAT traffic through a MediaTek MT7621 devices from a host
behind PPPoE to a host directly connected via ethernet, the TCP throughput
that the device is able to handle improves from ~130 Mbit/s to ~630 Mbit/s,
using fraglist GRO.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
v8: no changes
v7: https://lore.kernel.org/netdev/20260428064717.74794-1-qingfang.deng@linux.dev
- Use PPPOE_SES_HLEN macro instead of +2 magic number
v6: https://lore.kernel.org/netdev/20260326081127.61229-1-dqfext@gmail.com
- avoid phdr->length field overflow
- restore skb_is_gso() check
- do not register GRO if INET=n
- do not check for PPP_IPV6 if IPV6=n
- tail call gro_complete
drivers/net/ppp/pppoe.c | 165 +++++++++++++++++++++++++++++++++++++++-
net/ipv4/af_inet.c | 2 +
net/ipv6/ip6_offload.c | 2 +
3 files changed, 168 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index bdd61c504a1c..363204e0c49a 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -77,6 +77,7 @@
#include <net/net_namespace.h>
#include <net/netns/generic.h>
#include <net/sock.h>
+#include <net/gro.h>
#include <linux/uaccess.h>
@@ -409,7 +410,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
if (ppp_skb_is_compressed_proto(skb))
goto drop;
- if (pskb_trim_rcsum(skb, len))
+ if (!skb_is_gso(skb) && pskb_trim_rcsum(skb, len))
goto drop;
ph = pppoe_hdr(skb);
@@ -1103,6 +1104,164 @@ static struct pernet_operations pppoe_net_ops = {
.size = sizeof(struct pppoe_net),
};
+static u16
+compare_pppoe_header(const struct pppoe_hdr *phdr,
+ const struct pppoe_hdr *phdr2)
+{
+ __be16 proto = *(const __be16 *)(phdr + 1);
+ __be16 proto2 = *(const __be16 *)(phdr2 + 1);
+
+ return (__force u16)((phdr->sid ^ phdr2->sid) | (proto ^ proto2));
+}
+
+static __be16 pppoe_hdr_proto(const struct pppoe_hdr *phdr)
+{
+ __be16 proto = *(const __be16 *)(phdr + 1);
+
+ switch (proto) {
+ case cpu_to_be16(PPP_IP):
+ return cpu_to_be16(ETH_P_IP);
+#if IS_ENABLED(CONFIG_IPV6)
+ case cpu_to_be16(PPP_IPV6):
+ return cpu_to_be16(ETH_P_IPV6);
+#endif
+ default:
+ return 0;
+ }
+}
+
+static struct sk_buff *pppoe_gro_receive(struct list_head *head,
+ struct sk_buff *skb)
+{
+ const struct packet_offload *ptype;
+ unsigned int hlen, off_pppoe;
+ const struct pppoe_hdr *phdr;
+ struct sk_buff *pp = NULL;
+ struct sk_buff *p;
+ int flush = 1;
+ __be16 type;
+
+ off_pppoe = skb_gro_offset(skb);
+ hlen = off_pppoe + PPPOE_SES_HLEN;
+ phdr = skb_gro_header(skb, hlen, off_pppoe);
+ if (unlikely(!phdr))
+ goto out;
+
+ /* filter for session packets (type:1, ver:1, code:0) */
+ if (*(const __be16 *)phdr != cpu_to_be16(0x1100))
+ goto out;
+
+ /* ignore packets with padding or invalid length */
+ if (skb_gro_len(skb) != be16_to_cpu(phdr->length) + sizeof(*phdr))
+ goto out;
+
+ type = pppoe_hdr_proto(phdr);
+ ptype = gro_find_receive_by_type(type);
+ if (!ptype)
+ goto out;
+
+ flush = 0;
+
+ list_for_each_entry(p, head, list) {
+ const struct pppoe_hdr *phdr2;
+
+ if (!NAPI_GRO_CB(p)->same_flow)
+ continue;
+
+ phdr2 = (const struct pppoe_hdr *)(p->data + off_pppoe);
+ if (compare_pppoe_header(phdr, phdr2))
+ NAPI_GRO_CB(p)->same_flow = 0;
+ }
+
+ skb_gro_pull(skb, PPPOE_SES_HLEN);
+ skb_gro_postpull_rcsum(skb, phdr, PPPOE_SES_HLEN);
+
+ pp = indirect_call_gro_receive_inet(ptype->callbacks.gro_receive,
+ ipv6_gro_receive, inet_gro_receive,
+ head, skb);
+
+out:
+ skb_gro_flush_final(skb, pp, flush);
+
+ return pp;
+}
+
+static int pppoe_gro_complete(struct sk_buff *skb, int nhoff)
+{
+ struct pppoe_hdr *phdr = (struct pppoe_hdr *)(skb->data + nhoff);
+ __be16 type = pppoe_hdr_proto(phdr);
+ struct packet_offload *ptype;
+ unsigned int len;
+
+ ptype = gro_find_complete_by_type(type);
+ if (!ptype)
+ return -ENOENT;
+
+ len = skb->len - (nhoff + sizeof(*phdr));
+ len = min(len, 0xFFFFU);
+ phdr->length = cpu_to_be16(len);
+
+ return INDIRECT_CALL_INET(ptype->callbacks.gro_complete,
+ ipv6_gro_complete, inet_gro_complete,
+ skb, nhoff + PPPOE_SES_HLEN);
+}
+
+static struct sk_buff *pppoe_gso_segment(struct sk_buff *skb,
+ netdev_features_t features)
+{
+ struct sk_buff *segs = ERR_PTR(-EINVAL);
+ u16 mac_offset = skb->mac_header;
+ struct packet_offload *ptype;
+ u16 mac_len = skb->mac_len;
+ struct pppoe_hdr *phdr;
+ __be16 orig_type, type;
+ int len, nhoff;
+
+ skb_reset_network_header(skb);
+ nhoff = skb_network_header(skb) - skb_mac_header(skb);
+
+ if (unlikely(!pskb_may_pull(skb, PPPOE_SES_HLEN)))
+ goto out;
+
+ phdr = (struct pppoe_hdr *)skb_network_header(skb);
+ type = pppoe_hdr_proto(phdr);
+ ptype = gro_find_complete_by_type(type);
+ if (!ptype)
+ goto out;
+
+ orig_type = skb->protocol;
+ __skb_pull(skb, PPPOE_SES_HLEN);
+ segs = ptype->callbacks.gso_segment(skb, features);
+ if (IS_ERR_OR_NULL(segs)) {
+ skb_gso_error_unwind(skb, orig_type, PPPOE_SES_HLEN, mac_offset,
+ mac_len);
+ goto out;
+ }
+
+ skb = segs;
+ do {
+ phdr = (struct pppoe_hdr *)(skb_mac_header(skb) + nhoff);
+ len = skb->len - (nhoff + sizeof(*phdr));
+ phdr->length = cpu_to_be16(len);
+ skb->network_header = (u8 *)phdr - skb->head;
+ skb->protocol = orig_type;
+ skb_reset_mac_len(skb);
+ } while ((skb = skb->next));
+
+out:
+ return segs;
+}
+
+static struct packet_offload pppoe_packet_offload __read_mostly = {
+ .type = cpu_to_be16(ETH_P_PPP_SES),
+ .priority = 20,
+ .callbacks = {
+ .gro_receive = pppoe_gro_receive,
+ .gro_complete = pppoe_gro_complete,
+ .gso_segment = pppoe_gso_segment,
+ },
+};
+
static int __init pppoe_init(void)
{
int err;
@@ -1119,6 +1278,8 @@ static int __init pppoe_init(void)
if (err)
goto out_unregister_pppoe_proto;
+ if (IS_ENABLED(CONFIG_INET))
+ dev_add_offload(&pppoe_packet_offload);
dev_add_pack(&pppoes_ptype);
dev_add_pack(&pppoed_ptype);
register_netdevice_notifier(&pppoe_notifier);
@@ -1138,6 +1299,8 @@ static void __exit pppoe_exit(void)
unregister_netdevice_notifier(&pppoe_notifier);
dev_remove_pack(&pppoed_ptype);
dev_remove_pack(&pppoes_ptype);
+ if (IS_ENABLED(CONFIG_INET))
+ dev_remove_offload(&pppoe_packet_offload);
unregister_pppox_proto(PX_PROTO_OE);
proto_unregister(&pppoe_sk_proto);
unregister_pernet_device(&pppoe_net_ops);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 0e62032e76b1..cbac072633bb 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1540,6 +1540,7 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
return pp;
}
+EXPORT_INDIRECT_CALLABLE(inet_gro_receive);
static struct sk_buff *ipip_gro_receive(struct list_head *head,
struct sk_buff *skb)
@@ -1625,6 +1626,7 @@ int inet_gro_complete(struct sk_buff *skb, int nhoff)
out:
return err;
}
+EXPORT_INDIRECT_CALLABLE(inet_gro_complete);
static int ipip_gro_complete(struct sk_buff *skb, int nhoff)
{
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index d8072ad6b8c4..78f50c93c536 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -297,6 +297,7 @@ INDIRECT_CALLABLE_SCOPE struct sk_buff *ipv6_gro_receive(struct list_head *head,
return pp;
}
+EXPORT_INDIRECT_CALLABLE(ipv6_gro_receive);
static struct sk_buff *sit_ip6ip6_gro_receive(struct list_head *head,
struct sk_buff *skb)
@@ -359,6 +360,7 @@ INDIRECT_CALLABLE_SCOPE int ipv6_gro_complete(struct sk_buff *skb, int nhoff)
out:
return err;
}
+EXPORT_INDIRECT_CALLABLE(ipv6_gro_complete);
static int sit_gro_complete(struct sk_buff *skb, int nhoff)
{
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net 5/7] ionic: fix adminq use-after-free on command timeout
From: Eric Joyner @ 2026-05-01 3:31 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: Creeley, Brett, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
In-Reply-To: <20260429210007.40015-6-eric.joyner@amd.com>
On 4/29/2026 2:00 PM, Joyner, Eric wrote:
> From: Brett Creeley <brett.creeley@amd.com>
>
> When ionic_adminq_wait() times out or detects FW reset, it
> returns an error to the caller, whose ionic_admin_ctx is typically
> on the stack. However, desc_info->ctx in the adminq still points
> to that ctx. If ionic_adminq_service() later runs in NAPI context,
> it dereferences the stale pointer to copy the completion and call
> complete_all(), causing a use-after-free.
>
> The timeout path partially addressed this via ionic_adminq_flush()
> in ionic_adminq_check_err(), which NULLs all pending desc_info->ctx
> entries. But there is a race window between the timeout detection
> and the flush where NAPI could fire and access the stale ctx. The
> FW reset path had no protection at all and returned directly
> without clearing desc_info->ctx.
>
> Add ionic_adminq_cancel() which takes adminq_lock and NULLs
> desc_info->ctx for the specific context being cancelled. This
> coordinates with ionic_adminq_service() which also runs under the
> same lock. Call it from both error paths in ionic_adminq_wait()
> before returning.
>
> Fixes: 938962d55229 ("ionic: Add adminq action")
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
> Signed-off-by: Eric Joyner <eric.joyner@amd.com>
> ---
> .../net/ethernet/pensando/ionic/ionic_main.c | 30 +++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> index 810cef0fec93..0971ca4d6650 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> @@ -190,6 +190,32 @@ static const char *ionic_opcode_to_str(enum ionic_cmd_opcode opcode)
> }
> }
>
> +static void ionic_adminq_cancel(struct ionic_lif *lif,
> + struct ionic_admin_ctx *ctx)
> +{
> + struct ionic_admin_desc_info *desc_info;
> + unsigned long irqflags;
> + struct ionic_queue *q;
> + int i;
> +
> + spin_lock_irqsave(&lif->adminq_lock, irqflags);
> + if (!lif->adminqcq) {
> + spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
> + return;
> + }
> +
> + q = &lif->adminqcq->q;
> +
> + for (i = 0; i < q->num_descs; i++) {
> + desc_info = &q->admin_info[i];
> + if (desc_info->ctx == ctx) {
> + desc_info->ctx = NULL;
> + break;
> + }
> + }
> + spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
> +}
> +
> static void ionic_adminq_flush(struct ionic_lif *lif)
> {
> struct ionic_admin_desc_info *desc_info;
> @@ -448,6 +474,7 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
> if (do_msg)
> netdev_warn(netdev, "%s (%d) interrupted, FW in reset\n",
> name, ctx->cmd.cmd.opcode);
> + ionic_adminq_cancel(lif, ctx);
> ctx->comp.comp.status = IONIC_RC_ERROR;
> return -ENXIO;
> }
> @@ -458,6 +485,9 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
> dev_dbg(lif->ionic->dev, "%s: elapsed %d msecs\n",
> __func__, jiffies_to_msecs(time_done - time_start));
>
> + if (time_after_eq(time_done, time_limit))
> + ionic_adminq_cancel(lif, ctx);
> +
> return ionic_adminq_check_err(lif, ctx,
> time_after_eq(time_done, time_limit),
> do_msg);
I took a look at the Sashiko output for patches 5 and 6, and it echoed concerns that we found
internally around ionic_adminq_cancel() and ionic_adminq_flush(). We might need to rework at
least those two.
https://sashiko.dev/#/message/20260429210007.40015-7-eric.joyner%40amd.com
- Eric
^ permalink raw reply
* Re: [PATCH net v4 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
From: Stanislav Fomichev @ 2026-05-01 3:29 UTC (permalink / raw)
To: Jason Xing
Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin, bpf, netdev, Jason Xing
In-Reply-To: <CAL+tcoBTO9QGM220EO=m1TdiXwod7QzG59j7vQL3qoVSrb2QEw@mail.gmail.com>
On 04/29, Jason Xing wrote:
> On Wed, Apr 29, 2026 at 6:14 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> >
> > On 04/29, Jason Xing wrote:
> > > On Wed, Apr 29, 2026 at 2:11 AM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> > > >
> > > > On 04/24, Jason Xing wrote:
> > > > > From: Jason Xing <kernelxing@tencent.com>
> > > > >
> > > > > In copy mode TX, xsk_skb_destructor_set_addr() stores the 64-bit
> > > > > descriptor address into skb_shinfo(skb)->destructor_arg (void *) via a
> > > > > uintptr_t cast:
> > > > >
> > > > > skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
> > > > >
> > > > > On 32-bit architectures uintptr_t is 32 bits, so the upper 32 bits of
> > > > > the descriptor address are silently dropped. In unaligned mode the chunk
> > > > > offset is encoded in bits 48-63 of the descriptor address
> > > > > (XSK_UNALIGNED_BUF_OFFSET_SHIFT = 48), meaning the offset is lost
> > > > > entirely. The completion queue then returns a truncated address to
> > > > > userspace, making buffer recycling impossible.
> > > > >
> > > > > Fix this by handling the 32-bit case in the destructor_arg helpers:
> > > > >
> > > > > - xsk_skb_destructor_set_addr(): on !CONFIG_64BIT, allocate an
> > > > > xsk_addrs struct via kmem_cache_zalloc() to store the full u64
> > > > > address. Leave num_descs as 0 (zalloc) so that the subsequent
> > > > > xsk_inc_num_desc() brings it to the correct count of 1.
> > > > >
> > > > > - xsk_skb_destructor_is_addr(): on !CONFIG_64BIT, return true only
> > > > > when destructor_arg is NULL (not yet set), false when it points to
> > > > > an xsk_addrs struct.
> > > > >
> > > > > - xsk_skb_init_misc(): call xsk_skb_destructor_set_addr() first
> > > > > before touching any other skb fields; on failure return early so
> > > > > the skb destructor is never changed from sock_wfree.
> > > > >
> > > > > The existing xsk_consume_skb() already handles 32-bit correctly after
> > > > > these changes: xsk_skb_destructor_is_addr() returns false for any
> > > > > allocated xsk_addrs, so the kmem_cache_free path is always taken.
> > > > >
> > > > > The overhead is one extra kmem_cache_zalloc per first descriptor on
> > > > > 32-bit only; 64-bit builds are completely unchanged.
> > > > >
> > > > > Closes: https://lore.kernel.org/all/20260419045824.D9E5EC2BCAF@smtp.kernel.org/
> > > > > Fixes: 0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number")
> > > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > > > ---
> > > > > net/xdp/xsk.c | 38 +++++++++++++++++++++++++++++++-------
> > > > > 1 file changed, 31 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > > > > index ed96f6ec8ff2..fe88f47741b5 100644
> > > > > --- a/net/xdp/xsk.c
> > > > > +++ b/net/xdp/xsk.c
> > > > > @@ -558,7 +558,10 @@ static int xsk_cq_reserve_locked(struct xsk_buff_pool *pool)
> > > > >
> > > > > static bool xsk_skb_destructor_is_addr(struct sk_buff *skb)
> > > > > {
> > > > > - return (uintptr_t)skb_shinfo(skb)->destructor_arg & 0x1UL;
> > > > > + if (IS_ENABLED(CONFIG_64BIT))
> > > > > + return (uintptr_t)skb_shinfo(skb)->destructor_arg & 0x1UL;
> > > > > + else
> > > > > + return !skb_shinfo(skb)->destructor_arg;
> > > >
> > > > Don't understand why we need to special case CONFIG_64BIT here?
> > > > Shouldn't the same existing condition work on 32bit?
> > >
> > > Because 0x1UL is the particular semantic applied on a 64-bit arch.
> > > xsk_skb_destructor_set_addr() sets it while
> > > xsk_skb_destructor_is_addr() recognizes it. They are a pair.
> > >
> > > As you noticed, one liner works but is not that appropriate: on a
> > > 32-bit arch, this member should be either a NULL point or a valid
> > > pointer pointing to a memory region. Testing if it's NULL can be
> > > helpful as to the long term maintenance because of its readability and
> > > robustness/safety.
> > >
> > > The error path in allocation of skb is really complex, which is why
> > > I'm so cautious to take care of it :)
> >
> > Let's cleanup the error path instead of adding more complexity? Similar to what
> > you do with your "xsk: fix xsk_addrs slab leak on multi-buffer error path",
> > but maybe add a few NULL checks?
>
> Good suggestion. I think I can cook a follow up patch to do such a
> thing targetting net-next tree. This patch 8 belongs to net material
> which means it will be backported to the older stable kernel as soon
> as it gets merged. IIUC, the better way is to make it as simple as
> possible?
>
> >
> > Instead of 32 vs 64, I'd like to reason about whether destructor_arg
> > is an address or an allocated array (not whether we have 1 or >1
> > descriptors). And we special case 32 bit by always allocating it.
> >
> > Haven't checked, but maybe this is all you need (besides your _set_addr
> > changes)?
> >
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 6149f6a79897..03f217e85d31 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -589,6 +589,8 @@ static u32 xsk_get_num_desc(struct sk_buff *skb)
> > return 1;
> >
> > xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
> > + if (!xsk_addr)
> > + return 0;
> >
> > return xsk_addr->num_descs;
> > }
>
> Right, as I mentioned, how about posting a new cleanup patch with your
> suggested-by tag?
>
> >
> > > I've noticed the status has been changed to 'changes requested'. Does
> > > that mean one way or another I have to post a new version?
> >
> > That wasn't me :-) From my POW, patches 1-7 are good to go..
>
> Great! Thanks for the review. My hope is to get this series merged
> soon in the net tree.
>
> >
> > > >
> > > > > }
> > > > >
> > > > > static u64 xsk_skb_destructor_get_addr(struct sk_buff *skb)
> > > > > @@ -566,9 +569,21 @@ static u64 xsk_skb_destructor_get_addr(struct sk_buff *skb)
> > > > > return (u64)((uintptr_t)skb_shinfo(skb)->destructor_arg & ~0x1UL);
> > > > > }
> > > > >
> > > > > -static void xsk_skb_destructor_set_addr(struct sk_buff *skb, u64 addr)
> > > > > +static int xsk_skb_destructor_set_addr(struct sk_buff *skb, u64 addr)
> > > > > {
> > > >
> > > > [..]
> > > >
> > > > > + if (!IS_ENABLED(CONFIG_64BIT)) {
> > > > > + struct xsk_addrs *xsk_addr;
> > > > > +
> > > > > + xsk_addr = kmem_cache_zalloc(xsk_tx_generic_cache, GFP_KERNEL);
> > > > > + if (!xsk_addr)
> > > > > + return -ENOMEM;
> > > > > + xsk_addr->addrs[0] = addr;
> > > > > + skb_shinfo(skb)->destructor_arg = (void *)xsk_addr;
> > > > > + return 0;
> > > > > + }
> > > > > +
> > > > > skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
> > > > > + return 0;
> > > >
> > > > I think this is gonna be a 3rd copy paste of the same logic? Let's
> > > > move to a new helper and replace existing kmem_cache_zalloc places?
> > > >
> > > > xsk_skb_destructor_alloc_list(prev_addr) ?
> >
> > Any comments on this?
>
> I didn't comment on this because I thought I was not that sure if we
> needed to wrap it up in the stable kernels :)
>
> Of course, it would be easier for me to work on the net-next tree to
> make the code look
> more neat and elegant.
Are you concerned that you're gonna break something in the net tree? Why not
do it properly from the start? If you want, you can post 1-7 patches
separately and we follow up with this one but still into net?
^ permalink raw reply
* [PATCH net] pds_core: Fix potential invalid stack memory access
From: Eric Joyner @ 2026-05-01 3:19 UTC (permalink / raw)
To: netdev
Cc: Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Eric Joyner
pds_adminq_post() and pds_process_adminq() can run concurrently
together; either one can check if the current request is complete:
pds_adminq_post() does it after the loop inside it times out waiting
for the completion, and pds_process_adminq() when it sees the
uncompleted request on the ring.
However, since pds_adminq_post() does not do any synchronization
around checking for an incomplete command after it reaches its timeout
and marking the command as complete, there is a small window where both
pds_process_adminq() and pds_adminq_post() can execute their
if(!completion_done()) clauses when the completion_done() returns
false; if pdsc_adminq_post() finishes first and its callers return,
then pdsc_process_adminq() will attempt to access q_info->dest after
the address in there points to an invalid location.
(q_info->dest will be invalid after pdsc_adminq_post()'s call chain
exits because it is pointing to a stack variable "comp" in
pdsc_fw_rpc())
Fix this by locking around the completion_done() check and the
complete() contained in the if-statement with the same adminq_lock that
pds_process_adminq() uses, which will prevent this synchronization
issue from occurring.
Fixes: 3f77c3dfffc7 ("pds_core: make wait_context part of q_info")
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
drivers/net/ethernet/amd/pds_core/adminq.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/amd/pds_core/adminq.c b/drivers/net/ethernet/amd/pds_core/adminq.c
index 097bb092bdb8..6db7d29cdf5c 100644
--- a/drivers/net/ethernet/amd/pds_core/adminq.c
+++ b/drivers/net/ethernet/amd/pds_core/adminq.c
@@ -283,9 +283,15 @@ int pdsc_adminq_post(struct pdsc *pdsc,
__func__, jiffies_to_msecs(time_done - time_start));
/* Check the results and clear an un-completed timeout */
- if (time_after_eq(time_done, time_limit) && !completion_done(wc)) {
- err = -ETIMEDOUT;
- complete(wc);
+ if (time_after_eq(time_done, time_limit)) {
+ unsigned long irqflags;
+
+ spin_lock_irqsave(&pdsc->adminq_lock, irqflags);
+ if (!completion_done(wc)) {
+ err = -ETIMEDOUT;
+ complete(wc);
+ }
+ spin_unlock_irqrestore(&pdsc->adminq_lock, irqflags);
}
dev_dbg(pdsc->dev, "read admin queue completion idx %d:\n", index);
base-commit: e728258debd553c95d2e70f9cd97c9fde27c7130
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 4/5] ionic: Report rx_bits_phy stat to ethtool
From: Eric Joyner @ 2026-05-01 3:15 UTC (permalink / raw)
To: netdev
Cc: Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Eric Joyner
In-Reply-To: <20260501031555.43259-1-eric.joyner@amd.com>
This stat contains the number of total bits that the PHY has received;
it's useful for BER calculations. Add it to the ethtool stats output.
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
.../net/ethernet/pensando/ionic/ionic_stats.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_stats.c b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
index 8f39f687a39f..f64f52083301 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_stats.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
@@ -167,7 +167,7 @@ static const struct ionic_stat_desc ionic_rx_stats_desc[] = {
#define IONIC_NUM_PORT_STATS ARRAY_SIZE(ionic_port_stats_desc)
#define IONIC_NUM_TX_STATS ARRAY_SIZE(ionic_tx_stats_desc)
#define IONIC_NUM_RX_STATS ARRAY_SIZE(ionic_rx_stats_desc)
-#define IONIC_NUM_EXTRA_PORT_STATS 1
+#define IONIC_NUM_EXTRA_PORT_STATS 2
#define MAX_Q(lif) ((lif)->netdev->real_num_tx_queues)
@@ -281,7 +281,9 @@ static void ionic_sw_stats_get_strings(struct ionic_lif *lif, u8 **buf)
for (i = 0; i < IONIC_NUM_PORT_STATS; i++)
ethtool_puts(buf, ionic_port_stats_desc[i].name);
+ /* extra port stats */
ethtool_puts(buf, "link_down_events_phy");
+ ethtool_puts(buf, "rx_bits_phy");
for (q_num = 0; q_num < MAX_Q(lif); q_num++)
ionic_sw_stats_get_tx_strings(lif, buf, q_num);
@@ -324,6 +326,17 @@ static void ionic_sw_stats_get_rxq_values(struct ionic_lif *lif, u64 **buf,
}
}
+static void ionic_extra_port_stats_get_values(struct ionic_lif *lif, u64 **buf)
+{
+ struct ionic_port_info *port_info = lif->ionic->idev.port_info;
+
+ /* The # of stats added here == IONIC_NUM_EXTRA_PORT_STATS */
+ **buf = le16_to_cpu(port_info->status.link_down_count);
+ (*buf)++;
+ **buf = le64_to_cpu(port_info->extra_stats.rx_bits_phy);
+ (*buf)++;
+}
+
static void ionic_sw_stats_get_values(struct ionic_lif *lif, u64 **buf)
{
struct ionic_port_stats *port_stats;
@@ -343,8 +356,7 @@ static void ionic_sw_stats_get_values(struct ionic_lif *lif, u64 **buf)
&ionic_port_stats_desc[i]);
(*buf)++;
}
- **buf = le16_to_cpu(lif->ionic->idev.port_info->status.link_down_count);
- (*buf)++;
+ ionic_extra_port_stats_get_values(lif, buf);
for (q_num = 0; q_num < MAX_Q(lif); q_num++)
ionic_sw_stats_get_txq_values(lif, buf, q_num);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 5/5] ionic: Add .get_fec_stats ethtool handler
From: Eric Joyner @ 2026-05-01 3:15 UTC (permalink / raw)
To: netdev
Cc: Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Eric Joyner
In-Reply-To: <20260501031555.43259-1-eric.joyner@amd.com>
Several FEC error statistics being collected can be reported in a
dedicated ethtool callback for FEC errors, so implement the handler that
does so. This includes 802.3ck FEC histogram data that some newer
hardware collects.
Assisted-by: Claude:claude-4.6-sonnet
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
.../ethernet/pensando/ionic/ionic_ethtool.c | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index 78a802eb159f..fe1f753b6115 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -418,6 +418,56 @@ static int ionic_get_fecparam(struct net_device *netdev,
return 0;
}
+static const struct ethtool_fec_hist_range ionic_fec_ranges[] = {
+ { 0, 0},
+ { 1, 1},
+ { 2, 2},
+ { 3, 3},
+ { 4, 4},
+ { 5, 5},
+ { 6, 6},
+ { 7, 7},
+ { 8, 8},
+ { 9, 9},
+ { 10, 10},
+ { 11, 11},
+ { 12, 12},
+ { 13, 13},
+ { 14, 14},
+ { 15, 15},
+ { 0, 0},
+};
+
+static void
+ionic_fill_fec_hist(const struct ionic_port_extra_stats *extra_stats,
+ struct ethtool_fec_hist *hist)
+{
+ int i;
+
+ hist->ranges = ionic_fec_ranges;
+ for (i = 0; i < ETHTOOL_FEC_HIST_MAX - 1; i++)
+ hist->values[i].sum = extra_stats->fec_codeword_error_bin[i];
+}
+
+static void ionic_get_fec_stats(struct net_device *netdev,
+ struct ethtool_fec_stats *fec_stats,
+ struct ethtool_fec_hist *hist)
+{
+ struct ionic_port_extra_stats *extra_stats;
+ struct ionic_lif *lif = netdev_priv(netdev);
+
+ extra_stats = &lif->ionic->idev.port_info->extra_stats;
+
+ fec_stats->corrected_blocks.total =
+ le64_to_cpu(extra_stats->rsfec_correctable_blocks);
+ fec_stats->uncorrectable_blocks.total =
+ le64_to_cpu(extra_stats->rsfec_uncorrectable_blocks);
+ fec_stats->corrected_bits.total =
+ le64_to_cpu(extra_stats->fec_corrected_bits_total);
+
+ ionic_fill_fec_hist(extra_stats, hist);
+}
+
static int ionic_set_fecparam(struct net_device *netdev,
struct ethtool_fecparam *fec)
{
@@ -1154,6 +1204,7 @@ static const struct ethtool_ops ionic_ethtool_ops = {
.get_module_eeprom_by_page = ionic_get_module_eeprom_by_page,
.get_pauseparam = ionic_get_pauseparam,
.set_pauseparam = ionic_set_pauseparam,
+ .get_fec_stats = ionic_get_fec_stats,
.get_fecparam = ionic_get_fecparam,
.set_fecparam = ionic_set_fecparam,
.get_ts_info = ionic_get_ts_info,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/5] ionic: Report "link_down_events_phy" in ethtool statistics
From: Eric Joyner @ 2026-05-01 3:15 UTC (permalink / raw)
To: netdev
Cc: Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Eric Joyner
In-Reply-To: <20260501031555.43259-1-eric.joyner@amd.com>
The number of times that link has gone down at the port level is tracked
by the firmware and sent to the driver via regular DMA writes to an
instance of struct ionic_port_status in the driver's memory.
This statistic was never reported, but it is useful for diagnostics, so
add it to the "ethtool -S` stats output, grouped with the other
port-level stats that are contained in struct ionic_port_stats.
Assisted-by: Claude:claude-4.6-sonnet
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
drivers/net/ethernet/pensando/ionic/ionic_stats.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_stats.c b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
index 0107599a9dd4..8f39f687a39f 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_stats.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
@@ -167,6 +167,7 @@ static const struct ionic_stat_desc ionic_rx_stats_desc[] = {
#define IONIC_NUM_PORT_STATS ARRAY_SIZE(ionic_port_stats_desc)
#define IONIC_NUM_TX_STATS ARRAY_SIZE(ionic_tx_stats_desc)
#define IONIC_NUM_RX_STATS ARRAY_SIZE(ionic_rx_stats_desc)
+#define IONIC_NUM_EXTRA_PORT_STATS 1
#define MAX_Q(lif) ((lif)->netdev->real_num_tx_queues)
@@ -243,7 +244,7 @@ static u64 ionic_sw_stats_get_count(struct ionic_lif *lif)
rx_queues += 1;
total += IONIC_NUM_LIF_STATS;
- total += IONIC_NUM_PORT_STATS;
+ total += IONIC_NUM_PORT_STATS + IONIC_NUM_EXTRA_PORT_STATS;
total += tx_queues * IONIC_NUM_TX_STATS;
total += rx_queues * IONIC_NUM_RX_STATS;
@@ -280,6 +281,7 @@ static void ionic_sw_stats_get_strings(struct ionic_lif *lif, u8 **buf)
for (i = 0; i < IONIC_NUM_PORT_STATS; i++)
ethtool_puts(buf, ionic_port_stats_desc[i].name);
+ ethtool_puts(buf, "link_down_events_phy");
for (q_num = 0; q_num < MAX_Q(lif); q_num++)
ionic_sw_stats_get_tx_strings(lif, buf, q_num);
@@ -341,6 +343,8 @@ static void ionic_sw_stats_get_values(struct ionic_lif *lif, u64 **buf)
&ionic_port_stats_desc[i]);
(*buf)++;
}
+ **buf = le16_to_cpu(lif->ionic->idev.port_info->status.link_down_count);
+ (*buf)++;
for (q_num = 0; q_num < MAX_Q(lif); q_num++)
ionic_sw_stats_get_txq_values(lif, buf, q_num);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 3/5] ionic: Update ionic_if.h with new extra port stats structure
From: Eric Joyner @ 2026-05-01 3:15 UTC (permalink / raw)
To: netdev
Cc: Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Eric Joyner
In-Reply-To: <20260501031555.43259-1-eric.joyner@amd.com>
A new structure to report additional statistics from the firmware has
been added to struct ionic_port_info. It currently only contains FEC
related statistics, but new statistics collected by the firmware for
the port would go in it.
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
drivers/net/ethernet/pensando/ionic/ionic_if.h | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_if.h b/drivers/net/ethernet/pensando/ionic/ionic_if.h
index 23d6e2b4791e..52bcc9b11f6c 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_if.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_if.h
@@ -2855,6 +2855,14 @@ struct ionic_mgmt_port_stats {
__le64 frames_tx_pause;
};
+struct ionic_port_extra_stats {
+ __le64 rsfec_correctable_blocks;
+ __le64 rsfec_uncorrectable_blocks;
+ __le64 fec_corrected_bits_total;
+ __le64 rx_bits_phy;
+ __le64 fec_codeword_error_bin[16];
+};
+
enum ionic_pb_buffer_drop_stats {
IONIC_BUFFER_INTRINSIC_DROP = 0,
IONIC_BUFFER_DISCARDED,
@@ -2951,6 +2959,7 @@ union ionic_port_identity {
* @sprom_page17: Extended Transceiver sprom, page 17
* @rsvd: reserved byte(s)
* @pb_stats: uplink pb drop stats
+ * @extra_stats: Extra port statistics data
*/
struct ionic_port_info {
union ionic_port_config config;
@@ -2968,9 +2977,7 @@ struct ionic_port_info {
};
};
u8 rsvd[376];
-
- /* pb_stats must start at 2k offset */
- struct ionic_port_pb_stats pb_stats;
+ struct ionic_port_extra_stats extra_stats;
};
/*
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 1/5] ionic: Small improvements in devcmd retry logic
From: Eric Joyner @ 2026-05-01 3:15 UTC (permalink / raw)
To: netdev
Cc: Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Eric Joyner
In-Reply-To: <20260501031555.43259-1-eric.joyner@amd.com>
From: Brett Creeley <brett.creeley@amd.com>
If the timeout time is hit when the last attempt returned EAGAIN, the
driver returns -ETIMEDOUT. This causes the -EAGAIN result to be lost.
Fix this by returning -EAGAIN if the timeout time is hit and the
previous result matches.
Also, reduce the sleep between the write to done and doorbell
registers. The msleep(1000) was initially added in an arbitrary
manner. However, this long of a sleep is problematic because
it reduces the number of retries when -EAGAIN is returned, which
may result in the devmcd giving up early due to the timeout. Fix
this by reducing the sleep to msleep(50).
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
drivers/net/ethernet/pensando/ionic/ionic_main.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 3c5200e2fdb7..92f2ec0bd5af 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -554,6 +554,11 @@ static int __ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds,
if (!done && !time_before(jiffies, max_wait)) {
ionic_dev_cmd_clean(ionic);
+
+ /* allow caller to manage EAGAIN from previous attempt */
+ if (err == IONIC_RC_EAGAIN)
+ return -EAGAIN;
+
dev_warn(ionic->dev, "DEVCMD %s (%d) timeout after %ld secs\n",
ionic_opcode_to_str(opcode), opcode, max_seconds);
return -ETIMEDOUT;
@@ -568,7 +573,7 @@ static int __ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds,
ionic_error_to_str(err), err);
iowrite32(0, &idev->dev_cmd_regs->done);
- msleep(1000);
+ msleep(50);
iowrite32(1, &idev->dev_cmd_regs->doorbell);
goto try_again;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 0/5] Expose more port stats to ethtool
From: Eric Joyner @ 2026-05-01 3:15 UTC (permalink / raw)
To: netdev
Cc: Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Eric Joyner
Newer hardware collects a lot more FEC statistics than older hardware; these
include FEC histograms and corrected/uncorrected word and bit totals. This
patchset adds plumbing to pass these through to ethtool along with another
link_down_count stat that is another port-level stat. That link_down_count was
already being sent to the driver; it just wasn't exposed outside of the driver.
Brett's patch is a small unrelated improvement to devcmd handling that's
still nice to have.
Brett Creeley (1):
ionic: Small improvements in devcmd retry logic
Eric Joyner (4):
ionic: Report "link_down_events_phy" in ethtool statistics
ionic: Update ionic_if.h with new extra port stats structure
ionic: Report rx_bits_phy stat to ethtool
ionic: Add .get_fec_stats ethtool handler
.../ethernet/pensando/ionic/ionic_ethtool.c | 51 +++++++++++++++++++
.../net/ethernet/pensando/ionic/ionic_if.h | 13 +++--
.../net/ethernet/pensando/ionic/ionic_main.c | 7 ++-
.../net/ethernet/pensando/ionic/ionic_stats.c | 18 ++++++-
4 files changed, 84 insertions(+), 5 deletions(-)
base-commit: 09942ddedcb960f9e78fd817ec33f501d1040c5b
--
2.17.1
^ permalink raw reply
* [PATCH net, v3] net: mana: Fix crash from unvalidated SHM offset read from BAR0 during FLR
From: Dipayaan Roy @ 2026-05-01 2:47 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, leon, longli, kotaranov, horms, shradhagupta,
ssengar, ernis, shirazsaleem, linux-hyperv, netdev, linux-kernel,
linux-rdma, stephen, jacob.e.keller, dipayanroy, leitao, kees,
john.fastabend, hawk, bpf, daniel, ast, sdf, yury.norov
During Function Level Reset recovery, the MANA driver reads
hardware BAR0 registers that may temporarily contain garbage values.
The SHM (Shared Memory) offset read from GDMA_REG_SHM_OFFSET is used
to compute gc->shm_base, which is later dereferenced via readl() in
mana_smc_poll_register(). If the hardware returns an unaligned or
out-of-range value, the driver must not blindly use it, as this would
propagate the hardware error into a kernel crash.
The following crash was observed on an arm64 Hyper-V guest running
kernel 6.17.0-3013-azure during VF reset recovery triggered by HWC
timeout.
[13291.785274] Unable to handle kernel paging request at virtual address ffff8000a200001b
[13291.785311] Mem abort info:
[13291.785332] ESR = 0x0000000096000021
[13291.785343] EC = 0x25: DABT (current EL), IL = 32 bits
[13291.785355] SET = 0, FnV = 0
[13291.785363] EA = 0, S1PTW = 0
[13291.785372] FSC = 0x21: alignment fault
[13291.785382] Data abort info:
[13291.785391] ISV = 0, ISS = 0x00000021, ISS2 = 0x00000000
[13291.785404] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[13291.785412] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[13291.785421] swapper pgtable: 4k pages, 48-bit VAs, pgdp=00000014df3a1000
[13291.785432] [ffff8000a200001b] pgd=1000000100438403, p4d=1000000100438403, pud=1000000100439403, pmd=0068000fc2000711
[13291.785703] Internal error: Oops: 0000000096000021 [#1] SMP
[13291.830975] Modules linked in: tls qrtr mana_ib ib_uverbs ib_core xt_owner xt_tcpudp xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nft_compat nf_tables cfg80211 8021q garp mrp stp llc binfmt_misc joydev serio_raw nls_iso8859_1 hid_generic aes_ce_blk aes_ce_cipher polyval_ce ghash_ce sm4_ce_gcm sm4_ce_ccm sm4_ce sm4_ce_cipher hid_hyperv sm4 sm3_ce sha3_ce hv_netvsc hid vmgenid hyperv_keyboard hyperv_drm sch_fq_codel nvme_fabrics efi_pstore dm_multipath nfnetlink vsock_loopback vmw_vsock_virtio_transport_common hv_sock vmw_vsock_vmci_transport vmw_vmci vsock dmi_sysfs ip_tables x_tables autofs4
[13291.862630] CPU: 122 UID: 0 PID: 61796 Comm: kworker/122:2 Tainted: G W 6.17.0-3013-azure #13-Ubuntu VOLUNTARY
[13291.869902] Tainted: [W]=WARN
[13291.871901] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 01/08/2026
[13291.878086] Workqueue: events mana_serv_func
[13291.880718] pstate: 62400005 (nZCv daif +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
[13291.884835] pc : mana_smc_poll_register+0x48/0xb0
[13291.887902] lr : mana_smc_setup_hwc+0x70/0x1c0
[13291.890493] sp : ffff8000ab79bbb0
[13291.892364] x29: ffff8000ab79bbb0 x28: ffff00410c8b5900 x27: ffff00410d630680
[13291.896252] x26: ffff004171f9fd80 x25: 000000016ed55000 x24: 000000017f37e000
[13291.899990] x23: 0000000000000000 x22: 000000016ed55000 x21: 0000000000000000
[13291.904497] x20: ffff8000a200001b x19: 0000000000004e20 x18: ffff8000a6183050
[13291.908308] x17: 0000000000000000 x16: 0000000000000000 x15: 000000000000000a
[13291.912542] x14: 0000000000000004 x13: 0000000000000000 x12: 0000000000000000
[13291.916298] x11: 0000000000000000 x10: 0000000000000001 x9 : ffffc45006af1bd8
[13291.920945] x8 : ffff000151129000 x7 : 0000000000000000 x6 : 0000000000000000
[13291.925293] x5 : 000000015f214000 x4 : 000000017217a000 x3 : 000000016ed50000
[13291.930436] x2 : 000000016ed55000 x1 : 0000000000000000 x0 : ffff8000a1ffffff
[13291.934342] Call trace:
[13291.935736] mana_smc_poll_register+0x48/0xb0 (P)
[13291.938611] mana_smc_setup_hwc+0x70/0x1c0
[13291.941113] mana_hwc_create_channel+0x1a0/0x3a0
[13291.944283] mana_gd_setup+0x16c/0x398
[13291.946584] mana_gd_resume+0x24/0x70
[13291.948917] mana_do_service+0x13c/0x1d0
[13291.951583] mana_serv_func+0x34/0x68
[13291.953732] process_one_work+0x168/0x3d0
[13291.956745] worker_thread+0x2ac/0x480
[13291.959104] kthread+0xf8/0x110
[13291.961026] ret_from_fork+0x10/0x20
[13291.963560] Code: d2807d00 9417c551 71000673 54000220 (b9400281)
[13291.967299] ---[ end trace 0000000000000000 ]---
Disassembly of mana_smc_poll_register() around the crash site:
Disassembly of section .text:
00000000000047c8 <mana_smc_poll_register>:
47c8: d503201f nop
47cc: d503201f nop
47d0: d503233f paciasp
47d4: f800865e str x30, [x18], #8
47d8: a9bd7bfd stp x29, x30, [sp, #-48]!
47dc: 910003fd mov x29, sp
47e0: a90153f3 stp x19, x20, [sp, #16]
47e4: 91007014 add x20, x0, #0x1c
47e8: 5289c413 mov w19, #0x4e20
47ec: f90013f5 str x21, [sp, #32]
47f0: 12001c35 and w21, w1, #0xff
47f4: 14000008 b 4814 <mana_smc_poll_register+0x4c>
47f8: 36f801e1 tbz w1, #31, 4834 <mana_smc_poll_register+0x6c>
47fc: 52800042 mov w2, #0x2
4800: d280fa01 mov x1, #0x7d0
4804: d2807d00 mov x0, #0x3e8
4808: 94000000 bl 0 <usleep_range_state>
480c: 71000673 subs w19, w19, #0x1
4810: 54000200 b.eq 4850 <mana_smc_poll_register+0x88>
4814: b9400281 ldr w1, [x20] <-- **** CRASHED HERE *****
4818: d50331bf dmb oshld
481c: 2a0103e2 mov w2, w1
...
From the crash signature x20 = ffff8000a200001b, this address
ends in 0x1b which is not 4-byte aligned, so the 'ldr w1, [x20]'
instruction (readl) triggers the arm64 alignment fault (FSC = 0x21).
The root cause is in mana_gd_init_vf_regs(), which computes:
gc->shm_base = gc->bar0_va + mana_gd_r64(gc, GDMA_REG_SHM_OFFSET);
The offset is used without any validation. The same problem exists
in mana_gd_init_pf_regs() for sriov_base_off and sriov_shm_off.
Fix this by validating all offsets before use:
- VF: check shm_off is within BAR0, properly aligned to 4 bytes
(readl requirement), and leaves room for the full 256-bit
(32-byte) SMC aperture.
- PF: check sriov_base_off is within BAR0, aligned to 8 bytes
(readq requirement), and leaves room to safely read the
sriov_shm_off register at sriov_base_off + GDMA_PF_REG_SHM_OFF.
Then check sriov_shm_off leaves room for the full SMC aperture.
All arithmetic uses subtraction rather than addition to avoid
integer overflow on garbage values.
Define SMC_APERTURE_SIZE (32 bytes, derived from the 256-bit aperture
width)
Return -EPROTO on invalid values. The existing recovery path in
mana_serv_reset() already handles -EPROTO by falling through to PCI
device rescan, giving the hardware another chance to present valid
register values after reset.
Fixes: 9bf66036d686 ("net: mana: Handle hardware recovery events when probing the device")
Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
---
Changes in v3:
- Fixed commit message.
- Removed macro duplicates.
Changes in v2:
- Fix sriov_base_off alignment check: sizeof(u32) to sizeof(u64), since
mana_gd_r64() (readq) requires 8-byte alignment on arm64.
- Fix sriov_base_off bounds: also verify enough space remains in BAR0
to safely read sriov_shm_off at offset GDMA_PF_REG_SHM_OFF + 8 bytes.
- Fix integer overflow: rewrite bounds checks using subtraction
(remaining = bar0_size - base) instead of addition.
- Fix SMC aperture size: add gc->bar0_size - shm_off < SMC_APERTURE_SIZE
checks in both VF and PF paths; previously only the start address was
validated, but mana_smc_poll_register() accesses up to shm_base + 0x1c
(28 bytes from base, 32 bytes total).
- Export SMC_APERTURE_SIZE to shm_channel.h.
---
.../net/ethernet/microsoft/mana/gdma_main.c | 40 ++++++++++++++++---
.../net/ethernet/microsoft/mana/shm_channel.c | 5 ---
include/net/mana/shm_channel.h | 6 +++
3 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 098fbda0d128..d8e816882f02 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -43,8 +43,9 @@ static u64 mana_gd_r64(struct gdma_context *g, u64 offset)
static int mana_gd_init_pf_regs(struct pci_dev *pdev)
{
struct gdma_context *gc = pci_get_drvdata(pdev);
- void __iomem *sriov_base_va;
+ u64 remaining_barsize;
u64 sriov_base_off;
+ u64 sriov_shm_off;
gc->db_page_size = mana_gd_r32(gc, GDMA_PF_REG_DB_PAGE_SIZE) & 0xFFFF;
@@ -73,10 +74,28 @@ static int mana_gd_init_pf_regs(struct pci_dev *pdev)
gc->phys_db_page_base = gc->bar0_pa + gc->db_page_off;
sriov_base_off = mana_gd_r64(gc, GDMA_SRIOV_REG_CFG_BASE_OFF);
+ if (sriov_base_off >= gc->bar0_size ||
+ gc->bar0_size - sriov_base_off <
+ GDMA_PF_REG_SHM_OFF + sizeof(u64) ||
+ !IS_ALIGNED(sriov_base_off, sizeof(u64))) {
+ dev_err(gc->dev,
+ "SRIOV base offset 0x%llx out of range or unaligned (BAR0 size 0x%llx)\n",
+ sriov_base_off, (u64)gc->bar0_size);
+ return -EPROTO;
+ }
- sriov_base_va = gc->bar0_va + sriov_base_off;
- gc->shm_base = sriov_base_va +
- mana_gd_r64(gc, sriov_base_off + GDMA_PF_REG_SHM_OFF);
+ remaining_barsize = gc->bar0_size - sriov_base_off;
+ sriov_shm_off = mana_gd_r64(gc, sriov_base_off + GDMA_PF_REG_SHM_OFF);
+ if (sriov_shm_off >= remaining_barsize ||
+ remaining_barsize - sriov_shm_off < SMC_APERTURE_SIZE ||
+ !IS_ALIGNED(sriov_shm_off, sizeof(u32))) {
+ dev_err(gc->dev,
+ "SRIOV SHM offset 0x%llx out of range or unaligned (BAR0 size 0x%llx)\n",
+ sriov_shm_off, (u64)gc->bar0_size);
+ return -EPROTO;
+ }
+
+ gc->shm_base = gc->bar0_va + sriov_base_off + sriov_shm_off;
return 0;
}
@@ -84,6 +103,7 @@ static int mana_gd_init_pf_regs(struct pci_dev *pdev)
static int mana_gd_init_vf_regs(struct pci_dev *pdev)
{
struct gdma_context *gc = pci_get_drvdata(pdev);
+ u64 shm_off;
gc->db_page_size = mana_gd_r32(gc, GDMA_REG_DB_PAGE_SIZE) & 0xFFFF;
@@ -111,7 +131,17 @@ static int mana_gd_init_vf_regs(struct pci_dev *pdev)
gc->db_page_base = gc->bar0_va + gc->db_page_off;
gc->phys_db_page_base = gc->bar0_pa + gc->db_page_off;
- gc->shm_base = gc->bar0_va + mana_gd_r64(gc, GDMA_REG_SHM_OFFSET);
+ shm_off = mana_gd_r64(gc, GDMA_REG_SHM_OFFSET);
+ if (shm_off >= gc->bar0_size ||
+ gc->bar0_size - shm_off < SMC_APERTURE_SIZE ||
+ !IS_ALIGNED(shm_off, sizeof(u32))) {
+ dev_err(gc->dev,
+ "SHM offset 0x%llx out of range or unaligned (BAR0 size 0x%llx)\n",
+ shm_off, (u64)gc->bar0_size);
+ return -EPROTO;
+ }
+
+ gc->shm_base = gc->bar0_va + shm_off;
return 0;
}
diff --git a/drivers/net/ethernet/microsoft/mana/shm_channel.c b/drivers/net/ethernet/microsoft/mana/shm_channel.c
index 0f1679ebad96..d21b5db06e50 100644
--- a/drivers/net/ethernet/microsoft/mana/shm_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/shm_channel.c
@@ -61,11 +61,6 @@ union smc_proto_hdr {
};
}; /* HW DATA */
-#define SMC_APERTURE_BITS 256
-#define SMC_BASIC_UNIT (sizeof(u32))
-#define SMC_APERTURE_DWORDS (SMC_APERTURE_BITS / (SMC_BASIC_UNIT * 8))
-#define SMC_LAST_DWORD (SMC_APERTURE_DWORDS - 1)
-
static int mana_smc_poll_register(void __iomem *base, bool reset)
{
void __iomem *ptr = base + SMC_LAST_DWORD * SMC_BASIC_UNIT;
diff --git a/include/net/mana/shm_channel.h b/include/net/mana/shm_channel.h
index 5199b41497ff..dbabcfb95daf 100644
--- a/include/net/mana/shm_channel.h
+++ b/include/net/mana/shm_channel.h
@@ -4,6 +4,12 @@
#ifndef _SHM_CHANNEL_H
#define _SHM_CHANNEL_H
+#define SMC_APERTURE_BITS 256
+#define SMC_BASIC_UNIT (sizeof(u32))
+#define SMC_APERTURE_DWORDS (SMC_APERTURE_BITS / (SMC_BASIC_UNIT * 8))
+#define SMC_LAST_DWORD (SMC_APERTURE_DWORDS - 1)
+#define SMC_APERTURE_SIZE (SMC_APERTURE_BITS / 8)
+
struct shm_channel {
struct device *dev;
void __iomem *base;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v41 0/7] eea: Add basic driver framework for Alibaba Elastic Ethernet Adaptor
From: Jakub Kicinski @ 2026-05-01 2:13 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Wen Gu, Philo Lu, Vadim Fedorenko, Dong Yibo, Mingyu Wang,
Heiner Kallweit, Dust Li
In-Reply-To: <20260429023726.100908-1-xuanzhuo@linux.alibaba.com>
On Wed, 29 Apr 2026 10:37:19 +0800 Xuan Zhuo wrote:
> Add a driver framework for EEA that will be available in the future.
>
> This driver is currently quite minimal, implementing only fundamental
> core functionalities. Key features include: I/O queue management via
> adminq, basic PCI-layer operations, and essential RX/TX data
> communication capabilities. It also supports the creation,
> initialization, and management of network devices (netdev). Furthermore,
> the ring structures for both I/O queues and adminq have been abstracted
> into a simple, unified, and reusable library implementation,
> facilitating future extension and maintenance.
>
> v41:
> 1. make https://sashiko.dev/ happy
Does not look happy, some of the reports / asks look valid.
FWIW we have another sashiko instance now:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260429023726.100908-5-xuanzhuo@linux.alibaba.com
You should definitely follow the advice to rename the config,
and also failing probe because the FW doesn't like the kernel
version is not acceptable upstream. You can print a warning
but failure is not acceptable.
--
pw-bot: cr
^ permalink raw reply
* Re: [pull-request] mlx5-next updates 2026-04-29
From: patchwork-bot+netdevbpf @ 2026-05-01 2:10 UTC (permalink / raw)
To: Tariq Toukan
Cc: edumazet, kuba, pabeni, andrew+netdev, davem, leon, jgg, saeedm,
mbloch, moshe, shayd, parav, danielj, kees, ajayachandra, jiri,
ohartoov, horms, linux-rdma, linux-kernel, netdev, gal, dtatulea
In-Reply-To: <20260429212747.224411-1-tariqt@nvidia.com>
Hello:
This pull request was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 30 Apr 2026 00:27:47 +0300 you wrote:
> Hi,
>
> The following pull-request contains common mlx5 updates
> for your *net-next* tree.
> Please pull and let me know of any problem.
>
> Regards,
> Tariq
>
> [...]
Here is the summary with links:
- [pull-request] mlx5-next updates 2026-04-29
https://git.kernel.org/netdev/net-next/c/4e37987362bc
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v5 net 00/10] octeontx2-af: npc: cn20k: MCAM fixes
From: patchwork-bot+netdevbpf @ 2026-05-01 2:00 UTC (permalink / raw)
To: Ratheesh Kannoth
Cc: netdev, linux-kernel, sgoutham, davem, edumazet, kuba, pabeni,
andrew+netdev
In-Reply-To: <20260429022722.1110289-1-rkannoth@marvell.com>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Apr 2026 07:57:12 +0530 you wrote:
> This series tightens Marvell OcteonTX2 AF NPC support for CN20K silicon
> around MCAM key typing, optional debugfs setup, defrag allocation rollback,
> defrag entry relocation bookkeeping, logical MCAM clear and programming,
> default-rule index handling with explicit teardown, and NIXLF reserved-slot
> lookup when default rules are missing.
>
> Patches 1 through 3 focus on AF error handling: propagate
> npc_mcam_idx_2_key_type() failures through cn20k MCAM enable, config, copy,
> and read paths; treat cn20k NPC debugfs nodes as optional so probe does not
> fail when debugfs is unavailable; and fix defrag MCAM allocation rollback
> so allocation errno is not overwritten during subbank index resolution.
>
> [...]
Here is the summary with links:
- [v5,net,01/10] octeontx2-af: npc: cn20k: Propagate MCAM key-type errors on cn20k
https://git.kernel.org/netdev/net/c/aaadccde312f
- [v5,net,02/10] octeontx2-af: npc: cn20k: Drop debugfs_create_file() error checks in init
https://git.kernel.org/netdev/net/c/1100af13fd14
- [v5,net,03/10] octeontx2-af: npc: cn20k: Propagate errors in defrag MCAM alloc rollback
https://git.kernel.org/netdev/net/c/adb5ff41efbc
- [v5,net,04/10] octeontx2-af: npc: cn20k: Fix target map and rule
https://git.kernel.org/netdev/net/c/d7e5940c4c50
- [v5,net,05/10] octeontx2-af: npc: cn20k: Clear MCAM entries by index and key width
https://git.kernel.org/netdev/net/c/d2dabf09632c
- [v5,net,06/10] octeontx2-af: npc: cn20k: Fix bank value
https://git.kernel.org/netdev/net/c/2b6d6bb7282c
- [v5,net,07/10] octeontx2-af: npc: cn20k: Fix MCAM actions read
https://git.kernel.org/netdev/net/c/f6803eb070bf
- [v5,net,08/10] octeontx2-af: npc: cn20k: Initialize default-rule index outputs up front
https://git.kernel.org/netdev/net/c/afb474bd4ffc
- [v5,net,09/10] octeontx2-af: npc: cn20k: Tear down default MCAM rules explicitly on free
https://git.kernel.org/netdev/net/c/013717353c03
- [v5,net,10/10] octeontx2-af: npc: cn20k: Reject missing default-rule MCAM indices
https://git.kernel.org/netdev/net/c/bc968f61bf0a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net-next v3 5/5] selftests: net: add veth BQL stress test
From: Jakub Kicinski @ 2026-05-01 2:00 UTC (permalink / raw)
To: hawk
Cc: netdev, kernel-team, Jonas Köppeler, Breno Leitao,
David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Shuah Khan, linux-kernel, linux-kselftest
In-Reply-To: <20260429172036.1028526-6-hawk@kernel.org>
On Wed, 29 Apr 2026 19:20:32 +0200 hawk@kernel.org wrote:
> A companion wrapper (veth_bql_test_virtme.sh) launches the test inside
> a virtme-ng VM, with .config validation to prevent silent stalls.
>
> Usage:
> sudo ./veth_bql_test.sh [--duration 300] [--nrules 100]
> [--qdisc sfq] [--qdisc-opts '...']
> [--bql-disable] [--normal-napi]
> [--qdisc-replace]
Not convinced we need to carry this in the tree.
Honestly:
> + close(fd);
> + return 0;
> +}
> +CEOF
> +gcc -O2 -Wall -o "$TMPDIR"/udp_sink "$TMPDIR"/udp_sink.c -lm || exit $ksft_fail
> +}
> +
> +setup_veth() {
> + log_info "Setting up
This whole test looks like outrageous slop.
Did you even read this? :|
^ permalink raw reply
* Re: [PATCH net-next v3 0/5] veth: add Byte Queue Limits (BQL) support
From: Jakub Kicinski @ 2026-05-01 1:58 UTC (permalink / raw)
To: hawk
Cc: netdev, kernel-team, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Shuah Khan, linux-kselftest, Chris Arges,
Mike Freemon, Toke Høiland-Jørgensen,
Jonas Köppeler, Breno Leitao, Alexei Starovoitov,
Daniel Borkmann, John Fastabend, Stanislav Fomichev, bpf
In-Reply-To: <20260429172036.1028526-1-hawk@kernel.org>
On Wed, 29 Apr 2026 19:20:27 +0200 hawk@kernel.org wrote:
> This series adds BQL (Byte Queue Limits) to the veth driver, reducing
> latency by dynamically limiting in-flight packets in the ptr_ring and
> moving buffering into the qdisc where AQM algorithms can act on it.
Does not apply (am I remembering right that it's not the first time
this series doesn't apply?)
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net-next v8 6/7] net: bcmgenet: add XDP statistics counters
From: Jakub Kicinski @ 2026-05-01 1:42 UTC (permalink / raw)
To: nb
Cc: Jakub Kicinski, netdev, justin.chen, horms, mohsin.bashr, opendmb,
florian.fainelli, bcm-kernel-feedback-list, andrew+netdev,
edumazet, pabeni, davem, rajashekar.hudumula, vikas.gupta,
bhargava.marreddy, sashal, ebiggers, linux-kernel
In-Reply-To: <20260428205846.2625550-7-nb@tipi-net.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: bcmgenet: add XDP statistics counters
This commit adds six per-action XDP counters (xdp_pass, xdp_drop, xdp_tx,
xdp_tx_err, xdp_redirect, xdp_redirect_err) to bcmgenet via the existing
soft MIB infrastructure, exposing them through ethtool -S and
incrementing them from bcmgenet_run_xdp().
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 9dd258567824..02ad2f410d6c 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1169,6 +1169,13 @@ static const struct bcmgenet_stats bcmgenet_gstrings_stats[] = {
> STAT_GENET_SOFT_MIB("tx_realloc_tsb", mib.tx_realloc_tsb),
> STAT_GENET_SOFT_MIB("tx_realloc_tsb_failed",
> mib.tx_realloc_tsb_failed),
> + /* XDP counters */
> + STAT_GENET_SOFT_MIB("xdp_pass", mib.xdp_pass),
> + STAT_GENET_SOFT_MIB("xdp_drop", mib.xdp_drop),
> + STAT_GENET_SOFT_MIB("xdp_tx", mib.xdp_tx),
> + STAT_GENET_SOFT_MIB("xdp_tx_err", mib.xdp_tx_err),
> + STAT_GENET_SOFT_MIB("xdp_redirect", mib.xdp_redirect),
> + STAT_GENET_SOFT_MIB("xdp_redirect_err", mib.xdp_redirect_err),
Should these counters be exposed through the standard netdev netlink
qstats interface rather than as driver-private ethtool -S strings?
Per-action XDP statistics are a generic, cross-driver category that
already has a standard uAPI: the netdev netlink family exposes per-device
and per-queue XDP counters (rx-xdp-packets, rx-xdp-bytes, rx-xdp-errors,
tx-xdp-xmit-*, etc.) via the qstats ops, retrievable with ynl.
Documentation/networking/statistics.rst reserves ethtool -S for
driver-private counters, and the networking subsystem guidance is that
new counters should be plumbed through the standard interface rather
than added as private strings, even when the driver does not currently
implement that interface.
Jakub: no, this is wrong, there are no standard counters for XDP today
> /* Per TX queues */
> STAT_GENET_Q(0),
> STAT_GENET_Q(1),
> @@ -2428,6 +2435,7 @@ static unsigned int bcmgenet_run_xdp(struct bcmgenet_rx_ring *ring,
>
> switch (act) {
> case XDP_PASS:
> + priv->mib.xdp_pass++;
> return XDP_PASS;
> case XDP_TX:
> /* Prepend a zeroed TSB (Transmit Status Block). The GENET
> @@ -2440,6 +2448,7 @@ static unsigned int bcmgenet_run_xdp(struct bcmgenet_rx_ring *ring,
> sizeof(struct status_64) + sizeof(struct xdp_frame)) {
> page_pool_put_full_page(ring->page_pool, rx_page,
> true);
> + priv->mib.xdp_tx_err++;
> return XDP_DROP;
> }
> xdp->data -= sizeof(struct status_64);
> @@ -2459,19 +2468,24 @@ static unsigned int bcmgenet_run_xdp(struct bcmgenet_rx_ring *ring,
> xdpf, false))) {
> spin_unlock(&tx_ring->lock);
> xdp_return_frame_rx_napi(xdpf);
> + priv->mib.xdp_tx_err++;
> return XDP_DROP;
> }
> bcmgenet_xdp_ring_doorbell(priv, tx_ring);
> spin_unlock(&tx_ring->lock);
> + priv->mib.xdp_tx++;
> return XDP_TX;
Is the XDP_TX path missing an increment on one of its failure exits?
The headroom check and the bcmgenet_xdp_xmit_frame() failure both bump
xdp_tx_err, but the xdp_convert_buff_to_frame() failure in between:
xdpf = xdp_convert_buff_to_frame(xdp);
if (unlikely(!xdpf)) {
page_pool_put_full_page(ring->page_pool, rx_page,
true);
return XDP_DROP;
}
returns XDP_DROP without touching any counter.
With this path in place, does the identity
xdp_pass + xdp_drop + xdp_tx + xdp_tx_err +
xdp_redirect + xdp_redirect_err == packets_processed
still hold?
> case XDP_REDIRECT:
> if (unlikely(xdp_do_redirect(priv->dev, xdp, prog))) {
> + priv->mib.xdp_redirect_err++;
> page_pool_put_full_page(ring->page_pool, rx_page,
> true);
> return XDP_DROP;
> }
> + priv->mib.xdp_redirect++;
> return XDP_REDIRECT;
> case XDP_DROP:
> + priv->mib.xdp_drop++;
> page_pool_put_full_page(ring->page_pool, rx_page, true);
> return XDP_DROP;
> default:
> @@ -2479,6 +2493,7 @@ static unsigned int bcmgenet_run_xdp(struct bcmgenet_rx_ring *ring,
> fallthrough;
> case XDP_ABORTED:
> trace_xdp_exception(priv->dev, prog, act);
> + priv->mib.xdp_drop++;
> page_pool_put_full_page(ring->page_pool, rx_page, true);
> return XDP_ABORTED;
> }
Does folding XDP_ABORTED and the unsupported-action default case into
xdp_drop match the commit message's description of "per-action XDP
counters"?
The function returns XDP_ABORTED (not XDP_DROP) in those cases, and the
existing trace_xdp_exception() call distinguishes aborts and unknown
actions from intentional XDP_DROP.
With the current mapping, operators cannot tell an explicit XDP_DROP
from a program abort or an invalid action return when looking at
xdp_drop. Would a separate xdp_aborted (or xdp_errors) counter for the
XDP_ABORTED and default cases be more appropriate?
^ permalink raw reply
* Re: [PATCH net-next v8 5/7] net: bcmgenet: add XDP_REDIRECT and ndo_xdp_xmit support
From: Jakub Kicinski @ 2026-05-01 1:40 UTC (permalink / raw)
To: nb
Cc: Jakub Kicinski, netdev, justin.chen, horms, mohsin.bashr, opendmb,
florian.fainelli, bcm-kernel-feedback-list, andrew+netdev,
edumazet, pabeni, davem, rajashekar.hudumula, vikas.gupta,
bhargava.marreddy, sashal, ebiggers, linux-kernel
In-Reply-To: <20260428205846.2625550-6-nb@tipi-net.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: bcmgenet: add XDP_REDIRECT and ndo_xdp_xmit support
This patch adds XDP_REDIRECT support in the RX path and implements
ndo_xdp_xmit for receiving redirected frames from other devices, batching
frames into ring 16 under a single spinlock. It also advertises
NETDEV_XDP_ACT_REDIRECT and NETDEV_XDP_ACT_NDO_XMIT in xdp_features.
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 3c3b0c44ea8a..9dd258567824 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
[ ... ]
> @@ -3996,10 +4017,16 @@ static int bcmgenet_xdp_setup(struct net_device *dev,
> return -EOPNOTSUPP;
> }
>
> + if (!prog)
> + xdp_features_clear_redirect_target(dev);
> +
> old_prog = xchg(&priv->xdp_prog, prog);
> if (old_prog)
> bpf_prog_put(old_prog);
>
> + if (prog)
> + xdp_features_set_redirect_target(dev, false);
> +
Does this leave NETDEV_XDP_ACT_NDO_XMIT permanently cleared after a
single attach+detach cycle?
bcmgenet_probe() unconditionally sets:
dev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
NETDEV_XDP_ACT_NDO_XMIT;
but on any call to bcmgenet_xdp_setup() with prog == NULL this code
runs xdp_features_clear_redirect_target(dev), which clears both
NETDEV_XDP_ACT_NDO_XMIT and NETDEV_XDP_ACT_NDO_XMIT_SG in
xdp_features_clear_redirect_target_locked() (net/core/xdp.c).
bcmgenet_xdp_xmit() does not depend on priv->xdp_prog — it only needs
netif_running() and priv->xdp_tx_ring, which is initialized
unconditionally in bcmgenet_init_tx_queues(). So after attach+detach,
is_valid_dst() in kernel/bpf/devmap.c:__xdp_enqueue() will see
NETDEV_XDP_ACT_NDO_XMIT cleared and reject every redirected frame with
-EOPNOTSUPP until another program is attached.
The paired xdp_features_set_redirect_target(dev, false) call only fires
for prog != NULL, so it does not restore the bit after a no-op detach,
and it is redundant with the probe-time advertisement when prog is
non-NULL.
> return 0;
> }
>
> @@ -4013,6 +4040,36 @@ static int bcmgenet_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> }
> }
>
> +static int bcmgenet_xdp_xmit(struct net_device *dev, int num_frames,
> + struct xdp_frame **frames, u32 flags)
> +{
> + struct bcmgenet_priv *priv = netdev_priv(dev);
> + struct bcmgenet_tx_ring *ring = &priv->xdp_tx_ring;
> + int sent = 0;
> + int i;
> +
> + if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
> + return -EINVAL;
> +
> + if (unlikely(!netif_running(dev)))
> + return -ENETDOWN;
Is there a use-after-free window between this check and a concurrent
bcmgenet_close()?
netif_running() is an unlocked test_bit on __LINK_STATE_START.
__dev_close_many() clears that bit and then calls ndo_stop, which
reaches bcmgenet_close() -> bcmgenet_netif_stop() -> bcmgenet_fini_dma(),
where kfree(priv->tx_cbs) frees the backing array that ring->cbs points
into.
The synchronize_net() inside dev_deactivate_many() only waits for
outstanding qdisc enqueuers, not for a remote device's NAPI loop that
is inside xdp_do_flush() -> __dev_flush() -> bq_xmit_all() ->
bcmgenet_xdp_xmit() on another CPU.
A caller that passed the netif_running() test just before
__LINK_STATE_START was cleared can then execute bcmgenet_get_txcb() and
dmadesc_set() against freed tx_cbs memory after bcmgenet_fini_dma() has
run.
Would an RCU-protected "xmit enabled" flag cleared on close followed by
synchronize_rcu() before freeing the ring be needed here?
Jakub: I think this is worried that sync_net will be skipped if the device
has no qdiscs on down(). I don't think we care.
> +
> + spin_lock(&ring->lock);
> +
> + for (i = 0; i < num_frames; i++) {
> + if (!bcmgenet_xdp_xmit_frame(priv, ring, frames[i], true))
> + break;
> + sent++;
> + }
> +
> + if (sent)
> + bcmgenet_xdp_ring_doorbell(priv, ring);
> +
> + spin_unlock(&ring->lock);
> +
> + return sent;
> +}
How are completions reclaimed on priv->xdp_tx_ring (ring 16) when the
device has no local RX traffic?
The existing ring 16 has no TX interrupt and is reclaimed only from
bcmgenet_rx_poll(); the driver comment states:
/* ring 16 has no interrupt */
bcmgenet_xdp_xmit() itself has no completion path. When this device is
used purely as a redirect target (receiving frames via ndo_xdp_xmit
from other NICs) with little or no local RX traffic, bcmgenet_rx_poll()
is not scheduled, completed descriptors are never reclaimed,
ring->free_bds stays at 0 after the first burst, and
bcmgenet_xdp_xmit_frame() returns false for every subsequent frame.
bcmgenet_xdp_xmit() then returns 0 (all frames dropped) until unrelated
RX traffic happens to arrive.
The XDP_TX path does not hit this because it runs from inside
bcmgenet_rx_poll(), which drains the ring on each call.
Would scheduling tx reclaim from ndo_xdp_xmit, or installing a
completion interrupt or timer for ring 16, be appropriate here?
> +
> static const struct net_device_ops bcmgenet_netdev_ops = {
> .ndo_open = bcmgenet_open,
> .ndo_stop = bcmgenet_close,
[ ... ]
> @@ -4327,7 +4385,8 @@ static int bcmgenet_probe(struct platform_device *pdev)
> NETIF_F_RXCSUM;
> dev->hw_features |= dev->features;
> dev->vlan_features |= dev->features;
> - dev->xdp_features = NETDEV_XDP_ACT_BASIC;
> + dev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
> + NETDEV_XDP_ACT_NDO_XMIT;
>
> netdev_sw_irq_coalesce_default_on(dev);
^ permalink raw reply
* Re: [PATCH net-next v8 4/7] net: bcmgenet: add XDP_TX support
From: Jakub Kicinski @ 2026-05-01 1:39 UTC (permalink / raw)
To: nb
Cc: Jakub Kicinski, netdev, justin.chen, horms, mohsin.bashr, opendmb,
florian.fainelli, bcm-kernel-feedback-list, andrew+netdev,
edumazet, pabeni, davem, rajashekar.hudumula, vikas.gupta,
bhargava.marreddy, sashal, ebiggers, linux-kernel
In-Reply-To: <20260428205846.2625550-5-nb@tipi-net.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: bcmgenet: add XDP_TX support
This patch adds XDP_TX support to bcmgenet by dedicating ring 16
(DESC_INDEX) to XDP TX, carving 32 BDs from ring 0's allocation and
piggybacking TX completion on the RX NAPI poll. The page_pool DMA
direction is switched from DMA_FROM_DEVICE to DMA_BIDIRECTIONAL.
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 1b60571446e1..3c3b0c44ea8a 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
[ ... ]
> @@ -1940,8 +1956,15 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
>
> /* Reclaim transmitted buffers */
> while (txbds_processed < txbds_ready) {
> - skb = bcmgenet_free_tx_cb(&priv->pdev->dev,
> - &priv->tx_cbs[ring->clean_ptr]);
> + tx_cb = &priv->tx_cbs[ring->clean_ptr];
> + if (tx_cb->xdpf) {
> + pkts_compl++;
> + bytes_compl += tx_cb->xdp_dma_map
> + ? tx_cb->xdpf->len
> + : tx_cb->xdpf->len -
> + sizeof(struct status_64);
> + }
> + skb = bcmgenet_free_tx_cb(&priv->pdev->dev, tx_cb);
> if (skb) {
> pkts_compl++;
> bytes_compl += GENET_CB(skb)->bytes_sent;
> @@ -1963,8 +1986,11 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
> u64_stats_add(&stats->bytes, bytes_compl);
> u64_stats_update_end(&stats->syncp);
>
> - netdev_tx_completed_queue(netdev_get_tx_queue(dev, ring->index),
> - pkts_compl, bytes_compl);
> + /* Ring DESC_INDEX (XDP TX) has no netdev TX queue; skip BQL */
> + if (ring->index != DESC_INDEX)
> + netdev_tx_completed_queue(netdev_get_tx_queue(dev,
> + ring->index),
> + pkts_compl, bytes_compl);
>
> return txbds_processed;
> }
The guard here skips netdev_get_tx_queue() for the XDP TX ring, but the
outer wrapper bcmgenet_tx_reclaim() still calls:
netdev_tx_reset_queue(netdev_get_tx_queue(dev, ring->index));
...
bcmgenet_tdma_ring_writel(priv, ring->index, ring->prod_index, ...);
unconditionally when invoked with all=true.
For the XDP TX ring, ring->index == DESC_INDEX == 16, but bcmgenet_probe()
allocates the netdev via alloc_etherdev_mqs(..., GENET_MAX_MQ_CNT + 1,
GENET_MAX_MQ_CNT + 1), which is only 5 TX queues. So netdev_get_tx_queue(dev,
16) returns &dev->_tx[16], well past the end of the allocated _tx[] array.
On CONFIG_BQL=y (the default), netdev_tx_reset_queue() then writes into that
memory through dql_reset() and clear_bit(__QUEUE_STATE_STACK_XOFF, &q->state).
Can this path corrupt memory past dev->_tx[] whenever bcmgenet_tx_reclaim_all()
runs on the XDP TX ring? See the new call added below:
[ ... ]
> @@ -2043,6 +2069,9 @@ static void bcmgenet_tx_reclaim_all(struct net_device *dev)
> do {
> bcmgenet_tx_reclaim(dev, &priv->tx_rings[i++], true);
> } while (i <= priv->hw_params->tx_queues && netif_is_multiqueue(dev));
> +
> + /* Also reclaim XDP TX ring */
> + bcmgenet_tx_reclaim(dev, &priv->xdp_tx_ring, true);
> }
bcmgenet_tx_reclaim_all() is reached from bcmgenet_netif_stop() (ifconfig
down), the WoL/suspend reconfigure path, and open/reset-failure cleanup, so
this is triggerable from userspace on every interface-down.
Should the netdev_tx_reset_queue() in bcmgenet_tx_reclaim()'s all-branch be
guarded with the same ring->index != DESC_INDEX check that was added to
__bcmgenet_tx_reclaim()?
[ ... ]
> @@ -3008,14 +3161,18 @@ static int bcmgenet_rdma_disable(struct bcmgenet_priv *priv)
[ ... ]
> static void bcmgenet_init_tx_queues(struct net_device *dev)
> {
> @@ -3028,7 +3185,8 @@ static void bcmgenet_init_tx_queues(struct net_device *dev)
>
> /* Initialize Tx priority queues */
> for (i = 0; i <= priv->hw_params->tx_queues; i++) {
> - bcmgenet_init_tx_ring(priv, i, end - start, start, end);
> + bcmgenet_init_tx_ring(priv, &priv->tx_rings[i],
> + i, end - start, start, end);
> start = end;
> end += priv->hw_params->tx_bds_per_q;
> dma_priority[DMA_PRIO_REG_INDEX(i)] |=
> @@ -3036,13 +3194,19 @@ static void bcmgenet_init_tx_queues(struct net_device *dev)
> << DMA_PRIO_REG_SHIFT(i);
> }
>
> + /* Initialize ring 16 (descriptor ring) for XDP TX */
> + bcmgenet_init_tx_ring(priv, &priv->xdp_tx_ring,
> + DESC_INDEX, GENET_Q16_TX_BD_CNT,
> + TOTAL_DESC - GENET_Q16_TX_BD_CNT, TOTAL_DESC);
> +
> /* Set Tx queue priorities */
> bcmgenet_tdma_writel(priv, dma_priority[0], DMA_PRIORITY_0);
> bcmgenet_tdma_writel(priv, dma_priority[1], DMA_PRIORITY_1);
> bcmgenet_tdma_writel(priv, dma_priority[2], DMA_PRIORITY_2);
What priority does ring 16 end up with under strict-priority arbitration?
dma_priority[] is declared as:
u32 dma_priority[3] = {0, 0, 0};
and only rings 0..tx_queues populate it. Ring 16's priority field lives in
DMA_PRIORITY_2, bits [20..24] (DMA_PRIO_REG_INDEX(16) == 2,
DMA_PRIO_REG_SHIFT(16) == 20), and is left as zero.
In this driver priority 0 is the highest:
#define GENET_Q1_PRIORITY 0 /* Default highest priority queue */
#define GENET_Q0_PRIORITY 1
A few lines above, arbitration is set to strict priority:
bcmgenet_tdma_writel(priv, DMA_ARBITER_SP, DMA_ARB_CTRL);
So ring 16 ends up at priority 0, outranking Q0 (priority 1) and equal to
the user-configured high-priority queues Q1..Q4. Should ring 16 be given
an explicit priority (for example the same as Q0, or lower) so XDP_TX does
not preempt normal SKB TX under strict-priority arbitration?
^ permalink raw reply
* Re: [PATCH net-next v8 1/7] net: bcmgenet: convert RX path to page_pool
From: Jakub Kicinski @ 2026-05-01 1:37 UTC (permalink / raw)
To: nb
Cc: Jakub Kicinski, netdev, justin.chen, horms, mohsin.bashr, opendmb,
florian.fainelli, bcm-kernel-feedback-list, andrew+netdev,
edumazet, pabeni, davem, rajashekar.hudumula, vikas.gupta,
bhargava.marreddy, sashal, ebiggers, linux-kernel
In-Reply-To: <20260428205846.2625550-2-nb@tipi-net.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: bcmgenet: convert RX path to page_pool
This patch replaces the per-packet __netdev_alloc_skb() + dma_map_single()
in the bcmgenet RX path with page_pool, adding a page_pool per RX ring and
building SKBs from pages via napi_build_skb() with skb_mark_for_recycle().
A few questions on the changes below.
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 54f71b1e85fc..d013a3df9048 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -52,6 +52,13 @@
> #define RX_BUF_LENGTH 2048
> #define SKB_ALIGNMENT 32
>
> +/* Page pool RX buffer layout:
> + * XDP_PACKET_HEADROOM | RSB(64) + pad(2) | frame data | skb_shared_info
> + * The HW writes the 64B RSB + 2B alignment padding before the frame.
> + */
> +#define GENET_RSB_PAD (sizeof(struct status_64) + 2)
> +#define GENET_RX_HEADROOM (XDP_PACKET_HEADROOM + GENET_RSB_PAD)
Is GENET_RX_HEADROOM used anywhere? A grep of the tree after this patch
shows no references to it; all call sites use XDP_PACKET_HEADROOM and
GENET_RSB_PAD separately. Can this macro be dropped?
Jakub: false positive
Also, XDP_PACKET_HEADROOM is referenced here and in three other places in
bcmgenet.c (bcmgenet_rx_refill, bcmgenet_desc_rx, and
bcmgenet_rx_ring_create_pool), but the only new include added by the
patch is <net/page_pool/helpers.h> in bcmgenet.h. XDP_PACKET_HEADROOM is
defined in include/uapi/linux/bpf.h, and the helpers.h/types.h chain does
not include it. Would it be worth adding an explicit #include
<linux/bpf.h> to bcmgenet.c so this does not rely on transitive inclusion
(other drivers using this macro, e.g. bnxt, include it explicitly)?
> @@ -2250,46 +2249,30 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
> -static struct sk_buff *bcmgenet_rx_refill(struct bcmgenet_priv *priv,
> - struct enet_cb *cb)
> +static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring,
> + struct enet_cb *cb)
> {
[ ... ]
> - /* Allocate a new Rx skb */
> - skb = __netdev_alloc_skb(priv->dev, priv->rx_buf_len + SKB_ALIGNMENT,
> - GFP_ATOMIC | __GFP_NOWARN);
After this conversion, priv->rx_buf_len no longer has any readers; the
old dma_map_single/dma_unmap paths that consumed it are gone, and the new
code uses RX_BUF_LENGTH directly in pp_params.max_len and
page_pool_dma_sync_for_cpu(). bcmgenet_init() still writes the field.
Should priv->rx_buf_len be removed as part of this cleanup?
[ ... ]
> @@ -2341,25 +2324,28 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
> while ((rxpktprocessed < rxpkttoprocess) &&
> (rxpktprocessed < budget)) {
> struct status_64 *status;
> + struct page *rx_page;
> + unsigned int rx_off;
> __be16 rx_csum;
> + void *hard_start;
>
> cb = &priv->rx_cbs[ring->read_ptr];
[ ... ]
> + page_pool_dma_sync_for_cpu(ring->page_pool, rx_page, 0,
> + RX_BUF_LENGTH);
page_pool_dma_sync_for_cpu() is called unconditionally with a fixed size
of RX_BUF_LENGTH (2048) even when the actual received frame is much
smaller. The received length is available in the descriptor status word
once it is read. On the non-cache-coherent SoCs this driver targets
(for example BCM2711), is there a reason not to sync only the number of
bytes that were actually received?
[ ... ]
> @@ -2375,7 +2361,8 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
> if (unlikely(len > RX_BUF_LENGTH)) {
> netif_err(priv, rx_status, dev, "oversized packet\n");
> BCMGENET_STATS64_INC(stats, length_errors);
[ ... ]
> @@ -2411,24 +2399,47 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
[ ... ]
> - skb_put(skb, len);
> + /* Build SKB from the page - data starts at hard_start,
> + * frame begins after RSB(64) + pad(2) = 66 bytes.
> + */
> + skb = napi_build_skb(hard_start, PAGE_SIZE - XDP_PACKET_HEADROOM);
> + if (unlikely(!skb)) {
> + BCMGENET_STATS64_INC(stats, dropped);
> + page_pool_put_full_page(ring->page_pool, rx_page,
> + true);
> + goto next;
> + }
>
> - /* remove RSB and hardware 2bytes added for IP alignment */
> - skb_pull(skb, 66);
> - len -= 66;
> + skb_mark_for_recycle(skb);
> +
> + /* Reserve the RSB + pad, then set the data length */
> + skb_reserve(skb, GENET_RSB_PAD);
> + __skb_put(skb, len - GENET_RSB_PAD);
Can len here be smaller than GENET_RSB_PAD (66)? len is taken from
status->length_status >> DMA_BUFLENGTH_SHIFT and the only bound checked
above is the upper bound against RX_BUF_LENGTH:
if (unlikely(len > RX_BUF_LENGTH)) {
...
}
If HW ever reports a runt length below 66 (for example after a reset or
due to an errata path), len - GENET_RSB_PAD underflows as an unsigned
value. __skb_put() is the unchecked variant:
static inline void *__skb_put(struct sk_buff *skb, unsigned int len)
{
void *tmp = skb_tail_pointer(skb);
SKB_LINEAR_ASSERT(skb);
skb->tail += len;
skb->len += len;
return tmp;
}
so it will not trip skb_over_panic(); instead skb->tail/skb->len advance
past skb->end into the page. Since napi_build_skb() was given
frag_size = PAGE_SIZE - XDP_PACKET_HEADROOM, skb->end sits inside the
same page_pool-backed page and the SKB would then expose recycled
page_pool memory up the stack via napi_gro_receive().
The previous code used skb_put(skb, len) followed by skb_pull(skb, 66),
both of which are bounds-checked. Would it make sense to add an
explicit guard such as:
if (unlikely(len < GENET_RSB_PAD)) {
BCMGENET_STATS64_INC(stats, length_errors);
page_pool_put_full_page(ring->page_pool, rx_page, true);
goto next;
}
before the __skb_put() call?
Jakub: I suppose.. IDK if protecting against bad HW is required, up to you
> if (priv->crc_fwd_en) {
> - skb_trim(skb, len - ETH_FCS_LEN);
> - len -= ETH_FCS_LEN;
> + skb_trim(skb, skb->len - ETH_FCS_LEN);
> }
The same underflow concern applies here: if skb->len is less than
ETH_FCS_LEN, skb->len - ETH_FCS_LEN wraps to a very large unsigned
value, and skb_trim() does:
void skb_trim(struct sk_buff *skb, unsigned int len)
{
if (skb->len > len)
__skb_trim(skb, len);
}
so the trim is silently skipped and the FCS stays attached. Should this
path also validate skb->len before subtracting ETH_FCS_LEN?
[ ... ]
--
pw-bot: cr
^ permalink raw reply
* [PATCH iwl-next v4] libie: log more info when virtchnl fails
From: Li Li @ 2026-05-01 1:25 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, David S. Miller, Jakub Kicinski,
Eric Dumazet, intel-wired-lan
Cc: netdev, linux-kernel, David Decotigny, Anjali Singhai,
Sridhar Samudrala, Brian Vazquez, Li Li, emil.s.tantilov
Virtchnl failures can be hard to debug without logs. Logging the details
of virtchnl transactions can be useful for debugging virtchnl-related
issues.
Tested: Built & booted on a test machine and synthetically produced a
virtual failure to produce the following log:
idpf 0000:01:00.0: Non-zero virtchnl ret val 6 (msg op: 1, data_len: 8);
xn id: 0, cookie: 0
idpf 0000:01:00.0: Transaction failed (op 1, xn state:
3, id: 0, cookie: 0, size: 8)
Signed-off-by: Li Li <boolli@google.com>
---
v4:
- Simplify logging to reduce redundant "ret val"s.
- Use %u for xn->state.
v3:
- Use dev_err_ratelimited in both logs.
- Move log placement to after virtchnl field validation.
- Remove redundant op/cookie fields since they were validated.
v2:
- Use dev_warn_ratelimited instead of dev_notice_ratelimited based on
reviewer feedback.
drivers/net/ethernet/intel/libie/controlq.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
index ebc05355e39d..e1bc19d6cdbf 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -766,6 +766,14 @@ libie_ctlq_xn_process_recv(struct libie_ctlq_xn_recv_params *params,
msg_cookie != xn->cookie)
return false;
+ if (ctlq_msg->chnl_retval) {
+ dev_err_ratelimited(
+ params->ctlq->dev,
+ "Non-zero virtchnl ret val %u (msg op: %u, data_len: %u); xn id: %u, cookie: %u\n",
+ ctlq_msg->chnl_retval, ctlq_msg->chnl_opcode,
+ ctlq_msg->data_len, xn->index, xn->cookie);
+ }
+
spin_lock(&xn->xn_lock);
if (xn->state != LIBIE_CTLQ_XN_ASYNC &&
xn->state != LIBIE_CTLQ_XN_WAITING) {
@@ -1011,6 +1019,11 @@ int libie_ctlq_xn_send(struct libie_ctlq_xn_send_params *params)
params->recv_mem = xn->recv_mem;
break;
default:
+ dev_err_ratelimited(
+ params->ctlq->dev,
+ "Transaction failed (op %u, xn state: %u, id: %u, cookie: %u, size: %zu)\n",
+ params->chnl_opcode, xn->state, xn->index, xn->cookie,
+ xn->recv_mem.iov_len);
ret = -EBADMSG;
break;
}
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply related
* Re: [PATCH net 1/2] net: libwx: fix VF illegal register access
From: patchwork-bot+netdevbpf @ 2026-05-01 1:20 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, mengyuanlou, andrew+netdev, davem, edumazet, kuba, pabeni,
horms, kees, stable
In-Reply-To: <4D1F4452D21DE107+20260429083743.88961-1-jiawenwu@trustnetic.com>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Apr 2026 16:37:42 +0800 you wrote:
> Register WX_CFG_PORT_ST is a PF restricted register. When a VF is
> initialized, attempting to read this register triggers an illegal
> register access, which lead to a system hang.
>
> When the device is VF, the bus function ID can be obtained directly from
> the PCI_FUNC(pdev->devfn).
>
> [...]
Here is the summary with links:
- [net,1/2] net: libwx: fix VF illegal register access
https://git.kernel.org/netdev/net/c/694de316f607
- [net,2/2] net: libwx: use request_irq for VF misc interrupt
https://git.kernel.org/netdev/net/c/7a33345153ee
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v3 net] net: enetc: fix VSI mailbox timeout handling and DMA lifecycle
From: patchwork-bot+netdevbpf @ 2026-05-01 1:20 UTC (permalink / raw)
To: Wei Fang
Cc: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
davem, edumazet, kuba, pabeni, netdev, linux-kernel, imx
In-Reply-To: <20260429081930.3259824-1-wei.fang@nxp.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Apr 2026 16:19:30 +0800 you wrote:
> In the current VSI mailbox implementation, the VSI allocates a DMA buffer
> to store the message sent to the PSI. When the PSI receives the message
> request from the VSI, the hardware copies the message data from this DMA
> buffer to PSI's DMA buffer for processing.
>
> When enetc_msg_vsi_send() times out, two scenarios can occur:
>
> [...]
Here is the summary with links:
- [v3,net] net: enetc: fix VSI mailbox timeout handling and DMA lifecycle
https://git.kernel.org/netdev/net/c/26ebd12e67bf
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net v2] net: airoha: Move entries to queue head in case of DMA mapping failure in airoha_dev_xmit()
From: patchwork-bot+netdevbpf @ 2026-05-01 1:20 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, jacob.e.keller,
horms, linux-arm-kernel, linux-mediatek, netdev
In-Reply-To: <20260429-airoha-xmit-unmap-error-path-v2-1-32e43b7c6d25@kernel.org>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Apr 2026 14:02:31 +0200 you wrote:
> In order to respect the original descriptor order and avoid any
> potential IOMMU fault or memory corruption, move pending queue entries
> to the head of hw queue tx_list if the DMA mapping of current inflight
> packet fails in airoha_dev_xmit routine.
>
> Fixes: 3f47e67dff1f7 ("net: airoha: Add the capability to consume out-of-order DMA tx descriptors")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>
> [...]
Here is the summary with links:
- [net,v2] net: airoha: Move entries to queue head in case of DMA mapping failure in airoha_dev_xmit()
https://git.kernel.org/netdev/net/c/75df490c9e84
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH iproute2-next 5/5] netshaper: Add group command for creating scheduling hierarchies
From: Mohsin Bashir @ 2026-05-01 1:16 UTC (permalink / raw)
To: netdev; +Cc: dsahern, stephen, pabeni, kuba, ernis, mohsin.bashr
In-Reply-To: <20260501011611.3533573-1-mohsin.bashr@gmail.com>
Add the group command to create and update scheduling groups via the
NET_SHAPER_CMD_GROUP netlink operation. This enables building shaper
hierarchies by specifying a node handle, parent scope, rate parameters,
and a set of leaf shapers (queues or nodes) to attach.
Example usage:
netshaper group dev eth0 handle scope node parent scope netdev \
bw-max 1gbit leaves scope queue id 0 scope queue id 1
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
netshaper/netshaper.c | 206 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 205 insertions(+), 1 deletion(-)
diff --git a/netshaper/netshaper.c b/netshaper/netshaper.c
index fc68e735..7ec8220c 100644
--- a/netshaper/netshaper.c
+++ b/netshaper/netshaper.c
@@ -31,9 +31,15 @@ static void usage(void)
fprintf(stderr,
"Usage: netshaper [ OPTIONS ] { COMMAND | help }\n"
"OPTIONS := { -V[ersion] | -c[olor] | -help }\n"
- "COMMAND := { set | get | delete } dev DEVNAME\n"
+ "COMMAND := { set | get | delete | group } dev DEVNAME\n"
" handle scope HANDLE_SCOPE [id HANDLE_ID]\n"
" [bw-max BW_MAX] [bw-min BW_MIN] [weight WEIGHT]\n"
+ "\n"
+ "netshaper group dev DEVNAME handle scope SCOPE [ id ID ]\n"
+ " parent scope SCOPE [ id ID ]\n"
+ " [ bw-max BW ] [ bw-min BW ] [ weight WEIGHT ]\n"
+ " leaves { scope SCOPE id ID } [ ... ]\n"
+ "\n"
"Where: DEVNAME := STRING\n"
" HANDLE_SCOPE := { netdev | queue | node }\n"
" HANDLE_ID := UINT (required for queue/node, optional for netdev)\n"
@@ -264,6 +270,202 @@ static int do_cmd(int argc, char **argv, int cmd)
return err;
}
+static int do_group(int argc, char **argv)
+{
+ GENL_REQUEST(req, 4096, genl_family, 0, NET_SHAPER_FAMILY_VERSION,
+ NET_SHAPER_CMD_GROUP, NLM_F_REQUEST | NLM_F_ACK);
+
+ bool has_bw_max = false, has_bw_min = false, has_weight = false;
+ bool parsing_leaves = false, has_hid = false, has_pid = false;
+ int parent_scope = -1, ifindex = -1, num_leaves = 0;
+ int err, handle_scope = NET_SHAPER_SCOPE_UNSPEC;
+ __u32 handle_id = 0, parent_id = 0, weight = 0;
+ __u64 bw_max_bps = 0, bw_min_bps = 0;
+ struct nlmsghdr *answer;
+
+ struct {
+ int scope;
+ __u32 id;
+ } leaves[128];
+
+ while (argc > 0) {
+ if (parsing_leaves) {
+ if (strcmp(*argv, "scope") == 0) {
+ int lscope;
+
+ NEXT_ARG();
+ lscope = parse_scope(*argv);
+ if (lscope < 0) {
+ fprintf(stderr, "Invalid leaf scope \"%s\"\n",
+ *argv);
+ return -1;
+ }
+
+ NEXT_ARG();
+ if (strcmp(*argv, "id") != 0) {
+ fprintf(stderr, "Expected \"id\" after leaf scope\n");
+ return -1;
+ }
+
+ NEXT_ARG();
+ leaves[num_leaves].scope = lscope;
+ if (get_unsigned(&leaves[num_leaves].id, *argv, 10)) {
+ fprintf(stderr, "Invalid leaf id\n");
+ return -1;
+ }
+ num_leaves++;
+ argc--;
+ argv++;
+ continue;
+ }
+ parsing_leaves = false;
+ }
+
+ if (strcmp(*argv, "dev") == 0) {
+ NEXT_ARG();
+ ifindex = ll_name_to_index(*argv);
+ if (ifindex == 0) {
+ fprintf(stderr, "Device \"%s\" not found\n", *argv);
+ return -1;
+ }
+ } else if (strcmp(*argv, "bw-max") == 0) {
+ NEXT_ARG();
+ if (parse_rate(*argv, &bw_max_bps))
+ return -1;
+ has_bw_max = true;
+ } else if (strcmp(*argv, "bw-min") == 0) {
+ NEXT_ARG();
+ if (parse_rate(*argv, &bw_min_bps))
+ return -1;
+ has_bw_min = true;
+ } else if (strcmp(*argv, "weight") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&weight, *argv, 10)) {
+ fprintf(stderr, "Invalid weight value\n");
+ return -1;
+ }
+ has_weight = true;
+ } else if (strcmp(*argv, "handle") == 0) {
+ NEXT_ARG();
+ if (strcmp(*argv, "scope") != 0) {
+ fprintf(stderr, "Expected \"scope\" after \"handle\"\n");
+ return -1;
+ }
+ NEXT_ARG();
+ handle_scope = parse_scope(*argv);
+ if (handle_scope < 0) {
+ fprintf(stderr, "Invalid handle scope \"%s\"\n",
+ *argv);
+ return -1;
+ }
+
+ if (argc > 1 && strcmp(argv[1], "id") == 0) {
+ NEXT_ARG();
+ NEXT_ARG();
+ if (get_unsigned(&handle_id, *argv, 10)) {
+ fprintf(stderr, "Invalid handle id\n");
+ return -1;
+ }
+ has_hid = true;
+ }
+ } else if (strcmp(*argv, "parent") == 0) {
+ NEXT_ARG();
+ if (strcmp(*argv, "scope") != 0) {
+ fprintf(stderr, "Expected \"scope\" after \"parent\"\n");
+ return -1;
+ }
+ NEXT_ARG();
+ parent_scope = parse_scope(*argv);
+ if (parent_scope < 0) {
+ fprintf(stderr, "Invalid parent scope \"%s\"\n",
+ *argv);
+ return -1;
+ }
+
+ if (argc > 1 && strcmp(argv[1], "id") == 0) {
+ NEXT_ARG();
+ NEXT_ARG();
+ if (get_unsigned(&parent_id, *argv, 10)) {
+ fprintf(stderr, "Invalid parent id\n");
+ return -1;
+ }
+ has_pid = true;
+ }
+ } else if (strcmp(*argv, "leaves") == 0) {
+ parsing_leaves = true;
+ argc--;
+ argv++;
+ continue;
+ } else {
+ fprintf(stderr, "What is \"%s\"\n", *argv);
+ usage();
+ return -1;
+ }
+ argc--;
+ argv++;
+ }
+
+ if (ifindex == -1)
+ missarg("dev");
+ if (handle_scope == NET_SHAPER_SCOPE_UNSPEC)
+ missarg("handle");
+ if (parent_scope < 0)
+ missarg("parent");
+ if (num_leaves == 0)
+ missarg("leaves");
+
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_IFINDEX, ifindex);
+
+ struct rtattr *parent = addattr_nest(&req.n, sizeof(req),
+ NET_SHAPER_A_PARENT | NLA_F_NESTED);
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_SCOPE, parent_scope);
+ if (has_pid)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID, parent_id);
+ addattr_nest_end(&req.n, parent);
+
+ struct rtattr *handle = addattr_nest(&req.n, sizeof(req),
+ NET_SHAPER_A_HANDLE | NLA_F_NESTED);
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_SCOPE, handle_scope);
+ if (has_hid)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID, handle_id);
+ addattr_nest_end(&req.n, handle);
+
+ if (has_bw_max)
+ addattr64(&req.n, sizeof(req), NET_SHAPER_A_BW_MAX, bw_max_bps);
+ if (has_bw_min)
+ addattr64(&req.n, sizeof(req), NET_SHAPER_A_BW_MIN, bw_min_bps);
+ if (has_weight)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_WEIGHT, weight);
+
+ if (has_bw_max || has_bw_min)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_METRIC,
+ NET_SHAPER_METRIC_BPS);
+
+ for (int i = 0; i < num_leaves; i++) {
+ struct rtattr *leaf, *leaf_handle;
+
+ leaf = addattr_nest(&req.n, sizeof(req),
+ NET_SHAPER_A_LEAVES | NLA_F_NESTED);
+ leaf_handle = addattr_nest(&req.n, sizeof(req),
+ NET_SHAPER_A_HANDLE | NLA_F_NESTED);
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_SCOPE,
+ leaves[i].scope);
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID,
+ leaves[i].id);
+ addattr_nest_end(&req.n, leaf_handle);
+ addattr_nest_end(&req.n, leaf);
+ }
+
+ err = rtnl_talk(&gen_rth, &req.n, &answer);
+ if (err < 0) {
+ fprintf(stderr, "Kernel command failed: %d\n", err);
+ return err;
+ }
+
+ print_netshaper_attrs(answer);
+ return 0;
+}
+
int main(int argc, char **argv)
{
int color = default_color_opt();
@@ -308,6 +510,8 @@ int main(int argc, char **argv)
return do_cmd(argc - 1, argv + 1, NET_SHAPER_CMD_DELETE);
if (strcmp(*argv, "show") == 0)
return do_cmd(argc - 1, argv + 1, NET_SHAPER_CMD_GET);
+ if (strcmp(*argv, "group") == 0)
+ return do_group(argc - 1, argv + 1);
if (strcmp(*argv, "help") == 0) {
usage();
return 0;
--
2.52.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).