* [PATCH net v4 1/5] net: mana: Init link_change_work before potential error paths in probe
From: Erni Sri Satya Vennela @ 2026-04-20 12:47 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, ernis, ssengar, dipayanroy, gargaditya,
shirazsaleem, kees, kotaranov, leon, shacharr, stephen,
linux-hyperv, netdev, linux-kernel
In-Reply-To: <20260420124741.1056179-1-ernis@linux.microsoft.com>
Move INIT_WORK(link_change_work) to right after the mana_context
allocation, before any error path that could reach mana_remove().
Previously, if mana_create_eq() or mana_query_device_cfg() failed,
mana_probe() would jump to the error path which calls mana_remove().
mana_remove() unconditionally calls disable_work_sync(link_change_work),
but the work struct had not been initialized yet. This can trigger
CONFIG_DEBUG_OBJECTS_WORK enabled.
Fixes: 54133f9b4b53 ("net: mana: Support HW link state events")
Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
---
Changes in v3,v4:
* No change.
Changes in v2:
* Apply the patch in net instead of net-next.
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 6302432b9bf6..e3e4b6de6668 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -3631,6 +3631,8 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
ac->gdma_dev = gd;
gd->driver_data = ac;
+
+ INIT_WORK(&ac->link_change_work, mana_link_state_handle);
}
err = mana_create_eq(ac);
@@ -3648,8 +3650,6 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
if (!resuming) {
ac->num_ports = num_ports;
-
- INIT_WORK(&ac->link_change_work, mana_link_state_handle);
} else {
if (ac->num_ports != num_ports) {
dev_err(dev, "The number of vPorts changed: %d->%d\n",
--
2.34.1
^ permalink raw reply related
* [PATCH net v4 0/5] net: mana: Fix probe/remove error path bugs
From: Erni Sri Satya Vennela @ 2026-04-20 12:47 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, ernis, ssengar, dipayanroy, gargaditya,
shirazsaleem, kees, kotaranov, leon, shacharr, stephen,
linux-hyperv, netdev, linux-kernel
Fix five bugs in mana_probe()/mana_remove() error handling that can
cause warnings on uninitialized work structs, NULL pointer dereferences,
masked errors, and resource leaks when early probe steps fail.
Patches 1-2 move work struct initialization (link_change_work and
gf_stats_work) to before any error path that could trigger
mana_remove(), preventing WARN_ON in __flush_work() or debug object
warnings when sync cancellation runs on uninitialized work structs.
Patch 3 guards mana_remove() against double invocation. If PM resume
fails, mana_probe() calls mana_remove() which sets gdma_context and
driver_data to NULL. A failed resume does not unbind the driver, so
when the device is eventually unbound, mana_remove() is called again
and dereferences NULL, causing a kernel panic. An early return on
NULL gdma_context or driver_data makes the second call harmless.
Patch 4 prevents add_adev() from overwriting a port probe error,
which could leave the driver in a broken state with NULL ports while
reporting success.
Patch 5 changes 'goto out' to 'break' in mana_remove()'s port loop
so that mana_destroy_eq() is always reached, preventing EQ leaks when
a NULL port is encountered.
---
Changes in v4:
* Correct Fixes tag from ca9c54d2d6a5 to 635096a86edb
* Correct Fixes tag from ced82fce77e9 to a69839d4327d
Changes in v3:
* Add patch 3: net: mana: Guard mana_remove against double invocation.
* Fix inaccurate comments.
* Correct Fixes tag from ca9c54d2d6a5 to 1e2d0824a9c3.
Changes in v2:
* Apply the patchset in net instead of net-next.
---
Erni Sri Satya Vennela (5):
net: mana: Init link_change_work before potential error paths in probe
net: mana: Init gf_stats_work before potential error paths in probe
net: mana: Guard mana_remove against double invocation
net: mana: Don't overwrite port probe error with add_adev result
net: mana: Fix EQ leak in mana_remove on NULL port
drivers/net/ethernet/microsoft/mana/mana_en.c | 35 +++++++++++--------
1 file changed, 20 insertions(+), 15 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: Re: [PATCH] net: hamachi: fix divide by zero in hamachi_init_one
From: Andrew Lunn @ 2026-04-20 12:46 UTC (permalink / raw)
To: 王明煜
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, tglx, mingo, netdev,
linux-kernel
In-Reply-To: <2543c18f.5194.19da8c3b8d1.Coremail.25181214217@stu.xidian.edu.cn>
So what is your security model here? How did i get the hacked card
into the machine? If i have physical access, isn't it already too
late?
As far as i can see, this devices does not load firmware. Even if it
did load firmware from /lib/firmware, i need root to put a hacked
version there.
The PacketEngine Hamachi was in use around year 2000. It needs a PCI
slot. Not PCIe but PCI. Do they even exist any more? Both the card and
a machine with a PCI slot?
Maybe, rather than fix this /0, you should look around and see if you
can find any indication this hardware is still in use, and if not,
submit a patch removing the whole driver?
> +++ b/drivers/net/ethernet/packetengines/hamachi.c
> @@ -745,6 +745,14 @@ static int hamachi_init_one(struct pci_dev *pdev,
> dev->name, chip_tbl[chip_id].name, readl(ioaddr + ChipRev),
> ioaddr, dev->dev_addr, irq);
> i = readb(ioaddr + PCIClkMeas);
> +
> + if ((i & 0x80) && !(i & 0x7f)) {
> + dev_err(&pdev->dev, "Invalid PCIClkMeas value (0x%02x), hardware untrusted.\n", i);
> + unregister_netdev(dev);
> + ret = -EIO;
> + goto err_out_unmap_rx;
> + }
> +
> printk(KERN_INFO "%s: %d-bit %d Mhz PCI bus (%d), Virtual Jumpers "
> "%2.2x, LPA %4.4x.\n",
> dev->name, readw(ioaddr + MiscStatus) & 1 ? 64 : 32,
If you do decide it is worth keeping the driver, please add another
label at the end of the function, and do the unregister_netdev there.
Could i also suggest you consider more likely scenarios. It is much
easier to produce a hacked USB dongle than a PCI card. Users plug in
USB dongles without thinking, where as few users plug in a PCI
card. You are more likely to fix security problems which affect real
systems if you look at USB devices. I would also suggest that / 0 is
not that important. But can you trigger a buffer overrun?
Andrew
---
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net v3 3/5] net: mana: Guard mana_remove against double invocation
From: Erni Sri Satya Vennela @ 2026-04-20 12:45 UTC (permalink / raw)
To: Simon Horman
Cc: haiyangz, davem, longli, ssengar, linux-kernel, kuba, kotaranov,
stephen, andrew+netdev, kys, wei.liu, shirazsaleem, kees, pabeni,
linux-hyperv, edumazet, decui, leon, gargaditya, dipayanroy,
netdev, shacharr
In-Reply-To: <20260417140953.218832-1-horms@kernel.org>
On Fri, Apr 17, 2026 at 03:09:53PM +0100, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: mana: Guard mana_remove against double invocation
>
> If PM resume fails (e.g., mana_attach() returns an error), mana_probe()
> calls mana_remove(), which tears down the device and sets
> gd->gdma_context = NULL and gd->driver_data = NULL. However, a failed
> resume callback does not automatically unbind the driver. When the device
> is eventually unbound, mana_remove() is invoked a second time. Without a
> NULL check, it dereferences gc->dev with gc == NULL, causing a kernel
> panic. Add an early return if gdma_context or driver_data is NULL so the
> second invocation is harmless.
>
> > Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
>
> The Fixes: tag points to ca9c54d2d6a5, which introduced mana_remove()
> without NULL checks. However, the exploitable double invocation scenario
> was actually introduced by commit 635096a86edb ("net: mana: Support
> hibernation and kexec"), which added suspend/resume support and made
> mana_probe() call mana_remove() on error.
>
> Should the Fixes: tag be:
>
> Fixes: 635096a86edb ("net: mana: Support hibernation and kexec")
>
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > index 9d095a6fb56c..2ab7c89e2fed 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > @@ -3685,11 +3685,16 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
> > struct gdma_context *gc = gd->gdma_context;
> > struct mana_context *ac = gd->driver_data;
> > struct mana_port_context *apc;
> > - struct device *dev = gc->dev;
> > + struct device *dev;
> > struct net_device *ndev;
> > int err;
> > int i;
> >
> > + if (!gc || !ac)
> > + return;
> > +
> > + dev = gc->dev;
> > +
> > disable_work_sync(&ac->link_change_work);
> > cancel_delayed_work_sync(&ac->gf_stats_work);
Thankyou for the correction, Simon.
I'll make this change in the next version.
- Vennela
^ permalink raw reply
* Re: [PATCH net v3 4/5] net: mana: Don't overwrite port probe error with add_adev result
From: Erni Sri Satya Vennela @ 2026-04-20 12:45 UTC (permalink / raw)
To: Simon Horman
Cc: haiyangz, davem, longli, ssengar, linux-kernel, kuba, kotaranov,
stephen, andrew+netdev, kys, wei.liu, shirazsaleem, kees, pabeni,
linux-hyperv, edumazet, decui, leon, gargaditya, dipayanroy,
netdev, shacharr
In-Reply-To: <20260417141014.218936-1-horms@kernel.org>
On Fri, Apr 17, 2026 at 03:10:14PM +0100, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: mana: Don't overwrite port probe error with add_adev result
>
> In mana_probe(), if mana_probe_port() fails for any port, the error
> is stored in 'err' and the loop breaks. However, the subsequent
> unconditional 'err = add_adev(gd, "eth")' overwrites this error.
> If add_adev() succeeds, mana_probe() returns success despite ports
> being left in a partially initialized state (ac->ports[i] == NULL).
> Only call add_adev() when there is no prior error, so the probe
> correctly fails and triggers mana_remove() cleanup.
>
> > In mana_probe(), if mana_probe_port() fails for any port, the error
> > is stored in 'err' and the loop breaks. However, the subsequent
> > unconditional 'err = add_adev(gd, "eth")' overwrites this error.
> > If add_adev() succeeds, mana_probe() returns success despite ports
> > being left in a partially initialized state (ac->ports[i] == NULL).
> >
> > Only call add_adev() when there is no prior error, so the probe
> > correctly fails and triggers mana_remove() cleanup.
> >
> > Fixes: ced82fce77e9 ("net: mana: Probe rdma device in mana driver")
>
> The existing Fixes: tag points to commit ced82fce77e9 ("net: mana: Probe
> rdma device in mana driver"), but was the bug actually introduced there?
>
> Looking at the commit history, commit a69839d4327d ("net: mana: Add support
> for auxiliary device") added the unconditional 'err = add_adev(gd);' call
> that overwrites the error from mana_probe_port(). Commit ced82fce77e9 only
> modified the add_adev signature from add_adev(gd) to add_adev(gd, "eth")
> but did not introduce the buggy pattern.
>
> Should the Fixes: tag be:
> Fixes: a69839d4327d ("net: mana: Add support for auxiliary device")
Thankyou for the correction, Simon.
I'll make this change in the next version.
- Vennela
^ permalink raw reply
* Re: [PATCH net v1] net: validate skb->napi_id in RX tracepoints
From: Kohei Enju @ 2026-04-20 11:54 UTC (permalink / raw)
To: Jiayuan Chen
Cc: netdev, linux-trace-kernel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <b943ec1e-417c-4157-ab19-b34aa6d63688@linux.dev>
On 04/20 19:27, Jiayuan Chen wrote:
>
> On 4/20/26 6:54 PM, Kohei Enju wrote:
> > Since commit 2bd82484bb4c ("xps: fix xps for stacked devices"),
> > skb->napi_id shares storage with sender_cpu. RX tracepoints using
> > net_dev_rx_verbose_template read skb->napi_id directly and can therefore
> > report sender_cpu values as if they were NAPI IDs.
> >
> > For example, on the loopback path this can report 1 as napi_id, where 1
> So I think veth_forward_skb->__netif_rx could be affected as well?
Yes. Just in case, I've confirmed the same behavior in the veth path.
The mentioned loopback path is just a single example of possibly
affected paths.
Thanks,
Kohei
> > comes from raw_smp_processor_id() + 1 in the XPS path:
> >
> > # bpftrace -e 'tracepoint:net:netif_rx_entry{ print(args->napi_id); }'
> > # taskset -c 0 ping -c 1 ::1
> >
> > Report only valid NAPI IDs in these tracepoints and use 0 otherwise.
> >
> > Fixes: 2bd82484bb4c ("xps: fix xps for stacked devices")
> > Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> > ---
> > include/trace/events/net.h | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> > index fdd9ad474ce3..dbc2c5598e35 100644
> > --- a/include/trace/events/net.h
> > +++ b/include/trace/events/net.h
> > @@ -10,6 +10,7 @@
> > #include <linux/if_vlan.h>
> > #include <linux/ip.h>
> > #include <linux/tracepoint.h>
> > +#include <net/busy_poll.h>
> > TRACE_EVENT(net_dev_start_xmit,
> > @@ -208,7 +209,8 @@ DECLARE_EVENT_CLASS(net_dev_rx_verbose_template,
> > TP_fast_assign(
> > __assign_str(name);
> > #ifdef CONFIG_NET_RX_BUSY_POLL
> > - __entry->napi_id = skb->napi_id;
> > + __entry->napi_id = napi_id_valid(skb->napi_id) ?
> > + skb->napi_id : 0;
> > #else
> > __entry->napi_id = 0;
> > #endif
^ permalink raw reply
* [PATCH] docs: maintainer-netdev: fix typo in "targeting"
From: Ariful Islam Shoikot @ 2026-04-20 11:45 UTC (permalink / raw)
To: netdev; +Cc: linux-doc, workflows, linux-kernel, Ariful Islam Shoikot
Fix spelling mistake "targgeting" -> "targeting" in
maintainer-netdev.rst
No functional change.
Signed-off-by: Ariful Islam Shoikot <islamarifulshoikat@gmail.com>
---
Documentation/process/maintainer-netdev.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
index bda93b459a05..ec7b9aa2877f 100644
--- a/Documentation/process/maintainer-netdev.rst
+++ b/Documentation/process/maintainer-netdev.rst
@@ -528,7 +528,7 @@ The exact rules a driver must follow to acquire the ``Supported`` status:
status will be withdrawn.
5. Test failures due to bugs either in the driver or the test itself,
- or lack of support for the feature the test is targgeting are
+ or lack of support for the feature the test is targeting are
*not* a basis for losing the ``Supported`` status.
netdev CI will maintain an official page of supported devices, listing their
--
2.43.0
^ permalink raw reply related
* Question/proposal for DPLL NCO (DCO) mode
From: Ivan Vecera @ 2026-04-20 11:42 UTC (permalink / raw)
To: netdev@vger.kernel.org, Jiri Pirko, Arkadiusz Kubalewski,
Vadim Fedorenko, Jakub Kicinski
Hi all,
I am currently working on adding PTP clock support (PHC) to the ZL3073x
driver, and I would like to discuss an architectural addition to the
DPLL subsystem enum dpll_mode before sending the formal patch series.
To support IEEE 1588 PTP, the hardware DPLL must be decoupled from
tracking physical reference pins. Instead, its output frequency needs to
be continuously steered by a software PTP stack (e.g., linuxptp) using
the .adjfine() callback. In the industry, this specific hardware state
is widely known as NCO (Numerically Controlled Oscillator) or DCO
(Digitally Controlled Oscillator) mode.
Currently, the DPLL subsystem defines two modes in enum dpll_mode:
MANUAL: The user explicitly selects a physical input pin to track.
AUTOMATIC: The hardware automatically selects the best physical input
pin based on priorities.
Neither of these accurately represents the NCO/DCO state, where physical
pins are ignored, and the loop is fully software-steered.
Without a dedicated mode, drivers are forced into "automagic"
workarounds. A driver might implicitly switch the hardware to NCO mode
in the background as soon as .adjfine() is called, while misreporting
its mode as MANUAL to the DPLL subsystem.
I strongly believe this hidden "automagic" behavior is a bad design. The
user-space should have full visibility and explicit control over what
the DPLL state machine is actually doing.
The Proposal:
I would like to propose adding a new mode: DPLL_MODE_NCO (or
DPLL_MODE_DCO, I am open to suggestions regarding the terminology).
The expected workflow would be explicit:
If a user wants to actively steer the DPLL frequency via software (for
PTP or other Time-over-Packet applications), they must explicitly set
the DPLL device to this mode via Netlink first. Only then will the
hardware accept frequency adjustments via .adjfine().
I would appreciate any feedback from the DPLL subsystem maintainers and
the community on this approach.
Thanks,
Ivan
^ permalink raw reply
* Re: [PATCH bpf-next v4 3/6] bpf: add bpf_icmp_send_unreach kfunc
From: bot+bpf-ci @ 2026-04-20 11:36 UTC (permalink / raw)
To: mahe.tardy, mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260420105816.72168-4-mahe.tardy@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3581 bytes --]
> diff --git a/net/core/filter.c b/net/core/filter.c
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
[ ... ]
> @@ -12423,6 +12427,86 @@
> return 0;
> }
>
> +static DEFINE_PER_CPU(bool, bpf_icmp_send_in_progress);
> +
> +/**
> + * bpf_icmp_send_unreach - Send ICMP destination unreachable error
> + * @skb: Packet that triggered the error
> + * @code: ICMP unreachable code (0-15 for IPv4, 0-6 for IPv6)
> + *
> + * Sends an ICMP destination unreachable message in response to the
> + * packet. The original packet is cloned before sending the ICMP error,
> + * so the BPF program can still let the packet pass if desired.
> + *
> + * Recursion protection: If called from a context that would trigger
> + * recursion (e.g., root cgroup processing its own ICMP packets),
> + * returns -EBUSY on re-entry.
> + *
> + * Return: 0 on success, negative error code on failure:
> + * -EINVAL: Invalid code parameter
> + * -ENOMEM: Memory allocation failed
> + * -EHOSTUNREACH: Routing lookup failed
> + * -EBUSY: Recursion detected
> + * -EPROTONOSUPPORT: Non-IP protocol
> + */
> +__bpf_kfunc int bpf_icmp_send_unreach(struct __sk_buff *__skb, int code)
> +{
> + struct sk_buff *skb = (struct sk_buff *)__skb;
> + struct sk_buff *nskb;
> + bool *in_progress;
> +
> + in_progress = this_cpu_ptr(&bpf_icmp_send_in_progress);
> + if (*in_progress)
> + return -EBUSY;
> +
> + switch (skb->protocol) {
> +#if IS_ENABLED(CONFIG_INET)
> + case htons(ETH_P_IP):
> + if (code < 0 || code > NR_ICMP_UNREACH)
> + return -EINVAL;
> +
> + nskb = skb_clone(skb, GFP_ATOMIC);
> + if (!nskb)
> + return -ENOMEM;
> +
> + if (!skb_dst(nskb) && ip_route_reply_fetch_dst(nskb) < 0) {
> + kfree_skb(nskb);
> + return -EHOSTUNREACH;
> + }
> +
> + *in_progress = true;
> + icmp_send(nskb, ICMP_DEST_UNREACH, code, 0);
> + *in_progress = false;
> + kfree_skb(nskb);
> + break;
> +#endif
> +#if IS_ENABLED(CONFIG_IPV6)
> + case htons(ETH_P_IPV6):
> + if (code < 0 || code > ICMPV6_REJECT_ROUTE)
> + return -EINVAL;
> +
> + nskb = skb_clone(skb, GFP_ATOMIC);
> + if (!nskb)
> + return -ENOMEM;
> +
> + if (!skb_dst(nskb) && ip6_route_reply_fetch_dst(nskb) < 0) {
> + kfree_skb(nskb);
> + return -EHOSTUNREACH;
> + }
Is the route lookup needed here? Looking at icmpv6_send() in
net/ipv6/icmp.c, it fills fl6.{daddr,saddr} and passes them to
icmpv6_route_lookup() which performs its own route lookup. The function
doesn't appear to use skb_dst() from the original skb:
icmpv6_send() {
...
fl6.daddr = hdr->saddr;
fl6.saddr = saddr;
...
dst = icmpv6_route_lookup(net, skb, sk, &fl6);
...
}
The same applies to the IPv4 path with icmp_send(). Additionally, at
cgroup/egress the skb->_skb_refdst should already be set, and the same
should be true for cgroup/ingress for inet proto.
Does the kfunc work without ip_route_reply_fetch_dst() and
ip6_route_reply_fetch_dst()? If so, can these calls be removed?
Reference: Martin KaFai Lau's comment in v3 review
https://lore.kernel.org/bpf/df4b0996-3e88-4ea4-983b-82866455a6fc@linux.dev/
> +
> + *in_progress = true;
> + icmpv6_send(nskb, ICMPV6_DEST_UNREACH, code, 0);
> + *in_progress = false;
> + kfree_skb(nskb);
> + break;
> +#endif
> + default:
> + return -EPROTONOSUPPORT;
> + }
> +
> + return 0;
> +}
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24663313503
^ permalink raw reply
* Re: [PATCH bpf-next v4 4/6] selftests/bpf: add icmp_send_unreach kfunc tests
From: bot+bpf-ci @ 2026-04-20 11:36 UTC (permalink / raw)
To: mahe.tardy, mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260420105816.72168-5-mahe.tardy@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]
> diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
> new file mode 100644
> index 000000000000..24d5e01cfe80
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <network_helpers.h>
> +#include <linux/errqueue.h>
> +#include "icmp_send_unreach.skel.h"
> +
> +#define TIMEOUT_MS 1000
> +#define SRV_PORT 54321
> +
> +#define ICMP_DEST_UNREACH 3
> +
> +#define ICMP_FRAG_NEEDED 4
> +#define NR_ICMP_UNREACH 15
Jordan Rife previously suggested in v3 that these ICMP constants should be
imported from <linux/icmp.h> rather than redefined here. Would it be cleaner
to include the header and avoid the redefinitions?
Reference: https://lore.kernel.org/bpf/usz5bhydsiejr37owgt3zypckzh7fa7ygmhsyaaiprsljx7iy5@ipopnr5n4ds7/
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24663313503
^ permalink raw reply
* Re: [PATCH bpf-next v4 1/6] net: move netfilter nf_reject_fill_skb_dst to core ipv4
From: bot+bpf-ci @ 2026-04-20 11:36 UTC (permalink / raw)
To: mahe.tardy, mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260420105816.72168-2-mahe.tardy@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2271 bytes --]
> diff --git a/include/net/route.h b/include/net/route.h
> index f90106f383c5..ec2466fd0bec 100644
> --- a/include/net/route.h
> +++ b/include/net/route.h
> @@ -173,6 +173,7 @@ struct rtable *ip_route_output_flow(struct net *, struct flowi4 *flp,
> const struct sock *sk);
> struct dst_entry *ipv4_blackhole_route(struct net *net,
> struct dst_entry *dst_orig);
> +int ip_route_reply_fetch_dst(struct sk_buff *skb);
>
> static inline struct rtable *ip_route_output_key(struct net *net, struct flowi4 *flp)
> {
[ ... ]
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index bc1296f0ea69..7091ef936073 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2945,6 +2945,21 @@ struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
> }
> EXPORT_SYMBOL_GPL(ip_route_output_flow);
>
> +int ip_route_reply_fetch_dst(struct sk_buff *skb)
> +{
> + struct rtable *rt;
> + struct flowi4 fl4 = {
> + .daddr = ip_hdr(skb)->saddr
> + };
> +
> + rt = ip_route_output_key(dev_net(skb->dev), &fl4);
> + if (IS_ERR(rt))
> + return PTR_ERR(rt);
> + skb_dst_set(skb, &rt->dst);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ip_route_reply_fetch_dst);
Is ip_route_reply_fetch_dst() actually needed before calling icmp_send()?
Martin KaFai Lau raised this question in v3 of the patch series and
noted that icmpv6_send() does its own route lookup and doesn't use
skb_dst() of the original skb. He also pointed out that at
cgroup/egress, skb->_skb_refdst should already be set.
Looking at icmpv6_send(), the fl6.{daddr,saddr} are filled and passed
to icmpv6_route_lookup, which performs the reverse/invert route lookup.
The function doesn't appear to use skb_dst() of the original skb.
Since this patch moves ip_route_reply_fetch_dst() from netfilter to
core specifically for the BPF kfunc, can you clarify why this routing
step is necessary? Does the kfunc not work without
ip[6]_route_reply_fetch_dst()?
Reference: https://lore.kernel.org/bpf/df4b0996-3e88-4ea4-983b-82866455a6fc@linux.dev/
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24663313503
^ permalink raw reply
* Re: [PATCH net v3 1/1] net: l3mdev: Reject non-L3 uppers in slave helpers
From: Ido Schimmel @ 2026-04-20 11:32 UTC (permalink / raw)
To: Ren Wei
Cc: netdev, idosch, dsahern, davem, edumazet, kuba, pabeni, horms,
jiri, yifanwucs, tomapufckgml, yuantan098, bird, royenheart
In-Reply-To: <20260419145332.3988923-1-n05ec@lzu.edu.cn>
On Sun, Apr 19, 2026 at 10:53:32PM +0800, Ren Wei wrote:
> From: Haoze Xie <royenheart@gmail.com>
>
> Several l3mdev slave-side helpers resolve an upper device and then use
> l3mdev_ops without first proving that the resolved device is still a
> valid L3 master.
>
> During slave transition, an RCU reader can transiently observe an upper
> that is not an L3 master. Guard the affected slave-resolved paths by
> requiring the resolved upper to still be an L3 master before using
> l3mdev_ops, while keeping existing L3 RX handler providers intact.
>
> Fixes: fdeea7be88b1 ("net: vrf: Set slave's private flag before linking")
> Cc: stable@kernel.org
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
I think it's fine for net:
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
We can try a more general approach in net-next which ensures that RCU
readers don't see different master devices within an RCU read-side
critical section. Something like [1]. I tested it (without your patch)
using the reproducer and was not able to trigger the bug.
I am aware that there are more places where we can unlink a device from
its master (e.g., deleting a master device), but most / all of them
(needs auditing) result in a synchronize_rcu() before releasing RTNL.
On my machine, the time it takes to unlink 1k devices from their master
increased from 0.96 seconds to 1.22 seconds (on average), which is
probably fine.
[1]
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 69daba3ddaf0..23657a11c66d 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2970,6 +2970,7 @@ static int do_set_master(struct net_device *dev, int ifindex,
netdev_lock_ops(dev);
if (err)
return err;
+ synchronize_net();
} else {
return -EOPNOTSUPP;
}
^ permalink raw reply related
* Re: [PATCH net v1] net: validate skb->napi_id in RX tracepoints
From: Jiayuan Chen @ 2026-04-20 11:27 UTC (permalink / raw)
To: Kohei Enju, netdev, linux-trace-kernel
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20260420105427.162816-1-kohei@enjuk.jp>
On 4/20/26 6:54 PM, Kohei Enju wrote:
> Since commit 2bd82484bb4c ("xps: fix xps for stacked devices"),
> skb->napi_id shares storage with sender_cpu. RX tracepoints using
> net_dev_rx_verbose_template read skb->napi_id directly and can therefore
> report sender_cpu values as if they were NAPI IDs.
>
> For example, on the loopback path this can report 1 as napi_id, where 1
So I think veth_forward_skb->__netif_rx could be affected as well?
> comes from raw_smp_processor_id() + 1 in the XPS path:
>
> # bpftrace -e 'tracepoint:net:netif_rx_entry{ print(args->napi_id); }'
> # taskset -c 0 ping -c 1 ::1
>
> Report only valid NAPI IDs in these tracepoints and use 0 otherwise.
>
> Fixes: 2bd82484bb4c ("xps: fix xps for stacked devices")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> ---
> include/trace/events/net.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> index fdd9ad474ce3..dbc2c5598e35 100644
> --- a/include/trace/events/net.h
> +++ b/include/trace/events/net.h
> @@ -10,6 +10,7 @@
> #include <linux/if_vlan.h>
> #include <linux/ip.h>
> #include <linux/tracepoint.h>
> +#include <net/busy_poll.h>
>
> TRACE_EVENT(net_dev_start_xmit,
>
> @@ -208,7 +209,8 @@ DECLARE_EVENT_CLASS(net_dev_rx_verbose_template,
> TP_fast_assign(
> __assign_str(name);
> #ifdef CONFIG_NET_RX_BUSY_POLL
> - __entry->napi_id = skb->napi_id;
> + __entry->napi_id = napi_id_valid(skb->napi_id) ?
> + skb->napi_id : 0;
> #else
> __entry->napi_id = 0;
> #endif
^ permalink raw reply
* Re: [PATCH 2/9] x86/extable: switch to using FIELD_GET_SIGNED()
From: Peter Zijlstra @ 2026-04-20 11:24 UTC (permalink / raw)
To: Yury Norov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, Ping-Ke Shih, Richard Cochran,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexandre Belloni, Yury Norov, Rasmus Villemoes,
Hans de Goede, Linus Walleij, Sakari Ailus, Salah Triki,
Achim Gratz, Ben Collins, linux-kernel, linux-iio, linux-wireless,
netdev, linux-rtc
In-Reply-To: <20260417173621.368914-3-ynorov@nvidia.com>
On Fri, Apr 17, 2026 at 01:36:13PM -0400, Yury Norov wrote:
> The EX_DATA register is laid out such that EX_DATA_IMM occupied MSB.
> It's done to make sure that FIELD_GET() will sign-extend the IMM
> field during extraction.
>
> To enforce that, all EX_DATA masks are made signed integers. This
> works, but relies on the particular implementation of FIELD_GET(),
> i.e. masking then shifting, not vice versa; and the particular
> placement of the fields in the register.
I don't think the order of the mask and shift matters in this case. If
we were to first shift down and then mask, it would still work (after
all, the mask would also need to be shifted and would also get sign
extended, effectively ending up as -1).
But yes, this very much depends on the signed field being the topmost
field and including the MSB.
^ permalink raw reply
* Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()
From: Peter Zijlstra @ 2026-04-20 11:19 UTC (permalink / raw)
To: Yury Norov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, Ping-Ke Shih, Richard Cochran,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Alexandre Belloni, Yury Norov, Rasmus Villemoes,
Hans de Goede, Linus Walleij, Sakari Ailus, Salah Triki,
Achim Gratz, Ben Collins, linux-kernel, linux-iio, linux-wireless,
netdev, linux-rtc
In-Reply-To: <20260417173621.368914-2-ynorov@nvidia.com>
On Fri, Apr 17, 2026 at 01:36:12PM -0400, Yury Norov wrote:
> The bitfields are designed in assumption that fields contain unsigned
> integer values, thus extracting the values from the field implies
> zero-extending.
>
> Some drivers need to sign-extend their fields, and currently do it like:
>
> dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
>
> It's error-prone because it relies on user to provide the correct
> index of the most significant bit and proper 32 vs 64 function flavor.
>
> Thus, introduce a FIELD_GET_SIGNED() macro, which is the more
> convenient and compiles (on x86_64) to just a couple instructions:
> shl and sar.
>
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
> ---
> include/linux/bitfield.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 54aeeef1f0ec..35ef63972810 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -178,6 +178,22 @@
> __FIELD_GET(_mask, _reg, "FIELD_GET: "); \
> })
>
> +/**
> + * FIELD_GET_SIGNED() - extract a signed bitfield element
> + * @mask: shifted mask defining the field's length and position
> + * @reg: value of entire bitfield
> + *
> + * Returns the sign-extended field specified by @_mask from the
> + * bitfield passed in as @_reg by masking and shifting it down.
> + */
> +#define FIELD_GET_SIGNED(mask, reg) \
> + ({ \
> + __BF_FIELD_CHECK(mask, reg, 0U, "FIELD_GET_SIGNED: "); \
> + ((__signed_scalar_typeof(mask))((long long)(reg) << \
> + __builtin_clzll(mask) >> (__builtin_clzll(mask) + \
> + __builtin_ctzll(mask))));\
> + })
IIRC clz is count-leading-zeros and ctz is count-trailing-zeros. Most of
the other FIELD things use __bf_shf() which is defined in terms of ffs -
1 (which is another way of writing ctz).
So how about you start by redefining __bf_shf() in ctz, and then add
another helper for the clz and write the thing something like:
((long long)(reg) << __bf_clz(mask)) >> (__bf_clz(mask) + __bf_shf(mask));
Also, since the order of the shifts is rather important, I think it
makes sense to add this extra pair of (), even when not strictly needed,
just to make it easier to read.
^ permalink raw reply
* Re: [PATCH v5 5/6] bpf: clear decap tunnel GSO state in skb_adjust_room
From: bot+bpf-ci @ 2026-04-20 11:19 UTC (permalink / raw)
To: nhudson, bpf, netdev, willemb, martin.lau
Cc: nhudson, mtottenh, aglasgal, ast, daniel, andrii, eddyz87, memxor,
davem, edumazet, kuba, pabeni, linux-kernel, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260420104051.1528843-6-nhudson@akamai.com>
[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]
> diff --git a/net/core/filter.c b/net/core/filter.c
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3666,6 +3666,39 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
> if (!(flags & BPF_F_ADJ_ROOM_FIXED_GSO))
> skb_increase_gso_size(shinfo, len_diff);
>
> + /* Selective GSO flag clearing based on decap type.
> + * Only clear the flags for the tunnel layer being removed.
> + */
This isn't a bug, but the opening /* should be on its own line following BPF
subsystem convention for multi-line comments.
> + if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_UDP) &&
> + (shinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
> + SKB_GSO_UDP_TUNNEL_CSUM)))
> + shinfo->gso_type &= ~(SKB_GSO_UDP_TUNNEL |
> + SKB_GSO_UDP_TUNNEL_CSUM);
> + if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_GRE) &&
> + (shinfo->gso_type & (SKB_GSO_GRE | SKB_GSO_GRE_CSUM)))
> + shinfo->gso_type &= ~(SKB_GSO_GRE |
> + SKB_GSO_GRE_CSUM);
> + if ((flags & BPF_F_ADJ_ROOM_DECAP_IPXIP4) &&
> + (shinfo->gso_type & SKB_GSO_IPXIP4))
> + shinfo->gso_type &= ~SKB_GSO_IPXIP4;
> + if ((flags & BPF_F_ADJ_ROOM_DECAP_IPXIP6) &&
> + (shinfo->gso_type & SKB_GSO_IPXIP6))
> + shinfo->gso_type &= ~SKB_GSO_IPXIP6;
> +
> + /* Clear encapsulation flag only when no tunnel GSO flags remain */
> + if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
> + BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) {
> + if (!(shinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
> + SKB_GSO_UDP_TUNNEL_CSUM |
> + SKB_GSO_GRE |
> + SKB_GSO_GRE_CSUM |
> + SKB_GSO_IPXIP4 |
> + SKB_GSO_IPXIP6 |
> + SKB_GSO_ESP)))
> + if (skb->encapsulation)
> + skb->encapsulation = 0;
> + }
All of this new GSO flag clearing logic is added inside the if (skb_is_gso(skb))
block in bpf_skb_net_shrink(). Does this mean non-GSO packets bypass the
decapsulation state clearing?
BPF programs can call bpf_skb_adjust_room() to decapsulate non-GSO packets
as well. If a non-GSO packet undergoes decapsulation with the tunnel headers
stripped, but skb->encapsulation remains set, could this cause downstream
network stack components to misinterpret the packet as still being
encapsulated?
> +
> /* Header must be checked, and gso_segs recomputed. */
> shinfo->gso_type |= SKB_GSO_DODGY;
> shinfo->gso_segs = 0;
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24662733093
^ permalink raw reply
* Re: [PATCH v5 6/6] selftests/bpf: tc_tunnel validate decap GSO state
From: bot+bpf-ci @ 2026-04-20 11:19 UTC (permalink / raw)
To: nhudson, bpf, netdev, willemb, martin.lau
Cc: nhudson, ast, daniel, andrii, eddyz87, memxor, shuah,
linux-kselftest, linux-kernel, ast, andrii, daniel, martin.lau,
eddyz87, yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260420104051.1528843-7-nhudson@akamai.com>
[-- Attachment #1: Type: text/plain, Size: 3501 bytes --]
> diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> index 7376df405a6b..511022630bb1 100644
> --- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> +++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
[ ... ]
> @@ -621,6 +640,11 @@ static int decap_internal(struct __sk_buff *skb, int off, int len, char proto)
> break;
> case IPPROTO_GRE:
> olen += sizeof(struct gre_hdr);
> + if (!bpf_core_enum_value_exists(enum bpf_adj_room_flags,
> + BPF_F_ADJ_ROOM_DECAP_L4_GRE))
> + return TC_ACT_SHOT;
> + flags |= BPF_F_ADJ_ROOM_DECAP_L4_GRE;
> +
[ ... ]
> @@ -634,6 +658,10 @@ static int decap_internal(struct __sk_buff *skb, int off, int len, char proto)
> break;
> case IPPROTO_UDP:
> olen += sizeof(struct udphdr);
> + if (!bpf_core_enum_value_exists(enum bpf_adj_room_flags,
> + BPF_F_ADJ_ROOM_DECAP_L4_UDP))
> + return TC_ACT_SHOT;
> + flags |= BPF_F_ADJ_ROOM_DECAP_L4_UDP;
[ ... ]
> @@ -655,6 +683,35 @@ static int decap_internal(struct __sk_buff *skb, int off, int len, char proto)
> if (bpf_skb_adjust_room(skb, -olen, BPF_ADJ_ROOM_MAC, flags))
> return TC_ACT_SHOT;
>
> + kskb = bpf_cast_to_kern_ctx(skb);
> + shinfo = bpf_core_cast(kskb->head + kskb->end, struct skb_shared_info);
Could the pointer arithmetic here cause compilation issues on 32-bit
architectures?
On 32-bit architectures without NET_SKBUFF_DATA_USES_OFFSET, sk_buff::end
is defined as a pointer (unsigned char *) rather than an offset. Adding
two pointers is invalid in C and would fail to compile.
Martin KaFai Lau noted that other tests use this pattern and suggested it
could be addressed with bpf_core_field_size(kskb->end) if 32-bit support
becomes necessary in the future.
Ref: https://lore.kernel.org/bpf/20260416124324.B0CE6C2BCAF@smtp.kernel.org/
> + if (!shinfo->gso_size)
> + return TC_ACT_OK;
> +
> + if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_UDP) &&
> + (shinfo->gso_type & SKB_GSO_UDP_TUNNEL_MASK))
> + return TC_ACT_SHOT;
> +
> + if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_GRE) &&
> + (shinfo->gso_type & (SKB_GSO_GRE | SKB_GSO_GRE_CSUM)))
> + return TC_ACT_SHOT;
> +
> + if ((flags & BPF_F_ADJ_ROOM_DECAP_IPXIP4) &&
> + (shinfo->gso_type & SKB_GSO_IPXIP4))
> + return TC_ACT_SHOT;
> +
> + if ((flags & BPF_F_ADJ_ROOM_DECAP_IPXIP6) &&
> + (shinfo->gso_type & SKB_GSO_IPXIP6))
> + return TC_ACT_SHOT;
Are these IPXIP4 and IPXIP6 checks reachable?
Looking earlier in decap_internal(), for IPPROTO_IPIP and IPPROTO_IPV6
cases, the function only sets BPF_F_ADJ_ROOM_DECAP_L3_IPV4 and
BPF_F_ADJ_ROOM_DECAP_L3_IPV6 on the flags variable (around the protocol
switch statement).
Because BPF_F_ADJ_ROOM_DECAP_IPXIP4 and BPF_F_ADJ_ROOM_DECAP_IPXIP6 are
never added to flags, wouldn't these validation checks always be skipped?
Ref: https://lore.kernel.org/bpf/20260416124324.B0CE6C2BCAF@smtp.kernel.org/
> +
> + if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
> + BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) {
> + if ((shinfo->gso_type & SKB_GSO_TUNNEL_MASK) && !kskb->encapsulation)
> + return TC_ACT_SHOT;
> + if (!(shinfo->gso_type & SKB_GSO_TUNNEL_MASK) && kskb->encapsulation)
> + return TC_ACT_SHOT;
> + }
> +
> return TC_ACT_OK;
> }
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24662733093
^ permalink raw reply
* Re: [RFC Patch net-next v1 0/9] r8169: add RSS support for RTL8127
From: FUKAUMI Naoki @ 2026-04-20 11:06 UTC (permalink / raw)
To: javen, hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, horms
Cc: netdev, linux-kernel
In-Reply-To: <20260420021957.1756-1-javen_xu@realsil.com.cn>
Hi Javen,
Thank you very much for your nice work!
On 4/20/26 11:19, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
>
> This series patch adds RSS support for RTL8127 in the r8169 driver.
>
> Currently, without RSS support, a single CPU core handles all incoming
> traffic. Under heavy loads, this single core becomes a bottleneck, causing
> high softirq usage and leading to unstable and degraded network throughput.
>
> As a result, we add rss support for RTL8127. This RFC patch is just for
> discussing. And we do some experiments on AMD platform. Below is the
> result.
>
> Platform: AMD Ryzen Embedded R2514 with Radeon Graphics(4 Cores/8 Threads)
> Arch: x86_64
> Test command:
> Server: iperf3 -s
> Client: iperf3 -c 192.168.2.1 -P 20 -t 3600
> Monitor: mpstat -P ALL 1
>
> Before this patch (Without RSS):
> Throughput: Unstable, fluctuating between 3.76 Gbits/sec and
> 8.2 Gbits/sec.
> CPU Usage: A single CPU core is fully occupied with softirq reaching
> up to 96%.
>
> After this patch (With RSS enabled):
> Throughput: Stable at 9.42 Gbits/sec.
> CPU Usage: The traffic load is evenly distributed across multiple CPU
> cores. The maximum softirq on a single core dropped to 63%.
Platform: Radxa ROCK 5T (RK3588: 4x Cortex-A76, 4x Cortex-A55)
Arch: aarch64
Configuration: smp_affinity is set to use only the big cores.
Vanilla Linux v7.0:
Throughput: 5.5 Gbps (4.3 Gbps with -P 20)
CPU Usage: ~100% on a single A76 core.
Linux v7.0 + this patch series:
Throughput: 9.4 Gbps with -P 20
CPU Usage: distributed across all four A76 cores.
Looks good to me!
Feel free to use:
Tested-by: FUKAUMI Naoki <naoki@radxa.com>
Best regards,
--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.
> Patch summary:
> Patch 1: Adds necessary macro and register definitions for RSS.
> Patch 2-4: Support NAPI and multi RX/TX queues.
> Patch 5-6: Support MSI-X and enables it specifically for RTL8127.
> Patch 7: Enables RSS for RTL8127.
> Patch 8-9: Adds ethtool support to configure the number of RX queues.
>
> Javen Xu (9):
> r8169: add some register definitions
> r8169: add napi and irq support
> r8169: add support for multi tx queues
> r8169: add support for multi rx queues
> r8169: add support for msix
> r8169: enable msix for RTL8127
> r8169: add support and enable rss
> r8169: move struct ethtool_ops
> r8169: add support for ethtool
>
> drivers/net/ethernet/realtek/r8169_main.c | 1437 ++++++++++++++++++---
> 1 file changed, 1238 insertions(+), 199 deletions(-)
^ permalink raw reply
* Re: [PATCH bpf] bpf: Fix NULL pointer dereference in bpf_skb_fib_lookup()
From: Jiayuan Chen @ 2026-04-20 11:01 UTC (permalink / raw)
To: Weiming Shi, Martin KaFai Lau, Daniel Borkmann,
Alexei Starovoitov, Andrii Nakryiko, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: John Fastabend, Stanislav Fomichev, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Hao Luo, Jiri Olsa, Simon Horman,
Jesper Dangaard Brouer, bpf, netdev, Xiang Mei
In-Reply-To: <20260419170131.3899757-2-bestswngs@gmail.com>
On 4/20/26 1:01 AM, Weiming Shi wrote:
> When tot_len is not provided by the user, bpf_skb_fib_lookup()
> resolves the FIB result's output device via dev_get_by_index_rcu()
> to check skb forwardability and fill in mtu_result. The returned
> pointer is dereferenced without a NULL check. If the device is
> concurrently unregistered, dev_get_by_index_rcu() returns NULL and
> is_skb_forwardable() crashes at dev->flags:
>
> KASAN: null-ptr-deref in range
> [0x00000000000000b0-0x00000000000000b7]
> Call Trace:
> is_skb_forwardable (include/linux/netdevice.h:4365)
> bpf_skb_fib_lookup (net/core/filter.c:6446)
> bpf_prog_test_run_skb (net/bpf/test_run.c)
> __sys_bpf (kernel/bpf/syscall.c)
>
> Add the missing NULL check, returning -ENODEV to be consistent
> with how bpf_ipv4_fib_lookup() and bpf_ipv6_fib_lookup() handle
> the same condition.
>
> Fixes: e1850ea9bd9e ("bpf: bpf_fib_lookup return MTU value as output when looked up")
Is it correct to blame this commit?
I find such code block 'if (!is_skb_forwardable(dev, skb))' was
introduced by 4f74fede40df
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> net/core/filter.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 78b548158fb0..3e56b567bd18 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -6450,6 +6450,8 @@ BPF_CALL_4(bpf_skb_fib_lookup, struct sk_buff *, skb,
> * against MTU of FIB lookup resulting net_device
> */
> dev = dev_get_by_index_rcu(net, params->ifindex);
> + if (!dev)
> + return -ENODEV;
> if (!is_skb_forwardable(dev, skb))
> rc = BPF_FIB_LKUP_RET_FRAG_NEEDED;
>
^ permalink raw reply
* [PATCH bpf-next v4 6/6] selftests/bpf: add icmp_send_unreach_recursion test
From: Mahe Tardy @ 2026-04-20 10:58 UTC (permalink / raw)
To: mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo
In-Reply-To: <20260420105816.72168-1-mahe.tardy@gmail.com>
This test is similar to icmp_send_unreach_kfunc but checks that, in case
of recursion, meaning that the BPF program calling the kfunc was
re-triggered by the icmp_send done by the kfunc, the kfunc will stop
early and return -EBUSY.
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
.../bpf/prog_tests/icmp_send_unreach_kfunc.c | 43 +++++++++++++++++++
.../selftests/bpf/progs/icmp_send_unreach.c | 30 +++++++++++++
2 files changed, 73 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
index 047bfd4d80f7..a4f4324b2b99 100644
--- a/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
+++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <test_progs.h>
#include <network_helpers.h>
+#include <cgroup_helpers.h>
#include <linux/errqueue.h>
#include "icmp_send_unreach.skel.h"
@@ -10,6 +11,7 @@
#define ICMP_DEST_UNREACH 3
#define ICMPV6_DEST_UNREACH 1
+#define ICMP_HOST_UNREACH 1
#define ICMP_FRAG_NEEDED 4
#define NR_ICMP_UNREACH 15
#define NR_ICMPV6_UNREACH 6
@@ -157,3 +159,44 @@ void test_icmp_send_unreach_kfunc(void)
icmp_send_unreach__destroy(skel);
close(cgroup_fd);
}
+
+void test_icmp_send_unreach_recursion(void)
+{
+ struct icmp_send_unreach *skel;
+ int cgroup_fd = -1;
+ int *code;
+
+ skel = icmp_send_unreach__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ goto cleanup;
+
+ if (setup_cgroup_environment()) {
+ fprintf(stderr, "Failed to setup cgroup environment\n");
+ goto cleanup;
+ }
+
+ cgroup_fd = get_root_cgroup();
+ if (!ASSERT_GE(cgroup_fd, 0, "get_root_cgroup"))
+ goto cleanup;
+
+ skel->links.recursion =
+ bpf_program__attach_cgroup(skel->progs.recursion, cgroup_fd);
+ if (!ASSERT_OK_PTR(skel->links.recursion, "prog_attach_cgroup"))
+ goto cleanup;
+
+ code = &skel->bss->unreach_code;
+ *code = ICMP_HOST_UNREACH;
+
+ trigger_prog_read_icmp_errqueue(code, AF_INET, "127.0.0.1");
+
+ /* Because there's recursion involved, the first call will return at
+ * index 1 since it will return the second, and the second call will
+ * return at index 0 since it will return the first.
+ */
+ ASSERT_EQ(skel->data->rec_kfunc_rets[1], 0, "kfunc_rets[1]");
+ ASSERT_EQ(skel->data->rec_kfunc_rets[0], -EBUSY, "kfunc_rets[0]");
+
+cleanup:
+ icmp_send_unreach__destroy(skel);
+ close(cgroup_fd);
+}
diff --git a/tools/testing/selftests/bpf/progs/icmp_send_unreach.c b/tools/testing/selftests/bpf/progs/icmp_send_unreach.c
index 112b9cbfab6f..9aca7c0b12e1 100644
--- a/tools/testing/selftests/bpf/progs/icmp_send_unreach.c
+++ b/tools/testing/selftests/bpf/progs/icmp_send_unreach.c
@@ -15,6 +15,9 @@
int unreach_code = 0;
int kfunc_ret = -1;
+uint rec_count = 0;
+int rec_kfunc_rets[] = { -1, -1 };
+
SEC("cgroup_skb/egress")
int egress(struct __sk_buff *skb)
{
@@ -67,4 +70,31 @@ int egress(struct __sk_buff *skb)
return SK_DROP;
}
+SEC("cgroup_skb/egress")
+int recursion(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ void *data_end = (void *)(long)skb->data_end;
+ struct iphdr *iph;
+
+ iph = data;
+ if ((void *)(iph + 1) > data_end || iph->version != 4)
+ return SK_PASS;
+
+ /* This call will provoke a recursion: the ICMP package generated by the
+ * kfunc will re-trigger this program since we are in the root cgroup in
+ * which the kernel ICMP socket belongs. However when re-entering the
+ * kfunc, it should return EBUSY.
+ */
+ rec_kfunc_rets[rec_count & 1] =
+ bpf_icmp_send_unreach(skb, unreach_code);
+ __sync_fetch_and_add(&rec_count, 1);
+
+ /* Let the first ICMP error message pass */
+ if (iph->protocol == IPPROTO_ICMP)
+ return SK_PASS;
+
+ return SK_DROP;
+}
+
char LICENSE[] SEC("license") = "Dual BSD/GPL";
--
2.34.1
^ permalink raw reply related
* [PATCH bpf-next v4 5/6] selftests/bpf: add icmp_send_unreach kfunc IPv6 tests
From: Mahe Tardy @ 2026-04-20 10:58 UTC (permalink / raw)
To: mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo
In-Reply-To: <20260420105816.72168-1-mahe.tardy@gmail.com>
This test extend the existing IPv4 tests to IPv6.
Note that we need to set IP_RECVERR on the socket for IPv6 in
connect_to_fd_nonblock otherwise the error will be ignored even if we
are in the middle of the TCP handshake. See in
net/ipv6/datagram.c:ipv6_icmp_error line 313 for more details.
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
.../bpf/prog_tests/icmp_send_unreach_kfunc.c | 83 ++++++++++++-------
.../selftests/bpf/progs/icmp_send_unreach.c | 46 ++++++++--
2 files changed, 93 insertions(+), 36 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
index 24d5e01cfe80..047bfd4d80f7 100644
--- a/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
+++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
@@ -8,15 +8,17 @@
#define SRV_PORT 54321
#define ICMP_DEST_UNREACH 3
+#define ICMPV6_DEST_UNREACH 1
#define ICMP_FRAG_NEEDED 4
#define NR_ICMP_UNREACH 15
+#define NR_ICMPV6_UNREACH 6
static int connect_to_fd_nonblock(int server_fd)
{
struct sockaddr_storage addr;
socklen_t len = sizeof(addr);
- int fd, err;
+ int fd, err, on = 1;
if (getsockname(server_fd, (struct sockaddr *)&addr, &len))
return -1;
@@ -25,6 +27,12 @@ static int connect_to_fd_nonblock(int server_fd)
if (fd < 0)
return -1;
+ if (addr.ss_family == AF_INET6 &&
+ setsockopt(fd, IPPROTO_IPV6, IPV6_RECVERR, &on, sizeof(on)) < 0) {
+ close(fd);
+ return -1;
+ }
+
err = connect(fd, (struct sockaddr *)&addr, len);
if (err < 0 && errno != EINPROGRESS) {
close(fd);
@@ -34,7 +42,7 @@ static int connect_to_fd_nonblock(int server_fd)
return fd;
}
-static void read_icmp_errqueue(int sockfd, int expected_code)
+static void read_icmp_errqueue(int sockfd, int expected_code, int af)
{
ssize_t n;
struct sock_extended_err *sock_err;
@@ -44,6 +52,12 @@ static void read_icmp_errqueue(int sockfd, int expected_code)
.msg_control = ctrl_buf,
.msg_controllen = sizeof(ctrl_buf),
};
+ int expected_level = (af == AF_INET) ? IPPROTO_IP : IPPROTO_IPV6;
+ int expected_type = (af == AF_INET) ? IP_RECVERR : IPV6_RECVERR;
+ int expected_origin = (af == AF_INET) ? SO_EE_ORIGIN_ICMP :
+ SO_EE_ORIGIN_ICMP6;
+ int expected_ee_type = (af == AF_INET) ? ICMP_DEST_UNREACH :
+ ICMPV6_DEST_UNREACH;
n = recvmsg(sockfd, &msg, MSG_ERRQUEUE);
if (!ASSERT_GE(n, 0, "recvmsg_errqueue"))
@@ -54,28 +68,27 @@ static void read_icmp_errqueue(int sockfd, int expected_code)
return;
for (; cm; cm = CMSG_NXTHDR(&msg, cm)) {
- if (!ASSERT_EQ(cm->cmsg_level, IPPROTO_IP, "cmsg_type") ||
- !ASSERT_EQ(cm->cmsg_type, IP_RECVERR, "cmsg_level"))
+ if (!ASSERT_EQ(cm->cmsg_level, expected_level, "cmsg_level") ||
+ !ASSERT_EQ(cm->cmsg_type, expected_type, "cmsg_type"))
continue;
sock_err = (struct sock_extended_err *)CMSG_DATA(cm);
- if (!ASSERT_EQ(sock_err->ee_origin, SO_EE_ORIGIN_ICMP,
- "sock_err_origin_icmp"))
+ if (!ASSERT_EQ(sock_err->ee_origin, expected_origin,
+ "sock_err_origin"))
return;
- if (!ASSERT_EQ(sock_err->ee_type, ICMP_DEST_UNREACH,
+ if (!ASSERT_EQ(sock_err->ee_type, expected_ee_type,
"sock_err_type_dest_unreach"))
return;
ASSERT_EQ(sock_err->ee_code, expected_code, "sock_err_code");
}
}
-static void trigger_prog_read_icmp_errqueue(int *code)
+static void trigger_prog_read_icmp_errqueue(int *code, int af, const char *addr)
{
int srv_fd = -1, client_fd = -1;
- srv_fd = start_server(AF_INET, SOCK_STREAM, "127.0.0.1", SRV_PORT,
- TIMEOUT_MS);
+ srv_fd = start_server(af, SOCK_STREAM, addr, SRV_PORT, TIMEOUT_MS);
if (!ASSERT_GE(srv_fd, 0, "start_server"))
return;
@@ -86,18 +99,40 @@ static void trigger_prog_read_icmp_errqueue(int *code)
}
/* Skip reading ICMP error queue if code is invalid */
- if (*code >= 0 && *code <= NR_ICMP_UNREACH)
- read_icmp_errqueue(client_fd, *code);
+ if (*code >= 0 && ((af == AF_INET && *code <= NR_ICMP_UNREACH) ||
+ (af == AF_INET6 && *code <= NR_ICMPV6_UNREACH)))
+ read_icmp_errqueue(client_fd, *code, af);
- close(srv_fd);
close(client_fd);
+ close(srv_fd);
+}
+
+static void run_icmp_test(struct icmp_send_unreach *skel, int af,
+ const char *addr, int max_code)
+{
+ int *code = &skel->bss->unreach_code;
+
+ for (*code = 0; *code <= max_code; (*code)++) {
+ /* The TCP stack reacts differently when asking for
+ * fragmentation, let's ignore it for now.
+ */
+ if (af == AF_INET && *code == ICMP_FRAG_NEEDED)
+ continue;
+
+ trigger_prog_read_icmp_errqueue(code, af, addr);
+ ASSERT_EQ(skel->data->kfunc_ret, 0, "kfunc_ret");
+ }
+
+ /* Test an invalid code */
+ *code = -1;
+ trigger_prog_read_icmp_errqueue(code, af, addr);
+ ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
}
void test_icmp_send_unreach_kfunc(void)
{
struct icmp_send_unreach *skel;
int cgroup_fd = -1;
- int *code;
skel = icmp_send_unreach__open_and_load();
if (!ASSERT_OK_PTR(skel, "skel_open"))
@@ -112,23 +147,11 @@ void test_icmp_send_unreach_kfunc(void)
if (!ASSERT_OK_PTR(skel->links.egress, "prog_attach_cgroup"))
goto cleanup;
- code = &skel->bss->unreach_code;
-
- for (*code = 0; *code <= NR_ICMP_UNREACH; (*code)++) {
- /* The TCP stack reacts differently when asking for
- * fragmentation, let's ignore it for now.
- */
- if (*code == ICMP_FRAG_NEEDED)
- continue;
-
- trigger_prog_read_icmp_errqueue(code);
- ASSERT_EQ(skel->data->kfunc_ret, 0, "kfunc_ret");
- }
+ if (test__start_subtest("ipv4"))
+ run_icmp_test(skel, AF_INET, "127.0.0.1", NR_ICMP_UNREACH);
- /* Test an invalid code */
- *code = -1;
- trigger_prog_read_icmp_errqueue(code);
- ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+ if (test__start_subtest("ipv6"))
+ run_icmp_test(skel, AF_INET6, "::1", NR_ICMPV6_UNREACH);
cleanup:
icmp_send_unreach__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/icmp_send_unreach.c b/tools/testing/selftests/bpf/progs/icmp_send_unreach.c
index 6fc5595f08aa..112b9cbfab6f 100644
--- a/tools/testing/selftests/bpf/progs/icmp_send_unreach.c
+++ b/tools/testing/selftests/bpf/progs/icmp_send_unreach.c
@@ -6,6 +6,11 @@
#define SERVER_PORT 54321
/* 127.0.0.1 in network byte order */
#define SERVER_IP 0x7F000001
+/* ::1 in network byte order */
+#define SERVER_IP6_0 0x00000000
+#define SERVER_IP6_1 0x00000000
+#define SERVER_IP6_2 0x00000000
+#define SERVER_IP6_3 0x01000000
int unreach_code = 0;
int kfunc_ret = -1;
@@ -16,17 +21,46 @@ int egress(struct __sk_buff *skb)
void *data = (void *)(long)skb->data;
void *data_end = (void *)(long)skb->data_end;
struct iphdr *iph;
+ struct ipv6hdr *ip6h;
struct tcphdr *tcph;
+ __u8 version;
- iph = data;
- if ((void *)(iph + 1) > data_end || iph->version != 4 ||
- iph->protocol != IPPROTO_TCP || iph->daddr != bpf_htonl(SERVER_IP))
+ if (data + 1 > data_end)
return SK_PASS;
- tcph = (void *)iph + iph->ihl * 4;
- if ((void *)(tcph + 1) > data_end ||
- tcph->dest != bpf_htons(SERVER_PORT))
+ version = (*((__u8 *)data)) >> 4;
+
+ if (version == 4) {
+ iph = data;
+ if ((void *)(iph + 1) > data_end ||
+ iph->protocol != IPPROTO_TCP ||
+ iph->daddr != bpf_htonl(SERVER_IP))
+ return SK_PASS;
+
+ tcph = (void *)iph + iph->ihl * 4;
+ if ((void *)(tcph + 1) > data_end ||
+ tcph->dest != bpf_htons(SERVER_PORT))
+ return SK_PASS;
+
+ } else if (version == 6) {
+ ip6h = data;
+ if ((void *)(ip6h + 1) > data_end ||
+ ip6h->nexthdr != IPPROTO_TCP)
+ return SK_PASS;
+
+ if (ip6h->daddr.in6_u.u6_addr32[0] != SERVER_IP6_0 ||
+ ip6h->daddr.in6_u.u6_addr32[1] != SERVER_IP6_1 ||
+ ip6h->daddr.in6_u.u6_addr32[2] != SERVER_IP6_2 ||
+ ip6h->daddr.in6_u.u6_addr32[3] != SERVER_IP6_3)
+ return SK_PASS;
+
+ tcph = (void *)(ip6h + 1);
+ if ((void *)(tcph + 1) > data_end ||
+ tcph->dest != bpf_htons(SERVER_PORT))
+ return SK_PASS;
+ } else {
return SK_PASS;
+ }
kfunc_ret = bpf_icmp_send_unreach(skb, unreach_code);
--
2.34.1
^ permalink raw reply related
* [PATCH bpf-next v4 4/6] selftests/bpf: add icmp_send_unreach kfunc tests
From: Mahe Tardy @ 2026-04-20 10:58 UTC (permalink / raw)
To: mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo
In-Reply-To: <20260420105816.72168-1-mahe.tardy@gmail.com>
This test opens a server and client, enters a new cgroup, attach a
cgroup_skb program on egress and calls the icmp_send_unreach function
from the client egress so that an ICMP unreach control message is sent
back to the client. It then fetches the message from the error queue to
confirm the correct ICMP unreach code has been sent.
Note that, for the client, we have to connect in non-blocking mode to
let the test execute faster. Otherwise, we need to wait for the TCP
three-way handshake to timeout in the kernel before reading the errno.
Also note that we don't set IP_RECVERR on the socket in
connect_to_fd_nonblock since the error will be transferred anyway in our
test because the connection is rejected at the beginning of the TCP
handshake. See in net/ipv4/tcp_ipv4.c:tcp_v4_err line 615 to 655 for
more details.
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
.../bpf/prog_tests/icmp_send_unreach_kfunc.c | 136 ++++++++++++++++++
.../selftests/bpf/progs/icmp_send_unreach.c | 36 +++++
2 files changed, 172 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
create mode 100644 tools/testing/selftests/bpf/progs/icmp_send_unreach.c
diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
new file mode 100644
index 000000000000..24d5e01cfe80
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_unreach_kfunc.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include <linux/errqueue.h>
+#include "icmp_send_unreach.skel.h"
+
+#define TIMEOUT_MS 1000
+#define SRV_PORT 54321
+
+#define ICMP_DEST_UNREACH 3
+
+#define ICMP_FRAG_NEEDED 4
+#define NR_ICMP_UNREACH 15
+
+static int connect_to_fd_nonblock(int server_fd)
+{
+ struct sockaddr_storage addr;
+ socklen_t len = sizeof(addr);
+ int fd, err;
+
+ if (getsockname(server_fd, (struct sockaddr *)&addr, &len))
+ return -1;
+
+ fd = socket(addr.ss_family, SOCK_STREAM | SOCK_NONBLOCK, 0);
+ if (fd < 0)
+ return -1;
+
+ err = connect(fd, (struct sockaddr *)&addr, len);
+ if (err < 0 && errno != EINPROGRESS) {
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
+
+static void read_icmp_errqueue(int sockfd, int expected_code)
+{
+ ssize_t n;
+ struct sock_extended_err *sock_err;
+ struct cmsghdr *cm;
+ char ctrl_buf[512];
+ struct msghdr msg = {
+ .msg_control = ctrl_buf,
+ .msg_controllen = sizeof(ctrl_buf),
+ };
+
+ n = recvmsg(sockfd, &msg, MSG_ERRQUEUE);
+ if (!ASSERT_GE(n, 0, "recvmsg_errqueue"))
+ return;
+
+ cm = CMSG_FIRSTHDR(&msg);
+ if (!ASSERT_NEQ(cm, NULL, "cm_firsthdr_null"))
+ return;
+
+ for (; cm; cm = CMSG_NXTHDR(&msg, cm)) {
+ if (!ASSERT_EQ(cm->cmsg_level, IPPROTO_IP, "cmsg_type") ||
+ !ASSERT_EQ(cm->cmsg_type, IP_RECVERR, "cmsg_level"))
+ continue;
+
+ sock_err = (struct sock_extended_err *)CMSG_DATA(cm);
+
+ if (!ASSERT_EQ(sock_err->ee_origin, SO_EE_ORIGIN_ICMP,
+ "sock_err_origin_icmp"))
+ return;
+ if (!ASSERT_EQ(sock_err->ee_type, ICMP_DEST_UNREACH,
+ "sock_err_type_dest_unreach"))
+ return;
+ ASSERT_EQ(sock_err->ee_code, expected_code, "sock_err_code");
+ }
+}
+
+static void trigger_prog_read_icmp_errqueue(int *code)
+{
+ int srv_fd = -1, client_fd = -1;
+
+ srv_fd = start_server(AF_INET, SOCK_STREAM, "127.0.0.1", SRV_PORT,
+ TIMEOUT_MS);
+ if (!ASSERT_GE(srv_fd, 0, "start_server"))
+ return;
+
+ client_fd = connect_to_fd_nonblock(srv_fd);
+ if (!ASSERT_GE(client_fd, 0, "client_connect_nonblock")) {
+ close(srv_fd);
+ return;
+ }
+
+ /* Skip reading ICMP error queue if code is invalid */
+ if (*code >= 0 && *code <= NR_ICMP_UNREACH)
+ read_icmp_errqueue(client_fd, *code);
+
+ close(srv_fd);
+ close(client_fd);
+}
+
+void test_icmp_send_unreach_kfunc(void)
+{
+ struct icmp_send_unreach *skel;
+ int cgroup_fd = -1;
+ int *code;
+
+ skel = icmp_send_unreach__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ goto cleanup;
+
+ cgroup_fd = test__join_cgroup("/icmp_send_unreach_cgroup");
+ if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup"))
+ goto cleanup;
+
+ skel->links.egress =
+ bpf_program__attach_cgroup(skel->progs.egress, cgroup_fd);
+ if (!ASSERT_OK_PTR(skel->links.egress, "prog_attach_cgroup"))
+ goto cleanup;
+
+ code = &skel->bss->unreach_code;
+
+ for (*code = 0; *code <= NR_ICMP_UNREACH; (*code)++) {
+ /* The TCP stack reacts differently when asking for
+ * fragmentation, let's ignore it for now.
+ */
+ if (*code == ICMP_FRAG_NEEDED)
+ continue;
+
+ trigger_prog_read_icmp_errqueue(code);
+ ASSERT_EQ(skel->data->kfunc_ret, 0, "kfunc_ret");
+ }
+
+ /* Test an invalid code */
+ *code = -1;
+ trigger_prog_read_icmp_errqueue(code);
+ ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+
+cleanup:
+ icmp_send_unreach__destroy(skel);
+ close(cgroup_fd);
+}
diff --git a/tools/testing/selftests/bpf/progs/icmp_send_unreach.c b/tools/testing/selftests/bpf/progs/icmp_send_unreach.c
new file mode 100644
index 000000000000..6fc5595f08aa
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/icmp_send_unreach.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
+
+#define SERVER_PORT 54321
+/* 127.0.0.1 in network byte order */
+#define SERVER_IP 0x7F000001
+
+int unreach_code = 0;
+int kfunc_ret = -1;
+
+SEC("cgroup_skb/egress")
+int egress(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ void *data_end = (void *)(long)skb->data_end;
+ struct iphdr *iph;
+ struct tcphdr *tcph;
+
+ iph = data;
+ if ((void *)(iph + 1) > data_end || iph->version != 4 ||
+ iph->protocol != IPPROTO_TCP || iph->daddr != bpf_htonl(SERVER_IP))
+ return SK_PASS;
+
+ tcph = (void *)iph + iph->ihl * 4;
+ if ((void *)(tcph + 1) > data_end ||
+ tcph->dest != bpf_htons(SERVER_PORT))
+ return SK_PASS;
+
+ kfunc_ret = bpf_icmp_send_unreach(skb, unreach_code);
+
+ return SK_DROP;
+}
+
+char LICENSE[] SEC("license") = "Dual BSD/GPL";
--
2.34.1
^ permalink raw reply related
* [PATCH bpf-next v4 3/6] bpf: add bpf_icmp_send_unreach kfunc
From: Mahe Tardy @ 2026-04-20 10:58 UTC (permalink / raw)
To: mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo
In-Reply-To: <20260420105816.72168-1-mahe.tardy@gmail.com>
This is needed in the context of Tetragon to provide improved feedback
(in contrast to just dropping packets) to east-west traffic when blocked
by policies using cgroup_skb programs.
This reuse concepts from netfilter reject target codepath with the
differences that:
* Packets are cloned since the BPF user can still let the packet pass
(SK_PASS from the cgroup_skb progs for example) and the current skb
need to stay untouched (cgroup_skb hooks only allow read-only skb
payload). The kfunc set the dst of the cloned skb by using the saddr
as the daddr and routing it.
* Checksums are not computed or verified and IPv4 fragmentation is not
checked early (icmp_send will check).
* We protect against recursion since the kfunc, by generating an ICMP
error message could retrigger the BPF prog that invoked it.
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
net/core/filter.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index fcfcb72663ca..a6c3b9145c93 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -84,6 +84,10 @@
#include <linux/un.h>
#include <net/xdp_sock_drv.h>
#include <net/inet_dscp.h>
+#include <linux/icmp.h>
+#include <net/icmp.h>
+#include <net/route.h>
+#include <net/ip6_route.h>
#include "dev.h"
@@ -12423,6 +12427,86 @@ __bpf_kfunc int bpf_xdp_pull_data(struct xdp_md *x, u32 len)
return 0;
}
+static DEFINE_PER_CPU(bool, bpf_icmp_send_in_progress);
+
+/**
+ * bpf_icmp_send_unreach - Send ICMP destination unreachable error
+ * @skb: Packet that triggered the error
+ * @code: ICMP unreachable code (0-15 for IPv4, 0-6 for IPv6)
+ *
+ * Sends an ICMP destination unreachable message in response to the
+ * packet. The original packet is cloned before sending the ICMP error,
+ * so the BPF program can still let the packet pass if desired.
+ *
+ * Recursion protection: If called from a context that would trigger
+ * recursion (e.g., root cgroup processing its own ICMP packets),
+ * returns -EBUSY on re-entry.
+ *
+ * Return: 0 on success, negative error code on failure:
+ * -EINVAL: Invalid code parameter
+ * -ENOMEM: Memory allocation failed
+ * -EHOSTUNREACH: Routing lookup failed
+ * -EBUSY: Recursion detected
+ * -EPROTONOSUPPORT: Non-IP protocol
+ */
+__bpf_kfunc int bpf_icmp_send_unreach(struct __sk_buff *__skb, int code)
+{
+ struct sk_buff *skb = (struct sk_buff *)__skb;
+ struct sk_buff *nskb;
+ bool *in_progress;
+
+ in_progress = this_cpu_ptr(&bpf_icmp_send_in_progress);
+ if (*in_progress)
+ return -EBUSY;
+
+ switch (skb->protocol) {
+#if IS_ENABLED(CONFIG_INET)
+ case htons(ETH_P_IP):
+ if (code < 0 || code > NR_ICMP_UNREACH)
+ return -EINVAL;
+
+ nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
+ return -ENOMEM;
+
+ if (!skb_dst(nskb) && ip_route_reply_fetch_dst(nskb) < 0) {
+ kfree_skb(nskb);
+ return -EHOSTUNREACH;
+ }
+
+ *in_progress = true;
+ icmp_send(nskb, ICMP_DEST_UNREACH, code, 0);
+ *in_progress = false;
+ kfree_skb(nskb);
+ break;
+#endif
+#if IS_ENABLED(CONFIG_IPV6)
+ case htons(ETH_P_IPV6):
+ if (code < 0 || code > ICMPV6_REJECT_ROUTE)
+ return -EINVAL;
+
+ nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
+ return -ENOMEM;
+
+ if (!skb_dst(nskb) && ip6_route_reply_fetch_dst(nskb) < 0) {
+ kfree_skb(nskb);
+ return -EHOSTUNREACH;
+ }
+
+ *in_progress = true;
+ icmpv6_send(nskb, ICMPV6_DEST_UNREACH, code, 0);
+ *in_progress = false;
+ kfree_skb(nskb);
+ break;
+#endif
+ default:
+ return -EPROTONOSUPPORT;
+ }
+
+ return 0;
+}
+
__bpf_kfunc_end_defs();
int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
@@ -12442,6 +12526,7 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
BTF_KFUNCS_START(bpf_kfunc_check_set_skb)
BTF_ID_FLAGS(func, bpf_dynptr_from_skb)
+BTF_ID_FLAGS(func, bpf_icmp_send_unreach)
BTF_KFUNCS_END(bpf_kfunc_check_set_skb)
BTF_KFUNCS_START(bpf_kfunc_check_set_skb_meta)
--
2.34.1
^ permalink raw reply related
* [PATCH bpf-next v4 2/6] net: move netfilter nf_reject6_fill_skb_dst to core ipv6
From: Mahe Tardy @ 2026-04-20 10:58 UTC (permalink / raw)
To: mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo
In-Reply-To: <20260420105816.72168-1-mahe.tardy@gmail.com>
Move and rename nf_reject6_fill_skb_dst from
ipv6/netfilter/nf_reject_ipv6 to ip6_route_reply_fetch_dst in
ipv6/route.c so that it can be reused in the following patches by BPF
kfuncs.
Netfilter uses nf_ip6_route that is almost a transparent wrapper around
ip6_route_output so this patch inlines it.
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
include/net/ip6_route.h | 2 ++
net/ipv6/netfilter/nf_reject_ipv6.c | 17 +----------------
net/ipv6/route.c | 18 ++++++++++++++++++
3 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 09ffe0f13ce7..3652efec7081 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -100,6 +100,8 @@ static inline struct dst_entry *ip6_route_output(struct net *net,
return ip6_route_output_flags(net, sk, fl6, 0);
}
+int ip6_route_reply_fetch_dst(struct sk_buff *skb);
+
/* Only conditionally release dst if flags indicates
* !RT6_LOOKUP_F_DST_NOREF or dst is in uncached_list.
*/
diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c
index ef5b7e85cffa..9663d1db6d80 100644
--- a/net/ipv6/netfilter/nf_reject_ipv6.c
+++ b/net/ipv6/netfilter/nf_reject_ipv6.c
@@ -293,21 +293,6 @@ nf_reject_ip6_tcphdr_put(struct sk_buff *nskb,
sizeof(struct tcphdr), 0));
}
-static int nf_reject6_fill_skb_dst(struct sk_buff *skb_in)
-{
- struct dst_entry *dst = NULL;
- struct flowi fl;
-
- memset(&fl, 0, sizeof(struct flowi));
- fl.u.ip6.daddr = ipv6_hdr(skb_in)->saddr;
- nf_ip6_route(dev_net(skb_in->dev), &dst, &fl, false);
- if (!dst)
- return -1;
-
- skb_dst_set(skb_in, dst);
- return 0;
-}
-
void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
int hook)
{
@@ -440,7 +425,7 @@ void nf_send_unreach6(struct net *net, struct sk_buff *skb_in,
if (hooknum == NF_INET_LOCAL_OUT && skb_in->dev == NULL)
skb_in->dev = net->loopback_dev;
- if (!skb_dst(skb_in) && nf_reject6_fill_skb_dst(skb_in) < 0)
+ if (!skb_dst(skb_in) && ip6_route_reply_fetch_dst(skb_in) < 0)
return;
icmpv6_send(skb_in, ICMPV6_DEST_UNREACH, code, 0);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 19eb6b702227..41871fddec4d 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2721,6 +2721,24 @@ struct dst_entry *ip6_route_output_flags(struct net *net,
}
EXPORT_SYMBOL_GPL(ip6_route_output_flags);
+int ip6_route_reply_fetch_dst(struct sk_buff *skb)
+{
+ struct dst_entry *result;
+ struct flowi6 fl = {
+ .daddr = ipv6_hdr(skb)->saddr
+ };
+ int err;
+
+ result = ip6_route_output(dev_net(skb->dev), NULL, &fl);
+ err = result->error;
+ if (err)
+ dst_release(result);
+ else
+ skb_dst_set(skb, result);
+ return err;
+}
+EXPORT_SYMBOL_GPL(ip6_route_reply_fetch_dst);
+
struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_orig)
{
struct rt6_info *rt, *ort = dst_rt6_info(dst_orig);
--
2.34.1
^ permalink raw reply related
* [PATCH bpf-next v4 1/6] net: move netfilter nf_reject_fill_skb_dst to core ipv4
From: Mahe Tardy @ 2026-04-20 10:58 UTC (permalink / raw)
To: mahe.tardy
Cc: alexei.starovoitov, andrii, ast, bpf, coreteam, daniel, fw,
john.fastabend, lkp, martin.lau, netdev, netfilter-devel,
oe-kbuild-all, pablo
In-Reply-To: <20260420105816.72168-1-mahe.tardy@gmail.com>
Move and rename nf_reject_fill_skb_dst from
ipv4/netfilter/nf_reject_ipv4 to ip_route_reply_fetch_dst in
ipv4/route.c so that it can be reused in the following patches by BPF
kfuncs.
Netfilter uses nf_ip_route that is almost a transparent wrapper around
ip_route_output_key so this patch inlines it.
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
include/net/route.h | 1 +
net/ipv4/netfilter/nf_reject_ipv4.c | 19 ++-----------------
net/ipv4/route.c | 15 +++++++++++++++
3 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/include/net/route.h b/include/net/route.h
index f90106f383c5..ec2466fd0bec 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -173,6 +173,7 @@ struct rtable *ip_route_output_flow(struct net *, struct flowi4 *flp,
const struct sock *sk);
struct dst_entry *ipv4_blackhole_route(struct net *net,
struct dst_entry *dst_orig);
+int ip_route_reply_fetch_dst(struct sk_buff *skb);
static inline struct rtable *ip_route_output_key(struct net *net, struct flowi4 *flp)
{
diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
index fecf6621f679..2290451ed122 100644
--- a/net/ipv4/netfilter/nf_reject_ipv4.c
+++ b/net/ipv4/netfilter/nf_reject_ipv4.c
@@ -252,21 +252,6 @@ static void nf_reject_ip_tcphdr_put(struct sk_buff *nskb, const struct sk_buff *
nskb->csum_offset = offsetof(struct tcphdr, check);
}
-static int nf_reject_fill_skb_dst(struct sk_buff *skb_in)
-{
- struct dst_entry *dst = NULL;
- struct flowi fl;
-
- memset(&fl, 0, sizeof(struct flowi));
- fl.u.ip4.daddr = ip_hdr(skb_in)->saddr;
- nf_ip_route(dev_net(skb_in->dev), &dst, &fl, false);
- if (!dst)
- return -1;
-
- skb_dst_set(skb_in, dst);
- return 0;
-}
-
/* Send RST reply */
void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
int hook)
@@ -279,7 +264,7 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
if (!oth)
return;
- if (!skb_dst(oldskb) && nf_reject_fill_skb_dst(oldskb) < 0)
+ if (!skb_dst(oldskb) && ip_route_reply_fetch_dst(oldskb) < 0)
return;
if (skb_rtable(oldskb)->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))
@@ -352,7 +337,7 @@ void nf_send_unreach(struct sk_buff *skb_in, int code, int hook)
if (iph->frag_off & htons(IP_OFFSET))
return;
- if (!skb_dst(skb_in) && nf_reject_fill_skb_dst(skb_in) < 0)
+ if (!skb_dst(skb_in) && ip_route_reply_fetch_dst(skb_in) < 0)
return;
if (skb_csum_unnecessary(skb_in) ||
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index bc1296f0ea69..7091ef936073 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2945,6 +2945,21 @@ struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
}
EXPORT_SYMBOL_GPL(ip_route_output_flow);
+int ip_route_reply_fetch_dst(struct sk_buff *skb)
+{
+ struct rtable *rt;
+ struct flowi4 fl4 = {
+ .daddr = ip_hdr(skb)->saddr
+ };
+
+ rt = ip_route_output_key(dev_net(skb->dev), &fl4);
+ if (IS_ERR(rt))
+ return PTR_ERR(rt);
+ skb_dst_set(skb, &rt->dst);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ip_route_reply_fetch_dst);
+
/* called with rcu_read_lock held */
static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
struct rtable *rt, u32 table_id, dscp_t dscp,
--
2.34.1
^ 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