* [PATCH bpf-next v3] bpf: reject short IPv4/IPv6 inputs in bpf_prog_test_run_skb
From: Sun Jian @ 2026-04-02 16:01 UTC (permalink / raw)
To: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, edumazet,
kuba, pabeni, horms, shuah
Cc: syzbot+619b9ef527f510a57cfc, bpf, netdev, linux-kernel,
linux-kselftest, Sun Jian
bpf_prog_test_run_skb() calls eth_type_trans() first and then uses
skb->protocol to initialize sk family and address fields for the test
run.
For IPv4 and IPv6 packets, it may access ip_hdr(skb) or ipv6_hdr(skb)
to initialize sk fields. Reject the input earlier if the Ethernet frame
carries IPv4/IPv6 EtherType but the L3 header is too short.
Fold the IPv4/IPv6 header length checks into the existing protocol
switch and return -EINVAL before accessing the network headers.
Also extend empty_skb selftests with ETH_HLEN-sized packets carrying
IPv4/IPv6 EtherType but no L3 header.
Reported-by: syzbot+619b9ef527f510a57cfc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=619b9ef527f510a57cfc
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
Changes in v3:
- Rework the fix by moving the checks into the existing protocol switch
in bpf_prog_test_run_skb() instead of duplicating the switch.
- Add empty_skb selftests for ETH_HLEN-sized packets with IPv4/IPv6
EtherType but no L3 header.
- Retarget to bpf-next.
Link: https://lore.kernel.org/bpf/20260329161751.1914272-1-sun.jian.kdev@gmail.com/
net/bpf/test_run.c | 20 ++++++++-----
.../selftests/bpf/prog_tests/empty_skb.c | 29 +++++++++++++++++++
2 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 178c4738e63b..300e2bfc5a62 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1120,19 +1120,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
switch (skb->protocol) {
case htons(ETH_P_IP):
- sk->sk_family = AF_INET;
- if (sizeof(struct iphdr) <= skb_headlen(skb)) {
- sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
- sk->sk_daddr = ip_hdr(skb)->daddr;
+ if (skb_headlen(skb) < sizeof(struct iphdr)) {
+ ret = -EINVAL;
+ goto out;
}
+ sk->sk_family = AF_INET;
+ sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
+ sk->sk_daddr = ip_hdr(skb)->daddr;
break;
#if IS_ENABLED(CONFIG_IPV6)
case htons(ETH_P_IPV6):
- sk->sk_family = AF_INET6;
- if (sizeof(struct ipv6hdr) <= skb_headlen(skb)) {
- sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
- sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
+ if (skb_headlen(skb) < sizeof(struct ipv6hdr)) {
+ ret = -EINVAL;
+ goto out;
}
+ sk->sk_family = AF_INET6;
+ sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
+ sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
break;
#endif
default:
diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
index 438583e1f2d1..d53567e9cd77 100644
--- a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
+++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
@@ -12,6 +12,8 @@ void test_empty_skb(void)
struct bpf_program *prog;
char eth_hlen_pp[15];
char eth_hlen[14];
+ char ipv4_eth_hlen[14];
+ char ipv6_eth_hlen[14];
int veth_ifindex;
int ipip_ifindex;
int err;
@@ -46,6 +48,24 @@ void test_empty_skb(void)
.err = -EINVAL,
},
+ /* ETH_HLEN-sized packets with IPv4/IPv6 EtherType but
+ * no L3 header are rejected.
+ */
+ {
+ .msg = "veth short IPv4 ingress packet",
+ .data_in = ipv4_eth_hlen,
+ .data_size_in = sizeof(ipv4_eth_hlen),
+ .ifindex = &veth_ifindex,
+ .err = -EINVAL,
+ },
+ {
+ .msg = "veth short IPv6 ingress packet",
+ .data_in = ipv6_eth_hlen,
+ .data_size_in = sizeof(ipv6_eth_hlen),
+ .ifindex = &veth_ifindex,
+ .err = -EINVAL,
+ },
+
/* ETH_HLEN-sized packets:
* - can not be redirected at LWT_XMIT
* - can be redirected at TC to non-tunneling dest
@@ -108,6 +128,15 @@ void test_empty_skb(void)
SYS(out, "ip addr add 192.168.1.1/16 dev ipip0");
ipip_ifindex = if_nametoindex("ipip0");
+ memset(ipv4_eth_hlen, 0, sizeof(ipv4_eth_hlen));
+ memset(ipv6_eth_hlen, 0, sizeof(ipv6_eth_hlen));
+
+ ipv4_eth_hlen[12] = 0x08;
+ ipv4_eth_hlen[13] = 0x00;
+
+ ipv6_eth_hlen[12] = 0x86;
+ ipv6_eth_hlen[13] = 0xdd;
+
bpf_obj = empty_skb__open_and_load();
if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
goto out;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] usb: rtl8150: avoid using uninitialized CSCR value
From: Petko Manolov @ 2026-04-02 15:51 UTC (permalink / raw)
To: Morduan Zang
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-usb, netdev, linux-kernel,
syzbot+9db6c624635564ad813c
In-Reply-To: <93FF85BB9F33CD2B+20260402070743.20641-1-zhangdandan@uniontech.com>
On 26-04-02 15:07:43, Morduan Zang wrote:
> Check get_registers() when reading CSCR in set_carrier().
> If the control transfer fails, don't use the stack value.
>
> Reported-by: syzbot+9db6c624635564ad813c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9db6c624635564ad813c
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
> ---
> drivers/net/usb/rtl8150.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 4cda0643afb6..7e32726d3e6f 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -722,7 +722,11 @@ static void set_carrier(struct net_device *netdev)
> rtl8150_t *dev = netdev_priv(netdev);
> short tmp;
>
> - get_registers(dev, CSCR, 2, &tmp);
> + if (get_registers(dev, CSCR, 2, &tmp) < 0) {
> + netif_carrier_off(netdev);
> + return;
> + }
> +
> if (tmp & CSCR_LINK_STATUS)
> netif_carrier_on(netdev);
> else
> --
Nice catch. You can add my Acked-by.
cheers,
Petko
^ permalink raw reply
* Re: [PATCH net-next v2 3/3] gve: implement PTP gettimex64
From: Naman Gulati @ 2026-04-02 15:53 UTC (permalink / raw)
To: Jordan Rhee
Cc: Jakub Kicinski, hramamurthy, netdev, joshwash, andrew+netdev,
davem, edumazet, pabeni, richardcochran, willemb, nktgrg, jfraker,
ziweixiao, maolson, thostet, jefrogers, alok.a.tiwari, yyd,
linux-kernel
In-Reply-To: <CA+mzVtv-91ru7v6GxOS80xihE4ZbwGqdyQdKPTHv+woKaeOhuQ@mail.gmail.com>
On Thu, Apr 2, 2026 at 8:38 AM Jordan Rhee <jordanrhee@google.com> wrote:
>
> We know that gve_adminq_report_nic_ts() will cause a VM exit, which
> will act as a full barrier. Additional barriers are not strictly
> necessary from a functional perspective. Can we rely on this fact or
> do we need explicit barriers?
I don't think that's enough. Without a barrier get_cycles can be
speculatively executed before gve_adminq_report_nic_ts.
>
>
> On Tue, Mar 31, 2026 at 11:05 AM Naman Gulati <namangulati@google.com> wrote:
> >
> > On Sun, Mar 29, 2026 at 9:31 PM Jordan Rhee <jordanrhee@google.com> wrote:
> > >
> > > On Sun, Mar 29, 2026 at 2:20 PM Jakub Kicinski <kuba@kernel.org> 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.
> > > > ---
> > > > Subject: gve: implement PTP gettimex64
> > > >
> > > > This commit implements the PTP gettimex64 function to enable chrony and
> > > > phc2sys to synchronize the system clock to the NIC clock. It reads the
> > > > NIC timestamp along with pre and post system cycle counters to minimize
> > > > the uncertainty window.
> > > >
> > > > > diff --git a/drivers/net/ethernet/google/gve/gve_ptp.c b/drivers/net/ethernet/google/gve/gve_ptp.c
> > > > > index 140b8fbce4f4..bea794541f30 100644
> > > > > --- a/drivers/net/ethernet/google/gve/gve_ptp.c
> > > > > +++ b/drivers/net/ethernet/google/gve/gve_ptp.c
> > > > > @@ -10,28 +10,203 @@
> > > >
> > > > [ ... ]
> > > >
> > > > > +static int gve_adminq_read_timestamp(struct gve_priv *priv,
> > > > > + cycles_t *pre_cycles,
> > > > > + cycles_t *post_cycles)
> > > > > +{
> > > > > + unsigned long delay_us = 1000;
> > > > > + int retry_count = 0;
> > > > > + int err;
> > > > > +
> > > > > + lockdep_assert_held(&priv->nic_ts_read_lock);
> > > > > +
> > > > > + do {
> > > > > + *pre_cycles = get_cycles();
> > > > > + err = gve_adminq_report_nic_ts(priv, priv->nic_ts_report_bus);
> > > > > +
> > > > > + /* Ensure cycle counter is sampled after AdminQ cmd returns */
> > > > > + rmb();
> > > > > + *post_cycles = get_cycles();
> > > >
> > > > Are execution barriers needed around get_cycles() here? rmb() orders
> > > > memory accesses, but the CPU might still speculatively execute the cycle
> > > > counter reads out of order with respect to the AdminQ commands.
> > > >
> > > > Could this speculative reordering cause the strict bounds validation in
> > > > gve_ptp_gettimex64() to fail spuriously?
> > >
> > > I will add mb() barriers to be totally safe.
> >
> > I'm not so sure that rmb() is insufficient here. In x86_64 rmb() is an
> > lfence. lfence is used to prevent speculative execution (per the x86
> > barrier_nospec implementation) and rdtsc_ordered, which is called by
> > the ktime* helpers, also prefaces rdtsc with an lfence. On ARM the
> > get_cycles implementation adds an isb instruction before reading
> > cntvct_el0.
> >
> >
> > >
> > > > [ ... ]
> > > >
> > > > > static int gve_ptp_gettimex64(struct ptp_clock_info *info,
> > > > > struct timespec64 *ts,
> > > > > struct ptp_system_timestamp *sts)
> > > > > {
> > > > > - return -EOPNOTSUPP;
> > > > > + struct gve_ptp *ptp = container_of(info, struct gve_ptp, info);
> > > > > + struct gve_sysclock_sample sysclock = {0};
> > > > > + struct gve_priv *priv = ptp->priv;
> > > > > + struct system_time_snapshot snap;
> > > > > + u64 nic_ts;
> > > > > + int err;
> > > > > +
> > > > > + /* Take system clock snapshot before sampling cycle counters */
> > > > > + if (sts)
> > > > > + ktime_get_snapshot(&snap);
> > > > > +
> > > > > + err = gve_clock_nic_ts_read(priv, &nic_ts, &sysclock);
> > > >
> > > > Could the system time snapshot become stale here?
> > > >
> > > > If priv->nic_ts_read_lock is contended, or if gve_adminq_read_timestamp()
> > > > retries and sleeps, the snapshot might be taken significantly before the
> > > > actual hardware read.
> > > >
> > > > Would it be more accurate to take the snapshot immediately before reading the
> > > > hardware, inside the mutex?
> > >
> > > Ack, will fix.
> > >
> > > >
> > > > > + if (err)
> > > > > + return err;
> > > > > +
> > > > > + if (sts) {
> > > > > + /* Reject samples with out of order system clock values */
> > > > > + if (!(sysclock.host_pre_cycles <= sysclock.nic_pre_cycles &&
> > > > > + sysclock.nic_pre_cycles <= sysclock.nic_post_cycles &&
> > > > > + sysclock.nic_post_cycles <= sysclock.host_post_cycles)) {
> > > > > + dev_err_ratelimited(&priv->pdev->dev,
> > > > > + "AdminQ system clock cycle counts out of order. Expecting %llu <= %llu <= %llu <= %llu\n",
> > > > > + (u64)sysclock.host_pre_cycles,
> > > > > + sysclock.nic_pre_cycles,
> > > > > + sysclock.nic_post_cycles,
> > > > > + (u64)sysclock.host_post_cycles);
> > > > > + return -EBADMSG;
> > > >
> > > > How does this behave on architectures where get_cycles() is not implemented
> > > > and returns 0, or with older firmware that leaves nic_pre_cycles and
> > > > nic_post_cycles as 0?
> > >
> > > Firmware is not allowed to return 0. 0 should trigger the check.
> > >
> > > > It looks like the bounds check would unconditionally fail and return -EBADMSG.
> > > > Since userspace tools like chrony usually expect -EOPNOTSUPP for unsupported
> > > > extended ioctls to fall back to basic PTP ioctls, will returning -EBADMSG
> > > > cause a hard failure for clock synchronization?
> > >
> > > Will fix. GVE is only supported on x86_64 and arm64, and will fail on
> > > other architectures with -EOPNOTSUPP due to the check in
> > > gve_cycles_to_clock_fn(). However, if an architecture returns
> > > get_cycles() as 0, it will fail here first with -EBADMSG. I will add
> > > an explicit check for x86 || arm64 earlier in this function.
> > >
> > > >
> > > > [ ... ]
> > > >
> > > > > @@ -132,7 +307,7 @@ int gve_init_clock(struct gve_priv *priv)
> > > > > goto release_ptp;
> > > > > }
> > > > > mutex_init(&priv->nic_ts_read_lock);
> > > > > - err = gve_clock_nic_ts_read(priv, &nic_raw);
> > > > > + err = gve_clock_nic_ts_read(priv, &nic_raw, NULL);
> > > > > if (err) {
> > > > > dev_err(&priv->pdev->dev, "failed to read NIC clock %d\n", err);
> > > > > goto release_nic_ts_report;
> > > >
> > > > Is there a race condition exposing the PTP device to userspace before its
> > > > resources are fully initialized?
> > > >
> > > > Looking at gve_init_clock(), gve_ptp_init() is called before allocating
> > > > nic_ts_report and initializing nic_ts_read_lock:
> > > >
> > > > err = gve_ptp_init(priv);
> > > > if (err)
> > > > return err;
> > > >
> > > > priv->nic_ts_report = dma_alloc_coherent(...);
> > > > ...
> > > > mutex_init(&priv->nic_ts_read_lock);
> > > >
> > > > If a concurrent userspace process immediately invokes the
> > > > PTP_SYS_OFFSET_EXTENDED ioctl after gve_ptp_init() registers the /dev/ptpX
> > > > device, could it call gve_ptp_gettimex64() and attempt to lock the
> > > > uninitialized mutex or dereference the NULL nic_ts_report pointer?
> > > >
> > > > Additionally, in the error path for gve_init_clock():
> > > >
> > > > release_nic_ts_report:
> > > > mutex_destroy(&priv->nic_ts_read_lock);
> > > > dma_free_coherent(...);
> > > > priv->nic_ts_report = NULL;
> > > > release_ptp:
> > > > gve_ptp_release(priv);
> > > >
> > > > Could destroying the mutex and freeing the memory before gve_ptp_release()
> > > > create a use-after-free window if an ioctl is currently running?
> > >
> > > Will be fixed in the previous patch in the series.
^ permalink raw reply
* [PATCH v6 net 8/8] selftests: bpf: adjust rx_dropped xskxceiver's test to respect tailroom
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260402154958.562179-1-maciej.fijalkowski@intel.com>
Since we have changed how big user defined headroom in umem can be,
change the logic in testapp_stats_rx_dropped() so we pass updated
headroom validation in xdp_umem_reg() and still drop half of frames.
Test works on non-mbuf setup so __xsk_pool_get_rx_frame_size() that is
called on xsk_rcv_check() will not account skb_shared_info size. Taking
the tailroom size into account in test being fixed is needed as
xdp_umem_reg() defaults to respect it.
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
tools/testing/selftests/bpf/prog_tests/test_xsk.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index ee60bcc22ee4..7950c504ed28 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -1959,15 +1959,17 @@ int testapp_headroom(struct test_spec *test)
int testapp_stats_rx_dropped(struct test_spec *test)
{
+ u32 umem_tr = test->ifobj_tx->umem_tailroom;
+
if (test->mode == TEST_MODE_ZC) {
ksft_print_msg("Can not run RX_DROPPED test for ZC mode\n");
return TEST_SKIP;
}
- if (pkt_stream_replace_half(test, MIN_PKT_SIZE * 4, 0))
+ if (pkt_stream_replace_half(test, (MIN_PKT_SIZE * 3) + umem_tr, 0))
return TEST_FAILURE;
test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
- XDP_PACKET_HEADROOM - MIN_PKT_SIZE * 3;
+ XDP_PACKET_HEADROOM - (MIN_PKT_SIZE * 2) - umem_tr;
if (pkt_stream_receive_half(test))
return TEST_FAILURE;
test->ifobj_rx->validation_func = validate_rx_dropped;
--
2.43.0
^ permalink raw reply related
* [PATCH v6 net 7/8] selftests: bpf: have a separate variable for drop test
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260402154958.562179-1-maciej.fijalkowski@intel.com>
Currently two different XDP programs share a static variable for
different purposes (picking where to redirect on shared umem test &
whether to drop a packet). This can be a problem when running full test
suite - idx can be written by shared umem test and this value can cause
a false behavior within XDP drop half test.
Introduce a dedicated variable for drop half test so that these two
don't step on each other toes. There is no real need for using
__sync_fetch_and_add here as XSK tests are executed on single CPU.
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
tools/testing/selftests/bpf/progs/xsk_xdp_progs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
index 683306db8594..023d8befd4ca 100644
--- a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
+++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
@@ -26,8 +26,10 @@ SEC("xdp.frags") int xsk_def_prog(struct xdp_md *xdp)
SEC("xdp.frags") int xsk_xdp_drop(struct xdp_md *xdp)
{
+ static unsigned int drop_idx;
+
/* Drop every other packet */
- if (idx++ % 2)
+ if (drop_idx++ % 2)
return XDP_DROP;
return bpf_redirect_map(&xsk, 0, XDP_DROP);
--
2.43.0
^ permalink raw reply related
* [PATCH v6 net 6/8] selftests: bpf: fix pkt grow tests
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260402154958.562179-1-maciej.fijalkowski@intel.com>
Skip tail adjust tests in xskxceiver for SKB mode as it is not very
friendly for it. multi-buffer case does not work as xdp_rxq_info that is
registered for generic XDP does not report ::frag_size. The non-mbuf
path copies packet via skb_pp_cow_data() which only accounts for
headroom, leaving us with no tailroom and causing underlying XDP prog to
drop packets therefore.
For multi-buffer test on other modes, change the amount of bytes we use
for growth, assume worst-case scenario and take care of headroom and
tailroom.
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
.../selftests/bpf/prog_tests/test_xsk.c | 24 ++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 62118ffba661..ee60bcc22ee4 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -2528,16 +2528,34 @@ int testapp_adjust_tail_shrink_mb(struct test_spec *test)
int testapp_adjust_tail_grow(struct test_spec *test)
{
+ if (test->mode == TEST_MODE_SKB)
+ return TEST_SKIP;
+
/* Grow by 4 bytes for testing purpose */
return testapp_adjust_tail(test, 4, MIN_PKT_SIZE * 2);
}
int testapp_adjust_tail_grow_mb(struct test_spec *test)
{
+ u32 grow_size;
+
+ if (test->mode == TEST_MODE_SKB)
+ return TEST_SKIP;
+
+ /* worst case scenario is when underlying setup will work on 3k
+ * buffers, let us account for it; given that we will use 6k as
+ * pkt_len, expect that it will be broken down to 2 descs each
+ * with 3k payload;
+ *
+ * 4k is truesize, 3k payload, 256 HR, 320 TR;
+ */
+ grow_size = XSK_UMEM__MAX_FRAME_SIZE -
+ XSK_UMEM__LARGE_FRAME_SIZE -
+ XDP_PACKET_HEADROOM -
+ test->ifobj_tx->umem_tailroom;
test->mtu = MAX_ETH_JUMBO_SIZE;
- /* Grow by (frag_size - last_frag_Size) - 1 to stay inside the last fragment */
- return testapp_adjust_tail(test, (XSK_UMEM__MAX_FRAME_SIZE / 2) - 1,
- XSK_UMEM__LARGE_FRAME_SIZE * 2);
+
+ return testapp_adjust_tail(test, grow_size, XSK_UMEM__LARGE_FRAME_SIZE * 2);
}
int testapp_tx_queue_consumer(struct test_spec *test)
--
2.43.0
^ permalink raw reply related
* [PATCH v6 net 5/8] selftests: bpf: introduce a common routine for reading procfs
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260402154958.562179-1-maciej.fijalkowski@intel.com>
Parametrize current way of getting MAX_SKB_FRAGS value from {sys,proc}fs
so that it can be re-used to get cache line size of system's CPU. All
that just to mimic and compute size of kernel's struct skb_shared_info
which for xsk and test suite interpret as tailroom.
Introduce two variables to ifobject struct that will carry count of skb
frags and tailroom size. Do the reading and computing once, at the
beginning of test suite execution in xskxceiver, but for test_progs such
way is not possible as in this environment each test setups and torns
down ifobject structs.
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
.../selftests/bpf/prog_tests/test_xsk.c | 25 +------------------
.../selftests/bpf/prog_tests/test_xsk.h | 23 +++++++++++++++++
tools/testing/selftests/bpf/prog_tests/xsk.c | 19 ++++++++++++++
tools/testing/selftests/bpf/xskxceiver.c | 23 +++++++++++++++++
4 files changed, 66 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 7e38ec6e656b..62118ffba661 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -179,25 +179,6 @@ int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem
return xsk_socket__create(&xsk->xsk, ifobject->ifindex, 0, umem->umem, rxr, txr, &cfg);
}
-#define MAX_SKB_FRAGS_PATH "/proc/sys/net/core/max_skb_frags"
-static unsigned int get_max_skb_frags(void)
-{
- unsigned int max_skb_frags = 0;
- FILE *file;
-
- file = fopen(MAX_SKB_FRAGS_PATH, "r");
- if (!file) {
- ksft_print_msg("Error opening %s\n", MAX_SKB_FRAGS_PATH);
- return 0;
- }
-
- if (fscanf(file, "%u", &max_skb_frags) != 1)
- ksft_print_msg("Error reading %s\n", MAX_SKB_FRAGS_PATH);
-
- fclose(file);
- return max_skb_frags;
-}
-
static int set_ring_size(struct ifobject *ifobj)
{
int ret;
@@ -2242,11 +2223,7 @@ int testapp_too_many_frags(struct test_spec *test)
if (test->mode == TEST_MODE_ZC) {
max_frags = test->ifobj_tx->xdp_zc_max_segs;
} else {
- max_frags = get_max_skb_frags();
- if (!max_frags) {
- ksft_print_msg("Can't get MAX_SKB_FRAGS from system, using default (17)\n");
- max_frags = 17;
- }
+ max_frags = test->ifobj_tx->max_skb_frags;
max_frags += 1;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.h b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
index 8fc78a057de0..1ab8aee4ce56 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.h
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
@@ -31,6 +31,9 @@
#define SOCK_RECONF_CTR 10
#define USLEEP_MAX 10000
+#define MAX_SKB_FRAGS_PATH "/proc/sys/net/core/max_skb_frags"
+#define SMP_CACHE_BYTES_PATH "/sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size"
+
extern bool opt_verbose;
#define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
@@ -45,6 +48,24 @@ static inline u64 ceil_u64(u64 a, u64 b)
return (a + b - 1) / b;
}
+static inline unsigned int read_procfs_val(const char *path)
+{
+ unsigned int read_val = 0;
+ FILE *file;
+
+ file = fopen(path, "r");
+ if (!file) {
+ ksft_print_msg("Error opening %s\n", path);
+ return 0;
+ }
+
+ if (fscanf(file, "%u", &read_val) != 1)
+ ksft_print_msg("Error reading %s\n", path);
+
+ fclose(file);
+ return read_val;
+}
+
/* Simple test */
enum test_mode {
TEST_MODE_SKB,
@@ -115,6 +136,8 @@ struct ifobject {
int mtu;
u32 bind_flags;
u32 xdp_zc_max_segs;
+ u32 umem_tailroom;
+ u32 max_skb_frags;
bool tx_on;
bool rx_on;
bool use_poll;
diff --git a/tools/testing/selftests/bpf/prog_tests/xsk.c b/tools/testing/selftests/bpf/prog_tests/xsk.c
index dd4c35c0e428..6e2f63ee2a6c 100644
--- a/tools/testing/selftests/bpf/prog_tests/xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/xsk.c
@@ -62,6 +62,7 @@ int configure_ifobj(struct ifobject *tx, struct ifobject *rx)
static void test_xsk(const struct test_spec *test_to_run, enum test_mode mode)
{
+ u32 max_frags, umem_tailroom, cache_line_size;
struct ifobject *ifobj_tx, *ifobj_rx;
struct test_spec test;
int ret;
@@ -84,6 +85,24 @@ static void test_xsk(const struct test_spec *test_to_run, enum test_mode mode)
ifobj_tx->set_ring.default_rx = ifobj_tx->ring.rx_pending;
}
+ cache_line_size = read_procfs_val(SMP_CACHE_BYTES_PATH);
+ if (!cache_line_size)
+ cache_line_size = 64;
+
+ max_frags = read_procfs_val(MAX_SKB_FRAGS_PATH);
+ if (!max_frags)
+ max_frags = 17;
+
+ ifobj_tx->max_skb_frags = max_frags;
+ ifobj_rx->max_skb_frags = max_frags;
+
+ /* 48 bytes is a part of skb_shared_info w/o frags array;
+ * 16 bytes is sizeof(skb_frag_t)
+ */
+ umem_tailroom = ALIGN(48 + (max_frags * 16), cache_line_size);
+ ifobj_tx->umem_tailroom = umem_tailroom;
+ ifobj_rx->umem_tailroom = umem_tailroom;
+
if (!ASSERT_OK(init_iface(ifobj_rx, worker_testapp_validate_rx), "init RX"))
goto delete_rx;
if (!ASSERT_OK(init_iface(ifobj_tx, worker_testapp_validate_tx), "init TX"))
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 05b3cebc5ca9..7dad8556a722 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -80,6 +80,7 @@
#include <linux/mman.h>
#include <linux/netdev.h>
#include <linux/ethtool.h>
+#include <linux/align.h>
#include <arpa/inet.h>
#include <net/if.h>
#include <locale.h>
@@ -333,6 +334,7 @@ static void print_tests(void)
int main(int argc, char **argv)
{
const size_t total_tests = ARRAY_SIZE(tests) + ARRAY_SIZE(ci_skip_tests);
+ u32 cache_line_size, max_frags, umem_tailroom;
struct pkt_stream *rx_pkt_stream_default;
struct pkt_stream *tx_pkt_stream_default;
struct ifobject *ifobj_tx, *ifobj_rx;
@@ -354,6 +356,27 @@ int main(int argc, char **argv)
setlocale(LC_ALL, "");
+ cache_line_size = read_procfs_val(SMP_CACHE_BYTES_PATH);
+ if (!cache_line_size) {
+ ksft_print_msg("Can't get SMP_CACHE_BYTES from system, using default (64)\n");
+ cache_line_size = 64;
+ }
+
+ max_frags = read_procfs_val(MAX_SKB_FRAGS_PATH);
+ if (!max_frags) {
+ ksft_print_msg("Can't get MAX_SKB_FRAGS from system, using default (17)\n");
+ max_frags = 17;
+ }
+ ifobj_tx->max_skb_frags = max_frags;
+ ifobj_rx->max_skb_frags = max_frags;
+
+ /* 48 bytes is a part of skb_shared_info w/o frags array;
+ * 16 bytes is sizeof(skb_frag_t)
+ */
+ umem_tailroom = ALIGN(48 + (max_frags * 16), cache_line_size);
+ ifobj_tx->umem_tailroom = umem_tailroom;
+ ifobj_rx->umem_tailroom = umem_tailroom;
+
parse_command_line(ifobj_tx, ifobj_rx, argc, argv);
if (opt_print_tests) {
--
2.43.0
^ permalink raw reply related
* [PATCH v6 net 4/8] xsk: validate MTU against usable frame size on bind
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260402154958.562179-1-maciej.fijalkowski@intel.com>
AF_XDP bind currently accepts zero-copy pool configurations without
verifying that the device MTU fits into the usable frame space provided
by the UMEM chunk.
This becomes a problem since we started to respect tailroom which is
subtracted from chunk_size (among with headroom). 2k chunk size might
not provide enough space for standard 1500 MTU, so let us catch such
settings at bind time. Furthermore, validate whether underlying HW will
be able to satisfy configured MTU wrt XSK's frame size multiplied by
supported Rx buffer chain length (that is exposed via
net_device::xdp_zc_max_segs).
Fixes: 24ea50127ecf ("xsk: support mbuf on ZC RX")
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
net/xdp/xsk_buff_pool.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 729602a3cec0..cd7bc50872f6 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -10,6 +10,8 @@
#include "xdp_umem.h"
#include "xsk.h"
+#define ETH_PAD_LEN (ETH_HLEN + 2 * VLAN_HLEN + ETH_FCS_LEN)
+
void xp_add_xsk(struct xsk_buff_pool *pool, struct xdp_sock *xs)
{
if (!xs->tx)
@@ -157,8 +159,12 @@ static void xp_disable_drv_zc(struct xsk_buff_pool *pool)
int xp_assign_dev(struct xsk_buff_pool *pool,
struct net_device *netdev, u16 queue_id, u16 flags)
{
+ u32 needed = netdev->mtu + ETH_PAD_LEN;
+ u32 segs = netdev->xdp_zc_max_segs;
+ bool mbuf = flags & XDP_USE_SG;
bool force_zc, force_copy;
struct netdev_bpf bpf;
+ u32 frame_size;
int err = 0;
ASSERT_RTNL();
@@ -178,7 +184,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
if (err)
return err;
- if (flags & XDP_USE_SG)
+ if (mbuf)
pool->umem->flags |= XDP_UMEM_SG_FLAG;
if (flags & XDP_USE_NEED_WAKEUP)
@@ -200,8 +206,24 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
goto err_unreg_pool;
}
- if (netdev->xdp_zc_max_segs == 1 && (flags & XDP_USE_SG)) {
- err = -EOPNOTSUPP;
+ if (mbuf) {
+ if (segs == 1) {
+ err = -EOPNOTSUPP;
+ goto err_unreg_pool;
+ }
+ } else {
+ segs = 1;
+ }
+
+ /* open-code xsk_pool_get_rx_frame_size() as pool->dev is not
+ * set yet at this point; we are before getting down to driver
+ */
+ frame_size = __xsk_pool_get_rx_frame_size(pool) -
+ xsk_pool_get_tailroom(mbuf);
+ frame_size = ALIGN_DOWN(frame_size, 128);
+
+ if (needed > frame_size * segs) {
+ err = -EINVAL;
goto err_unreg_pool;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v6 net 3/8] xsk: fix XDP_UMEM_SG_FLAG issues
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260402154958.562179-1-maciej.fijalkowski@intel.com>
Currently xp_assign_dev_shared() is missing XDP_USE_SG being propagated
to flags so set it in order to preserve mtu check that is supposed to be
done only when no multi-buffer setup is in picture.
Also, this flag has the same value as XDP_UMEM_TX_SW_CSUM so we could
get unexpected SG setups for software Tx checksums. Since csum flag is
UAPI, modify value of XDP_UMEM_SG_FLAG.
Fixes: d609f3d228a8 ("xsk: add multi-buffer support for sockets sharing umem")
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
include/net/xdp_sock.h | 2 +-
net/xdp/xsk_buff_pool.c | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 23e8861e8b25..ebac60a3d8a1 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -14,7 +14,7 @@
#include <linux/mm.h>
#include <net/sock.h>
-#define XDP_UMEM_SG_FLAG (1 << 1)
+#define XDP_UMEM_SG_FLAG BIT(3)
struct net_device;
struct xsk_queue;
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 37b7a68b89b3..729602a3cec0 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -247,6 +247,10 @@ int xp_assign_dev_shared(struct xsk_buff_pool *pool, struct xdp_sock *umem_xs,
struct xdp_umem *umem = umem_xs->umem;
flags = umem->zc ? XDP_ZEROCOPY : XDP_COPY;
+
+ if (umem->flags & XDP_UMEM_SG_FLAG)
+ flags |= XDP_USE_SG;
+
if (umem_xs->pool->uses_need_wakeup)
flags |= XDP_USE_NEED_WAKEUP;
--
2.43.0
^ permalink raw reply related
* [PATCH v6 net 2/8] xsk: respect tailroom for ZC setups
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski,
Stanislav Fomichev
In-Reply-To: <20260402154958.562179-1-maciej.fijalkowski@intel.com>
Multi-buffer XDP stores information about frags in skb_shared_info that
sits at the tailroom of a packet. The storage space is reserved via
xdp_data_hard_end():
((xdp)->data_hard_start + (xdp)->frame_sz - \
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
and then we refer to it via macro below:
static inline struct skb_shared_info *
xdp_get_shared_info_from_buff(const struct xdp_buff *xdp)
{
return (struct skb_shared_info *)xdp_data_hard_end(xdp);
}
Currently we do not respect this tailroom space in multi-buffer AF_XDP
ZC scenario. To address this, introduce xsk_pool_get_tailroom() and use
it within xsk_pool_get_rx_frame_size() which is used in ZC drivers to
configure length of HW Rx buffer.
Typically drivers on Rx Hw buffers side work on 128 byte alignment so
let us align the value returned by xsk_pool_get_rx_frame_size() in order
to avoid addressing this on driver's side. This addresses the fact that
idpf uses mentioned function *before* pool->dev being set so we were at
risk that after subtracting tailroom we would not provide 128-byte
aligned value to HW.
Since xsk_pool_get_rx_frame_size() is actively used in xsk_rcv_check()
and __xsk_rcv(), add a variant of this routine that will not include 128
byte alignment and therefore old behavior is preserved.
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Fixes: 24ea50127ecf ("xsk: support mbuf on ZC RX")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
include/net/xdp_sock_drv.h | 23 ++++++++++++++++++++++-
net/xdp/xsk.c | 4 ++--
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
index 6b9ebae2dc95..46797645a0c2 100644
--- a/include/net/xdp_sock_drv.h
+++ b/include/net/xdp_sock_drv.h
@@ -41,16 +41,37 @@ static inline u32 xsk_pool_get_headroom(struct xsk_buff_pool *pool)
return XDP_PACKET_HEADROOM + pool->headroom;
}
+static inline u32 xsk_pool_get_tailroom(bool mbuf)
+{
+ return mbuf ? SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) : 0;
+}
+
static inline u32 xsk_pool_get_chunk_size(struct xsk_buff_pool *pool)
{
return pool->chunk_size;
}
-static inline u32 xsk_pool_get_rx_frame_size(struct xsk_buff_pool *pool)
+static inline u32 __xsk_pool_get_rx_frame_size(struct xsk_buff_pool *pool)
{
return xsk_pool_get_chunk_size(pool) - xsk_pool_get_headroom(pool);
}
+static inline u32 xsk_pool_get_rx_frame_size(struct xsk_buff_pool *pool)
+{
+ u32 frame_size = __xsk_pool_get_rx_frame_size(pool);
+ struct xdp_umem *umem = pool->umem;
+ bool mbuf;
+
+ /* Reserve tailroom only for zero-copy pools that opted into
+ * multi-buffer. The reserved area is used for skb_shared_info,
+ * matching the XDP core's xdp_data_hard_end() layout.
+ */
+ mbuf = pool->dev && (umem->flags & XDP_UMEM_SG_FLAG);
+ frame_size -= xsk_pool_get_tailroom(mbuf);
+
+ return ALIGN_DOWN(frame_size, 128);
+}
+
static inline u32 xsk_pool_get_rx_frag_step(struct xsk_buff_pool *pool)
{
return pool->unaligned ? 0 : xsk_pool_get_chunk_size(pool);
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index c078c9e4b243..000cbfb66d5b 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -250,7 +250,7 @@ static u32 xsk_copy_xdp(void *to, void **from, u32 to_len,
static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
{
- u32 frame_size = xsk_pool_get_rx_frame_size(xs->pool);
+ u32 frame_size = __xsk_pool_get_rx_frame_size(xs->pool);
void *copy_from = xsk_copy_xdp_start(xdp), *copy_to;
u32 from_len, meta_len, rem, num_desc;
struct xdp_buff_xsk *xskb;
@@ -350,7 +350,7 @@ static int xsk_rcv_check(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index)
return -EINVAL;
- if (len > xsk_pool_get_rx_frame_size(xs->pool) && !xs->sg) {
+ if (len > __xsk_pool_get_rx_frame_size(xs->pool) && !xs->sg) {
xs->rx_dropped++;
return -ENOSPC;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v6 net 1/8] xsk: tighten UMEM headroom validation to account for tailroom and min frame
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski,
Stanislav Fomichev
In-Reply-To: <20260402154958.562179-1-maciej.fijalkowski@intel.com>
The current headroom validation in xdp_umem_reg() could leave us with
insufficient space dedicated to even receive minimum-sized ethernet
frame. Furthermore if multi-buffer would come to play then
skb_shared_info stored at the end of XSK frame would be corrupted.
HW typically works with 128-aligned sizes so let us provide this value
as bare minimum.
Multi-buffer setting is known later in the configuration process so
besides accounting for 128 bytes, let us also take care of tailroom space
upfront.
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Fixes: 99e3a236dd43 ("xsk: Add missing check on user supplied headroom size")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
net/xdp/xdp_umem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 066ce07c506d..58da2f4f4397 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -203,7 +203,8 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
if (!unaligned_chunks && chunks_rem)
return -EINVAL;
- if (headroom >= chunk_size - XDP_PACKET_HEADROOM)
+ if (headroom > chunk_size - XDP_PACKET_HEADROOM -
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - 128)
return -EINVAL;
if (mr->flags & XDP_UMEM_TX_METADATA_LEN) {
--
2.43.0
^ permalink raw reply related
* [PATCH v6 net 0/8] xsk: tailroom reservation and MTU validation
From: Maciej Fijalkowski @ 2026-04-02 15:49 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
v5->v6:
- refrain from relying on umem refcnt when disabling zc on error path
(Sashiko)
- have a separate __xsk_pool_get_rx_frame_size() which preserves old
behavior and use it on copy path (Sashiko)
- drop driver cleanups
v4->v5:
- add further review tags from Bjorn
- fix ./test_progs -t xsk
* do so by making the procfs reading common so test_xsk.c can use it
- get back to idea of relying on pool->umem->zc in
xsk_pool_get_tailroom(); that is because we now call its caller
(xsk_pool_get_rx_frame_size()) before getting down to driver via
ndo_bpf - we do it to rule out invalid setups in terms of MTU vs xsk
frame size/number of exposed segments by driver
- bring back explicit rejection of mbuf && segs == 1 combo
v3->v4:
- allow exact 128 bytes of space when user defined headroom is deducted
from total frame size
- provide a routine for reading procfs entries within xskxceiver
* use it to fetch cache line size and calculate skb_shared_info size
on our own
- clean up gve and igc xsk pool enablement routines
- include mtu vs frame size * max zc segments validation in
xp_assign_dev()
v2->v3:
- add tags from Bjorn/Stan
- provide at least 128 bytes instead ETH_ZLEN when validating frame
headroom
* this way we can get rid of i40e/ice changes
* make sure xsk_pool_get_rx_frame_size() returns value 128b-aligned
* and remove pre-check from idpf
- separate XDP_UMEM_SG_FLAG fixes from MTU validation in xsk_bind()
- make drop_idx a local variable in xsk's xdp drop prog
- adjust rx_dropped to new 128b related values
- move ugly placed define (Bjorn)
- remove READ_ONCE when fetching netdev->mtu (Bjorn)
v1->v2:
- remove xsk_pool_get_tailroom() definition for !CONFIG_XDP_SOCKETS
(Stan)
- do not rely on pool->umem->zc when configuring tailroom (Stan, Bjorn)
- simplify dbuff setting in ZC drivers (Bjorn)
- use defines for {head,tail}room in tests (Bjorn)
- return EINVAL instead of EOPNOTSUPP when mtu setting is wrong (Bjorn)
- include vlan headers and fcs length when validating mtu (Olek)
- tighten umem headroom validation when registering umem (Sashiko AI)
- set XDP_USE_SG in xp_assign_dev_shared() (Sashiko AI)
- adjust rx dropped xskxceiver test
Hi,
here we fix a long-standing issue regarding multi-buffer scenario in ZC
mode - we have not been providing space at the end of the buffer where
multi-buffer XDP works on skb_shared_info. This has been brought to our
attention via [0].
Unaligned mode does not get any specific treatment, it is user's
responsibility to properly handle XSK addresses in queues.
With adjustments included here in this set against xskxceiver I have
been able to pass the full test suite on ice.
Thanks,
Maciej
[0]: https://community.intel.com/t5/Ethernet-Products/X710-XDP-Packet-Corruption-Issue-DRV-MODE-Zero-Copy-Multi-Buffer/m-p/1724208
Maciej Fijalkowski (8):
xsk: tighten UMEM headroom validation to account for tailroom and min
frame
xsk: respect tailroom for ZC setups
xsk: fix XDP_UMEM_SG_FLAG issues
xsk: validate MTU against usable frame size on bind
selftests: bpf: introduce a common routine for reading procfs
selftests: bpf: fix pkt grow tests
selftests: bpf: have a separate variable for drop test
selftests: bpf: adjust rx_dropped xskxceiver's test to respect
tailroom
include/net/xdp_sock.h | 2 +-
include/net/xdp_sock_drv.h | 23 +++++++-
net/xdp/xdp_umem.c | 3 +-
net/xdp/xsk.c | 4 +-
net/xdp/xsk_buff_pool.c | 32 ++++++++++-
.../selftests/bpf/prog_tests/test_xsk.c | 55 +++++++++----------
.../selftests/bpf/prog_tests/test_xsk.h | 23 ++++++++
tools/testing/selftests/bpf/prog_tests/xsk.c | 19 +++++++
.../selftests/bpf/progs/xsk_xdp_progs.c | 4 +-
tools/testing/selftests/bpf/xskxceiver.c | 23 ++++++++
10 files changed, 150 insertions(+), 38 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH net 1/1] batman-adv: reject oversized global TT response buffers
From: Sven Eckelmann @ 2026-04-02 15:40 UTC (permalink / raw)
To: b.a.t.m.a.n, netdev, Ren Wei
Cc: Sven Eckelmann, marek.lindner, antonio, davem, edumazet, kuba,
pabeni, horms, yifanwucs, tomapufckgml, yuantan098, bird,
enjou1224z, caoruide123
In-Reply-To: <f1ae21c92be31b48651378f1ceba0dbbb43c2847.1774947926.git.caoruide123@gmail.com>
On Thu, 02 Apr 2026 23:12:31 +0800, Ren Wei wrote:
> batman-adv: reject oversized global TT response buffers
Applied, thanks!
[1/1] batman-adv: reject oversized global TT response buffers
https://git.open-mesh.org/linux-merge.git/commit/?h=batadv/net&id=3a359bf5c61d52e7f09754108309d637532164a6
Best regards,
--
Sven Eckelmann <sven@narfation.org>
^ permalink raw reply
* Re: [PATCH net] ipv6: avoid overflows in ip6_datagram_send_ctl()
From: patchwork-bot+netdevbpf @ 2026-04-02 15:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, horms, dsahern, netdev, eric.dumazet,
yimingqian591
In-Reply-To: <20260401154721.3740056-1-edumazet@google.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 1 Apr 2026 15:47:21 +0000 you wrote:
> Yiming Qian reported :
> <quote>
> I believe I found a locally triggerable kernel bug in the IPv6 sendmsg
> ancillary-data path that can panic the kernel via `skb_under_panic()`
> (local DoS).
>
> The core issue is a mismatch between:
>
> [...]
Here is the summary with links:
- [net] ipv6: avoid overflows in ip6_datagram_send_ctl()
https://git.kernel.org/netdev/net/c/4e453375561f
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] eth: fbnic: Increase FBNIC_QUEUE_SIZE_MIN to 64
From: patchwork-bot+netdevbpf @ 2026-04-02 15:40 UTC (permalink / raw)
To: Dimitri Daskalakis
Cc: davem, alexanderduyck, kuba, kernel-team, andrew+netdev, edumazet,
pabeni, mohsin.bashr, horms, jacob.e.keller, mike.marciniszyn,
daskald, bobbyeshleman, netdev
In-Reply-To: <20260401162848.2335350-1-dimitri.daskalakis1@gmail.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 1 Apr 2026 09:28:48 -0700 you wrote:
> From: Dimitri Daskalakis <daskald@meta.com>
>
> On systems with 64K pages, RX queues will be wedged if users set the
> descriptor count to the current minimum (16). Fbnic fragments large
> pages into 4K chunks, and scales down the ring size accordingly. With
> 64K pages and 16 descriptors, the ring size mask is 0 and will never
> be filled.
>
> [...]
Here is the summary with links:
- [net] eth: fbnic: Increase FBNIC_QUEUE_SIZE_MIN to 64
https://git.kernel.org/netdev/net/c/ec7067e66119
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 v4 0/2] net: hsr: fixes for PRP duplication and VLAN unwind
From: patchwork-bot+netdevbpf @ 2026-04-02 15:40 UTC (permalink / raw)
To: Luka Gejak
Cc: davem, edumazet, kuba, pabeni, netdev, fmaurer, horms, bigeasy,
m-karicheri2
In-Reply-To: <20260401092243.52121-1-luka.gejak@linux.dev>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 1 Apr 2026 11:22:41 +0200 you wrote:
> From: Luka Gejak <luka.gejak@linux.dev>
>
> This series addresses two logic bugs in the HSR/PRP implementation
> identified during a protocol audit. These are targeted for the 'net'
> tree as they fix potential memory corruption and state inconsistency.
>
> The primary change resolves a race condition in the node merging path by
> implementing address-based lock ordering. This ensures that concurrent
> mutations of sequence blocks do not lead to state corruption or
> deadlocks.
>
> [...]
Here is the summary with links:
- [net,v4,1/2] net: hsr: serialize seq_blocks merge across nodes
https://git.kernel.org/netdev/net/c/f5df2990c364
- [net,v4,2/2] net: hsr: fix VLAN add unwind on slave errors
https://git.kernel.org/netdev/net/c/2e3514e63bfb
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] net-timestamp: take track of the skb when wait_for_space occurs
From: Eric Dumazet @ 2026-04-02 15:39 UTC (permalink / raw)
To: Jason Xing
Cc: davem, kuba, pabeni, horms, willemb, netdev, Jason Xing,
Yushan Zhou
In-Reply-To: <CAL+tcoDiyptBUch5LZuRKn5BbJK_6TcgSi91D31axRtjqpSzzw@mail.gmail.com>
On Thu, Apr 2, 2026 at 8:03 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> On Thu, Apr 2, 2026 at 10:24 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Thu, Apr 2, 2026 at 1:58 AM Jason Xing <kerneljasonxing@gmail.com> wrote:
> > >
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > Tag the skb in tcp_sendmsg_locked() when wait_for_space occurs even
> > > though it might not carry the last byte of the sendmsg.
> > >
> > > If we don't do so, we might be faced with no single timestamp that
> > > can be received by application from the error queue. The following steps
> > > reproduce this:
> > > 1) skb A is the current last skb before entering wait_for_space process
> > > 2) tcp_push() pushes A without any tag
> > > 3) A is transmitted from TCP to driver without putting any skb carring
> > > timestamps in the error queue, like SCHED, DRV/HARDWARE.
> > > 4) sk_stream_wait_memory() sleeps for a while and then returns with an
> > > error code. Note that the socket lock is released.
> > > 5) skb A finally gets acked and removed from the rtx queue.
> > > 6) continue with the rest of tcp_sendmsg_locked(): it will jump to(goto)
> > > 'do_error' label and then 'out' label.
> > > 7) at this moment, skb A turns out to be the last one in this send
> > > syscall, and miss the following tcp_tx_timestamp() opportunity before
> > > the final tcp_push
> > > 8) application receives no timestamps this time
> > >
> > > The original commit ad02c4f54782 ("tcp: provide timestamps for partial writes")
> > > says it is best effort. Now it's time to cover the only potential point
> > > to avoid missing record.
> > >
> > > The side effect is obvious that we might record more than one time for a
> > > single send syscall since the skb that we keep track of in this scenario
> > > might not be the last one. But tracing more than one skb is not a bad
> > > thing since there is an emerging/promissing trend to do a detailed
> > > packet granularity monitor.
> >
> > This is rather weak/lazy, especially if you do not know how long the
> > thread is put to sleep?
>
> Thanks for the review!
>
> I actually thought about how to recognize which one should be the last
> skb in each send syscall. But it turns out that much more complicated
> work needs to be done which hurts the stack and common structures like
> tcp_sock. That's why I gave up and instead chose a much simpler way to
> minimize the impact.
>
> >
> > >
> > > Thanks to the great ID, namely, tskey, application that is responsible
> > > for the collect/sort of timestamps leverages it to put that record in
> > > between two consecutive send syscalls correctly.
> > >
> > > Signed-off-by: Yushan Zhou <katrinzhou@tencent.com>
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > ---
> > > net/ipv4/tcp.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > > index 516087c622ad..2db80d75cfa4 100644
> > > --- a/net/ipv4/tcp.c
> > > +++ b/net/ipv4/tcp.c
> > > @@ -1411,9 +1411,11 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> > > wait_for_space:
> > > set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
> > > tcp_remove_empty_skb(sk);
> > > - if (copied)
> > > + if (copied) {
> > > + tcp_tx_timestamp(sk, &sockc);
> > > tcp_push(sk, flags & ~MSG_MORE, mss_now,
> > > TCP_NAGLE_PUSH, size_goal);
> > > + }
> > >
> > > err = sk_stream_wait_memory(sk, &timeo);
> > > if (err != 0)
> >
> > I think non blocking model should be used by modern applications,
> > especially ones caring about timestamps.
>
> Please note that BPF timestamping has already been merged. It's a
> standalone script that monitors all kinds of flows and discovers those
> strange/unexpected behaviors. So we normally don't take a look at the
> implementation of each application before spotting any exceptions.
>
> Besides, sometimes monitoring the latency of the host doesn't have
> much to do with the mode of application. People might want to observe
> more deeper into the underlying layer. Right now, without the patch,
> the monitor possibly ends up missing the record of this send syscall,
> which has been reliably reproduced by Yushan.
>
> >
> > This patch has performance implications.
> >
> > tcp_tx_timestamp() is quite big and was inlined because it had a single caller.
> >
> > After this patch, it is no longer inlined.
> >
> > scripts/bloat-o-meter -t vmlinux.before vmlinux.after
> > add/remove: 2/0 grow/shrink: 0/1 up/down: 239/-192 (47)
> > Function old new delta
> > tcp_tx_timestamp - 223 +223
> > __pfx_tcp_tx_timestamp - 16 +16
> > tcp_sendmsg_locked 4560 4368 -192
> > Total: Before=29652698, After=29652745, chg +0.00%
>
> That's right and I really appreciate your recent great effort trying
> to mitigate the impact of function calls.
>
> My take is that it is a trade off. Adding one more track of the skb
> (which adds a function call) actually doesn't hurt much especially for
> this scenario and the last skb scenario. This path basically is not
> that hot. There are some left things to be done to improve the
> socket/BPF timestamping feature. And I plan to push more patches
> (sure, very carefully) to enhance the ability of BPF timestamping to
> observe TCP which is inevitable to add some extra functions and if
> statements.
>
> I'm wondering if the above makes sense to you.
If you mostly care about BPF, I would suggest:
iff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index bd2c3c4587e133897aa057bd3b26c29df050607f..444254b8870c4406a7105cd3ce177745fb1b6c0a
100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -506,6 +506,20 @@ static void tcp_tx_timestamp(struct sock *sk,
struct sockcm_cookie *sockc)
bpf_skops_tx_timestamping(sk, skb,
BPF_SOCK_OPS_TSTAMP_SENDMSG_CB);
}
+static void tcp_tx_timestamp_before_wait_memory(struct sock *sk)
+{
+ struct sk_buff *skb;
+
+ if (!cgroup_bpf_enabled(CGROUP_SOCK_OPS) ||
+ !SK_BPF_CB_FLAG_TEST(sk, SK_BPF_CB_TX_TIMESTAMPING))
+ return;
+
+ skb = tcp_write_queue_tail(sk);
+ if (skb)
+ bpf_skops_tx_timestamping(sk, skb,
+ BPF_SOCK_OPS_TSTAMP_SENDMSG_CB);
+}
+
/* @wake is one when sk_stream_write_space() calls us.
* This sends EPOLLOUT only if notsent_bytes is half the limit.
* This mimics the strategy used in sock_def_write_space().
@@ -1403,10 +1417,11 @@ int tcp_sendmsg_locked(struct sock *sk, struct
msghdr *msg, size_t size)
wait_for_space:
set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
tcp_remove_empty_skb(sk);
- if (copied)
+ if (copied) {
+ tcp_tx_timestamp_before_wait_memory(sk);
tcp_push(sk, flags & ~MSG_MORE, mss_now,
TCP_NAGLE_PUSH, size_goal);
-
+ }
err = sk_stream_wait_memory(sk, &timeo);
if (err != 0)
goto do_error;
Note: You also could use a different skops to differentiate cases in
your monitoring.
$ scripts/bloat-o-meter -t vmlinux.old vmlinux.new
add/remove: 0/0 grow/shrink: 1/0 up/down: 69/0 (69)
Function old new delta
tcp_sendmsg_locked 4848 4917 +69
Total: Before=29651496, After=29651565, chg +0.00%
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] net: call getsockopt_iter if available
From: Breno Leitao @ 2026-04-02 15:39 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Willem de Bruijn, metze, axboe,
Stanislav Fomichev, io-uring, bpf, netdev, Linus Torvalds,
linux-kernel, kernel-team
In-Reply-To: <ac1fjvVDfatpXwPY@mini-arch>
Hello Stanislav,
On Wed, Apr 01, 2026 at 11:10:22AM -0700, Stanislav Fomichev wrote:
> So maybe something like this is better to communicate your long term intent?
>
> } else if (ops->getsockopt_iter) {
> optval = sockptr_to_iter(optval)
> optlen = sockptr_to_iter(optlen)
> do_sock_getsockopt_iter(...) /* does not know what sockpt_t is */
> }
>
> ?
>
> Then your new do_sock_getsockopt_iter is sockptr-free from the beginning
> and at some point we'll just drop/move those sockptr_to_iter calls?
Sure, that would work as well. It would look like the following, from my
current implemention:
+static int sockptr_to_sockopt(sockopt_t *opt, sockptr_t optval,
+ sockptr_t optlen, struct kvec *kvec)
+{
+ int koptlen;
+
+ if (copy_from_sockptr(&koptlen, optlen, sizeof(int)))
+ return -EFAULT;
+
+ if (optval.is_kernel) {
+ kvec->iov_base = optval.kernel;
+ kvec->iov_len = koptlen;
+ iov_iter_kvec(&opt->iter_out, ITER_DEST, kvec, 1, koptlen);
+ iov_iter_kvec(&opt->iter_in, ITER_SOURCE, kvec, 1, koptlen);
+ } else {
+ iov_iter_ubuf(&opt->iter_out, ITER_DEST, optval.user, koptlen);
+ iov_iter_ubuf(&opt->iter_in, ITER_SOURCE, optval.user,
+ koptlen);
+ }
+ opt->optlen = koptlen;
+
+ return 0;
+}
+
int do_sock_getsockopt(struct socket *sock, bool compat, int level,
int optname, sockptr_t optval, sockptr_t optlen)
{
@@ -2366,15 +2390,31 @@ int do_sock_getsockopt(struct socket *sock, bool compat, int level,
+ } else if (ops->getsockopt_iter) {
+ struct kvec kvec;
+ sockopt_t opt;
+
+ err = sockptr_to_sockopt(&opt, optval, optlen, &kvec);
+ if (err)
+ return err;
+
+ err = ops->getsockopt_iter(sock, level, optname, &opt);
+
+ /* Always write back optlen, even on failure. Some protocols
+ * (e.g. CAN raw) return -ERANGE and set optlen to the
+ * required buffer size so userspace can discover it.
+ */
+ if (copy_to_sockptr(optlen, &opt.optlen, sizeof(int)))
+ return -EFAULT;
+ } else if (ops->getsockopt) {
....
> I hope this way it will be easier to review protocol handler changes.
>
> For example, looking at your AF_PACKET patch, you won't have to care
> about flipping the source and doing the revert. Most/all of the changes will
> be simple:
> - s/get_user(len, optlen)/len = opt->optlen/
> - s/put_user(len, optlen)/opt->optlen = len/
> - s/copy_from_user(xxx, optval, len)/copy_from_iter(xxx, len, &opt->iter_in)/
> - s/copy_to_user(optval, xxx, len)/copy_to_iter(xxx, len, &opt->iter_out)/
That is, in fact, a great proposal. It will make the protocol changes review
way easier.
This is what I have right now.
typedef struct sockopt {
struct iov_iter iter_out;
struct iov_iter iter_in;
int optlen;
} sockopt_t;
And then, the drivers change would be as simple as:
static int packet_getsockopt(struct socket *sock, int level, int optname,
- char __user *optval, int __user *optlen)
+ sockopt_t *opt)
{
int len;
int val, lv = sizeof(val);
@@ -4065,8 +4066,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
if (level != SOL_PACKET)
return -ENOPROTOOPT;
- if (get_user(len, optlen))
- return -EFAULT;
+ len = opt->optlen;
if (len < 0)
return -EINVAL;
@@ -4115,7 +4115,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
len = sizeof(int);
if (len < sizeof(int))
return -EINVAL;
- if (copy_from_user(&val, optval, len))
+ if (copy_from_iter(&val, len, &opt->iter_in) != len)
return -EFAULT;
switch (val) {
case TPACKET_V1:
@@ -4171,9 +4171,8 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
if (len > lv)
len = lv;
- if (put_user(len, optlen))
- return -EFAULT;
- if (copy_to_user(optval, data, len))
+ opt->optlen = len;
+ if (copy_to_iter(data, len, &opt->iter_out) != len)
return -EFAULT;
return 0;
This is not fully tested yet, but, in case you want to see how this looks like
so far, I have it in https://github.com/leitao/linux/tree/b4/getsockopt_v3.
I will submit a newer version after I am done with the testing.
Thanks for the insights,
--breno
^ permalink raw reply
* Re: [PATCH net-next v2 3/3] gve: implement PTP gettimex64
From: Jordan Rhee @ 2026-04-02 15:38 UTC (permalink / raw)
To: Naman Gulati
Cc: Jakub Kicinski, hramamurthy, netdev, joshwash, andrew+netdev,
davem, edumazet, pabeni, richardcochran, willemb, nktgrg, jfraker,
ziweixiao, maolson, thostet, jefrogers, alok.a.tiwari, yyd,
linux-kernel
In-Reply-To: <CAMP57yWZkgU989sN38ycxiR1p5w48mssAwycZY4BZS28nDObRw@mail.gmail.com>
We know that gve_adminq_report_nic_ts() will cause a VM exit, which
will act as a full barrier. Additional barriers are not strictly
necessary from a functional perspective. Can we rely on this fact or
do we need explicit barriers?
On Tue, Mar 31, 2026 at 11:05 AM Naman Gulati <namangulati@google.com> wrote:
>
> On Sun, Mar 29, 2026 at 9:31 PM Jordan Rhee <jordanrhee@google.com> wrote:
> >
> > On Sun, Mar 29, 2026 at 2:20 PM Jakub Kicinski <kuba@kernel.org> 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.
> > > ---
> > > Subject: gve: implement PTP gettimex64
> > >
> > > This commit implements the PTP gettimex64 function to enable chrony and
> > > phc2sys to synchronize the system clock to the NIC clock. It reads the
> > > NIC timestamp along with pre and post system cycle counters to minimize
> > > the uncertainty window.
> > >
> > > > diff --git a/drivers/net/ethernet/google/gve/gve_ptp.c b/drivers/net/ethernet/google/gve/gve_ptp.c
> > > > index 140b8fbce4f4..bea794541f30 100644
> > > > --- a/drivers/net/ethernet/google/gve/gve_ptp.c
> > > > +++ b/drivers/net/ethernet/google/gve/gve_ptp.c
> > > > @@ -10,28 +10,203 @@
> > >
> > > [ ... ]
> > >
> > > > +static int gve_adminq_read_timestamp(struct gve_priv *priv,
> > > > + cycles_t *pre_cycles,
> > > > + cycles_t *post_cycles)
> > > > +{
> > > > + unsigned long delay_us = 1000;
> > > > + int retry_count = 0;
> > > > + int err;
> > > > +
> > > > + lockdep_assert_held(&priv->nic_ts_read_lock);
> > > > +
> > > > + do {
> > > > + *pre_cycles = get_cycles();
> > > > + err = gve_adminq_report_nic_ts(priv, priv->nic_ts_report_bus);
> > > > +
> > > > + /* Ensure cycle counter is sampled after AdminQ cmd returns */
> > > > + rmb();
> > > > + *post_cycles = get_cycles();
> > >
> > > Are execution barriers needed around get_cycles() here? rmb() orders
> > > memory accesses, but the CPU might still speculatively execute the cycle
> > > counter reads out of order with respect to the AdminQ commands.
> > >
> > > Could this speculative reordering cause the strict bounds validation in
> > > gve_ptp_gettimex64() to fail spuriously?
> >
> > I will add mb() barriers to be totally safe.
>
> I'm not so sure that rmb() is insufficient here. In x86_64 rmb() is an
> lfence. lfence is used to prevent speculative execution (per the x86
> barrier_nospec implementation) and rdtsc_ordered, which is called by
> the ktime* helpers, also prefaces rdtsc with an lfence. On ARM the
> get_cycles implementation adds an isb instruction before reading
> cntvct_el0.
>
>
> >
> > > [ ... ]
> > >
> > > > static int gve_ptp_gettimex64(struct ptp_clock_info *info,
> > > > struct timespec64 *ts,
> > > > struct ptp_system_timestamp *sts)
> > > > {
> > > > - return -EOPNOTSUPP;
> > > > + struct gve_ptp *ptp = container_of(info, struct gve_ptp, info);
> > > > + struct gve_sysclock_sample sysclock = {0};
> > > > + struct gve_priv *priv = ptp->priv;
> > > > + struct system_time_snapshot snap;
> > > > + u64 nic_ts;
> > > > + int err;
> > > > +
> > > > + /* Take system clock snapshot before sampling cycle counters */
> > > > + if (sts)
> > > > + ktime_get_snapshot(&snap);
> > > > +
> > > > + err = gve_clock_nic_ts_read(priv, &nic_ts, &sysclock);
> > >
> > > Could the system time snapshot become stale here?
> > >
> > > If priv->nic_ts_read_lock is contended, or if gve_adminq_read_timestamp()
> > > retries and sleeps, the snapshot might be taken significantly before the
> > > actual hardware read.
> > >
> > > Would it be more accurate to take the snapshot immediately before reading the
> > > hardware, inside the mutex?
> >
> > Ack, will fix.
> >
> > >
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + if (sts) {
> > > > + /* Reject samples with out of order system clock values */
> > > > + if (!(sysclock.host_pre_cycles <= sysclock.nic_pre_cycles &&
> > > > + sysclock.nic_pre_cycles <= sysclock.nic_post_cycles &&
> > > > + sysclock.nic_post_cycles <= sysclock.host_post_cycles)) {
> > > > + dev_err_ratelimited(&priv->pdev->dev,
> > > > + "AdminQ system clock cycle counts out of order. Expecting %llu <= %llu <= %llu <= %llu\n",
> > > > + (u64)sysclock.host_pre_cycles,
> > > > + sysclock.nic_pre_cycles,
> > > > + sysclock.nic_post_cycles,
> > > > + (u64)sysclock.host_post_cycles);
> > > > + return -EBADMSG;
> > >
> > > How does this behave on architectures where get_cycles() is not implemented
> > > and returns 0, or with older firmware that leaves nic_pre_cycles and
> > > nic_post_cycles as 0?
> >
> > Firmware is not allowed to return 0. 0 should trigger the check.
> >
> > > It looks like the bounds check would unconditionally fail and return -EBADMSG.
> > > Since userspace tools like chrony usually expect -EOPNOTSUPP for unsupported
> > > extended ioctls to fall back to basic PTP ioctls, will returning -EBADMSG
> > > cause a hard failure for clock synchronization?
> >
> > Will fix. GVE is only supported on x86_64 and arm64, and will fail on
> > other architectures with -EOPNOTSUPP due to the check in
> > gve_cycles_to_clock_fn(). However, if an architecture returns
> > get_cycles() as 0, it will fail here first with -EBADMSG. I will add
> > an explicit check for x86 || arm64 earlier in this function.
> >
> > >
> > > [ ... ]
> > >
> > > > @@ -132,7 +307,7 @@ int gve_init_clock(struct gve_priv *priv)
> > > > goto release_ptp;
> > > > }
> > > > mutex_init(&priv->nic_ts_read_lock);
> > > > - err = gve_clock_nic_ts_read(priv, &nic_raw);
> > > > + err = gve_clock_nic_ts_read(priv, &nic_raw, NULL);
> > > > if (err) {
> > > > dev_err(&priv->pdev->dev, "failed to read NIC clock %d\n", err);
> > > > goto release_nic_ts_report;
> > >
> > > Is there a race condition exposing the PTP device to userspace before its
> > > resources are fully initialized?
> > >
> > > Looking at gve_init_clock(), gve_ptp_init() is called before allocating
> > > nic_ts_report and initializing nic_ts_read_lock:
> > >
> > > err = gve_ptp_init(priv);
> > > if (err)
> > > return err;
> > >
> > > priv->nic_ts_report = dma_alloc_coherent(...);
> > > ...
> > > mutex_init(&priv->nic_ts_read_lock);
> > >
> > > If a concurrent userspace process immediately invokes the
> > > PTP_SYS_OFFSET_EXTENDED ioctl after gve_ptp_init() registers the /dev/ptpX
> > > device, could it call gve_ptp_gettimex64() and attempt to lock the
> > > uninitialized mutex or dereference the NULL nic_ts_report pointer?
> > >
> > > Additionally, in the error path for gve_init_clock():
> > >
> > > release_nic_ts_report:
> > > mutex_destroy(&priv->nic_ts_read_lock);
> > > dma_free_coherent(...);
> > > priv->nic_ts_report = NULL;
> > > release_ptp:
> > > gve_ptp_release(priv);
> > >
> > > Could destroying the mutex and freeing the memory before gve_ptp_release()
> > > create a use-after-free window if an ioctl is currently running?
> >
> > Will be fixed in the previous patch in the series.
^ permalink raw reply
* Re: [PATCH net 2/3] net: enetc: pad short frames in software
From: Jakub Kicinski @ 2026-04-02 15:37 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, Zefir Kurtisi, Claudiu Manoil, Wei Fang, Clark Wang,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Ioana Ciornei, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
Simon Horman, bpf, imx, linux-kernel
In-Reply-To: <20260401172246.1075883-3-vladimir.oltean@nxp.com>
On Wed, 1 Apr 2026 20:22:45 +0300 Vladimir Oltean wrote:
> The ENETC does not support BUF_LEN or FRM_LEN in TX buffer descriptors
> less than 16. This is written in the reference manual of all SoCs
> supported by the driver: LS1028A, i.MX943, i.MX95 etc.
>
> Frames must not have a FRM_LEN that is less than 16 bytes. Frames of
> 0-15 bytes are not supported.
> (...)
> The first descriptor in a chain must not have a BUFF_LEN that is less
> than 16 bytes.
>
> I don't think proper attention was paid to this during development, we
> found the text at the end of a bug investigation. Therefore, the driver
> does not enforce this.
AI points out that the frame may be longer than 16B but fragmented.
Only the Ethernet header is guaranteed to be in the linear part.
So you may need to also toss something like pskb_may_pull(16) after
that padding call.
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net] vsock: initialize child_ns_mode_locked in vsock_net_init()
From: patchwork-bot+netdevbpf @ 2026-04-02 15:30 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, davem, linux-kernel, bobbyeshleman, kuba, horms,
virtualization, edumazet, pabeni, jinl
In-Reply-To: <20260401092153.28462-1-sgarzare@redhat.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 1 Apr 2026 11:21:53 +0200 you wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> The `child_ns_mode_locked` field lives in `struct net`, which persists
> across vsock module reloads. When the module is unloaded and reloaded,
> `vsock_net_init()` resets `mode` and `child_ns_mode` back to their
> default values, but does not reset `child_ns_mode_locked`.
>
> [...]
Here is the summary with links:
- [net] vsock: initialize child_ns_mode_locked in vsock_net_init()
https://git.kernel.org/netdev/net/c/b18c83388874
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH net-next] net: always inline some skb helpers
From: Eric Dumazet @ 2026-04-02 15:26 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, netdev, eric.dumazet,
Eric Dumazet
Some performance critical helpers from include/linux/skbuff.h
are not inlined by clang.
Use __always_inline hint for:
- __skb_fill_netmem_desc()
- __skb_fill_page_desc()
- skb_fill_netmem_desc()
- skb_fill_page_desc()
- __skb_pull()
- pskb_may_pull_reason()
- pskb_may_pull()
- pskb_pull()
- pskb_trim()
- skb_orphan()
- skb_postpull_rcsum()
- skb_header_pointer()
- skb_clear_delivery_time()
- skb_tstamp_cond()
- skb_warn_if_lro()
This increases performance and saves ~1200 bytes of text.
$ scripts/bloat-o-meter -t vmlinux.old vmlinux.new
add/remove: 4/24 grow/shrink: 66/12 up/down: 4104/-5306 (-1202)
Function old new delta
ip_multipath_l3_keys - 303 +303
tcp_sendmsg_locked 4560 4848 +288
xfrm_input 6240 6455 +215
esp_output_head 1516 1711 +195
skb_try_coalesce 696 866 +170
bpf_prog_test_run_skb 1951 2091 +140
tls_strp_read_copy 528 667 +139
gue_udp_recv 738 871 +133
__ip6_append_data 4159 4279 +120
__bond_xmit_hash 1019 1122 +103
ip6_multipath_l3_keys 394 495 +101
bpf_lwt_seg6_action 1096 1197 +101
input_action_end_dx2 344 442 +98
vxlan_remcsum 487 581 +94
udpv6_queue_rcv_skb 393 480 +87
udp_queue_rcv_skb 385 471 +86
gue_remcsum 453 539 +86
udp_lib_checksum_complete 84 168 +84
vxlan_xmit 2777 2857 +80
nf_reset_ct 456 532 +76
igmp_rcv 1902 1978 +76
mpls_forward 1097 1169 +72
tcp_add_backlog 1226 1292 +66
nfulnl_log_packet 3091 3156 +65
tcp_rcv_established 1966 2026 +60
__strp_recv 1547 1603 +56
eth_type_trans 357 411 +54
bond_flow_ip 392 444 +52
__icmp_send 1584 1630 +46
ip_defrag 1636 1681 +45
tpacket_rcv 2793 2837 +44
refcount_add 132 176 +44
nf_ct_frag6_gather 1959 2003 +44
napi_skb_free_stolen_head 199 240 +41
__pskb_trim - 41 +41
napi_reuse_skb 319 358 +39
icmpv6_rcv 1877 1916 +39
br_handle_frame_finish 1672 1711 +39
ip_rcv_core 841 879 +38
ip_check_defrag 377 415 +38
br_stp_rcv 909 947 +38
qdisc_pkt_len_segs_init 366 399 +33
mld_query_work 2945 2975 +30
bpf_sk_assign_tcp_reqsk 607 637 +30
udp_gro_receive 1657 1686 +29
ip6_rcv_core 1170 1193 +23
ah_input 1176 1197 +21
tun_get_user 5174 5194 +20
llc_rcv 815 834 +19
__pfx_udp_lib_checksum_complete 16 32 +16
__pfx_refcount_add 48 64 +16
__pfx_nf_reset_ct 96 112 +16
__pfx_ip_multipath_l3_keys - 16 +16
__pfx___pskb_trim - 16 +16
packet_sendmsg 5771 5781 +10
esp_output_tail 1460 1470 +10
alloc_skb_with_frags 433 443 +10
xsk_generic_xmit 3477 3486 +9
mptcp_sendmsg_frag 2250 2259 +9
__ip_append_data 4166 4175 +9
__ip6_tnl_rcv 1159 1168 +9
skb_zerocopy 1215 1220 +5
gre_parse_header 1358 1362 +4
__iptunnel_pull_header 405 407 +2
skb_vlan_untag 692 693 +1
psp_dev_rcv 701 702 +1
netkit_xmit 1263 1264 +1
gre_rcv 2776 2777 +1
gre_gso_segment 1521 1522 +1
bpf_skb_net_hdr_pop 535 536 +1
udp6_ufo_fragment 888 884 -4
br_multicast_rcv 9154 9148 -6
snap_rcv 312 305 -7
skb_copy_ubufs 1841 1834 -7
__pfx_skb_tstamp_cond 16 - -16
__pfx_skb_clear_delivery_time 16 - -16
__pfx_pskb_trim 16 - -16
__pfx_pskb_pull 16 - -16
ipv6_gso_segment 1400 1383 -17
ipv6_frag_rcv 2511 2492 -19
erspan_xmit 1221 1190 -31
__pfx_skb_warn_if_lro 32 - -32
__pfx___skb_fill_page_desc 32 - -32
skb_tstamp_cond 42 - -42
pskb_trim 46 - -46
__pfx_skb_postpull_rcsum 48 - -48
tcp_gso_segment 1524 1475 -49
skb_clear_delivery_time 54 - -54
__pfx_skb_fill_page_desc 64 - -64
__pfx_skb_header_pointer 80 - -80
pskb_pull 91 - -91
skb_warn_if_lro 110 - -110
tcp_v6_rcv 3288 3170 -118
__pfx___skb_pull 128 - -128
__pfx_skb_orphan 144 - -144
__pfx_pskb_may_pull 160 - -160
tcp_v4_rcv 3334 3153 -181
__skb_fill_page_desc 231 - -231
udp_rcv 1809 1553 -256
skb_postpull_rcsum 318 - -318
skb_header_pointer 367 - -367
fib_multipath_hash 3399 3018 -381
skb_orphan 513 - -513
skb_fill_page_desc 534 - -534
__skb_pull 568 - -568
pskb_may_pull 604 - -604
Total: Before=29652698, After=29651496, chg -0.00%
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/skbuff.h | 46 ++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9cc98f850f1d7cd01eed3fe9d17b59116b49958e..98e87bda9fa5add3e778d704373b300fc86e474a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2605,8 +2605,9 @@ static inline void skb_len_add(struct sk_buff *skb, int delta)
*
* Does not take any additional reference on the fragment.
*/
-static inline void __skb_fill_netmem_desc(struct sk_buff *skb, int i,
- netmem_ref netmem, int off, int size)
+static __always_inline void
+__skb_fill_netmem_desc(struct sk_buff *skb, int i, netmem_ref netmem,
+ int off, int size)
{
struct page *page;
@@ -2628,14 +2629,16 @@ static inline void __skb_fill_netmem_desc(struct sk_buff *skb, int i,
skb->pfmemalloc = true;
}
-static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
- struct page *page, int off, int size)
+static __always_inline void
+__skb_fill_page_desc(struct sk_buff *skb, int i, struct page *page,
+ int off, int size)
{
__skb_fill_netmem_desc(skb, i, page_to_netmem(page), off, size);
}
-static inline void skb_fill_netmem_desc(struct sk_buff *skb, int i,
- netmem_ref netmem, int off, int size)
+static __always_inline void
+skb_fill_netmem_desc(struct sk_buff *skb, int i, netmem_ref netmem,
+ int off, int size)
{
__skb_fill_netmem_desc(skb, i, netmem, off, size);
skb_shinfo(skb)->nr_frags = i + 1;
@@ -2655,8 +2658,9 @@ static inline void skb_fill_netmem_desc(struct sk_buff *skb, int i,
*
* Does not take any additional reference on the fragment.
*/
-static inline void skb_fill_page_desc(struct sk_buff *skb, int i,
- struct page *page, int off, int size)
+static __always_inline void
+skb_fill_page_desc(struct sk_buff *skb, int i, struct page *page,
+ int off, int size)
{
skb_fill_netmem_desc(skb, i, page_to_netmem(page), off, size);
}
@@ -2828,7 +2832,7 @@ static inline void *__skb_push(struct sk_buff *skb, unsigned int len)
}
void *skb_pull(struct sk_buff *skb, unsigned int len);
-static inline void *__skb_pull(struct sk_buff *skb, unsigned int len)
+static __always_inline void *__skb_pull(struct sk_buff *skb, unsigned int len)
{
DEBUG_NET_WARN_ON_ONCE(len > INT_MAX);
@@ -2853,7 +2857,7 @@ void *skb_pull_data(struct sk_buff *skb, size_t len);
void *__pskb_pull_tail(struct sk_buff *skb, int delta);
-static inline enum skb_drop_reason
+static __always_inline enum skb_drop_reason
pskb_may_pull_reason(struct sk_buff *skb, unsigned int len)
{
DEBUG_NET_WARN_ON_ONCE(len > INT_MAX);
@@ -2871,12 +2875,13 @@ pskb_may_pull_reason(struct sk_buff *skb, unsigned int len)
return SKB_NOT_DROPPED_YET;
}
-static inline bool pskb_may_pull(struct sk_buff *skb, unsigned int len)
+static __always_inline bool
+pskb_may_pull(struct sk_buff *skb, unsigned int len)
{
return pskb_may_pull_reason(skb, len) == SKB_NOT_DROPPED_YET;
}
-static inline void *pskb_pull(struct sk_buff *skb, unsigned int len)
+static __always_inline void *pskb_pull(struct sk_buff *skb, unsigned int len)
{
if (!pskb_may_pull(skb, len))
return NULL;
@@ -3337,7 +3342,7 @@ static inline int __pskb_trim(struct sk_buff *skb, unsigned int len)
return 0;
}
-static inline int pskb_trim(struct sk_buff *skb, unsigned int len)
+static __always_inline int pskb_trim(struct sk_buff *skb, unsigned int len)
{
skb_might_realloc(skb);
return (len < skb->len) ? __pskb_trim(skb, len) : 0;
@@ -3380,7 +3385,7 @@ static inline int __skb_grow(struct sk_buff *skb, unsigned int len)
* destructor function and make the @skb unowned. The buffer continues
* to exist but is no longer charged to its former owner.
*/
-static inline void skb_orphan(struct sk_buff *skb)
+static __always_inline void skb_orphan(struct sk_buff *skb)
{
if (skb->destructor) {
skb->destructor(skb);
@@ -4044,8 +4049,8 @@ __skb_postpull_rcsum(struct sk_buff *skb, const void *start, unsigned int len,
* update the CHECKSUM_COMPLETE checksum, or set ip_summed to
* CHECKSUM_NONE so that it can be recomputed from scratch.
*/
-static inline void skb_postpull_rcsum(struct sk_buff *skb,
- const void *start, unsigned int len)
+static __always_inline void
+skb_postpull_rcsum(struct sk_buff *skb, const void *start, unsigned int len)
{
if (skb->ip_summed == CHECKSUM_COMPLETE)
skb->csum = wsum_negate(csum_partial(start, len,
@@ -4304,7 +4309,7 @@ __skb_header_pointer(const struct sk_buff *skb, int offset, int len,
return buffer;
}
-static inline void * __must_check
+static __always_inline void * __must_check
skb_header_pointer(const struct sk_buff *skb, int offset, int len, void *buffer)
{
return __skb_header_pointer(skb, offset, len, skb->data,
@@ -4476,7 +4481,7 @@ DECLARE_STATIC_KEY_FALSE(netstamp_needed_key);
/* It is used in the ingress path to clear the delivery_time.
* If needed, set the skb->tstamp to the (rcv) timestamp.
*/
-static inline void skb_clear_delivery_time(struct sk_buff *skb)
+static __always_inline void skb_clear_delivery_time(struct sk_buff *skb)
{
if (skb->tstamp_type) {
skb->tstamp_type = SKB_CLOCK_REALTIME;
@@ -4503,7 +4508,8 @@ static inline ktime_t skb_tstamp(const struct sk_buff *skb)
return skb->tstamp;
}
-static inline ktime_t skb_tstamp_cond(const struct sk_buff *skb, bool cond)
+static __always_inline ktime_t
+skb_tstamp_cond(const struct sk_buff *skb, bool cond)
{
if (skb->tstamp_type != SKB_CLOCK_MONOTONIC && skb->tstamp)
return skb->tstamp;
@@ -5292,7 +5298,7 @@ static inline void skb_decrease_gso_size(struct skb_shared_info *shinfo,
void __skb_warn_lro_forwarding(const struct sk_buff *skb);
-static inline bool skb_warn_if_lro(const struct sk_buff *skb)
+static __always_inline bool skb_warn_if_lro(const struct sk_buff *skb)
{
/* LRO sets gso_size but not gso_type, whereas if GSO is really
* wanted then gso_type will be set. */
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* Re: [net-next v36] mctp pcc: Implement MCTP over PCC Transport
From: Adam Young @ 2026-04-02 15:26 UTC (permalink / raw)
To: Jeremy Kerr, Adam Young, Matt Johnston, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Sudeep Holla, Jonathan Cameron, Huisong Li
In-Reply-To: <4349e9f1668e443f49f193e1ab69cef669941c61.camel@codeconstruct.com.au>
On 4/1/26 22:16, Jeremy Kerr wrote:
> Hi Adam,
>
>> Implementation of network driver for
>> Management Component Transport Protocol(MCTP)
>> over Platform Communication Channel(PCC)
>>
>> DMTF DSP:0292
>> Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0292_1.0.0WIP50.pdf
>>
>> The transport mechanism is called Platform Communication Channels (PCC)
>> is part of the ACPI spec:
>>
>> Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html
>>
>> The PCC mechanism is managed via a mailbox implemented at
>> drivers/mailbox/pcc.c
>>
>> MCTP devices are specified via ACPI by entries in DSDT/SSDT and
>> reference channels specified in the PCCT. Messages are sent on a type
>> 3 and received on a type 4 channel. Communication with other devices
>> use the PCC based doorbell mechanism; a shared memory segment with a
>> corresponding interrupt and a memory register used to trigger remote
>> interrupts.
>>
>> The shared buffer must be at least 68 bytes long as that is the minimum
>> MTU as defined by the MCTP specification.
>>
>> Unlike the existing PCC Type 2 based drivers, the mssg parameter to
>> mbox_send_msg is actively used. The data section of the struct sk_buff
>> that contains the outgoing packet is sent to the mailbox, already
>> properly formatted as a PCC exctended message.
>>
>> If the mailbox ring buffer is full, the driver stops the incoming
>> packet queues until a message has been sent, freeing space in the
>> ring buffer.
>>
>> When the Type 3 channel outbox receives a txdone response interrupt,
>> it consumes the outgoing sk_buff, allowing it to be freed.
>>
>> Bringing up an interface creates the channel between the network driver
>> and the mailbox driver. This enables communication with the remote
>> endpoint, to include the receipt of new messages. Bringing down an
>> interface removes the channel, and no new messages can be delivered.
>> Stopping the interface will leave any packets that are cached in the
>> mailbox ringbuffer. They cannot safely be freed until the PCC mailbox
>> attempts to deliver them and has removed them from the ring buffer.
>>
>> PCC is based on a shared buffer and a set of I/O mapped memory locations
>> that the Spec calls registers. This mechanism exists regardless of the
>> existence of the driver. If the user has the ability to map these
>> physical location to virtual locations, they have the ability to drive the
>> hardware. Thus, there is a security aspect to this mechanism that extends
>> beyond the responsibilities of the operating system.
>>
>> If the hardware does not expose the PCC in the ACPI table, this device
>> will never be enabled. Thus it is only an issue on hardware that does
>> support PCC. In that case, it is up to the remote controller to sanitize
>> communication; MCTP will be exposed as a socket interface, and userland
>> can send any crafted packet it wants. It would also be incumbent on
>> the hardware manufacturer to allow the end user to disable MCTP over PCC
>> communication if they did not want to expose it.
>>
>> Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
> No changelogs anymore? This makes things more difficult to review. Just
> to be clear, when I mentioned changing to a single-patch series, that
> did not mean to throw out the changelog - it's still a key part of
> review.
Sorry about that. Your list below is accurate.
>
> So, for v36, it looks like you have
>
> * added the CONFIG_PCC dependency
Yes. THis was from Jakub's review
> * altered the RX min length check to include at least the command and
> a MCTP header
Yes. Again, from Jakub's review as well as thinking through what the
most minimal acceptable message would be.
> * added a RX max-length check
Correct. If the max-length is greater than the buffer something is
misconfigured.
> * added a minimum MTU check
> * altered the commit message to reflect PCC mailbox free behaviour.
>
> On the latter, could you expand on what happens on close? Does the PCC
> channel end up calling tx_done() on each pending TX during the channel
> free? I'm not familiar with the PCC paths, but it doesn't look like it
> (or the mailbox core) has a case to deal with this on free.
>
> Maybe I am missing something, but could this leak skbs?
Yes, it could, but since they are held by another subsystem, and there
is no way to access them, it is safer to leak than to free. The Ring
Buffer in the Mailbox layer is not accessable from the MCTP Client.
Additionally, there is no way to force a flush of ring buffer. This
should be a vanishingly rare case, I suspect where someone would have to
deliberately send a bunch of messages and then immediately bring the
link down. Bringing the link back up would immediately send the
remaining skbs and cause them to be free, so they are not frozen in
perpetuity with no chance of being sent.
Whether the backend could handle this is a different story.
There is a potential for this kind of leak even if we were to transfer
the data out of SKB: the two options are to either leave it in the ring
buffer or risk a use-after-free event.
>
> [...]
>
>> +static int initialize_MTU(struct net_device *ndev)
>> +{
>> + struct mctp_pcc_ndev *mctp_pcc_ndev;
>> + struct mctp_pcc_mailbox *outbox;
>> + struct pcc_mbox_chan *pchan;
>> + int mctp_pcc_mtu;
>> +
>> + mctp_pcc_mtu = MCTP_MIN_MTU;
>> + mctp_pcc_ndev = netdev_priv(ndev);
>> + outbox = &mctp_pcc_ndev->outbox;
>> + pchan = pcc_mbox_request_channel(&outbox->client, outbox->index);
>> + if (IS_ERR(pchan))
>> + return -1;
>> + if (pchan->shmem_size < MCTP_MIN_MTU)
>> + return -1;
> Looks like this would leak the channel? You may want to shift this check
> to after the free_channel().
True.
>
> (but also, should this check also include the size of the pcc header?)
Yes, good catch.
>
> Cheers,
>
>
> Jeremy
^ permalink raw reply
* [PATCH net 1/1] batman-adv: reject oversized global TT response buffers
From: Ren Wei @ 2026-04-02 15:12 UTC (permalink / raw)
To: b.a.t.m.a.n, netdev
Cc: marek.lindner, sw, antonio, sven, davem, edumazet, kuba, pabeni,
horms, yifanwucs, tomapufckgml, yuantan098, bird, enjou1224z,
caoruide123, n05ec
In-Reply-To: <cover.1774947926.git.caoruide123@gmail.com>
From: Ruide Cao <caoruide123@gmail.com>
batadv_tt_prepare_tvlv_global_data() builds the allocation length for a
global TT response in 16-bit temporaries. When a remote originator
advertises a large enough global TT, the TT payload length plus the VLAN
header offset can exceed 65535 and wrap before kmalloc().
The full-table response path still uses the original TT payload length when
it fills tt_change, so the wrapped allocation is too small and
batadv_tt_prepare_tvlv_global_data() writes past the end of the heap object
before the later packet-size check runs.
Fix this by rejecting TT responses whose TVLV value length cannot fit in
the 16-bit TVLV payload length field.
Fixes: 7ea7b4a14275 ("batman-adv: make the TT CRC logic VLAN specific")
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: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Ruide Cao <caoruide123@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
net/batman-adv/translation-table.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 6e95e883c2bf..05cddcf994f6 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -798,8 +798,8 @@ batadv_tt_prepare_tvlv_global_data(struct batadv_orig_node *orig_node,
{
u16 num_vlan = 0;
u16 num_entries = 0;
- u16 change_offset;
- u16 tvlv_len;
+ u16 tvlv_len = 0;
+ unsigned int change_offset;
struct batadv_tvlv_tt_vlan_data *tt_vlan;
struct batadv_orig_node_vlan *vlan;
u8 *tt_change_ptr;
@@ -816,6 +816,11 @@ batadv_tt_prepare_tvlv_global_data(struct batadv_orig_node *orig_node,
if (*tt_len < 0)
*tt_len = batadv_tt_len(num_entries);
+ if (change_offset > U16_MAX || *tt_len > U16_MAX - change_offset) {
+ *tt_len = 0;
+ goto out;
+ }
+
tvlv_len = *tt_len;
tvlv_len += change_offset;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH net-next v4 00/14] net: sleepable ndo_set_rx_mode
From: Stanislav Fomichev @ 2026-04-02 15:06 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Stanislav Fomichev, netdev, davem, edumazet, pabeni
In-Reply-To: <20260401203831.6deee0e4@kernel.org>
On 04/01, Jakub Kicinski wrote:
> On Tue, 31 Mar 2026 18:19:00 -0700 Stanislav Fomichev wrote:
> > sleepable ndo_set_rx_mode
>
> Appears not to be "buildable" on 32bit x86 tho..
> --
> pw-bot: cr
This is what I get for writing unit tests :-(
^ permalink raw reply
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