* Re: [PATCH net-next] pppoe: update Kconfig URLs
From: Dianne Skoll @ 2026-03-31 14:25 UTC (permalink / raw)
To: Jaco Kroon
Cc: Qingfang Deng, linux-ppp, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Eric Biggers, netdev,
linux-kernel, Paul Mackerras, James Carlson
In-Reply-To: <1bba860e-a204-4a5b-9b8f-4d55a559d01e@uls.co.za>
Hi, all,
> My bad, my apologies. It was the userspace code you dropped then, which
> does NOT require the kernel options?
I was considering doing that, but I kept the userspace code. The reason is
that on some platforms, such as uclinux on a processor without an MMU,
the dlopen() system call is not implemented, so there's no way to use
a plugin with pppd. One user asked me to keep the userspace code, so I
did.
But you are correct in that 99.99% of Linux users will not need to download
rp-pppoe if all they want to do is connect to the Internet using a PPPoE
client.
Regards,
Dianne.
^ permalink raw reply
* Re: [EXTERNAL] Re: [PATCH net,v5] virtio_net: clamp rss_max_key_size to NETDEV_RSS_KEY_LEN
From: Paolo Abeni @ 2026-03-31 14:39 UTC (permalink / raw)
To: Srujana Challa, netdev@vger.kernel.org,
virtualization@lists.linux.dev
Cc: mst@redhat.com, jasowang@redhat.com, xuanzhuo@linux.alibaba.com,
eperezma@redhat.com, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, Nithin Kumar Dabilpuram, Shiva Shankar Kommula,
stable@vger.kernel.org
In-Reply-To: <CH3PR18MB6379D39BA068565667CF2B06A053A@CH3PR18MB6379.namprd18.prod.outlook.com>
On 3/31/26 3:29 PM, Srujana Challa wrote:
>> On 3/26/26 3:23 PM, Srujana Challa wrote:
>>> rss_max_key_size in the virtio spec is the maximum key size supported
>>> by the device, not a mandatory size the driver must use. Also the
>>> value 40 is a spec minimum, not a spec maximum.
>>>
>>> The current code rejects RSS and can fail probe when the device
>>> reports a larger rss_max_key_size than the driver buffer limit.
>>> Instead, clamp the effective key length to min(device
>>> rss_max_key_size, NETDEV_RSS_KEY_LEN) and keep RSS enabled.
>>>
>>> This keeps probe working on devices that advertise larger maximum key
>>> sizes while respecting the netdev RSS key buffer size limit.
>>>
>>> Fixes: 3f7d9c1964fc ("virtio_net: Add hash_key_length check")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Srujana Challa <schalla@marvell.com>
>>> ---
>>> v3:
>>> - Moved RSS key validation checks to virtnet_validate.
>>> - Add fixes: tag and CC -stable
>>> v4:
>>> - Use NETDEV_RSS_KEY_LEN instead of type_max for the maximum rss key
>> size.
>>> v5:
>>> - Interpret rss_max_key_size as a maximum and clamp it to
>> NETDEV_RSS_KEY_LEN.
>>> - Do not disable RSS/HASH_REPORT when device rss_max_key_size exceeds
>> NETDEV_RSS_KEY_LEN.
>>> - Drop the separate patch that replaced the runtime check with
>> BUILD_BUG_ON.
>>>
>>> drivers/net/virtio_net.c | 20 +++++++++-----------
>>> 1 file changed, 9 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index
>>> 022f60728721..b241c8dbb4e1 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -373,8 +373,6 @@ struct receive_queue {
>>> struct xdp_buff **xsk_buffs;
>>> };
>>>
>>> -#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
>>> -
>>> /* Control VQ buffers: protected by the rtnl lock */ struct
>>> control_buf {
>>> struct virtio_net_ctrl_hdr hdr;
>>> @@ -478,7 +476,7 @@ struct virtnet_info {
>>>
>>> /* Must be last as it ends in a flexible-array member. */
>>> TRAILING_OVERLAP(struct virtio_net_rss_config_trailer, rss_trailer,
>> hash_key_data,
>>> - u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];
>>> + u8 rss_hash_key_data[NETDEV_RSS_KEY_LEN];
>>> );
>>> };
>>> static_assert(offsetof(struct virtnet_info,
>>> rss_trailer.hash_key_data) == @@ -6717,6 +6715,7 @@ static int
>> virtnet_probe(struct virtio_device *vdev)
>>> struct virtnet_info *vi;
>>> u16 max_queue_pairs;
>>> int mtu = 0;
>>> + u16 key_sz;
>>>
>>> /* Find if host supports multiqueue/rss virtio_net device */
>>> max_queue_pairs = 1;
>>> @@ -6851,14 +6850,13 @@ static int virtnet_probe(struct virtio_device
>> *vdev)
>>> }
>>>
>>> if (vi->has_rss || vi->has_rss_hash_report) {
>>> - vi->rss_key_size =
>>> - virtio_cread8(vdev, offsetof(struct virtio_net_config,
>> rss_max_key_size));
>>> - if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
>>> - dev_err(&vdev->dev, "rss_max_key_size=%u exceeds
>> the limit %u.\n",
>>> - vi->rss_key_size,
>> VIRTIO_NET_RSS_MAX_KEY_SIZE);
>>> - err = -EINVAL;
>>> - goto free;
>>> - }
>>> + key_sz = virtio_cread8(vdev, offsetof(struct virtio_net_config,
>>> +rss_max_key_size));
>>> +
>>> + vi->rss_key_size = min_t(u16, key_sz, NETDEV_RSS_KEY_LEN);
>>> + if (key_sz > vi->rss_key_size)
>>> + dev_warn(&vdev->dev,
>>> + "rss_max_key_size=%u exceeds driver limit
>> %u, clamping\n",
>>> + key_sz, vi->rss_key_size);
>>
>> NETDEV_RSS_KEY_LEN is 256 and virtio_cread8() returns a u8. The check is
>> not needed, and the warning will never be printed. I think that the
>> BUILD_BUG_ON() you used in v4 would be better than the above chunk.
>>
> Thank you for the feedback. In net-next, NETDEV_RSS_KEY_LEN is 256. This fix is
> also intended for stable kernels, where NETDEV_RSS_KEY_LEN is 52, and
> I added the message to make clamping visible in that case.
> I will remove the check and send the next version.
I'm sorry, I haven't looked at the historical context when I wrote my
previous reply.
IMHO the additional check does not make sense in the current net tree.
On the flip side stable trees will need it. I suggest:
- dropping the check for the 'net' patch
- also dropping CC: stable tag
- explicitly sending to stable the fix variant including the size check.
@Michael: WDYT?
/P
^ permalink raw reply
* Re: [PATCH net] ipv4: nexthop: allocate skb dynamically in rtm_get_nexthop()
From: Fernando Fernandez Mancera @ 2026-03-31 14:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, horms, pabeni, kuba, davem, dsahern, Yiming Qian
In-Reply-To: <CANn89iJ12g+AEr-9o1xbFEcSoTMYYYB4yXQW7pNRH0tei=KZug@mail.gmail.com>
On 3/31/26 3:38 PM, Eric Dumazet wrote:
> On Tue, Mar 31, 2026 at 5:50 AM Fernando Fernandez Mancera
> <fmancera@suse.de> wrote:
>>
>> On 3/31/26 2:13 PM, Eric Dumazet wrote:
>>> On Tue, Mar 31, 2026 at 5:00 AM Fernando Fernandez Mancera
>>> <fmancera@suse.de> wrote:
>>>>
>>>> When querying a nexthop object via RTM_GETNEXTHOP, the kernel currently
>>>> allocates a fixed-size skb using NLMSG_GOODSIZE. While sufficient for
>>>> single nexthops and small Equal-Cost Multi-Path groups, this fixed
>>>> allocation fails for large nexthop groups like 512+ nexthops.
>>>>
>>>> This results in the following warning splat:
>>>>
>>>> WARNING: net/ipv4/nexthop.c:3395 at rtm_get_nexthop+0x176/0x1c0, CPU#19: rep/9282
>>>> [...]
>>>> RIP: 0010:rtm_get_nexthop+0x176/0x1c0
>>>> [...]
>>>> Call Trace:
>>>> <TASK>
>>>> rtnetlink_rcv_msg+0x168/0x670
>>>> netlink_rcv_skb+0x5c/0x110
>>>> netlink_unicast+0x203/0x2e0
>>>> netlink_sendmsg+0x222/0x460
>>>> ____sys_sendmsg+0x35a/0x380
>>>> ___sys_sendmsg+0x99/0xe0
>>>> __sys_sendmsg+0x8a/0xf0
>>>> do_syscall_64+0x12f/0x1590
>>>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>> </TASK>
>>>
>>> I find these stack traces without symbols not very useful.
>>
>> Hi Eric,
>>
>> sure, here is the decoded trace:
>>
>> WARNING: net/ipv4/nexthop.c:3395 at rtm_get_nexthop+0x176/0x1c0,
>> CPU#20: rep/4608
>> RIP: 0010:rtm_get_nexthop (net/ipv4/nexthop.c:3395 (discriminator 2))
>> Call Trace:
>> <TASK>
>> rtnetlink_rcv_msg (net/core/rtnetlink.c:6989)
>> netlink_rcv_skb (net/netlink/af_netlink.c:2550)
>> netlink_unicast (net/netlink/af_netlink.c:1319
>> net/netlink/af_netlink.c:1344)
>> netlink_sendmsg (net/netlink/af_netlink.c:1894)
>> ____sys_sendmsg (net/socket.c:721 (discriminator 16) net/socket.c:736
>> (discriminator 16) net/socket.c:2585 (discriminator 16))
>> ___sys_sendmsg (net/socket.c:2641)
>> __sys_sendmsg (net/socket.c:2671 (discriminator 1))
>> do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1)
>> arch/x86/entry/syscall_64.c:94 (discriminator 1))
>> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>> </TASK>
>>
>>>
>>> Any reason you have not used scripts/decode_stacktrace.sh ?
>>>
>>
>> Not really, in the past I checked other commits with `git log --grep
>> "Call Trace"` and found out that most of the traces did not have symbols
>> so I decided to attach the raw trace.
>>
>> I do not really have a personal preference here. If trace with symbols
>> is preferred I will do it like that in the future. I could also send a
>> V2 of this patch if needed.
>
> Please send a V2 (after the ~24 hours delay) as we prefer this
> whenever possible.
>
Yes, in addition nh_nlmsg_size() isn't accounting the bytes for stats
dump so this needs a v2 anyway.
Thanks,
Fernando.
^ permalink raw reply
* Re: [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
From: Nikolaos Gkarlis @ 2026-03-31 14:43 UTC (permalink / raw)
To: netdev; +Cc: kuba, kuniyu
In-Reply-To: <20260328213338.450601-1-nickgarlis@gmail.com>
Just following up on this patch; is it okay as is, or does anything
need to be addressed?
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: mdio: add a driver for PIC64-HPSC/HX MDIO controller
From: Charles Perry @ 2026-03-31 14:43 UTC (permalink / raw)
To: Andrew Lunn
Cc: Russell King (Oracle), Charles Perry, netdev, Maxime Chevallier,
Heiner Kallweit, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-kernel
In-Reply-To: <88d652b9-ed4f-4bf4-b0ab-8703c15ce58b@lunn.ch>
On Tue, Mar 31, 2026 at 04:20:55PM +0200, Andrew Lunn wrote:
> On Tue, Mar 31, 2026 at 03:05:28PM +0100, Russell King (Oracle) wrote:
> > On Tue, Mar 31, 2026 at 06:42:02AM -0700, Charles Perry wrote:
> > > I don't know if there's any value in waiting for write completion here as
> > > write completion doesn't mean that the effects of the write are available
> > > right now. I also didn't run into any issues in my testing. Let me know if
> > > you know of a use case where this wouldn't work.
> > >
> > > I can add a wait for transaction completion if that's expected by phylib.
> >
> > Consider a PHY using a shared interrupt line, and the interrupt being
> > disabled in at the PHY before being torn down... wouldn't we want the
> > write to the register which enables interrupts to complete before we
> > unregister the interrupt handler for the particular PHY?
> >
> > I do notice that other MDIO drivers don't wait. Some PHY drivers don't
> > access the PHY after the write to disable interrupts either. So, maybe
> > phy_free_interrupt() should read-back from the PHY before calling
> > free_irq() to guarantee that the write has completed?
> >
> > Andrew?
>
> The general pattern is to not wait on write.
Ok, then it's status quo for this.
I'll send a v4 later this week for the other return issue.
Thanks,
Charles
^ permalink raw reply
* Re: [EXTERNAL] Re: [PATCH net,v5] virtio_net: clamp rss_max_key_size to NETDEV_RSS_KEY_LEN
From: Michael S. Tsirkin @ 2026-03-31 14:48 UTC (permalink / raw)
To: Paolo Abeni
Cc: Srujana Challa, netdev@vger.kernel.org,
virtualization@lists.linux.dev, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
Nithin Kumar Dabilpuram, Shiva Shankar Kommula,
stable@vger.kernel.org
In-Reply-To: <68ca0a8c-27f9-45f1-94cc-7e3c7936181f@redhat.com>
On Tue, Mar 31, 2026 at 04:39:02PM +0200, Paolo Abeni wrote:
> On 3/31/26 3:29 PM, Srujana Challa wrote:
> >> On 3/26/26 3:23 PM, Srujana Challa wrote:
> >>> rss_max_key_size in the virtio spec is the maximum key size supported
> >>> by the device, not a mandatory size the driver must use. Also the
> >>> value 40 is a spec minimum, not a spec maximum.
> >>>
> >>> The current code rejects RSS and can fail probe when the device
> >>> reports a larger rss_max_key_size than the driver buffer limit.
> >>> Instead, clamp the effective key length to min(device
> >>> rss_max_key_size, NETDEV_RSS_KEY_LEN) and keep RSS enabled.
> >>>
> >>> This keeps probe working on devices that advertise larger maximum key
> >>> sizes while respecting the netdev RSS key buffer size limit.
> >>>
> >>> Fixes: 3f7d9c1964fc ("virtio_net: Add hash_key_length check")
> >>> Cc: stable@vger.kernel.org
> >>> Signed-off-by: Srujana Challa <schalla@marvell.com>
> >>> ---
> >>> v3:
> >>> - Moved RSS key validation checks to virtnet_validate.
> >>> - Add fixes: tag and CC -stable
> >>> v4:
> >>> - Use NETDEV_RSS_KEY_LEN instead of type_max for the maximum rss key
> >> size.
> >>> v5:
> >>> - Interpret rss_max_key_size as a maximum and clamp it to
> >> NETDEV_RSS_KEY_LEN.
> >>> - Do not disable RSS/HASH_REPORT when device rss_max_key_size exceeds
> >> NETDEV_RSS_KEY_LEN.
> >>> - Drop the separate patch that replaced the runtime check with
> >> BUILD_BUG_ON.
> >>>
> >>> drivers/net/virtio_net.c | 20 +++++++++-----------
> >>> 1 file changed, 9 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index
> >>> 022f60728721..b241c8dbb4e1 100644
> >>> --- a/drivers/net/virtio_net.c
> >>> +++ b/drivers/net/virtio_net.c
> >>> @@ -373,8 +373,6 @@ struct receive_queue {
> >>> struct xdp_buff **xsk_buffs;
> >>> };
> >>>
> >>> -#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
> >>> -
> >>> /* Control VQ buffers: protected by the rtnl lock */ struct
> >>> control_buf {
> >>> struct virtio_net_ctrl_hdr hdr;
> >>> @@ -478,7 +476,7 @@ struct virtnet_info {
> >>>
> >>> /* Must be last as it ends in a flexible-array member. */
> >>> TRAILING_OVERLAP(struct virtio_net_rss_config_trailer, rss_trailer,
> >> hash_key_data,
> >>> - u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> >>> + u8 rss_hash_key_data[NETDEV_RSS_KEY_LEN];
> >>> );
> >>> };
> >>> static_assert(offsetof(struct virtnet_info,
> >>> rss_trailer.hash_key_data) == @@ -6717,6 +6715,7 @@ static int
> >> virtnet_probe(struct virtio_device *vdev)
> >>> struct virtnet_info *vi;
> >>> u16 max_queue_pairs;
> >>> int mtu = 0;
> >>> + u16 key_sz;
> >>>
> >>> /* Find if host supports multiqueue/rss virtio_net device */
> >>> max_queue_pairs = 1;
> >>> @@ -6851,14 +6850,13 @@ static int virtnet_probe(struct virtio_device
> >> *vdev)
> >>> }
> >>>
> >>> if (vi->has_rss || vi->has_rss_hash_report) {
> >>> - vi->rss_key_size =
> >>> - virtio_cread8(vdev, offsetof(struct virtio_net_config,
> >> rss_max_key_size));
> >>> - if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
> >>> - dev_err(&vdev->dev, "rss_max_key_size=%u exceeds
> >> the limit %u.\n",
> >>> - vi->rss_key_size,
> >> VIRTIO_NET_RSS_MAX_KEY_SIZE);
> >>> - err = -EINVAL;
> >>> - goto free;
> >>> - }
> >>> + key_sz = virtio_cread8(vdev, offsetof(struct virtio_net_config,
> >>> +rss_max_key_size));
> >>> +
> >>> + vi->rss_key_size = min_t(u16, key_sz, NETDEV_RSS_KEY_LEN);
> >>> + if (key_sz > vi->rss_key_size)
> >>> + dev_warn(&vdev->dev,
> >>> + "rss_max_key_size=%u exceeds driver limit
> >> %u, clamping\n",
> >>> + key_sz, vi->rss_key_size);
> >>
> >> NETDEV_RSS_KEY_LEN is 256 and virtio_cread8() returns a u8. The check is
> >> not needed, and the warning will never be printed. I think that the
> >> BUILD_BUG_ON() you used in v4 would be better than the above chunk.
> >>
> > Thank you for the feedback. In net-next, NETDEV_RSS_KEY_LEN is 256. This fix is
> > also intended for stable kernels, where NETDEV_RSS_KEY_LEN is 52, and
> > I added the message to make clamping visible in that case.
> > I will remove the check and send the next version.
>
> I'm sorry, I haven't looked at the historical context when I wrote my
> previous reply.
>
> IMHO the additional check does not make sense in the current net tree.
> On the flip side stable trees will need it. I suggest:
>
> - dropping the check for the 'net' patch
> - also dropping CC: stable tag
> - explicitly sending to stable the fix variant including the size check.
>
> @Michael: WDYT?
>
> /P
I was the one who suggested it, the extra check is harmless, I'm
inclined to always have it. Less work than maintaining two patches.
--
MST
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: mdio: add a driver for PIC64-HPSC/HX MDIO controller
From: Russell King (Oracle) @ 2026-03-31 14:57 UTC (permalink / raw)
To: Charles Perry
Cc: Andrew Lunn, netdev, Maxime Chevallier, Heiner Kallweit,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-kernel
In-Reply-To: <acvdkBF4ZjZaCHwd@bby-cbu-swbuild03.eng.microchip.com>
On Tue, Mar 31, 2026 at 07:43:28AM -0700, Charles Perry wrote:
> On Tue, Mar 31, 2026 at 04:20:55PM +0200, Andrew Lunn wrote:
> > On Tue, Mar 31, 2026 at 03:05:28PM +0100, Russell King (Oracle) wrote:
> > > On Tue, Mar 31, 2026 at 06:42:02AM -0700, Charles Perry wrote:
> > > > I don't know if there's any value in waiting for write completion here as
> > > > write completion doesn't mean that the effects of the write are available
> > > > right now. I also didn't run into any issues in my testing. Let me know if
> > > > you know of a use case where this wouldn't work.
> > > >
> > > > I can add a wait for transaction completion if that's expected by phylib.
> > >
> > > Consider a PHY using a shared interrupt line, and the interrupt being
> > > disabled in at the PHY before being torn down... wouldn't we want the
> > > write to the register which enables interrupts to complete before we
> > > unregister the interrupt handler for the particular PHY?
> > >
> > > I do notice that other MDIO drivers don't wait. Some PHY drivers don't
> > > access the PHY after the write to disable interrupts either. So, maybe
> > > phy_free_interrupt() should read-back from the PHY before calling
> > > free_irq() to guarantee that the write has completed?
> > >
> > > Andrew?
> >
> > The general pattern is to not wait on write.
>
> Ok, then it's status quo for this.
Except someone needs to add that read-back - don't look at me, I
generate too many patches for netdev, so have no capacity for yet
more patches.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* [PATCH v5 net 00/11] xsk: tailroom reservation and MTU validation
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
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 (11):
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
idpf: remove xsk frame size check against alignment
igc: remove home-grown xsk's frame size validation
gve: remove home-grown xsk's frame size validation
drivers/net/ethernet/google/gve/gve_main.c | 5 --
drivers/net/ethernet/intel/idpf/xsk.c | 10 ----
drivers/net/ethernet/intel/igc/igc_xdp.c | 11 ----
include/net/xdp_sock.h | 2 +-
include/net/xdp_sock_drv.h | 17 +++++-
net/xdp/xdp_umem.c | 3 +-
net/xdp/xsk_buff_pool.c | 25 ++++++++-
.../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 ++++++++
12 files changed, 135 insertions(+), 62 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v5 net 01/11] xsk: tighten UMEM headroom validation to account for tailroom and min frame
From: Maciej Fijalkowski @ 2026-03-31 15:02 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: <20260331150213.550797-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, 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 v5 net 02/11] xsk: respect tailroom for ZC setups
From: Maciej Fijalkowski @ 2026-03-31 15:02 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: <20260331150213.550797-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.
xsk_pool_get_tailroom() is only reserving necessary space when pool is
zc and underlying netdev supports zc multi-buffer. Rely on umem->zc
state when configuring tailroom. xsk_pool_get_rx_frame_size() is going
to be used in further MTU validation so move setting of umem->zc before
ndo_bpf() call and on error path clear only when there are no other
users of umem, as we want to preserve the setting for other active
sockets already bound to this entity.
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.
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 | 17 ++++++++++++++++-
net/xdp/xsk_buff_pool.c | 4 +++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
index 6b9ebae2dc95..cd9eeff536a6 100644
--- a/include/net/xdp_sock_drv.h
+++ b/include/net/xdp_sock_drv.h
@@ -41,6 +41,19 @@ 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(struct xsk_buff_pool *pool)
+{
+ struct xdp_umem *umem = pool->umem;
+
+ /* 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.
+ */
+ if (umem->zc && (umem->flags & XDP_UMEM_SG_FLAG))
+ return SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ return 0;
+}
+
static inline u32 xsk_pool_get_chunk_size(struct xsk_buff_pool *pool)
{
return pool->chunk_size;
@@ -48,7 +61,9 @@ static inline u32 xsk_pool_get_chunk_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);
+ return ALIGN_DOWN(xsk_pool_get_chunk_size(pool) -
+ xsk_pool_get_headroom(pool) -
+ xsk_pool_get_tailroom(pool), 128);
}
static inline u32 xsk_pool_get_rx_frag_step(struct xsk_buff_pool *pool)
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 37b7a68b89b3..0f40bee606d3 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -200,6 +200,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
goto err_unreg_pool;
}
+ pool->umem->zc = true;
if (netdev->xdp_zc_max_segs == 1 && (flags & XDP_USE_SG)) {
err = -EOPNOTSUPP;
goto err_unreg_pool;
@@ -224,13 +225,14 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
err = -EINVAL;
goto err_unreg_xsk;
}
- pool->umem->zc = true;
pool->xdp_zc_max_segs = netdev->xdp_zc_max_segs;
return 0;
err_unreg_xsk:
xp_disable_drv_zc(pool);
err_unreg_pool:
+ if (refcount_read(&pool->umem->users) == 1)
+ pool->umem->zc = false;
if (!force_zc)
err = 0; /* fallback to copy mode */
if (err) {
--
2.43.0
^ permalink raw reply related
* [PATCH v5 net 03/11] xsk: fix XDP_UMEM_SG_FLAG issues
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260331150213.550797-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 0f40bee606d3..d3f087fad551 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -249,6 +249,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 v5 net 04/11] xsk: validate MTU against usable frame size on bind
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260331150213.550797-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 | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index d3f087fad551..b55bbc79d3f7 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,6 +159,9 @@ 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;
int err = 0;
@@ -178,7 +183,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 +205,16 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
goto err_unreg_pool;
}
+ if (mbuf) {
+ if (segs == 1) {
+ err = -EOPNOTSUPP;
+ goto err_unreg_pool;
+ }
+ } else {
+ segs = 1;
+ }
pool->umem->zc = true;
- if (netdev->xdp_zc_max_segs == 1 && (flags & XDP_USE_SG)) {
+ if (needed > xsk_pool_get_rx_frame_size(pool) * segs) {
err = -EOPNOTSUPP;
goto err_unreg_pool;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v5 net 05/11] selftests: bpf: introduce a common routine for reading procfs
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260331150213.550797-1-maciej.fijalkowski@intel.com>
Parametrize current way of getting MAX_SKB_FRAGS value from procfs 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.
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 v5 net 06/11] selftests: bpf: fix pkt grow tests
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260331150213.550797-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 v5 net 07/11] selftests: bpf: have a separate variable for drop test
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260331150213.550797-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 v5 net 08/11] selftests: bpf: adjust rx_dropped xskxceiver's test to respect tailroom
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260331150213.550797-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..a96ca4b39d1e 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 * 2) + 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 - 1;
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 v5 net 09/11] idpf: remove xsk frame size check against alignment
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260331150213.550797-1-maciej.fijalkowski@intel.com>
We provide alignment within xsk_pool_get_rx_frame_size() now, so this
validation is redundant.
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
drivers/net/ethernet/intel/idpf/xsk.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
index d95d3efdfd36..a5b6177fd44c 100644
--- a/drivers/net/ethernet/intel/idpf/xsk.c
+++ b/drivers/net/ethernet/intel/idpf/xsk.c
@@ -558,16 +558,6 @@ int idpf_xsk_pool_setup(struct idpf_vport *vport, struct netdev_bpf *bpf)
bool restart;
int ret;
- if (pool && !IS_ALIGNED(xsk_pool_get_rx_frame_size(pool),
- LIBETH_RX_BUF_STRIDE)) {
- NL_SET_ERR_MSG_FMT_MOD(bpf->extack,
- "%s: HW doesn't support frames sizes not aligned to %u (qid %u: %u)",
- netdev_name(vport->netdev),
- LIBETH_RX_BUF_STRIDE, qid,
- xsk_pool_get_rx_frame_size(pool));
- return -EINVAL;
- }
-
restart = idpf_xdp_enabled(vport) && netif_running(vport->netdev);
if (!restart)
goto pool;
--
2.43.0
^ permalink raw reply related
* [PATCH v5 net 10/11] igc: remove home-grown xsk's frame size validation
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski
In-Reply-To: <20260331150213.550797-1-maciej.fijalkowski@intel.com>
Since this check is now present in core in xp_assign_dev(), remove
redundant statement from igc_xdp_enable_pool().
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
drivers/net/ethernet/intel/igc/igc_xdp.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_xdp.c b/drivers/net/ethernet/intel/igc/igc_xdp.c
index 9eb47b4beb06..4126173a0226 100644
--- a/drivers/net/ethernet/intel/igc/igc_xdp.c
+++ b/drivers/net/ethernet/intel/igc/igc_xdp.c
@@ -61,23 +61,12 @@ static int igc_xdp_enable_pool(struct igc_adapter *adapter,
struct igc_ring *rx_ring, *tx_ring;
struct napi_struct *napi;
bool needs_reset;
- u32 frame_size;
int err;
if (queue_id >= adapter->num_rx_queues ||
queue_id >= adapter->num_tx_queues)
return -EINVAL;
- frame_size = xsk_pool_get_rx_frame_size(pool);
- if (frame_size < ETH_FRAME_LEN + VLAN_HLEN * 2) {
- /* When XDP is enabled, the driver doesn't support frames that
- * span over multiple buffers. To avoid that, we check if xsk
- * frame size is big enough to fit the max ethernet frame size
- * + vlan double tagging.
- */
- return -EOPNOTSUPP;
- }
-
err = xsk_pool_dma_map(pool, dev, IGC_RX_DMA_ATTR);
if (err) {
netdev_err(ndev, "Failed to map xsk pool\n");
--
2.43.0
^ permalink raw reply related
* [PATCH v5 net 11/11] gve: remove home-grown xsk's frame size validation
From: Maciej Fijalkowski @ 2026-03-31 15:02 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Maciej Fijalkowski,
Joshua Washington
In-Reply-To: <20260331150213.550797-1-maciej.fijalkowski@intel.com>
Since this check is now present in core in xp_assign_dev(), remove
redundant statement from gve_xsk_pool_enable().
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
drivers/net/ethernet/google/gve/gve_main.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index 9eb4b3614c4f..b63e0a0459fb 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c
@@ -1596,11 +1596,6 @@ static int gve_xsk_pool_enable(struct net_device *dev,
dev_err(&priv->pdev->dev, "xsk pool invalid qid %d", qid);
return -EINVAL;
}
- if (xsk_pool_get_rx_frame_size(pool) <
- priv->dev->max_mtu + sizeof(struct ethhdr)) {
- dev_err(&priv->pdev->dev, "xsk pool frame_len too small");
- return -EINVAL;
- }
err = xsk_pool_dma_map(pool, &priv->pdev->dev,
DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v2 3/5] ice: migrate to netdev ops lock
From: Alexander Lobakin @ 2026-03-31 15:09 UTC (permalink / raw)
To: Kohei Enju, Simon Horman
Cc: Tony Nguyen, davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
jacob.e.keller, nxne.cnse.osdt.itp.upstreaming, horms,
maciej.fijalkowski, magnus.karlsson, ast, daniel, hawk,
john.fastabend, sdf, bpf, Aleksandr Loktionov
In-Reply-To: <aclmhA5yjXfzX_Jk@x1>
From: Kohei Enju <kohei@enjuk.jp>
Date: Mon, 30 Mar 2026 03:01:04 +0900
> On 03/25 13:06, Tony Nguyen wrote:
>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>>
>> Queue management ops unconditionally enable netdev locking. The same
>> lock is taken by default by several NAPI configuration functions,
>> such as napi_enable() and netif_napi_set_irq().
>> Request ops locking in advance and make sure we use the _locked
>> counterparts of those functions to avoid deadlocks, taking the lock
>> manually where needed (suspend/resume, queue rebuild and resets).
To Simon:
Yes, the DCB code was reported earlier and I fixed it already in this
series. The DCB core takes RTNL, not netdev, so now ice calls the
functions which take the netdev lock.
But seems like the E-Switch code should take the lock, yeah.
To Kohei:
Ufff, I'm sorry I missed one more part, this RTNL -> netdev lock
transition is when you change one bit of the code and then the whole
cascade in some other part breaks...
I'll send a new rev once I fix and test both.
Thanks!
Olek
^ permalink raw reply
* [PATCH] net/mlx5: Fix potential NULL dereference in PTP event handling
From: Prathamesh Deshpande @ 2026-03-31 15:31 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky, Tariq Toukan
Cc: Mark Bloch, Richard Cochran, netdev, linux-rdma, linux-kernel,
Prathamesh Deshpande
In mlx5_ptp_pps_event(), a TODO comment correctly identified that
clock->ptp can be NULL if ptp_clock_register() fails. However, the
code proceeded to call ptp_clock_event() with that NULL pointer,
leading to a potential kernel panic.
Fix this by adding a NULL check for clock->ptp before calling
ptp_clock_event(), ensuring the driver handles failed registration
gracefully.
Fixes: 7c39afb394c7 ("net/mlx5: PTP code migration to driver core section")
Signed-off-by: Prathamesh Deshpande <prathameshdeshpande7@gmail.com>
---
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
index bd4e042077af..9e9f844935b4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -1185,8 +1185,8 @@ static int mlx5_pps_event(struct notifier_block *nb,
} else {
ptp_event.type = PTP_CLOCK_EXTTS;
}
- /* TODOL clock->ptp can be NULL if ptp_clock_register fails */
- ptp_clock_event(clock->ptp, &ptp_event);
+ if (clock->ptp)
+ ptp_clock_event(clock->ptp, &ptp_event);
break;
case PTP_PF_PEROUT:
if (clock->shared) {
--
2.43.0
^ permalink raw reply related
* Re: [PATCHv3] atm: nicstar: simplify allocation
From: Gustavo A. R. Silva @ 2026-03-31 15:35 UTC (permalink / raw)
To: Rosen Penev, linux-atm-general
Cc: Chas Williams, Kees Cook, Gustavo A. R. Silva, open list:ATM,
open list,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
In-Reply-To: <20260327192407.365369-1-rosenp@gmail.com>
On 3/27/26 13:24, Rosen Penev wrote:
> Use a flexible array member with kzalloc_flex to combine allocations
> into one.
>
> Add __counted_by for extra runtime analysis.
Move counting variable
> assignment to right after allocation as required by __counted_by.
This is misinformation and should be phrased differently[1]
-Gustavo
[1] https://lore.kernel.org/linux-hardening/37378f49-437f-438b-ad6c-d60480feb306@embeddedor.com/
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> v3: reduce to 80 columns
> v2: use kzalloc_flex
> drivers/atm/nicstar.c | 18 ++++++------------
> drivers/atm/nicstar.h | 5 +++--
> 2 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
> index 24e51343df15..c0bed85daf96 100644
> --- a/drivers/atm/nicstar.c
> +++ b/drivers/atm/nicstar.c
> @@ -867,23 +867,18 @@ static scq_info *get_scq(ns_dev *card, int size, u32 scd)
> if (size != VBR_SCQSIZE && size != CBR_SCQSIZE)
> return NULL;
>
> - scq = kmalloc_obj(*scq);
> + scq = kzalloc_flex(*scq, skb, size / NS_SCQE_SIZE);
> if (!scq)
> return NULL;
> - scq->org = dma_alloc_coherent(&card->pcidev->dev,
> - 2 * size, &scq->dma, GFP_KERNEL);
> +
> + scq->num_entries = size / NS_SCQE_SIZE;
> +
> + scq->org = dma_alloc_coherent(&card->pcidev->dev, 2 * size, &scq->dma,
> + GFP_KERNEL);
> if (!scq->org) {
> kfree(scq);
> return NULL;
> }
> - scq->skb = kzalloc_objs(*scq->skb, size / NS_SCQE_SIZE);
> - if (!scq->skb) {
> - dma_free_coherent(&card->pcidev->dev,
> - 2 * size, scq->org, scq->dma);
> - kfree(scq);
> - return NULL;
> - }
> - scq->num_entries = size / NS_SCQE_SIZE;
> scq->base = PTR_ALIGN(scq->org, size);
> scq->next = scq->base;
> scq->last = scq->base + (scq->num_entries - 1);
> @@ -928,7 +923,6 @@ static void free_scq(ns_dev *card, scq_info *scq, struct atm_vcc *vcc)
> }
> }
> }
> - kfree(scq->skb);
> dma_free_coherent(&card->pcidev->dev,
> 2 * (scq->num_entries == VBR_SCQ_NUM_ENTRIES ?
> VBR_SCQSIZE : CBR_SCQSIZE),
> diff --git a/drivers/atm/nicstar.h b/drivers/atm/nicstar.h
> index 1b7f1dfc1735..76635d46016e 100644
> --- a/drivers/atm/nicstar.h
> +++ b/drivers/atm/nicstar.h
> @@ -667,14 +667,15 @@ typedef struct scq_info {
> ns_scqe *next;
> volatile ns_scqe *tail; /* Not related to the nicstar register */
> unsigned num_entries;
> - struct sk_buff **skb; /* Pointer to an array of pointers
> - to the sk_buffs used for tx */
> u32 scd; /* SRAM address of the corresponding
> SCD */
> int tbd_count; /* Only meaningful on variable rate */
> wait_queue_head_t scqfull_waitq;
> volatile char full; /* SCQ full indicator */
> spinlock_t lock; /* SCQ spinlock */
> + struct sk_buff *skb[] __counted_by(num_entries);
> + /* Pointer to an array of pointers
> + to the sk_buffs used for tx */
> } scq_info;
>
> typedef struct rsq_info {
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] r8152: Add support for 5Gbit Link Speeds and EEE
From: Birger Koblitz @ 2026-03-31 15:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew+netdev, davem, edumazet, pabeni, linux-usb, netdev,
linux-kernel
In-Reply-To: <20260329193449.2764517-1-kuba@kernel.org>
Hi Jakub,
thanks a lot for reviewing the patch!
On 3/29/26 21:34, Jakub Kicinski 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.
> ---
> r8152: Add support for 5Gbit Link Speeds and EEE
>
> This commit adds support for 5GBit link speeds and EEE to the RTL8157
> driver. It updates the speed and duplex settings, ethtool integration,
> and EEE advertisement parameters to handle the new 5G capabilities.
>
>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
>> index 3b6d4252d34c..bab8e10e5f99 100644
>> --- a/drivers/net/usb/r8152.c
>> +++ b/drivers/net/usb/r8152.c
>> @@ -604,6 +604,7 @@ enum spd_duplex {
>> FORCE_100M_FULL,
>> FORCE_1000M_FULL,
>> NWAY_2500M_FULL,
>> + NWAY_5000M_FULL,
>> };
>
> Are there missing switch cases for NWAY_5000M_FULL in the hardware UPS
> flag configurations, such as r8156_ups_flags?
This question has now been raised the third time, see here:
https://lkml.org/lkml/2026/3/24/1902 and here https://lkml.org/lkml/2026/3/19/1765
However, while I share the concern, I cannot provide a better answer than
the first time or second time.
> Is it intentional that the MAC idle speed down feature (IDLE_SPDWN_EN) is
> disabled for _2500bps but remains enabled for _5000bps?
Yes, this is intentional. IDLE_SPDWN_EN is indeed only disabled for _2500bps
in the out-of-tree driver (and not for 5GBit or 10GBit).
>
>> @@ -6343,15 +6356,20 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
>> r8152_mdio_write(tp, MII_CTRL1000, new1);
>> }
>>
>> - if (tp->support_2500full) {
>> + if (tp->support_2500full || tp->support_5000full) {
>> orig = ocp_reg_read(tp, OCP_10GBT_CTRL);
>> - new1 = orig & ~MDIO_AN_10GBT_CTRL_ADV2_5G;
>> + new1 = orig & ~(MDIO_AN_10GBT_CTRL_ADV2_5G | MDIO_AN_10GBT_CTRL_ADV5G);
>>
>> if (advertising & RTL_ADVERTISED_2500_FULL) {
>> new1 |= MDIO_AN_10GBT_CTRL_ADV2_5G;
>> tp->ups_info.speed_duplex = NWAY_2500M_FULL;
>> }
>>
>> + if (advertising & RTL_ADVERTISED_5000_FULL) {
>> + new1 |= MDIO_AN_10GBT_CTRL_ADV5G;
>> + tp->ups_info.speed_duplex = NWAY_5000M_FULL;
>> + }
>> +
>
> Could this unintentionally advertise 5GbE on a 2.5GbE-only adapter if
> userspace requests it?
>
> The earlier check in this function:
>
> if (!(advertising & support))
> return -EINVAL;
>
> only ensures that at least one supported speed is requested, but it does
> not strip unsupported speeds from the advertising mask. Since this block
> is now entered when support_2500full is true, checking the advertising mask
> for RTL_ADVERTISED_5000_FULL without verifying tp->support_5000full could
> erroneously write the 5G bit to the PHY and misconfigure the hardware
> state.
I will change this to:
advertising &= support;
if (!advertising)
return -EINVAL;
in v5, to be on the safe side.
> [ ... ]
>
>> @@ -8793,6 +8814,19 @@ int rtl8152_get_link_ksettings(struct net_device *netdev,
>> cmd->base.speed = SPEED_2500;
>> }
>>
>> + if (tp->support_5000full) {
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>> + cmd->link_modes.advertising,
>> + ocp_reg_read(tp, OCP_10GBT_CTRL) & MDIO_AN_10GBT_CTRL_ADV5G);
>> +
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>> + cmd->link_modes.lp_advertising,
>> + ocp_reg_read(tp, OCP_10GBT_STAT) & MDIO_AN_10GBT_STAT_LP5G);
>> +
>> + if (is_speed_5000(rtl8152_get_speed(tp)))
>> + cmd->base.speed = SPEED_5000;
>> + }
>
> Since 5G adapters like the RTL8157 will have both support_2500full and
> support_5000full set, will this result in duplicate blocking USB control
> transfers?
You are right. Also in view of future 10GBit support, I will change this to:
if (tp->support_2500full || tp->support_5000full) {
u16 ocp_10bt_ctrl = ocp_reg_read(tp, OCP_10GBT_CTR)L;
u16 ocp_10bt_stat = ocp_reg_read(tp, OCP_10GBT_STAT);
if (tp->support_2500full) {
...
}
if (tp->support_5000full) {
...
}
}
> In r8153_get_eee, the reading of OCP_EEE_ADV2 and OCP_EEE_LPABLE2 is
> performed inside the if (tp->support_2500full) block that precedes this new
> code.
>
> Since those registers contain the EEE capability bits for both 2.5G
> and 5G speeds, what happens if a device has support_5000full enabled but
> support_2500full disabled? It looks like the registers would never be read,
> and the 5Gbps EEE capabilities would fail to populate correctly.
The driver supports RTL815x-chips, and all RTL815x-chips, which support 5GBit also
support 2.5gbit (the RTL8157 and the RTL8159). My understanding is, that the
802.3bz standard even requires this fallback. However, to be on the safe side and
to also support possible future chips, I will change this to:
if (tp->support_2500full || tp->support_5000full) {
val = ocp_reg_read(tp, OCP_EEE_ADV2);
mii_eee_cap2_mod_linkmode_adv_t(eee->advertised, val);
val = ocp_reg_read(tp, OCP_EEE_LPABLE2);
mii_eee_cap2_mod_linkmode_adv_t(eee->lp_advertised, val);
}
if (tp->support_2500full) {
linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, eee->supported);
if (speed & _2500bps)
linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, common);
}
if (tp->support_5000full) {
linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, eee->supported);
if (speed & _5000bps)
linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, common);
}
Cheers,
Birger
^ permalink raw reply
* Re: [PATCH 6.1] net: enetc: fix PF !of_device_is_available() teardown path
From: Nathan Chancellor @ 2026-03-31 15:38 UTC (permalink / raw)
To: Vladimir Oltean
Cc: stable, Greg Kroah-Hartman, patches, netdev, Claudiu Manoil,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Wei Fang, Rahul Sharma, linux-kernel
In-Reply-To: <20260330081944.527545-1-vladimir.oltean@nxp.com>
On Mon, Mar 30, 2026 at 11:19:44AM +0300, Vladimir Oltean wrote:
> Upstream commit e15c5506dd39 ("net: enetc: allocate vf_state during PF
> probes") was backported incorrectly to kernels where enetc_pf_probe()
> still has to manually check whether the OF node of the PCI device is
> enabled.
>
> In kernels which contain commit bfce089ddd0e ("net: enetc: remove
> of_device_is_available() handling") and its dependent change, commit
> 6fffbc7ae137 ("PCI: Honor firmware's device disabled status"), the
> "err_device_disabled" label has disappeared. Yet, linux-6.1.y and
> earlier still contains it.
>
> The trouble is that upstream commit e15c5506dd39 ("net: enetc: allocate
> vf_state during PF probes"), backported as 35668e29e979 in linux-6.1.y,
> introduces new code for the err_setup_mac_addresses and err_alloc_netdev
> labels which calls kfree(pf->vf_state). This code must not execute for
> the err_device_disabled label, because at that stage, the pf structure
> has not yet been allocated, and is an uninitialized pointer.
>
> By moving the err_device_disabled label to undo just the previous
> operation, i.e. a successful enetc_psi_create() call with
> enetc_psi_destroy(), the dereference of uninitialized pf->vf_state is
> avoided.
>
> Fixes: 35668e29e979 ("net: enetc: allocate vf_state during PF probes")
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Closes: https://lore.kernel.org/linux-patches/20260330073356.GA1017537@ax162/
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org> # build
> ---
> drivers/net/ethernet/freescale/enetc/enetc_pf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index 99422c0b4a26..8cb4c759b165 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -1393,10 +1393,10 @@ static int enetc_pf_probe(struct pci_dev *pdev,
> si->ndev = NULL;
> free_netdev(ndev);
> err_alloc_netdev:
> -err_device_disabled:
> err_setup_mac_addresses:
> kfree(pf->vf_state);
> err_alloc_vf_state:
> +err_device_disabled:
> enetc_psi_destroy(pdev);
> err_psi_create:
> return err;
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] r8152: Add support for the RTL8157 hardware
From: Birger Koblitz @ 2026-03-31 15:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew+netdev, davem, edumazet, pabeni, linux-usb, netdev,
linux-kernel
In-Reply-To: <20260329193457.2764549-2-kuba@kernel.org>
On 3/29/26 21:34, Jakub Kicinski 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.
> ---
> r8152: Add support for the RTL8157 hardware
>
> This commit adds support for the RTL8157 chip, including its new 16-byte
> descriptor format, SRAM/ADV interfaces, and PHY setup. It reuses existing
> initialization functions where possible while branching for chip-specific
> logic.
>
>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
>> index bab8e10e5f99..010126e6dae4 100644
>> --- a/drivers/net/usb/r8152.c
>> +++ b/drivers/net/usb/r8152.c
>> @@ -1629,6 +1862,28 @@ static u16 sram_read(struct r8152 *tp, u16 addr)
>> return ocp_reg_read(tp, OCP_SRAM_DATA);
>> }
>>
>> +static void sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
>> +{
>> + u16 data = sram_read(tp, addr);
>> +
>> + data = (data & ~clear) | set;
>> + ocp_reg_write(tp, OCP_SRAM_DATA, data);
>> +}
>
> Does this sequence write the modified data back to the wrong address?
>
> Indirect SRAM access ports typically auto-increment their internal address
> pointer upon read. Since sram_read() reads OCP_SRAM_DATA, the address
> pointer likely advances, causing the subsequent ocp_reg_write() to corrupt
> the adjacent register instead of the intended one. Should this use
> sram_write(tp, addr, data) to reset the address before writing?
>
The SRAM access does not have this alleged behaviour. This optimization is
found in the out-of-tree driver by Realtek. I also read back the values written
into the SRAM address as well as the following address, and the code is correct.
>
> Is the same auto-increment issue present here with OCP_SRAM2_DATA?
No. Again, the optimization is also found in the out-of-tree driver and I explicitly
verified the data written.
>
> [ ... ]
>
>> @@ -4080,6 +4473,14 @@ static void rtl8156_runtime_enable(struct r8152 *tp, bool enable)
>> }
>> }
>>
>> +static void rtl8157_runtime_enable(struct r8152 *tp, bool enable)
>> +{
>> + r8153_queue_wake(tp, enable);
>> + r8153b_u1u2en(tp, false);
>> + r8157_u2p3en(tp, false);
>> + rtl_runtime_suspend_enable(tp, enable);
>> +}
>
> Does this function break power management upon the first suspend and resume
> cycle? Unlike rtl8156_runtime_enable() which checks the enable parameter,
> this implementation unconditionally calls r8153b_u1u2en(tp, false) and
> r8157_u2p3en(tp, false) even when resuming.
This appears to be a bug in the out-of-tree driver. I will change this
function to follow the behaviour of rtl8156_runtime_enable() in v5. I have also traced
the PM functions while entering and leaving S3 sleep, and the behavior is now correct.
>
> [ ... ]
>
>> @@ -6693,7 +7052,7 @@ static void rtl8156_up(struct r8152 *tp)
>> r8153_aldps_en(tp, true);
>> r8153_u2p3en(tp, true);
>>
>> - if (tp->udev->speed >= USB_SPEED_SUPER)
>> + if (tp->version != RTL_VER_16 && tp->udev->speed >= USB_SPEED_SUPER)
>> r8153b_u1u2en(tp, true);
>> }
>
> Because r8153_u2p3en(tp, true) is called unconditionally just before the
> modified lines, will RTL8157 incorrectly write to the legacy USB_U2P3_CTRL
> register instead of using its specific r8157_u2p3en() function?
This was a mistake. I will fix this to call r8153_u2p3en() only for non-RTL8157
chips.
>
> [ ... ]
>
>> @@ -6706,13 +7065,13 @@ static void rtl8156_down(struct r8152 *tp)
>> return;
>> }
>>
>> - ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3);
>> - ocp_data |= PLA_MCU_SPDWN_EN;
>> - ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, ocp_data);
>> + ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, PLA_MCU_SPDWN_EN);
>>
>> r8153b_u1u2en(tp, false);
>> - r8153_u2p3en(tp, false);
>> - r8153b_power_cut_en(tp, false);
>> + if (tp->version != RTL_VER_16) {
>> + r8153_u2p3en(tp, false);
>> + r8153b_power_cut_en(tp, false);
>> + }
>> r8153_aldps_en(tp, false);
>
> While the legacy registers are correctly avoided for RTL_VER_16 here, are we
> missing calls to r8157_u2p3en(tp, false) and r8157_power_cut_en(tp, false)?
> If omitted, these power states might remain permanently enabled when the
> interface goes down.
These calls are not in the out-of-tree driver. It appears the r8157_u2p3en()
is not used in the _up and _down functions. I measured the USB power
consumption with an interposed USB dongle on the port, and the power use is
better than for the RTL8153 when the link is down.
>
> [ ... ]
>
>> @@ -8362,10 +8825,21 @@ static void r8156b_init(struct r8152 *tp)
>>
>> set_bit(GREEN_ETHERNET, &tp->flags);
>>
>> - /* rx aggregation */
>> - ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
>> - ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
>> - ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
>> + /* rx aggregation / 16 bytes Rx descriptor */
>> + if (tp->version == RTL_VER_16)
>> + ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL, RX_AGG_DISABLE | RX_DESC_16B);
>
> Is it intended to clear the RX_DESC_16B bit for RTL_VER_16? The driver uses
> sizeof(struct rx_desc_v2) which is a 16-byte descriptor for RTL8157.
> Clearing this bit might configure the hardware to send 8-byte descriptors
> instead, leading to packet parsing corruption in rx_bottom().
The code is correct as per the out-of-tree driver. I also tested packet
transmission in various stress-tests.
>
> [ ... ]
>
>> @@ -9647,6 +10158,29 @@ static int rtl_ops_init(struct r8152 *tp)
>> tp->rx_buf_sz = 32 * 1024;
>> tp->eee_en = true;
>> tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
>> + r8152_desc_init(tp);
>> + break;
>> +
>> + case RTL_VER_16:
>> + tp->eee_en = true;
>> + tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
>> + tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
>> + ops->init = r8156_init;
>> + ops->enable = rtl8156_enable;
>> + ops->disable = rtl8153_disable;
>> + ops->up = rtl8156_up;
>> + ops->down = rtl8156_down;
>> + ops->unload = rtl8153_unload;
>
> Will using rtl8153_unload for RTL_VER_16 result in incorrect power cut
> teardown? rtl8153_unload calls the legacy r8153_power_cut_en(tp, false).
> RTL8157 seems to require r8157_power_cut_en(tp, false) to properly clear
> USB_MISC_2 bit 1 and PCUT_STATUS on module unload.
This behaviour is also found in the out-of-tree driver by Realtek.
The out-of-tree function reads:
static void rtl8157_unload(struct r8152 *tp)
{
if (test_bit(RTL8152_UNPLUG, &tp->flags))
return;
r8153_power_cut_en(tp, false);
/* Disable Interrupt Mitigation */
if (ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04,
BIT(0) | BIT(1) | BIT(2) | BIT(7)) < 0)
return;
}
However, I had forgotten to add the "Disable Interrupt Mitigation" part.
I therefore added it, depending on RTL_VER_16.
I tested unloading and re-loading the driver, again while measuring the USB power
consumption with an interposed USB dongle on the port, and the wattage
in the state where the driver is unloaded for the RTL8157 is lower than the
one for the RTL8153 (280mW vs 450mW). The driver behavior now appear
correct.
^ 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