* Re: [PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()
From: Kuniyuki Iwashima @ 2026-04-29 0:13 UTC (permalink / raw)
To: horms
Cc: andrew+netdev, bestswngs, davem, edumazet, kuba, martin.varghese,
netdev, pabeni, willemb, xmei5
In-Reply-To: <20260428160445.1336649-2-horms@kernel.org>
From: Simon Horman <horms@kernel.org>
Date: Tue, 28 Apr 2026 17:04:46 +0100
> From: 'Simon Horman' <horms@kernel.org>
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()
>
> This commit fixes a NULL pointer dereference in bareudp_fill_metadata_dst()
> by adding a check for the socket in the IPv6 path. If the device is down
> and the socket is NULL, it now returns -ESHUTDOWN.
>
> > Add a NULL check returning -ESHUTDOWN, consistent with the xmit paths
> > in the same driver.
>
> Does the IPv4 path also need to be updated to remain consistent?
No, because udp_tunnel_dst_lookup() does not take struct
sock while udp_tunnel6_dst_lookup() does.
And all the other fast paths (bareudp_xmit_skb() and
bareudp6_xmit_skb()) have NULL checks properly.
>
> While the IPv4 path lookup doesn't use the socket pointer directly, if the
> socket is NULL (meaning the device is down), shouldn't the IPv4 path also
> return -ESHUTDOWN instead of successfully performing a route lookup?
>
> > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
> > index 0df3208783ad..da5866ba0699 100644
> > --- a/drivers/net/bareudp.c
> > +++ b/drivers/net/bareudp.c
> > @@ -529,6 +529,9 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
>
> Is it possible for an IPv6 tunnel key to be incorrectly processed as IPv4
> earlier in this function?
I think this is orthogonal to this patch and can be a follow-up.
I have a series to remove all synchronize_rcu() from all UDP tunnel
users and udp_tunnel_sock_release(), but it conflict with this patch,
so I hope this land net-next this Thursday :)
>
> Looking at the preceding condition in this function:
>
> if (!ipv6_mod_enabled() || ip_tunnel_info_af(info) == AF_INET) {
> struct rtable *rt;
> ...
> info->key.u.ipv4.src = saddr;
> }
>
> If the IPv6 module is disabled at runtime, but an IPv6 tunnel key is provided
> (e.g., via OVS or TC), the code falls back to the IPv4 branch.
>
> Since key->u.ipv4.dst aliases exactly with bytes 4-7 of key->u.ipv6.src due
> to the union layout in struct ip_tunnel_key, could this result in an invalid
> IPv4 route lookup using portions of the IPv6 source address?
>
> Would it be safer to explicitly reject IPv6 tunnel keys when the IPv6 module
> is disabled?
^ permalink raw reply
* Re: [PATCH net-next 3/3] psp: validate IPv4 header fields in psp_dev_rcv()
From: Eric Dumazet @ 2026-04-29 0:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, pabeni, andrew+netdev, horms,
willemdebruijn.kernel, daniel.zahka
In-Reply-To: <20260428205352.1247325-4-kuba@kernel.org>
On Tue, Apr 28, 2026 at 1:53 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> psp_dev_rcv() is called from the NIC driver's RX completion path
> before the frame reaches ip_rcv_core(), so the IP header has not
> been validated in SW, yet. We expect that the device has done
> all this validation, but let's also add the SW checks, to avoid
> surprises.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net] bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()
From: Eric Dumazet @ 2026-04-29 0:18 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: horms, andrew+netdev, bestswngs, davem, kuba, martin.varghese,
netdev, pabeni, willemb, xmei5
In-Reply-To: <20260429001407.3358732-1-kuniyu@google.com>
On Tue, Apr 28, 2026 at 5:14 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> From: Simon Horman <horms@kernel.org>
> Date: Tue, 28 Apr 2026 17:04:46 +0100
> > From: 'Simon Horman' <horms@kernel.org>
> >
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://sashiko.dev
> > ---
> > bareudp: fix NULL pointer dereference in bareudp_fill_metadata_dst()
> >
> > This commit fixes a NULL pointer dereference in bareudp_fill_metadata_dst()
> > by adding a check for the socket in the IPv6 path. If the device is down
> > and the socket is NULL, it now returns -ESHUTDOWN.
> >
> > > Add a NULL check returning -ESHUTDOWN, consistent with the xmit paths
> > > in the same driver.
> >
> > Does the IPv4 path also need to be updated to remain consistent?
>
> No, because udp_tunnel_dst_lookup() does not take struct
> sock while udp_tunnel6_dst_lookup() does.
>
> And all the other fast paths (bareudp_xmit_skb() and
> bareudp6_xmit_skb()) have NULL checks properly.
>
>
> >
> > While the IPv4 path lookup doesn't use the socket pointer directly, if the
> > socket is NULL (meaning the device is down), shouldn't the IPv4 path also
> > return -ESHUTDOWN instead of successfully performing a route lookup?
> >
> > > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
> > > index 0df3208783ad..da5866ba0699 100644
> > > --- a/drivers/net/bareudp.c
> > > +++ b/drivers/net/bareudp.c
> > > @@ -529,6 +529,9 @@ static int bareudp_fill_metadata_dst(struct net_device *dev,
> >
> > Is it possible for an IPv6 tunnel key to be incorrectly processed as IPv4
> > earlier in this function?
>
> I think this is orthogonal to this patch and can be a follow-up.
>
> I have a series to remove all synchronize_rcu() from all UDP tunnel
> users and udp_tunnel_sock_release(), but it conflict with this patch,
> so I hope this land net-next this Thursday :)
Same here hope for me!
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next 3/3] psp: validate IPv4 header fields in psp_dev_rcv()
From: Willem de Bruijn @ 2026-04-29 0:22 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms,
willemdebruijn.kernel, daniel.zahka, Jakub Kicinski
In-Reply-To: <20260428205352.1247325-4-kuba@kernel.org>
Jakub Kicinski wrote:
> psp_dev_rcv() is called from the NIC driver's RX completion path
> before the frame reaches ip_rcv_core(), so the IP header has not
> been validated in SW, yet. We expect that the device has done
> all this validation, but let's also add the SW checks, to avoid
> surprises.
If devices are expected to have verified this, should these be more
noisy checks, similar to netdev_rx_csum_fault?
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> net/psp/psp_main.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
> index f069117c867a..524978dfb8fd 100644
> --- a/net/psp/psp_main.c
> +++ b/net/psp/psp_main.c
> @@ -300,6 +300,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
> if (proto == htons(ETH_P_IP)) {
> struct iphdr *iph = (struct iphdr *)(skb->data + l2_hlen);
>
> + if (unlikely(iph->ihl < 5))
> + return -EINVAL;
> +
> is_udp = iph->protocol == IPPROTO_UDP;
> l3_hlen = iph->ihl * 4;
> if (l3_hlen != sizeof(struct iphdr) &&
> @@ -335,6 +338,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
> if (proto == htons(ETH_P_IP)) {
> struct iphdr *iph = (struct iphdr *)(skb->data + l2_hlen);
>
> + if (unlikely(ntohs(iph->tot_len) < l3_hlen + encap))
> + return -EINVAL;
> +
> iph->protocol = psph->nexthdr;
> iph->tot_len = htons(ntohs(iph->tot_len) - encap);
> iph->check = 0;
> @@ -342,6 +348,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
> } else {
> struct ipv6hdr *ipv6h = (struct ipv6hdr *)(skb->data + l2_hlen);
>
> + if (unlikely(ntohs(ipv6h->payload_len) < encap))
> + return -EINVAL;
> +
> ipv6h->nexthdr = psph->nexthdr;
> ipv6h->payload_len = htons(ntohs(ipv6h->payload_len) - encap);
> }
> --
> 2.54.0
>
^ permalink raw reply
* Re: [PATCH net] ipv6: rpl: add NULL check for idev in ipv6_rpl_srh_rcv()
From: Eric Dumazet @ 2026-04-29 0:24 UTC (permalink / raw)
To: Andrea Mayer
Cc: David S . Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
Simon Horman, Alexander Aring, Justin Iurman, netdev,
linux-kernel, stable, stefano.salsano
In-Reply-To: <20260428224816.11223-1-andrea.mayer@uniroma2.it>
On Tue, Apr 28, 2026 at 3:48 PM Andrea Mayer <andrea.mayer@uniroma2.it> wrote:
>
> ipv6_rpl_srh_rcv() dereferences idev from __in6_dev_get() without
> a NULL check when reading idev->cnf.rpl_seg_enabled.
> When the device's MTU drops below IPV6_MIN_MTU, addrconf_ifdown()
> clears dev->ip6_ptr through RCU_INIT_POINTER(), which is immediately
> visible to concurrent readers. A packet that already passed the idev
> check in ip6_rcv_core() can race with this and hit a NULL pointer
> dereference.
>
> Reproduced by flooding traffic through a route with RPL source routing
> while rapidly flapping the receiving interface's MTU between 1500 and
> 1200:
>
> BUG: KASAN: null-ptr-deref in ipv6_rpl_srh_rcv+0xae/0x1050
> Read of size 4 at addr 00000000000006b4 by task ping6/318
>
> CPU: 0 UID: 0 PID: 318 Comm: ping6 Not tainted 7.1.0-rc1-micro-vm-dev-g46f74a3f7d57 #82 PREEMPT(full)
> Call Trace:
> <IRQ>
> kasan_report+0xc6/0x100
> ipv6_rpl_srh_rcv+0xae/0x1050
> ip6_protocol_deliver_rcu+0x717/0x960
> ip6_input_finish+0xa3/0x1b0
> ip6_input+0xdc/0x490
> ipv6_rcv+0x338/0x460
> __netif_receive_skb_one_core+0xd1/0x130
> process_backlog+0x2c7/0x9f0
> __napi_poll.constprop.0+0x51/0x270
> net_rx_action+0x322/0x730
> handle_softirqs+0x119/0x640
> do_softirq+0xae/0xe0
> </IRQ>
>
> Add a NULL check for idev after __in6_dev_get() and drop the skb if
> idev is NULL, consistent with the SRv6 fix in commit 064137935262
> ("ipv6: add NULL checks for idev in SRv6 paths").
>
> Fixes: 8610c7c6e3bd ("net: ipv6: add support for rpl sr exthdr")
> Cc: stable@vger.kernel.org
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> ---
> net/ipv6/exthdrs.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 03cbce842c1a..e398a8851031 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c
> @@ -499,6 +499,10 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb)
> u32 r;
>
> idev = __in6_dev_get(skb->dev);
> + if (!idev) {
> + kfree_skb(skb);
I suggest:
kfree_skb_reason(skb, SKB_DROP_REASON_IPV6DISABLED)
^ permalink raw reply
* Re: [RFC PATCH net-next 1/2] net: napi: Fix interrupts permanently disabled during busy poll
From: Jakub Kicinski @ 2026-04-29 0:31 UTC (permalink / raw)
To: Martin Karsten
Cc: Dragos Tatulea, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Daniel Borkmann, Björn Töpel,
Gal Pressman, Tariq Toukan, Joe Damato, Frederik Deweerdt, netdev,
linux-kernel
In-Reply-To: <4c45f423-ea41-4ae8-9cb0-7aca9157d8a4@uwaterloo.ca>
On Tue, 28 Apr 2026 20:04:13 -0400 Martin Karsten wrote:
> On 2026-04-28 19:40, Jakub Kicinski wrote:
> > On Tue, 28 Apr 2026 17:51:30 +0000 Dragos Tatulea wrote:
> >> Under certain conditions a queue can be left out with interrupts
> >> disabled and with the napi re-scheduling timer permanently stopped.
> >> This behaviour is triggered by the napi busy poll path when
> >> gro-flush-timeout and defer-hard-irq are set. Here's a sequence of
> >> operations:
> >>
> >> 1. Busy poll starts, NAPI_STATE_SCHED is set to avoid rescheduling napi
> >> from the timer.
> >>
> >> 2. During napi poll, driver disables interrupts due to being in poll
> >> mode (napi_complete_done() returns false because napi->state has
> >> NAPIF_STATE_IN_BUSY_POLL set).
> >
> > Why does the driver have IRQs disabled in busy poll?
>
> The problems occurs in irq deferral mode when both gro-flush-timeout and
> defer-hard-irqs are nonzero and NIC interrupts are disabled.
Okay.
> >> 3. At the end of the busy poll (busy_poll_stop()):
> >> 3.1 napi timer is scheduled and skip_schedule is set (due to config)
> >> 3.2 napi->poll() is called:
> >> - driver poll() processes exactly budget packets
> >> and exits early => napi not scheduled.
> >> (interrupts are still disabled at this point)
> >> 3.3 Since napi poll processed budget packets, __busy_poll_stop()
> >> is called with skip_schedule set => napi is not scheduled here
> >> either.
> >
> > with skip_schedule it calls:
> >
> > clear_bit(NAPI_STATE_SCHED, &napi->state);
> >
> >> 4. If the napi timer from 3.1 gets to be triggered due to slow napi poll
> >> or some other reason, the timer will run with no effect (due to
> >> NAPI_STATE_SCHED being set).
> >
> > And here you claim STATE_SCHED is still set?
>
> Labelling this with number 4. might be misleading, sorry! The concern is
> that a short enough timer (compared to the duration of the driver poll)
> can be triggered before the NAPI_STATE_SCHED bit is cleared at the end
> of Step 3.3.
Ah. Just say that :D Two pages of buggy text, y'all would have been
better off using this one paragraph as the commit message.
Please don't use AI for generating commit messages if that's the cause.
It really is spectacularly shit at it.
^ permalink raw reply
* Re: [RFC PATCH net-next 2/2] net: napi: Skip poll when arming GRO timer in busy poll
From: Jakub Kicinski @ 2026-04-29 0:37 UTC (permalink / raw)
To: Dragos Tatulea
Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Martin Karsten, Gal Pressman, Tariq Toukan, Joe Damato,
Frederik Deweerdt, netdev, linux-kernel
In-Reply-To: <20260428175134.1197036-4-dtatulea@nvidia.com>
On Tue, 28 Apr 2026 17:51:31 +0000 Dragos Tatulea wrote:
> From: Martin Karsten <mkarsten@uwaterloo.ca>
>
> As referenced in the previous patch, having the GRO timer scheduled
> while poll is running can lead to issues.
>
> Skip the extra call to napi->poll() when the GRO timer is armed. This
> removes the need for having a separate __busy_poll_stop routine and its
> code is moved directly into the relevant places in busy_poll_stop.
This needs to go in separately, the previous patch should be a Fix,
this is net-next material.
> @@ -6918,13 +6898,28 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock,
>
> if (flags & NAPI_F_PREFER_BUSY_POLL) {
> napi->defer_hard_irqs_count = napi_get_defer_hard_irqs(napi);
> - if (napi->defer_hard_irqs_count) {
> - /* Timer will be scheduled after napi poll to avoid
> - * firing during a slow poll which could cause the
> - * queue to get stuck with interrupts disabled and no
> - * scheduled timer.
> + timeout = napi_get_gro_flush_timeout(napi);
> + if (napi->defer_hard_irqs_count && timeout) {
> + unsigned long flags;
Feels like either timeout should also be declared in closest scope or
flags at function level
> + /* Drop prefer-busy state as in napi_complete_done(). */
sloppy comment
> + clear_bit(NAPI_STATE_PREFER_BUSY_POLL, &napi->state);
> + netpoll_poll_unlock(have_poll_lock);
> +
> + /* Flush too old packets. If HZ < 1000, flush all
> + * packets.
> + */
sloppy comment
> + gro_flush_normal(&napi->gro, HZ >= 1000);
> + local_irq_save(flags);
> + hrtimer_start(&napi->timer, ns_to_ktime(timeout),
> + HRTIMER_MODE_REL_PINNED);
> + clear_bit(NAPI_STATE_SCHED, &napi->state);
> + local_irq_restore(flags);
> +
> + /* Timer started, so need for another call to
> + * napi->poll().
> */
> - timeout = napi_get_gro_flush_timeout(napi);
> + goto out;
I think an else branch would do here? Let's not abuse goto
> }
> }
>
> @@ -6938,8 +6933,12 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock,
> */
> trace_napi_poll(napi, rc, budget);
> netpoll_poll_unlock(have_poll_lock);
> - if (rc == budget)
> - __busy_poll_stop(napi, timeout);
> + if (rc == budget) {
> + gro_normal_list(&napi->gro);
> + __napi_schedule(napi);
> + }
> +
> +out:
> bpf_net_ctx_clear(bpf_net_ctx);
> local_bh_enable();
> }
^ permalink raw reply
* Re: [RFC PATCH net-next 1/2] net: napi: Fix interrupts permanently disabled during busy poll
From: Jakub Kicinski @ 2026-04-29 0:38 UTC (permalink / raw)
To: Dragos Tatulea
Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Daniel Borkmann, Björn Töpel, Martin Karsten,
Gal Pressman, Tariq Toukan, Joe Damato, Frederik Deweerdt, netdev,
linux-kernel
In-Reply-To: <20260428175134.1197036-3-dtatulea@nvidia.com>
On Tue, 28 Apr 2026 17:51:30 +0000 Dragos Tatulea wrote:
> + local_irq_save(flags);
> + hrtimer_start(&napi->timer, ns_to_ktime(timeout),
> + HRTIMER_MODE_REL_PINNED);
> clear_bit(NAPI_STATE_SCHED, &napi->state);
> + local_irq_restore(flags);
I don't think disabling IRQ is necessary?
Isn't it legal to clear the bit first then schedule the timer?
The timer does not own the napi instance.
^ permalink raw reply
* RE: [PATCH v2 7/9] wifi: rtw89: switch to using FIELD_GET_SIGNED()
From: Ping-Ke Shih @ 2026-04-29 0:39 UTC (permalink / raw)
To: David Laight, Andy Shevchenko
Cc: Yury Norov, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Andy Lutomirski, Peter Zijlstra,
Jonathan Cameron, David Lechner, Johannes Berg, Nuno Sá,
Andy Shevchenko, Richard Cochran, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexandre Belloni,
Yury Norov, Rasmus Villemoes, Hans de Goede, Linus Walleij,
Sakari Ailus, Salah Triki, Achim Gratz, Ben Collins,
x86@kernel.org, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-rtc@vger.kernel.org
In-Reply-To: <20260428114342.0d674fd8@pumpkin>
David Laight <david.laight.linux@gmail.com> wrote:
>
> On Tue, 28 Apr 2026 10:10:22 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>
> > On Mon, Apr 27, 2026 at 05:41:24PM -0400, Yury Norov wrote:
> > > Switch from sign_extend32(FIELD_GET()) to the dedicated
> > > FIELD_GET_SIGNED() and don't calculate the fields length explicitly.
> >
> > ...
> >
> > > for (i = 0; i < ADDC_T_AVG; i++) {
> > > tmp = rtw89_phy_read32_mask(rtwdev, R_DBG32_D, MASKDWORD);
> > > - dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> > > - dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
> > > + dc_re += FIELD_GET_SIGNED(0xfff000, tmp);
> > > + dc_im += FIELD_GET_SIGNED(0xfff, tmp);
> >
> > In the same driver the GENMASK() is being used, why not doing it here while at it?
>
> To me those bit masks look more readable than the GENMASK() calls would be.
Either way is fine to me. As we rewrite these into rtw89, using GENMASK()
can be easier to check if vendor driver defines the masks correctly, no
overlap.
Ping-Ke
^ permalink raw reply
* RE: [PATCH v2 7/9] wifi: rtw89: switch to using FIELD_GET_SIGNED()
From: Ping-Ke Shih @ 2026-04-29 0:40 UTC (permalink / raw)
To: Yury Norov, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Andy Lutomirski, Peter Zijlstra,
Jonathan Cameron, David Lechner, Johannes Berg, David Laight,
Nuno Sá, Andy Shevchenko, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexandre Belloni, Yury Norov, Rasmus Villemoes, Hans de Goede,
Linus Walleij, Sakari Ailus, Salah Triki, Achim Gratz,
Ben Collins, x86@kernel.org, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-rtc@vger.kernel.org
In-Reply-To: <20260427214127.406067-8-ynorov@nvidia.com>
Yury Norov <ynorov@nvidia.com> wrote:
> Switch from sign_extend32(FIELD_GET()) to the dedicated
> FIELD_GET_SIGNED() and don't calculate the fields length explicitly.
>
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
^ permalink raw reply
* Re: [PATCH net-next] net: gianfar: use alloc_ethdev_mqs
From: Andrew Lunn @ 2026-04-29 0:41 UTC (permalink / raw)
To: Rosen Penev
Cc: netdev, Claudiu Manoil, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, open list
In-Reply-To: <CAKxU2N8yb9-M9zELSxdEjLz02GKFZ2mOmgHhc7NbW+oJsvtGXg@mail.gmail.com>
> > > + *pdev = alloc_etherdev_mqs(sizeof(*priv), num_tx_qs, num_rx_qs);
> > > dev = *pdev;
> > > if (NULL == dev)
> > > return -ENOMEM;
> > > @@ -679,10 +679,6 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
> > >
> > > priv->mode = mode;
> > >
> > > - priv->num_tx_queues = num_tx_qs;
> > > - netif_set_real_num_rx_queues(dev, num_rx_qs);
> > > - priv->num_rx_queues = num_rx_qs;
> >
> > Please add to the commit message an explanation of why these two
> > assignments can be removed, because it is not obvious.
> I didn't explain that _mqs sets them?
How can alloc_etherdev_mqs() set them? priv is opaque to the core. All
the core knows is the size of struct gfar_private, but nothing about
its layout, where num_tx_queues and num_rx_queues are within priv.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 2/2] net: napi: Skip poll when arming GRO timer in busy poll
From: Martin Karsten @ 2026-04-29 1:02 UTC (permalink / raw)
To: Jakub Kicinski, Dragos Tatulea
Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Gal Pressman, Tariq Toukan, Joe Damato, Frederik Deweerdt, netdev,
linux-kernel
In-Reply-To: <20260428173734.7f4ee58d@kernel.org>
On 2026-04-28 20:37, Jakub Kicinski wrote:
> On Tue, 28 Apr 2026 17:51:31 +0000 Dragos Tatulea wrote:
>> From: Martin Karsten <mkarsten@uwaterloo.ca>
>>
>> As referenced in the previous patch, having the GRO timer scheduled
>> while poll is running can lead to issues.
>>
>> Skip the extra call to napi->poll() when the GRO timer is armed. This
>> removes the need for having a separate __busy_poll_stop routine and its
>> code is moved directly into the relevant places in busy_poll_stop.
>
> This needs to go in separately, the previous patch should be a Fix,
> this is net-next material.
Ok, thanks for the recommendation!
>> @@ -6918,13 +6898,28 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock,
>>
>> if (flags & NAPI_F_PREFER_BUSY_POLL) {
>> napi->defer_hard_irqs_count = napi_get_defer_hard_irqs(napi);
>> - if (napi->defer_hard_irqs_count) {
>> - /* Timer will be scheduled after napi poll to avoid
>> - * firing during a slow poll which could cause the
>> - * queue to get stuck with interrupts disabled and no
>> - * scheduled timer.
>> + timeout = napi_get_gro_flush_timeout(napi);
>> + if (napi->defer_hard_irqs_count && timeout) {
>> + unsigned long flags;
>
> Feels like either timeout should also be declared in closest scope or
> flags at function level
Good point, thanks. I also just realize there's two flags - I somehow
missed that before. Maybe the 2nd flags isn't necessary after all.
>> + /* Drop prefer-busy state as in napi_complete_done(). */
>
> sloppy comment
Ok, will improve.
>> + clear_bit(NAPI_STATE_PREFER_BUSY_POLL, &napi->state);
>> + netpoll_poll_unlock(have_poll_lock);
>> +
>> + /* Flush too old packets. If HZ < 1000, flush all
>> + * packets.
>> + */
>
> sloppy comment
This one was just copied over from __busy_poll_stop. TBH, I don't know
exactly what's happening in the 'gro_flush_normal' call.
>> + gro_flush_normal(&napi->gro, HZ >= 1000);
>> + local_irq_save(flags);
>> + hrtimer_start(&napi->timer, ns_to_ktime(timeout),
>> + HRTIMER_MODE_REL_PINNED);
>> + clear_bit(NAPI_STATE_SCHED, &napi->state);
>> + local_irq_restore(flags);
>> +
>> + /* Timer started, so need for another call to
>> + * napi->poll().
>> */
>> - timeout = napi_get_gro_flush_timeout(napi);
>> + goto out;
>
> I think an else branch would do here? Let's not abuse goto
Ok, will rewrite. Among other things, I thought a goto avoids
indentation of the unchanged code and thus makes the patch smaller, but
I have no strong preference.
Thanks,
Martin
^ permalink raw reply
* Re: [PATCH net-next v7 2/4] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
From: Jakub Kicinski @ 2026-04-29 1:06 UTC (permalink / raw)
To: lizhi2
Cc: devicetree, andrew+netdev, davem, edumazet, robh, krzk+dt,
conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
rmk+kernel, pjw, palmer, aou, alex, linux-riscv, linux-stm32,
linux-arm-kernel, linux-kernel, maxime.chevallier, ningyu, linmin,
pinkesh.vaghela, pritesh.patel, weishangjuan, horms
In-Reply-To: <20260427072508.1151-1-lizhi2@eswincomputing.com>
On Mon, 27 Apr 2026 15:25:05 +0800 lizhi2@eswincomputing.com wrote:
> From: Zhi Li <lizhi2@eswincomputing.com>
>
> The second Ethernet controller (eth1) on the Eswin EIC7700 SoC may fail
> to sample RX data correctly at Gigabit speed due to EIC7700-specific
> receive clock to data skew at the MAC input in the silicon.
>
> The existing internal delay configuration does not provide sufficient
> adjustment range to compensate for this condition at 1000Mbps.
> Update the EIC7700 DWMAC glue driver to apply EIC7700-specific clock
> sampling inversion only during Gigabit operation on MAC instances
> that require it.
>
> TXD and RXD delay registers are explicitly cleared during initialization
> to override any residual configuration left by the bootloader. All HSP
> CSR register accesses are performed only after the required clocks are
> enabled.
>
> Fixes: ea77dbbdbc4e ("net: stmmac: add Eswin EIC7700 glue driver")
Why Fixes? If eth1 never worked this is not a fix but new functionality
If you want to make this a fix to prevent incompatibility - cut it down
just to the eth0 changes.
> Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
> ---
> .../ethernet/stmicro/stmmac/dwmac-eic7700.c | 183 ++++++++++++++----
> 1 file changed, 140 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> index bcb8e000e720..33144611da8d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> @@ -28,20 +28,40 @@
>
> /*
> * TX/RX Clock Delay Bit Masks:
> - * - TX Delay: bits [14:8] — TX_CLK delay (unit: 0.1ns per bit)
> - * - RX Delay: bits [30:24] — RX_CLK delay (unit: 0.1ns per bit)
> + * - TX Delay: bits [14:8] — TX_CLK delay (unit: 0.02ns per bit)
> + * - TX Invert : bit [15]
> + * - RX Delay: bits [30:24] — RX_CLK delay (unit: 0.02ns per bit)
> + * - RX Invert : bit [31]
> */
> #define EIC7700_ETH_TX_ADJ_DELAY GENMASK(14, 8)
> #define EIC7700_ETH_RX_ADJ_DELAY GENMASK(30, 24)
> +#define EIC7700_ETH_TX_INV_DELAY BIT(15)
> +#define EIC7700_ETH_RX_INV_DELAY BIT(31)
>
> -#define EIC7700_MAX_DELAY_UNIT 0x7F
> +#define EIC7700_MAX_DELAY_STEPS 0x7F
> +#define EIC7700_DELAY_STEP_PS 20
> +#define EIC7700_MAX_DELAY_PS \
> + (EIC7700_MAX_DELAY_STEPS * EIC7700_DELAY_STEP_PS)
AI says:
The step unit is being silently changed from 0.1 ns (delay_ps / 100)
to 0.02 ns (delay_ps / 20). The same DT value now programs 5x the number
of delay steps into the hardware.
> static const char * const eic7700_clk_names[] = {
> "tx", "axi", "cfg",
> };
>
> +struct eic7700_dwmac_data {
> + bool rgmii_rx_clk_invert;
> +};
> +
> struct eic7700_qos_priv {
> + struct device *dev;
> struct plat_stmmacenet_data *plat_dat;
> + struct regmap *eic7700_hsp_regmap;
> + u32 eth_axi_lp_ctrl_offset;
> + u32 eth_phy_ctrl_offset;
> + u32 eth_txd_offset;
> + u32 eth_clk_offset;
> + u32 eth_rxd_offset;
> + u32 eth_clk_dly_param;
> + bool eth_rx_clk_inv;
> };
>
> static int eic7700_clks_config(void *priv, bool enabled)
> @@ -61,8 +81,28 @@ static int eic7700_clks_config(void *priv, bool enabled)
> static int eic7700_dwmac_init(struct device *dev, void *priv)
> {
> struct eic7700_qos_priv *dwc = priv;
> + int ret;
> +
> + ret = eic7700_clks_config(dwc, true);
> + if (ret)
> + return ret;
> +
> + ret = regmap_set_bits(dwc->eic7700_hsp_regmap,
> + dwc->eth_phy_ctrl_offset,
> + EIC7700_ETH_TX_CLK_SEL |
> + EIC7700_ETH_PHY_INTF_SELI);
> + if (ret) {
> + eic7700_clks_config(dwc, false);
> + return ret;
> + }
> +
> + regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_axi_lp_ctrl_offset,
> + EIC7700_ETH_CSYSREQ_VAL);
> +
> + regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_txd_offset, 0);
> + regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_rxd_offset, 0);
>
> - return eic7700_clks_config(dwc, true);
> + return 0;
> }
>
> static void eic7700_dwmac_exit(struct device *dev, void *priv)
> @@ -88,18 +128,35 @@ static int eic7700_dwmac_resume(struct device *dev, void *priv)
> return ret;
> }
>
> +static void eic7700_dwmac_fix_speed(void *priv, phy_interface_t interface,
> + int speed, unsigned int mode)
> +{
> + struct eic7700_qos_priv *dwc = (struct eic7700_qos_priv *)priv;
> + u32 dly_param = dwc->eth_clk_dly_param;
> +
> + switch (speed) {
> + case SPEED_1000:
> + if (dwc->eth_rx_clk_inv)
> + dly_param |= EIC7700_ETH_RX_INV_DELAY;
> + break;
> + case SPEED_100:
> + case SPEED_10:
> + break;
> + default:
> + dev_err(dwc->dev, "invalid speed %u\n", speed);
> + break;
> + }
> +
> + regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_clk_offset, dly_param);
AI says
In the default case this logs "invalid speed %u" but then falls
through and still executes the regmap_write() with the base dly_param. An
unsupported speed reports an error and reprograms the hardware anyway.
Should the default path return without writing, or should the write be
moved into the valid cases only?
> +}
> +
> static int eic7700_dwmac_probe(struct platform_device *pdev)
> {
> + const struct eic7700_dwmac_data *data;
> struct plat_stmmacenet_data *plat_dat;
> struct stmmac_resources stmmac_res;
> struct eic7700_qos_priv *dwc_priv;
> - struct regmap *eic7700_hsp_regmap;
> - u32 eth_axi_lp_ctrl_offset;
> - u32 eth_phy_ctrl_offset;
> - u32 eth_phy_ctrl_regset;
> - u32 eth_rxd_dly_offset;
> - u32 eth_dly_param = 0;
> - u32 delay_ps;
> + u32 delay_ps, val;
> int i, ret;
>
> ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> @@ -116,70 +173,95 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
> if (!dwc_priv)
> return -ENOMEM;
>
> + dwc_priv->dev = &pdev->dev;
> +
> + data = device_get_match_data(&pdev->dev);
> + if (!data)
> + return dev_err_probe(&pdev->dev,
> + -EINVAL, "no match data found\n");
> +
> + dwc_priv->eth_rx_clk_inv = data->rgmii_rx_clk_invert;
> +
> /* Read rx-internal-delay-ps and update rx_clk delay */
> if (!of_property_read_u32(pdev->dev.of_node,
> "rx-internal-delay-ps", &delay_ps)) {
> - u32 val = min(delay_ps / 100, EIC7700_MAX_DELAY_UNIT);
> + if (delay_ps % EIC7700_DELAY_STEP_PS)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "rx delay must be multiple of %dps\n",
> + EIC7700_DELAY_STEP_PS);
> +
> + if (delay_ps > EIC7700_MAX_DELAY_PS)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "rx delay out of range\n");
>
> - eth_dly_param &= ~EIC7700_ETH_RX_ADJ_DELAY;
> - eth_dly_param |= FIELD_PREP(EIC7700_ETH_RX_ADJ_DELAY, val);
> - } else {
> - return dev_err_probe(&pdev->dev, -EINVAL,
> - "missing required property rx-internal-delay-ps\n");
> + val = delay_ps / EIC7700_DELAY_STEP_PS;
> +
> + dwc_priv->eth_clk_dly_param &= ~EIC7700_ETH_RX_ADJ_DELAY;
> + dwc_priv->eth_clk_dly_param |=
> + FIELD_PREP(EIC7700_ETH_RX_ADJ_DELAY, val);
> }
>
> /* Read tx-internal-delay-ps and update tx_clk delay */
> if (!of_property_read_u32(pdev->dev.of_node,
> "tx-internal-delay-ps", &delay_ps)) {
> - u32 val = min(delay_ps / 100, EIC7700_MAX_DELAY_UNIT);
> + if (delay_ps % EIC7700_DELAY_STEP_PS)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "tx delay must be multiple of %dps\n",
> + EIC7700_DELAY_STEP_PS);
> +
> + if (delay_ps > EIC7700_MAX_DELAY_PS)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "tx delay out of range\n");
> +
> + val = delay_ps / EIC7700_DELAY_STEP_PS;
>
> - eth_dly_param &= ~EIC7700_ETH_TX_ADJ_DELAY;
> - eth_dly_param |= FIELD_PREP(EIC7700_ETH_TX_ADJ_DELAY, val);
> - } else {
> - return dev_err_probe(&pdev->dev, -EINVAL,
> - "missing required property tx-internal-delay-ps\n");
> + dwc_priv->eth_clk_dly_param &= ~EIC7700_ETH_TX_ADJ_DELAY;
> + dwc_priv->eth_clk_dly_param |=
> + FIELD_PREP(EIC7700_ETH_TX_ADJ_DELAY, val);
> }
AI says:
First, rx-internal-delay-ps and tx-internal-delay-ps are silently
demoted from required to optional. The previous driver returned
-EINVAL with "missing required property ..." when either was absent;
the else branches have been removed, so missing properties now leave
eth_clk_dly_param at zero and fix_mac_speed() programs zero delay
without diagnostic. Is this intentional, and if so should the commit
message and the DT binding reflect it?
Second, the previous code silently clamped out-of-range values via
min() and only required non-zero step units by integer truncation.
The new code rejects non-multiples of 20 and values greater than 2540
ps with -EINVAL at probe time. DTBs that previously probed with
clamped delays will now fail probe. Should this stricter validation
also be called out in the commit message?
> - eic7700_hsp_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> - "eswin,hsp-sp-csr");
> - if (IS_ERR(eic7700_hsp_regmap))
> + dwc_priv->eic7700_hsp_regmap =
> + syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> + "eswin,hsp-sp-csr");
> + if (IS_ERR(dwc_priv->eic7700_hsp_regmap))
> return dev_err_probe(&pdev->dev,
> - PTR_ERR(eic7700_hsp_regmap),
> + PTR_ERR(dwc_priv->eic7700_hsp_regmap),
> "Failed to get hsp-sp-csr regmap\n");
>
> ret = of_property_read_u32_index(pdev->dev.of_node,
> "eswin,hsp-sp-csr",
> - 1, ð_phy_ctrl_offset);
> + 1, &dwc_priv->eth_phy_ctrl_offset);
> if (ret)
> return dev_err_probe(&pdev->dev, ret,
> "can't get eth_phy_ctrl_offset\n");
>
> - regmap_read(eic7700_hsp_regmap, eth_phy_ctrl_offset,
> - ð_phy_ctrl_regset);
> - eth_phy_ctrl_regset |=
> - (EIC7700_ETH_TX_CLK_SEL | EIC7700_ETH_PHY_INTF_SELI);
> - regmap_write(eic7700_hsp_regmap, eth_phy_ctrl_offset,
> - eth_phy_ctrl_regset);
> -
> ret = of_property_read_u32_index(pdev->dev.of_node,
> "eswin,hsp-sp-csr",
> - 2, ð_axi_lp_ctrl_offset);
> + 2, &dwc_priv->eth_axi_lp_ctrl_offset);
> if (ret)
> return dev_err_probe(&pdev->dev, ret,
> "can't get eth_axi_lp_ctrl_offset\n");
>
> - regmap_write(eic7700_hsp_regmap, eth_axi_lp_ctrl_offset,
> - EIC7700_ETH_CSYSREQ_VAL);
> + ret = of_property_read_u32_index(pdev->dev.of_node,
> + "eswin,hsp-sp-csr",
> + 3, &dwc_priv->eth_clk_offset);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "can't get eth_clk_offset\n");
>
> ret = of_property_read_u32_index(pdev->dev.of_node,
> "eswin,hsp-sp-csr",
> - 3, ð_rxd_dly_offset);
> + 4, &dwc_priv->eth_txd_offset);
> if (ret)
> return dev_err_probe(&pdev->dev, ret,
> - "can't get eth_rxd_dly_offset\n");
> + "can't get eth_txd_offset\n");
>
> - regmap_write(eic7700_hsp_regmap, eth_rxd_dly_offset,
> - eth_dly_param);
> + ret = of_property_read_u32_index(pdev->dev.of_node,
> + "eswin,hsp-sp-csr",
> + 5, &dwc_priv->eth_rxd_offset);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "can't get eth_rxd_offset\n");
AI says:
The eswin,hsp-sp-csr phandle-args layout is expanded from 4 cells
(phandle + 3 offsets) to 6 cells (phandle + 5 offsets), with new
indices 3/4/5 now required. Any DTB produced against the original
ea77dbbdbc4e binding will fail probe here with -EINVAL.
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH 0/9] Support for block device NVMEM providers
From: Andrew Lunn @ 2026-04-29 1:05 UTC (permalink / raw)
To: Loic Poulain
Cc: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Jens Axboe, Johannes Berg,
Jeff Johnson, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, linux-mmc, devicetree, linux-kernel, linux-arm-msm,
linux-block, linux-wireless, ath10k, linux-bluetooth, netdev,
daniel
In-Reply-To: <20260428-block-as-nvmem-v1-0-6ad23e75190a@oss.qualcomm.com>
> Note that this is currently limited to eMMC-backed block devices, as
> only the eMMC core associates a firmware node with the block device
> (add_disk_fwnode). This can be easily extended in the future to
> support additional block drivers.
Would this be
https://elixir.bootlin.com/linux/v7.0.1/source/drivers/mmc/core/block.c#L2641
Looking at that function, mmc_blk_alloc_req() i don't see it doing
anything different between an eMMC and MMC.
An eMMC you don't expect to go away, since it is soldered
down. However an MMC can be ejected. Is the code prepared for that?
Andrew
^ permalink raw reply
* [PATCH net-next] tcp: add tcp_mstamp_refresh_inline()
From: Eric Dumazet @ 2026-04-29 1:08 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Neal Cardwell, Kuniyuki Iwashima, netdev,
eric.dumazet, Eric Dumazet
We want to inline tcp_mstamp_refresh() in fast path only:
- tcp_rcv_established()
- tcp_write_xmit()
Add tcp_mstamp_refresh_inline() for this purpose.
Add noinline qualifier on tcp_mstamp_refresh() for the other paths,
to reduce bloat.
$ scripts/bloat-o-meter -t vmlinux.old vmlinux.new
add/remove: 0/0 grow/shrink: 1/4 up/down: 26/-123 (-97)
Function old new delta
tcp_rcv_established 2238 2264 +26
tcp_connect 4027 4003 -24
tcp_tsq_write 152 120 -32
tcp_send_active_reset 476 444 -32
tcp_send_window_probe 235 200 -35
Total: Before=25316710, After=25316613, chg -0.00%
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/tcp.h | 10 ++++++++++
net/ipv4/tcp_input.c | 2 +-
net/ipv4/tcp_output.c | 12 +++---------
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index ecbadcb3a7446cb18c245e670ba49ff574dfaff7..fb2bc9edc7de01bfc494afa828428723a352c02b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1015,6 +1015,16 @@ static inline u32 tcp_time_stamp_ts(const struct tcp_sock *tp)
return tcp_time_stamp_ms(tp);
}
+/* Refresh clocks of a TCP socket,
+ * ensuring monotically increasing values.
+ */
+static inline void tcp_mstamp_refresh_inline(struct tcp_sock *tp)
+{
+ u64 val = tcp_clock_ns();
+
+ tp->tcp_clock_cache = val;
+ tp->tcp_mstamp = div_u64(val, NSEC_PER_USEC);
+}
void tcp_mstamp_refresh(struct tcp_sock *tp);
static inline u32 tcp_stamp_us_delta(u64 t1, u64 t0)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d5c9e65d97606d8eb57aba8ebc2373adf1bed62b..7995a89bafc9d1d997b936eeac3964cdd7e7741a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6474,7 +6474,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
/* TCP congestion window tracking */
trace_tcp_probe(sk, skb);
- tcp_mstamp_refresh(tp);
+ tcp_mstamp_refresh_inline(tp);
if (unlikely(!rcu_access_pointer(sk->sk_rx_dst)))
inet_csk(sk)->icsk_af_ops->sk_rx_dst_set(sk, skb);
/*
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f9d8755705f762fe4da3064d2b1bfce4828ec0c1..c8d7b5d20d1874bb7840e1c480195076830f1058 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -52,15 +52,9 @@
#include <trace/events/tcp.h>
-/* Refresh clocks of a TCP socket,
- * ensuring monotically increasing values.
- */
-void tcp_mstamp_refresh(struct tcp_sock *tp)
+void noinline tcp_mstamp_refresh(struct tcp_sock *tp)
{
- u64 val = tcp_clock_ns();
-
- tp->tcp_clock_cache = val;
- tp->tcp_mstamp = div_u64(val, NSEC_PER_USEC);
+ tcp_mstamp_refresh_inline(tp);
}
static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
@@ -2971,7 +2965,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
sent_pkts = 0;
- tcp_mstamp_refresh(tp);
+ tcp_mstamp_refresh_inline(tp);
/* AccECN option beacon depends on mstamp, it may change mss */
if (tcp_ecn_mode_accecn(tp) && tcp_accecn_option_beacon_check(sk))
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply related
* Re: [PATCH 2/2] net: thunderbolt: Fix lockdep issue during tunnel tear down
From: Jakub Kicinski @ 2026-04-29 1:12 UTC (permalink / raw)
To: Mika Westerberg
Cc: netdev, Yehezkel Bernat, Andrew Lunn, David S . Miller,
Eric Dumazet, Paolo Abeni
In-Reply-To: <20260427081623.2338179-2-mika.westerberg@linux.intel.com>
On Mon, 27 Apr 2026 10:16:23 +0200 Mika Westerberg wrote:
> When unplugging cable with lockdep enabled it triggers following:
Sounds like a fix, should it have a Fixes tag?
I'll also send out the AI review but on a quick look not sure if any
of it is worth addressing.
^ permalink raw reply
* Re: [PATCH 1/2] net: thunderbolt: Call tbnet_stop() upon shutdown
From: Jakub Kicinski @ 2026-04-29 1:12 UTC (permalink / raw)
To: mika.westerberg
Cc: Jakub Kicinski, netdev, YehezkelShB, andrew+netdev, davem,
edumazet, pabeni
In-Reply-To: <20260427081623.2338179-1-mika.westerberg@linux.intel.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: thunderbolt: Call tbnet_stop() upon shutdown
This commit updates the shutdown callback for the Thunderbolt network service
driver to call tbnet_stop() if the interface is running. This ensures the
service drivers release the rings upon shutdown.
> diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> index 7aae5d915a1e..631af237998d 100644
> --- a/drivers/net/thunderbolt/main.c
> +++ b/drivers/net/thunderbolt/main.c
> @@ -1417,7 +1417,10 @@ static void tbnet_remove(struct tb_service *svc)
>
> static void tbnet_shutdown(struct tb_service *svc)
> {
> - tbnet_tear_down(tb_service_get_drvdata(svc), true);
> + struct tbnet *net = tb_service_get_drvdata(svc);
> +
> + if (netif_running(net->dev))
> + tbnet_stop(net->dev);
Does calling tbnet_stop() directly bypass the network stack teardown sequence?
tbnet_stop() appears to be the driver's ndo_stop callback. By bypassing
dev_close(), netif_tx_disable() is not called and the device's transmit queues
remain active.
tbnet_stop() destroys the DMA rings and explicitly sets net->tx_ring.ring
to NULL:
drivers/net/thunderbolt/main.c:tbnet_stop() {
...
tb_ring_free(net->tx_ring.ring);
net->tx_ring.ring = NULL;
...
}
If a concurrent transmission occurs, will tbnet_start_xmit() access the null
pointer and cause a crash?
Additionally, since this does not hold the rtnl_lock(), could this race
with concurrent administrative state changes (like 'ip link set dev down')
and lead to a double-execution of tbnet_stop()?
> }
>
> static int tbnet_suspend(struct device *dev)
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH 2/2] net: thunderbolt: Fix lockdep issue during tunnel tear down
From: Jakub Kicinski @ 2026-04-29 1:12 UTC (permalink / raw)
To: mika.westerberg
Cc: Jakub Kicinski, netdev, YehezkelShB, andrew+netdev, davem,
edumazet, pabeni
In-Reply-To: <20260427081623.2338179-2-mika.westerberg@linux.intel.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: thunderbolt: Fix lockdep issue during tunnel tear down
This commit attempts to fix a circular locking dependency warning in
tbnet_tear_down() by dropping the connection lock earlier and returning if the
connection isn't fully established.
> diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> index 631af237998d..d8fcf18fc55c 100644
> --- a/drivers/net/thunderbolt/main.c
> +++ b/drivers/net/thunderbolt/main.c
[ ... ]
> @@ -372,48 +372,47 @@ static void tbnet_free_buffers(struct tbnet_ring *ring)
>
> static void tbnet_tear_down(struct tbnet *net, bool send_logout)
> {
> + int ret, retries = TBNET_LOGOUT_RETRIES;
> +
> netif_carrier_off(net->dev);
> netif_stop_queue(net->dev);
>
> stop_login(net);
>
> mutex_lock(&net->connection_lock);
> + if (!net->login_sent || !net->login_received) {
> + mutex_unlock(&net->connection_lock);
> + return;
> + }
If the connection is in a partially established state (e.g. login_sent is
true, but login_received is false), this early return skips clearing the
connection states.
Could this stall the connection if a new TBIP_LOGIN arrives later?
tbnet_handle_packet() would see net->login_sent as true and might fail to
queue the login_work to reply.
> + net->login_sent = false;
> + net->login_received = false;
> + mutex_unlock(&net->connection_lock);
By releasing net->connection_lock here, the rest of the teardown process runs
without mutual exclusion. Since the network interface is still running, is it
possible for tbnet_handle_packet() to receive a new TBIP_LOGIN request right
after this lock is dropped?
If tbnet_handle_packet() acquires the lock, updates net->remote_transmit_path
and brings up a new connection concurrently:
CPU1
tbnet_tear_down()
mutex_unlock(&net->connection_lock);
CPU2
tbnet_handle_packet()
mutex_lock(&net->connection_lock);
net->remote_transmit_path = pkg->transmit_path;
mutex_unlock(&net->connection_lock);
CPU1
tb_xdomain_release_in_hopid(net->xd, net->remote_transmit_path);
Would CPU1 then erroneously release the newly established connection's hopid
and free active buffers?
>
> - if (net->login_sent && net->login_received) {
> - int ret, retries = TBNET_LOGOUT_RETRIES;
[ ... ]
> - tb_xdomain_release_in_hopid(net->xd, net->remote_transmit_path);
> - net->remote_transmit_path = 0;
> - }
> + tb_xdomain_release_in_hopid(net->xd, net->remote_transmit_path);
> + net->remote_transmit_path = 0;
>
> - net->login_retries = 0;
With the unconditional clearing of net->login_retries removed, does the
login_retries counter keep accumulating if tbnet_tear_down() returns early
above?
If it exceeds TBNET_LOGIN_RETRIES, could it permanently prevent new
connections until the module is reloaded?
> - net->login_sent = false;
> - net->login_received = false;
>
> netdev_dbg(net->dev, "network traffic stopped\n");
> -
> - mutex_unlock(&net->connection_lock);
> }
^ permalink raw reply
* Re: [Intel-wired-lan] [RFC PATCH] ice: allow creating VFs when !CONFIG_ICE_SWITCHDEV
From: Vincent Chen @ 2026-04-29 1:21 UTC (permalink / raw)
To: Paul Menzel
Cc: anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem,
edumazet, kuba, pabeni, intel-wired-lan, netdev
In-Reply-To: <f9fbc8c3-3b1d-456c-a389-4f95bf07822a@molgen.mpg.de>
On Tue, Apr 28, 2026 at 5:45 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Vincent,
>
>
> Thank you for your patch.
>
> Am 27.04.26 um 05:11 schrieb Vincent Chen via Intel-wired-lan:
> > Currently ice_eswitch_attach_vf() is called unconditionally in
> > ice_start_vfs() and ice_reset_all_vfs(), which causes VF creation
> > to fail when CONFIG_ICE_SWITCHDEV is not defined or switchdev mode
> > is not enabled at runtime.
>
> I’d add a blank line between paragraphs.
This is a good suggestion. I will modify it in my next version patch.
>
> > Fix this by adding switchdev mode checks at the call sites before
> > calling ice_eswitch_attach_vf(), consistent with how
> > ice_eswitch_attach_sf() is already handled in ice_devlink_port_new().
> > Also remove the redundant check inside ice_eswitch_attach_vf() itself.
>
> *Also* is a good indicator to make it a separate patch. I’d favor this
> in this case.
>
OK, I will move the code snippet used to remove the redundant check
inside ice_eswitch_attach_vf() to a separate patch in my next version
patch.
> > This is similar to commit aacca7a83b97 ("ice: allow creating VFs for
> > !CONFIG_NET_SWITCHDEV") which fixed the same issue for the previous
> > ice_eswitch_configure() API.
> >
> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > ---
> > drivers/net/ethernet/intel/ice/ice_eswitch.c | 3 ---
> > drivers/net/ethernet/intel/ice/ice_sriov.c | 14 ++++++++------
> > drivers/net/ethernet/intel/ice/ice_vf_lib.c | 3 ++-
> > 3 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> > index 2e4f0969035f..c709decb26d5 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> > @@ -512,9 +512,6 @@ int ice_eswitch_attach_vf(struct ice_pf *pf, struct ice_vf *vf)
> > struct ice_repr *repr;
> > int err;
> >
> > - if (!ice_is_eswitch_mode_switchdev(pf))
> > - return 0;
> > -
> > repr = ice_repr_create_vf(vf);
> > if (IS_ERR(repr))
> > return PTR_ERR(repr);
> > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > index 843e82fd3bf9..6a0b724e46f9 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > @@ -484,12 +484,14 @@ static int ice_start_vfs(struct ice_pf *pf)
> > goto teardown;
> > }
> >
> > - retval = ice_eswitch_attach_vf(pf, vf);
> > - if (retval) {
> > - dev_err(ice_pf_to_dev(pf), "Failed to attach VF %d to eswitch, error %d",
> > - vf->vf_id, retval);
> > - ice_vf_vsi_release(vf);
> > - goto teardown;
> > + if (ice_is_eswitch_mode_switchdev(pf)) {
> > + retval = ice_eswitch_attach_vf(pf, vf);
> > + if (retval) {
> > + dev_err(ice_pf_to_dev(pf), "Failed to attach VF %d to eswitch, error %d",
> > + vf->vf_id, retval);
> > + ice_vf_vsi_release(vf);
> > + goto teardown;
> > + }
> > }
> >
> > set_bit(ICE_VF_STATE_INIT, vf->vf_states);
> > diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> > index de9e81ccee66..71595410174c 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> > @@ -807,7 +807,8 @@ void ice_reset_all_vfs(struct ice_pf *pf)
> > ice_vf_rebuild_vsi(vf);
> > ice_vf_post_vsi_rebuild(vf);
> >
> > - ice_eswitch_attach_vf(pf, vf);
> > + if (ice_is_eswitch_mode_switchdev(pf))
> > + ice_eswitch_attach_vf(pf, vf);
> >
> > mutex_unlock(&vf->cfg_lock);
> > }
>
> The diff looks good.
>
>
> Kind regards,
>
> Paul
^ permalink raw reply
* Re: [PATCH v2] net: net_failover: Fix the deadlock in slave register
From: Jakub Kicinski @ 2026-04-29 1:19 UTC (permalink / raw)
To: faicker.mo
Cc: Jakub Kicinski, horms, sridhar.samudrala, andrew+netdev, davem,
edumazet, pabeni, sdf, netdev, linux-kernel
In-Reply-To: <20260427092923.93901-1-faicker.mo@gmail.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: net_failover: Fix the deadlock in slave register
This patch replaces dev_set_mtu(), dev_open() and dev_close() in
net_failover_slave_register() with their non-locking netif_* counterparts
to avoid a deadlock when the slave is registered from the NETDEV_REGISTER
notifier, where register_netdevice() already holds the per-device ops
lock.
> diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
> index d0361aaf25ef..3f7d31033bae 100644
> --- a/drivers/net/net_failover.c
> +++ b/drivers/net/net_failover.c
> @@ -502,7 +502,7 @@ static int net_failover_slave_register(struct net_device *slave_dev,
>
> /* Align MTU of slave with failover dev */
> orig_mtu = slave_dev->mtu;
> - err = dev_set_mtu(slave_dev, failover_dev->mtu);
> + err = netif_set_mtu(slave_dev, failover_dev->mtu);
Is net_failover_slave_register() only reachable from the NETDEV_REGISTER
notifier path?
Looking at net/core/failover.c, the fops->slave_register callback is
invoked from two places. One is failover_event() on NETDEV_REGISTER,
which matches the call trace in the commit message and where
register_netdevice() has already taken the ops lock via
netdev_lock_ops().
The other is failover_register() -> failover_existing_slave_register(),
which walks existing netdevs under rtnl_lock only:
net/core/failover.c:failover_existing_slave_register() {
rtnl_lock();
for_each_netdev(net, dev) {
if (netif_is_failover(dev))
continue;
if (ether_addr_equal(failover_dev->perm_addr, dev->perm_addr))
failover_slave_register(dev);
}
rtnl_unlock();
}
On that path, the slave's dev->lock is not held by the caller.
netif_set_mtu() -> netif_set_mtu_ext() calls netdev_ops_assert_locked(dev)
for devices with request_ops_lock (for example mlx5e, the driver in the
commit's own call trace).
> @@ -512,11 +512,11 @@ static int net_failover_slave_register(struct net_device *slave_dev,
> dev_hold(slave_dev);
>
> if (netif_running(failover_dev)) {
> - err = dev_open(slave_dev, NULL);
> + err = netif_open(slave_dev, NULL);
Same question for this call. netif_open() -> __dev_open() also does
netdev_ops_assert_locked(dev), and on the failover_existing_slave_register()
path the slave's ops lock is not held.
Before this change, dev_set_mtu()/dev_open()/dev_close() acquired the ops
lock internally via netdev_lock_ops(), so the pre-existing-slave path was
covered. Does switching unconditionally to the non-locking variants
regress that path for slaves whose drivers use the per-device ops lock,
both as a lockdep splat when CONFIG_PROVE_LOCKING is set and as a loss of
ops-lock serialization against concurrent ethtool, queue and netdev-genl
operations on the slave?
> @@ -562,10 +562,10 @@ static int net_failover_slave_register(struct net_device *slave_dev,
> err_vlan_add:
> dev_uc_unsync(slave_dev, failover_dev);
> dev_mc_unsync(slave_dev, failover_dev);
> - dev_close(slave_dev);
> -err_dev_open:
> + netif_close(slave_dev);
> +err_netif_open:
> dev_put(slave_dev);
> - dev_set_mtu(slave_dev, orig_mtu);
> + netif_set_mtu(slave_dev, orig_mtu);
Would it be more appropriate to acquire netdev_lock_ops(slave_dev) in
net_failover_slave_register() when the caller is not the NETDEV_REGISTER
notifier (where the ops lock is already held), instead of dropping the
locking unconditionally?
The commit message explains the NETDEV_REGISTER caller but does not
mention the failover_existing_slave_register() caller; could that
rationale be extended, or the other caller handled explicitly?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net v2] net: mctp i2c: check length before marking flow active
From: Jeremy Kerr @ 2026-04-29 1:23 UTC (permalink / raw)
To: William A.Kennington III, pabeni
Cc: matt, andrew+netdev, davem, edumazet, kuba, wsa, netdev,
linux-kernel
In-Reply-To: <177737520480.421689.2467854297040801595.git-patchwork-notify@kernel.org>
Hi Pablo,
> Here is the summary with links:
> - [net,v2] net: mctp i2c: check length before marking flow active
> https://git.kernel.org/netdev/net/c/4ca07b9239bd
>
> You are awesome, thank you!
4ca07b9239bd seems to have acquired an unrelated change:
$ git show 4ca07b9239bd | diffstat
drivers/net/mctp/mctp-i2c.c | 4 ++--
net/sched/cls_flower.c | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)
While the commit structure is probably not intentional, are the Flower
changes acceptable for the net tree?
Cheers,
Jeremy
^ permalink raw reply
* [PATCH net v4] ipv6: addrconf: skip ERRDAD transition when address already DEAD
From: Linmao Li @ 2026-04-29 1:26 UTC (permalink / raw)
To: davem, dsahern, edumazet, kuba, pabeni
Cc: horms, stephen, netdev, linux-kernel, Linmao Li
In-Reply-To: <20260421075033.1110816-1-lilinmao@kylinos.cn>
addrconf_dad_failure() transitions ifp->state from DAD to POSTDAD
via addrconf_dad_end(), which drops ifp->lock on return. The lock
is re-acquired after net_info_ratelimited(). A concurrent
ipv6_del_addr() can take the lock in that window, set ifp->state
to DEAD and run list_del_rcu(&ifp->if_list).
addrconf_dad_failure() then overwrites DEAD with ERRDAD at errdad:
and schedules a new dad_work. The work calls ipv6_del_addr()
again, hitting the already-poisoned list entry:
general protection fault: 0000 [#1] SMP NOPTI
CPU: 4 PID: 217 Comm: kworker/4:1
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:ipv6_del_addr+0xe9/0x280
RAX: dead000000000122
Call Trace:
addrconf_dad_stop+0x113/0x140
addrconf_dad_work+0x28c/0x430
process_one_work+0x1eb/0x3b0
worker_thread+0x4d/0x400
kthread+0x104/0x140
ret_from_fork+0x35/0x40
Fold the addrconf_dad_end() logic into addrconf_dad_failure()
under a single ifp->lock critical section. The STABLE_PRIVACY
branch temporarily drops ifp->lock around address regeneration,
so add a state-is-DEAD bail-out right after the lock is re-taken
at lock_errdad: for that remaining window.
Fixes: c15b1ccadb32 ("ipv6: move DAD and addrconf_verify processing to workqueue")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
net/ipv6/addrconf.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 5476b6536eb7..b58bd9f11606 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2166,16 +2166,18 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
struct net *net = dev_net(idev->dev);
int max_addresses;
- if (addrconf_dad_end(ifp)) {
+ spin_lock_bh(&ifp->lock);
+
+ if (ifp->state != INET6_IFADDR_STATE_DAD) {
+ spin_unlock_bh(&ifp->lock);
in6_ifa_put(ifp);
return;
}
+ ifp->state = INET6_IFADDR_STATE_POSTDAD;
net_info_ratelimited("%s: IPv6 duplicate address %pI6c used by %pM detected!\n",
ifp->idev->dev->name, &ifp->addr, eth_hdr(skb)->h_source);
- spin_lock_bh(&ifp->lock);
-
if (ifp->flags & IFA_F_STABLE_PRIVACY) {
struct in6_addr new_addr;
struct inet6_ifaddr *ifp2;
@@ -2223,6 +2225,11 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
in6_ifa_put(ifp2);
lock_errdad:
spin_lock_bh(&ifp->lock);
+ if (ifp->state == INET6_IFADDR_STATE_DEAD) {
+ spin_unlock_bh(&ifp->lock);
+ in6_ifa_put(ifp);
+ return;
+ }
}
errdad:
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 0/3] gve: add support for PTP gettimex64
From: Harshitha Ramamurthy @ 2026-04-29 1:28 UTC (permalink / raw)
To: netdev
Cc: joshwash, hramamurthy, andrew+netdev, davem, edumazet, kuba,
pabeni, richardcochran, jstultz, tglx, sboyd, willemb, nktgrg,
jfraker, ziweixiao, maolson, jordanrhee, thostet, alok.a.tiwari,
pkaligineedi, horms, dwmw2, jacob.e.keller, yyd, linux-kernel
From: Jordan Rhee <jordanrhee@google.com>
This patch series adds support to obtain near-simultaneous NIC and
system timestamps with gettimex64. This enables daemons like
chrony and phc2sys to synchronize the system clock to the NIC clock.
GVE does not have direct register access to the NIC hardware clock, so
it must issue an AdminQ command to read the NIC clock. Due to relatively
high latency of AQ commands (~100us), it is necessary for the hypervisor
to sample the system time sandwich inside the AQ command.
The first two patches pave the way for the PTP implementation by
quieting excessive logging and refactoring an existing routine for
thread safety.
This table shows a comparison in chrony tracking statistics between
the typical implementation pattern for gettimex64(), which is to call
ptp_read_system_prets() / ptp_read_system_postts(), and the
implementation in this patchset, which is to use the TSC values from
the device. The RMS offset is nearly 4 orders of magnitude smaller in
the chosen implementation.
| | TSC from VM | TSC from device (this patchset) |
| --------------- | --------------------- | ------------------------------- |
| System time | 0.000000005 s slow | 0.000000001 s fast |
| Last offset | +0.000005606 seconds | +0.000000001 seconds |
| RMS offset | 0.000009020 seconds | 0.000000002 seconds |
| Frequency | 4.115 ppm fast | 0.362 ppm fast |
| Residual freq | +2.515 ppm | +0.000 ppm |
| Skew | 18.480 ppm | 0.001 ppm |
| Root delay | 0.000000001 seconds | 0.000000001 seconds |
| Root dispersion | 0.000081905 seconds | 0.000001169 seconds |
| Update interval | 0.5 seconds | 0.5 seconds |
| Leap status | Normal | Normal |
---
Changelog:
V5:
- Reformulate retry loop in terms of total timeout instead of retry
count (Jakub Kicinski)
- Link to v4: https://lore.kernel.org/netdev/20260406234002.3610542-1-hramamurthy@google.com/
V4:
- Call out change to dev_err_ratelimited() in patch 1 commit message (Jacob Keller)
- Ensure only one log is emitted when command returns GVE_ADMINQ_COMMAND_UNSET (Jacob Keller)
- Link to v3: https://lore.kernel.org/netdev/20260403194427.1830609-1-hramamurthy@google.com/
V3:
- Take system time snapshot inside the mutex
- Return -EOPNOTSUPP if cross-timestamp is requested on an arch other
than x86 or arm64
- Fix initialization to only register PTP clock once all data is
initialized
- Link to v2: https://lore.kernel.org/netdev/20260326224527.1044097-1-hramamurthy@google.com/
V2:
- Fixed compilation warning on ARM by casting to u64
- Link to v1: https://lore.kernel.org/netdev/20260323234829.3185051-1-hramamurthy@google.com/
---
Ankit Garg (1):
gve: make nic clock reads thread safe
Jordan Rhee (2):
gve: skip error logging for retryable AdminQ commands
gve: implement PTP gettimex64
drivers/net/ethernet/google/gve/gve.h | 12 +-
drivers/net/ethernet/google/gve/gve_adminq.c | 27 +-
drivers/net/ethernet/google/gve/gve_adminq.h | 4 +-
drivers/net/ethernet/google/gve/gve_ethtool.c | 3 +-
drivers/net/ethernet/google/gve/gve_ptp.c | 316 +++++++++++++-----
5 files changed, 267 insertions(+), 95 deletions(-)
base-commit: 790ead9394860e7d70c5e0e50a35b243e909a618
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply
* [PATCH net-next v5 1/3] gve: skip error logging for retryable AdminQ commands
From: Harshitha Ramamurthy @ 2026-04-29 1:28 UTC (permalink / raw)
To: netdev
Cc: joshwash, hramamurthy, andrew+netdev, davem, edumazet, kuba,
pabeni, richardcochran, jstultz, tglx, sboyd, willemb, nktgrg,
jfraker, ziweixiao, maolson, jordanrhee, thostet, alok.a.tiwari,
pkaligineedi, horms, dwmw2, jacob.e.keller, yyd, linux-kernel
In-Reply-To: <20260429012819.3102675-1-hramamurthy@google.com>
From: Jordan Rhee <jordanrhee@google.com>
AdminQ commands may return -EAGAIN under certain transient conditions.
These commands are intended to be retried by the driver, so logging
a formal error to the system log is misleading and creates
unnecessary noise.
Modify the logging logic to skip the error message when the result
is -EAGAIN, and move logging to dev_err_ratelimited() to avoid
spamming the log.
Reviewed-by: Joshua Washington <joshwash@google.com>
Signed-off-by: Jordan Rhee <jordanrhee@google.com>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
---
Changes in v4:
- call out change to dev_err_ratelimited() in the commit message (Jacob Keller)
- remove extra print when adminQ status is GVE_ADMINQ_COMMAND_UNSET (Jacob Keller)
---
drivers/net/ethernet/google/gve/gve_adminq.c | 27 +++++++++++++++-----
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
index 08587bf40ed4..a65b14835aa0 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.c
+++ b/drivers/net/ethernet/google/gve/gve_adminq.c
@@ -416,16 +416,10 @@ static bool gve_adminq_wait_for_cmd(struct gve_priv *priv, u32 prod_cnt)
static int gve_adminq_parse_err(struct gve_priv *priv, u32 status)
{
- if (status != GVE_ADMINQ_COMMAND_PASSED &&
- status != GVE_ADMINQ_COMMAND_UNSET) {
- dev_err(&priv->pdev->dev, "AQ command failed with status %d\n", status);
- priv->adminq_cmd_fail++;
- }
switch (status) {
case GVE_ADMINQ_COMMAND_PASSED:
return 0;
case GVE_ADMINQ_COMMAND_UNSET:
- dev_err(&priv->pdev->dev, "parse_aq_err: err and status both unset, this should not be possible.\n");
return -EINVAL;
case GVE_ADMINQ_COMMAND_ERROR_ABORTED:
case GVE_ADMINQ_COMMAND_ERROR_CANCELLED:
@@ -455,6 +449,16 @@ static int gve_adminq_parse_err(struct gve_priv *priv, u32 status)
}
}
+static bool gve_adminq_is_retryable(enum gve_adminq_opcodes opcode)
+{
+ switch (opcode) {
+ case GVE_ADMINQ_REPORT_NIC_TIMESTAMP:
+ return true;
+ default:
+ return false;
+ }
+}
+
/* Flushes all AQ commands currently queued and waits for them to complete.
* If there are failures, it will return the first error.
*/
@@ -482,9 +486,18 @@ static int gve_adminq_kick_and_wait(struct gve_priv *priv)
cmd = &priv->adminq[i & priv->adminq_mask];
status = be32_to_cpu(READ_ONCE(cmd->status));
err = gve_adminq_parse_err(priv, status);
- if (err)
+ if (err) {
+ enum gve_adminq_opcodes opcode =
+ be32_to_cpu(READ_ONCE(cmd->opcode));
+ priv->adminq_cmd_fail++;
+ if (!gve_adminq_is_retryable(opcode) || err != -EAGAIN)
+ dev_err_ratelimited(&priv->pdev->dev,
+ "AQ command %d failed with status %d\n",
+ opcode, status);
+
// Return the first error if we failed.
return err;
+ }
}
return 0;
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply related
* [PATCH net-next v5 2/3] gve: make nic clock reads thread safe
From: Harshitha Ramamurthy @ 2026-04-29 1:28 UTC (permalink / raw)
To: netdev
Cc: joshwash, hramamurthy, andrew+netdev, davem, edumazet, kuba,
pabeni, richardcochran, jstultz, tglx, sboyd, willemb, nktgrg,
jfraker, ziweixiao, maolson, jordanrhee, thostet, alok.a.tiwari,
pkaligineedi, horms, dwmw2, jacob.e.keller, yyd, linux-kernel
In-Reply-To: <20260429012819.3102675-1-hramamurthy@google.com>
From: Ankit Garg <nktgrg@google.com>
Add a mutex to protect the shared DMA buffer that receives NIC
timestamp reports. The NIC timestamp will be read from two different
threads: the periodic worker and upcoming `gettimex64`.
Move clock registration to the last step of initialization to ensure
that all data needed by the clock module is initialized before
the clock is exposed to usermode.
Reviewed-by: Joshua Washington <joshwash@google.com>
Signed-off-by: Ankit Garg <nktgrg@google.com>
Signed-off-by: Jordan Rhee <jordanrhee@google.com>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
---
Changes in v3:
- Reorder init/teardown to register PTP clock last, and simplify code
- Move ptp-related members from gve_priv to gve_ptp
- Only assign priv->ptp after ptp module is successfully initialized
---
drivers/net/ethernet/google/gve/gve.h | 12 +-
drivers/net/ethernet/google/gve/gve_ethtool.c | 3 +-
drivers/net/ethernet/google/gve/gve_ptp.c | 134 ++++++++----------
3 files changed, 63 insertions(+), 86 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
index 1d66d3834f7e..7b69d0cfc0d5 100644
--- a/drivers/net/ethernet/google/gve/gve.h
+++ b/drivers/net/ethernet/google/gve/gve.h
@@ -792,6 +792,9 @@ struct gve_ptp {
struct ptp_clock_info info;
struct ptp_clock *clock;
struct gve_priv *priv;
+ struct mutex nic_ts_read_lock; /* Protects nic_ts_report */
+ struct gve_nic_ts_report *nic_ts_report;
+ dma_addr_t nic_ts_report_bus;
};
struct gve_priv {
@@ -923,8 +926,6 @@ struct gve_priv {
bool nic_timestamp_supported;
struct gve_ptp *ptp;
struct kernel_hwtstamp_config ts_config;
- struct gve_nic_ts_report *nic_ts_report;
- dma_addr_t nic_ts_report_bus;
u64 last_sync_nic_counter; /* Clock counter from last NIC TS report */
};
@@ -1201,7 +1202,7 @@ static inline bool gve_supports_xdp_xmit(struct gve_priv *priv)
static inline bool gve_is_clock_enabled(struct gve_priv *priv)
{
- return priv->nic_ts_report;
+ return priv->ptp;
}
/* gqi napi handler defined in gve_main.c */
@@ -1321,14 +1322,9 @@ int gve_flow_rules_reset(struct gve_priv *priv);
int gve_init_rss_config(struct gve_priv *priv, u16 num_queues);
/* PTP and timestamping */
#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
-int gve_clock_nic_ts_read(struct gve_priv *priv);
int gve_init_clock(struct gve_priv *priv);
void gve_teardown_clock(struct gve_priv *priv);
#else /* CONFIG_PTP_1588_CLOCK */
-static inline int gve_clock_nic_ts_read(struct gve_priv *priv)
-{
- return -EOPNOTSUPP;
-}
static inline int gve_init_clock(struct gve_priv *priv)
{
diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
index dc2213b5ce24..4fd7e8a442c5 100644
--- a/drivers/net/ethernet/google/gve/gve_ethtool.c
+++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
@@ -972,8 +972,7 @@ static int gve_get_ts_info(struct net_device *netdev,
info->rx_filters |= BIT(HWTSTAMP_FILTER_NONE) |
BIT(HWTSTAMP_FILTER_ALL);
- if (priv->ptp)
- info->phc_index = ptp_clock_index(priv->ptp->clock);
+ info->phc_index = ptp_clock_index(priv->ptp->clock);
}
return 0;
diff --git a/drivers/net/ethernet/google/gve/gve_ptp.c b/drivers/net/ethernet/google/gve/gve_ptp.c
index 06b1cf4a5efc..ad15f1209a83 100644
--- a/drivers/net/ethernet/google/gve/gve_ptp.c
+++ b/drivers/net/ethernet/google/gve/gve_ptp.c
@@ -11,19 +11,20 @@
#define GVE_NIC_TS_SYNC_INTERVAL_MS 250
/* Read the nic timestamp from hardware via the admin queue. */
-int gve_clock_nic_ts_read(struct gve_priv *priv)
+static int gve_clock_nic_ts_read(struct gve_ptp *ptp, u64 *nic_raw)
{
- u64 nic_raw;
int err;
- err = gve_adminq_report_nic_ts(priv, priv->nic_ts_report_bus);
+ mutex_lock(&ptp->nic_ts_read_lock);
+ err = gve_adminq_report_nic_ts(ptp->priv, ptp->nic_ts_report_bus);
if (err)
- return err;
+ goto out;
- nic_raw = be64_to_cpu(priv->nic_ts_report->nic_timestamp);
- WRITE_ONCE(priv->last_sync_nic_counter, nic_raw);
+ *nic_raw = be64_to_cpu(ptp->nic_ts_report->nic_timestamp);
- return 0;
+out:
+ mutex_unlock(&ptp->nic_ts_read_lock);
+ return err;
}
static int gve_ptp_gettimex64(struct ptp_clock_info *info,
@@ -41,17 +42,21 @@ static int gve_ptp_settime64(struct ptp_clock_info *info,
static long gve_ptp_do_aux_work(struct ptp_clock_info *info)
{
- const struct gve_ptp *ptp = container_of(info, struct gve_ptp, info);
+ struct gve_ptp *ptp = container_of(info, struct gve_ptp, info);
struct gve_priv *priv = ptp->priv;
+ u64 nic_raw;
int err;
if (gve_get_reset_in_progress(priv) || !gve_get_admin_queue_ok(priv))
goto out;
- err = gve_clock_nic_ts_read(priv);
- if (err && net_ratelimit())
- dev_err(&priv->pdev->dev,
- "%s read err %d\n", __func__, err);
+ err = gve_clock_nic_ts_read(ptp, &nic_raw);
+ if (err) {
+ dev_err_ratelimited(&priv->pdev->dev, "%s read err %d\n",
+ __func__, err);
+ goto out;
+ }
+ WRITE_ONCE(priv->last_sync_nic_counter, nic_raw);
out:
return msecs_to_jiffies(GVE_NIC_TS_SYNC_INTERVAL_MS);
@@ -65,94 +70,71 @@ static const struct ptp_clock_info gve_ptp_caps = {
.do_aux_work = gve_ptp_do_aux_work,
};
-static int gve_ptp_init(struct gve_priv *priv)
+int gve_init_clock(struct gve_priv *priv)
{
struct gve_ptp *ptp;
+ u64 nic_raw;
int err;
- priv->ptp = kzalloc_obj(*priv->ptp);
- if (!priv->ptp)
+ ptp = kzalloc_obj(*priv->ptp);
+ if (!ptp)
return -ENOMEM;
- ptp = priv->ptp;
ptp->info = gve_ptp_caps;
- ptp->clock = ptp_clock_register(&ptp->info, &priv->pdev->dev);
-
- if (IS_ERR(ptp->clock)) {
- dev_err(&priv->pdev->dev, "PTP clock registration failed\n");
- err = PTR_ERR(ptp->clock);
- goto free_ptp;
- }
-
ptp->priv = priv;
- return 0;
-
-free_ptp:
- kfree(ptp);
- priv->ptp = NULL;
- return err;
-}
-
-static void gve_ptp_release(struct gve_priv *priv)
-{
- struct gve_ptp *ptp = priv->ptp;
-
- if (!ptp)
- return;
-
- if (ptp->clock)
- ptp_clock_unregister(ptp->clock);
-
- kfree(ptp);
- priv->ptp = NULL;
-}
-
-int gve_init_clock(struct gve_priv *priv)
-{
- int err;
-
- err = gve_ptp_init(priv);
- if (err)
- return err;
-
- priv->nic_ts_report =
+ mutex_init(&ptp->nic_ts_read_lock);
+ ptp->nic_ts_report =
dma_alloc_coherent(&priv->pdev->dev,
sizeof(struct gve_nic_ts_report),
- &priv->nic_ts_report_bus,
- GFP_KERNEL);
- if (!priv->nic_ts_report) {
+ &ptp->nic_ts_report_bus, GFP_KERNEL);
+ if (!ptp->nic_ts_report) {
dev_err(&priv->pdev->dev, "%s dma alloc error\n", __func__);
err = -ENOMEM;
- goto release_ptp;
+ goto free_ptp;
}
- err = gve_clock_nic_ts_read(priv);
+
+ err = gve_clock_nic_ts_read(ptp, &nic_raw);
if (err) {
dev_err(&priv->pdev->dev, "failed to read NIC clock %d\n", err);
- goto release_nic_ts_report;
+ goto free_dma_mem;
}
- ptp_schedule_worker(priv->ptp->clock,
+ WRITE_ONCE(priv->last_sync_nic_counter, nic_raw);
+
+ ptp->clock = ptp_clock_register(&ptp->info, &priv->pdev->dev);
+ if (IS_ERR(ptp->clock)) {
+ dev_err(&priv->pdev->dev, "PTP clock registration failed\n");
+ err = PTR_ERR(ptp->clock);
+ goto free_dma_mem;
+ }
+
+ priv->ptp = ptp;
+ ptp_schedule_worker(ptp->clock,
msecs_to_jiffies(GVE_NIC_TS_SYNC_INTERVAL_MS));
return 0;
-release_nic_ts_report:
- dma_free_coherent(&priv->pdev->dev,
- sizeof(struct gve_nic_ts_report),
- priv->nic_ts_report, priv->nic_ts_report_bus);
- priv->nic_ts_report = NULL;
-release_ptp:
- gve_ptp_release(priv);
+free_dma_mem:
+ dma_free_coherent(&priv->pdev->dev, sizeof(struct gve_nic_ts_report),
+ ptp->nic_ts_report, ptp->nic_ts_report_bus);
+ ptp->nic_ts_report = NULL;
+free_ptp:
+ mutex_destroy(&ptp->nic_ts_read_lock);
+ kfree(ptp);
return err;
}
void gve_teardown_clock(struct gve_priv *priv)
{
- gve_ptp_release(priv);
+ struct gve_ptp *ptp = priv->ptp;
- if (priv->nic_ts_report) {
- dma_free_coherent(&priv->pdev->dev,
- sizeof(struct gve_nic_ts_report),
- priv->nic_ts_report, priv->nic_ts_report_bus);
- priv->nic_ts_report = NULL;
- }
+ if (!ptp)
+ return;
+
+ priv->ptp = NULL;
+ ptp_clock_unregister(ptp->clock);
+ dma_free_coherent(&priv->pdev->dev, sizeof(struct gve_nic_ts_report),
+ ptp->nic_ts_report, ptp->nic_ts_report_bus);
+ ptp->nic_ts_report = NULL;
+ mutex_destroy(&ptp->nic_ts_read_lock);
+ kfree(ptp);
}
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox