* [PATCH net v2 1/3] net/sched: cls_fw: fix NULL pointer dereference on shared blocks
From: Xiang Mei @ 2026-03-27 6:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, jiri, davem, edumazet, kuba, horms, shuah, bestswngs,
Xiang Mei
The old-method path in fw_classify() calls tcf_block_q() and
dereferences q->handle. Shared blocks leave block->q NULL, causing a
NULL deref when an empty cls_fw filter is attached to a shared block
and a packet with a nonzero major skb mark is classified.
Check tcf_block_shared() before accessing block->q and return -1 (no
match) for shared blocks, consistent with cls_u32's tc_u_common_ptr().
The fixed null-ptr-deref calling stack:
KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
RIP: 0010:fw_classify (net/sched/cls_fw.c:81)
Call Trace:
tcf_classify (./include/net/tc_wrapper.h:197 net/sched/cls_api.c:1764 net/sched/cls_api.c:1860)
tc_run (net/core/dev.c:4401)
__dev_queue_xmit (net/core/dev.c:4535 net/core/dev.c:4790)
Fixes: 1abf272022cf ("net: sched: tcindex, fw, flow: use tcf_block_q helper to get struct Qdisc")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
v2: Correct 3/3 selftest case
net/sched/cls_fw.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index be81c108179d..caf17ab3be87 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -74,9 +74,14 @@ TC_INDIRECT_SCOPE int fw_classify(struct sk_buff *skb,
}
}
} else {
- struct Qdisc *q = tcf_block_q(tp->chain->block);
+ struct tcf_block *block = tp->chain->block;
+ struct Qdisc *q;
+
+ if (tcf_block_shared(block))
+ return -1;
/* Old method: classify the packet using its skb mark. */
+ q = tcf_block_q(block);
if (id && (TC_H_MAJ(id) == 0 ||
!(TC_H_MAJ(id ^ q->handle)))) {
res->classid = id;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v5 phy-next 10/27] scsi: ufs: qcom: keep parallel track of PHY power state
From: Manivannan Sadhasivam @ 2026-03-27 6:52 UTC (permalink / raw)
To: Vladimir Oltean
Cc: linux-phy, Vinod Koul, Neil Armstrong, dri-devel, freedreno,
linux-arm-kernel, linux-arm-msm, linux-can, linux-gpio, linux-ide,
linux-kernel, linux-media, linux-pci, linux-renesas-soc,
linux-riscv, linux-rockchip, linux-samsung-soc, linux-scsi,
linux-sunxi, linux-tegra, linux-usb, netdev, spacemit,
UNGLinuxDriver, James E.J. Bottomley, Martin K. Petersen,
Nitin Rawat
In-Reply-To: <20260326080444.gbesciaa5zwvcgoy@skbuf>
On Thu, Mar 26, 2026 at 10:04:44AM +0200, Vladimir Oltean wrote:
> On Wed, Mar 25, 2026 at 01:57:31PM +0200, Vladimir Oltean wrote:
> > On Wed, Mar 25, 2026 at 05:21:14PM +0530, Manivannan Sadhasivam wrote:
> > > I believe I added the power_count check for phy_exit(). But since that got
> > > moved, the check becomes no longer necessary.
> >
> > FYI, the power_count keeps track of the balance of phy_power_on() and
> > phy_power_off() calls, whereas it is the init_count keeps track of
> > phy_init() and phy_exit() calls. They are only related to the extent
> > that you must respect the phy_init() -> phy_power_on() -> phy_power_off()
> > -> phy_exit() sequence. But in any case, both should be considered
> > PHY-internal fields. The "Order of API calls" section from
> > Documentation/driver-api/phy/phy.rst mentions the order that I just
> > described above, and consumers should just ensure they follow that.
>
> Ok, so we can close this topic of "checking the power_count not needed"
> by linking to the conversation which spun off here:
> https://lore.kernel.org/lkml/20260325120122.265973-1-manivannan.sadhasivam@oss.qualcomm.com/
>
Sure.
> Mani, I spent some more time to figure out what's really going on with
> this unexpected phy_power_off() call. Do you think you could
> regression-test the patch attached?
>
I tested the patch. But it fails ufs_qcom_power_up_sequence() if PHY was already
powered on:
[ 31.513321] qcom-qmp-ufs-phy 1d87000.phy: phy initialization timed-out
[ 31.513335] ufshcd-qcom 1d84000.ufshc: Failed to calibrate PHY: -110
[ 31.565273] ufshcd-qcom 1d84000.ufshc: Enabling the controller failed
Funny thing is, it didn't affect the functionality since the UFS core retries
ufshcd_hba_enable() and in the error path of ufs_qcom_power_up_sequence(),
phy_power_off() gets called and that causes the next try to succeed. So it is
evident that, if PHY was already powered ON, it should be powered off before
ufs_qcom_phy_power_on(). And due to the UFS driver design,
ufs_qcom_power_up_sequence() can get called multiple times. So we cannot just
remove phy_power_off().
Below diff on top of your patch fixes the issue:
```
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index ed067247d72a..2c9fe03f349e 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -567,6 +567,8 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
if (ret)
return ret;
+ ufs_qcom_phy_power_off(host);
+
ret = ufs_qcom_phy_set_gear(host, mode);
if (ret) {
dev_err(hba->dev, "%s: phy_set_mode_ext() failed, ret = %d\n",
```
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply related
* [PATCH iwl-next v4] ice: remove excessive memory allocation in ice_create_lag_recipe()
From: Aleksandr Loktionov @ 2026-03-27 6:48 UTC (permalink / raw)
To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov
Cc: netdev, Marcin Szycik, Joe Damato
From: Marcin Szycik <marcin.szycik@intel.com>
For some reason ice_create_lag_recipe() allocates an array of 64
struct ice_aqc_recipe_data_elem elements, while it only needs one (1).
Fix it, while also using kzalloc_obj().
Signed-off-by: Marcin Szycik <marcin.szycik@intel.com>
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Joe Damato <joe@dama.to>
---
v3 -> v4 corrected misspeled RB from Joe
v2 -> v3 use sizeof(*new_rcp) in memcpy() to match the allocation (Joe)
v1 -> v2 remove 'Fixes' from commit message because it's not a critical bug
---
drivers/net/ethernet/intel/ice/ice_lag.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
index 310e8fe..9ad19c3 100644
--- a/drivers/net/ethernet/intel/ice/ice_lag.c
+++ b/drivers/net/ethernet/intel/ice/ice_lag.c
@@ -2418,11 +2418,11 @@ static int ice_create_lag_recipe(struct ice_hw *hw, u16 *rid,
if (err)
return err;
- new_rcp = kzalloc(ICE_RECIPE_LEN * ICE_MAX_NUM_RECIPES, GFP_KERNEL);
+ new_rcp = kzalloc_obj(*new_rcp, GFP_KERNEL);
if (!new_rcp)
return -ENOMEM;
- memcpy(new_rcp, base_recipe, ICE_RECIPE_LEN);
+ memcpy(new_rcp, base_recipe, sizeof(*new_rcp));
new_rcp->content.act_ctrl_fwd_priority = prio;
new_rcp->content.rid = *rid | ICE_AQ_RECIPE_ID_IS_ROOT;
new_rcp->recipe_indx = *rid;
^ permalink raw reply related
* Re: [PATCH bpf-next 1/3] bpf: Disallow freplace on XDP with mismatched xdp_has_frags values
From: Leon Hwang @ 2026-03-27 6:42 UTC (permalink / raw)
To: Jakub Kicinski
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Shuah Khan, David S . Miller, Jesper Dangaard Brouer,
Toke Hoiland-Jorgensen, Lorenzo Bianconi, linux-kernel,
linux-kselftest, netdev, kernel-patches-bot
In-Reply-To: <20260326124205.1a3bb825@kernel.org>
On 27/3/26 03:42, Jakub Kicinski wrote:
> On Tue, 24 Mar 2026 23:04:42 +0800 Leon Hwang wrote:
>> The commit f45d5b6ce2e8 ("bpf: generalise tail call map compatibility check")
>> was to ensure backwards compatibility against tail calls. However, it
>> missed that XDP progs can be extended by freplace progs, which could break
>> the backwards compatibility, e.g. xdp_has_frags=true freplace progs are
>> allowed to attach to xdp_has_frags=false XDP progs.
>>
>> To avoid breaking the backwards compatibility via freplace, disallow
>> freplace on XDP programs with different xdp_has_frags values.
>
> It may be worth adding a selftest to
> tools/testing/selftests/drivers/net/xdp.py
> which sets MTU to 9k, tries to attach a non-frag-capable prog
> if that fails attaches a frag-capable prog and then checks if
> replacing the capable prog with non-capable fails.
> Drivers may be buggy in this regard.
Do you mean adding these two tests to xdp.py?
1. Verify the failure of attaching non-frag-capable prog to mtu=9k
driver.
2. Verify the failure of updating frag-capable prog with non-frag-
capable prog via libbpf.c::bpf_link__update_program().
As for test #2, it may require a helper to attach prog then update prog
using libbpf's APIs.
Thanks,
Leon
^ permalink raw reply
* Re: [PATCH 0/4] net: bridge: mcast: add multicast exponential field encoding
From: Nikolay Aleksandrov @ 2026-03-27 6:41 UTC (permalink / raw)
To: Ujjal Roy, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Ido Schimmel, David Ahern
Cc: Ujjal Roy, bridge, netdev, linux-kernel
In-Reply-To: <20260326150742.50289-1-royujjal@gmail.com>
On 26/03/2026 17:07, Ujjal Roy wrote:
> Description:
> This series addresses a mismatch in how multicast query
> intervals and response codes are handled across IPv4 (IGMPv3)
> and IPv6 (MLDv2). While decoding logic currently exists,
> the corresponding encoding logic is missing during query
> packet generation. This leads to incorrect intervals being
> transmitted when values exceed their linear thresholds.
>
> The patches introduce a unified floating-point encoding
> approach based on RFC3376 and RFC3810, ensuring that large
> intervals are correctly represented in QQIC and MRC fields
> using the exponent-mantissa format.
>
> Key Changes:
> * ipv4: igmp: get rid of IGMPV3_{QQIC,MRC} and simplify calculation
> Removes legacy macros in favor of a cleaner, unified
> calculation for retrieving intervals from encoded fields,
> improving code maintainability.
>
> * ipv6: mld: rename mldv2_mrc() and add mldv2_qqi()
> Standardizes MLDv2 terminology by renaming mldv2_mrc()
> to mldv2_mrd() (Maximum Response Delay) and introducing
> a new API mldv2_qqi for QQI calculation, improving code
> readability.
>
> * ipv4: igmp: encode multicast exponential fields
> Introduces the logic to dynamically calculate the exponent
> and mantissa using bit-scan (fls). This ensures QQIC and
> MRC fields (8-bit) are properly encoded when transmitting
> query packets with intervals that exceed their respective
> linear threshold value of 128 (for QQI/MRT).
>
> * ipv6: mld: encode multicast exponential fields
> Applies similar encoding logic for MLDv2. This ensures
> QQIC (8-bit) and MRC (16-bit) fields are properly encoded
> when transmitting query packets with intervals that exceed
> their respective linear thresholds (128 for QQI; 32768
> for MRD).
>
> Impact:
> These changes ensure that multicast queriers and listeners
> stay synchronized on timing intervals, preventing protocol
> timeouts or premature group membership expiration caused
> by incorrectly formatted packet headers.
>
Can you add selftests which cover these cases?
^ permalink raw reply
* Re: [PATCH net 3/3] vxlan: validate ND option lengths in vxlan_na_create
From: Nikolay Aleksandrov @ 2026-03-27 6:38 UTC (permalink / raw)
To: Yang Yang, davem, edumazet, kuba, pabeni, idosch
Cc: andrew+netdev, horms, florian.fainelli, roopa, dlstevens, nb,
netdev, bridge, linux-kernel, yifanwucs, tomapufckgml, tanyuan98,
bird
In-Reply-To: <20260326034441.2037420-4-n05ec@lzu.edu.cn>
On 26/03/2026 05:44, Yang Yang wrote:
> vxlan_na_create() walks ND options according to option-provided
> lengths. A malformed option can make the parser advance beyond the
> computed option span or use a too-short source LLADDR option payload.
>
> Validate option lengths against the remaining NS option area before
> advancing, and only read source LLADDR when the option is large enough
> for an Ethernet address.
>
> Fixes: 4b29dba9c085 ("vxlan: fix nonfunctional neigh_reduce()")
> Cc: stable@vger.kernel.org
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Tested-by: Ao Zhou <n05ec@lzu.edu.cn>
> Co-developed-by: Yuan Tan <tanyuan98@outlook.com>
> Signed-off-by: Yuan Tan <tanyuan98@outlook.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Signed-off-by: Yang Yang <n05ec@lzu.edu.cn>
> ---
> drivers/net/vxlan/vxlan_core.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 17c941aac32db..a94ac82a61364 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1965,12 +1965,14 @@ static struct sk_buff *vxlan_na_create(struct sk_buff *request,
> ns_olen = request->len - skb_network_offset(request) -
> sizeof(struct ipv6hdr) - sizeof(*ns);
> for (i = 0; i < ns_olen-1; i += (ns->opt[i+1]<<3)) {
> - if (!ns->opt[i + 1]) {
> + if (!ns->opt[i + 1] || i + (ns->opt[i + 1] << 3) > ns_olen) {
> kfree_skb(reply);
> return NULL;
> }
> if (ns->opt[i] == ND_OPT_SOURCE_LL_ADDR) {
> - daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
> + if ((ns->opt[i + 1] << 3) >=
> + sizeof(struct nd_opt_hdr) + ETH_ALEN)
> + daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
> break;
> }
> }
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
^ permalink raw reply
* Re: [PATCH net 2/3] bridge: br_nd_send: validate ND option lengths
From: Nikolay Aleksandrov @ 2026-03-27 6:37 UTC (permalink / raw)
To: Yang Yang, davem, edumazet, kuba, pabeni, idosch
Cc: andrew+netdev, horms, florian.fainelli, roopa, dlstevens, nb,
netdev, bridge, linux-kernel, yifanwucs, tomapufckgml, tanyuan98,
bird
In-Reply-To: <20260326034441.2037420-3-n05ec@lzu.edu.cn>
On 26/03/2026 05:44, Yang Yang wrote:
> br_nd_send() walks ND options according to option-provided lengths.
> A malformed option can make the parser advance beyond the computed
> option span or use a too-short source LLADDR option payload.
>
> Validate option lengths against the remaining NS option area before
> advancing, and only read source LLADDR when the option is large enough
> for an Ethernet address.
>
> Fixes: ed842faeb2bd ("bridge: suppress nd pkts on BR_NEIGH_SUPPRESS ports")
> Cc: stable@vger.kernel.org
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Tested-by: Ao Zhou <n05ec@lzu.edu.cn>
> Co-developed-by: Yuan Tan <tanyuan98@outlook.com>
> Signed-off-by: Yuan Tan <tanyuan98@outlook.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Signed-off-by: Yang Yang <n05ec@lzu.edu.cn>
> ---
> net/bridge/br_arp_nd_proxy.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
> index af3d1e33f50b8..6b5595868a39c 100644
> --- a/net/bridge/br_arp_nd_proxy.c
> +++ b/net/bridge/br_arp_nd_proxy.c
> @@ -288,12 +288,14 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
> ns_olen = request->len - (skb_network_offset(request) +
> sizeof(struct ipv6hdr)) - sizeof(*ns);
> for (i = 0; i < ns_olen - 1; i += (ns->opt[i + 1] << 3)) {
> - if (!ns->opt[i + 1]) {
> + if (!ns->opt[i + 1] || i + (ns->opt[i + 1] << 3) > ns_olen) {
> kfree_skb(reply);
> return;
> }
> if (ns->opt[i] == ND_OPT_SOURCE_LL_ADDR) {
> - daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
> + if ((ns->opt[i + 1] << 3) >=
> + sizeof(struct nd_opt_hdr) + ETH_ALEN)
> + daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
> break;
> }
> }
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
^ permalink raw reply
* Re: [PATCH net 1/3] bridge: br_nd_send: linearize skb before parsing ND options
From: Nikolay Aleksandrov @ 2026-03-27 6:37 UTC (permalink / raw)
To: Yang Yang, davem, edumazet, kuba, pabeni, idosch
Cc: andrew+netdev, horms, florian.fainelli, roopa, dlstevens, nb,
netdev, bridge, linux-kernel, yifanwucs, tomapufckgml, tanyuan98,
bird
In-Reply-To: <20260326034441.2037420-2-n05ec@lzu.edu.cn>
On 26/03/2026 05:44, Yang Yang wrote:
> br_nd_send() parses neighbour discovery options from ns->opt[] and
> assumes that these options are in the linear part of request.
>
> Its callers only guarantee that the ICMPv6 header and target address
> are available, so the option area can still be non-linear. Parsing
> ns->opt[] in that case can access data past the linear buffer.
>
> Linearize request before option parsing and derive ns from the linear
> network header.
>
> Fixes: ed842faeb2bd ("bridge: suppress nd pkts on BR_NEIGH_SUPPRESS ports")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Tested-by: Ao Zhou <n05ec@lzu.edu.cn>
> Co-developed-by: Yuan Tan <tanyuan98@outlook.com>
> Signed-off-by: Yuan Tan <tanyuan98@outlook.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Signed-off-by: Yang Yang <n05ec@lzu.edu.cn>
> ---
> net/bridge/br_arp_nd_proxy.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
> index 1e2b51769eec8..af3d1e33f50b8 100644
> --- a/net/bridge/br_arp_nd_proxy.c
> +++ b/net/bridge/br_arp_nd_proxy.c
> @@ -251,12 +251,12 @@ struct nd_msg *br_is_nd_neigh_msg(const struct sk_buff *skb, struct nd_msg *msg)
>
> static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
> struct sk_buff *request, struct neighbour *n,
> - __be16 vlan_proto, u16 vlan_tci, struct nd_msg *ns)
> + __be16 vlan_proto, u16 vlan_tci)
> {
> struct net_device *dev = request->dev;
> struct net_bridge_vlan_group *vg;
> + struct nd_msg *na, *ns;
> struct sk_buff *reply;
> - struct nd_msg *na;
> struct ipv6hdr *pip6;
> int na_olen = 8; /* opt hdr + ETH_ALEN for target */
> int ns_olen;
> @@ -264,7 +264,7 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
> u8 *daddr;
> u16 pvid;
>
> - if (!dev)
> + if (!dev || skb_linearize(request))
> return;
>
> len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +
> @@ -281,6 +281,8 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
> skb_set_mac_header(reply, 0);
>
> daddr = eth_hdr(request)->h_source;
> + ns = (struct nd_msg *)(skb_network_header(request) +
> + sizeof(struct ipv6hdr));
>
> /* Do we need option processing ? */
> ns_olen = request->len - (skb_network_offset(request) +
> @@ -472,9 +474,9 @@ void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
> if (vid != 0)
> br_nd_send(br, p, skb, n,
> skb->vlan_proto,
> - skb_vlan_tag_get(skb), msg);
> + skb_vlan_tag_get(skb));
> else
> - br_nd_send(br, p, skb, n, 0, 0, msg);
> + br_nd_send(br, p, skb, n, 0, 0);
> replied = true;
> }
>
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
^ permalink raw reply
* Re: Re: [PATCH] net/openvswitch: fix trigger-able BUG_ON after ovs_vport_cmd_fill_info
From: sunichi @ 2026-03-27 6:32 UTC (permalink / raw)
To: i.maximets
Cc: aconole, davem, dev, echaudro, edumazet, horms, kuba,
linux-kernel, netdev, pabeni, sunyiqixm
In-Reply-To: <3813e7fb-e234-42ec-b05c-7bcb609de3ba@ovn.org>
On 3/24/26 1:01 PM, Ilya Maximets wrote:
> On 3/23/26 8:14 AM, sunichi wrote:
> > ovs_vport_set_upcall_portids() does not validate the length of the
> > user-supplied OVS_VPORT_ATTR_UPCALL_PID netlink attribute. A
> > sufficiently large portid list can overflow the reply skb allocated
> > with NLMSG_DEFAULT_SIZE in causing ovs_vport_cmd_fill_info()
> > to return -EMSGSIZE and triggering the unconditional BUG_ON(),
> > which panics the kernel on most distributions.
> >
> > Any local user with CAP_NET_ADMIN (or an equivalent unprivileged
> > namespace capability where applicable) can exploit this to perform a
> > denial-of-service against the host.
> >
> > Replace BUG_ON with WARN_ON_ONCE to prevent kernel panic.
> >
> > Signed-off-by: sunichi <sunyiqixm@gmail.com>
> > ---
> > net/openvswitch/datapath.c | 17 ++++++++++++-----
> > 1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> > index e209099218b4..50c2945081a1 100644
> > --- a/net/openvswitch/datapath.c
> > +++ b/net/openvswitch/datapath.c
> > @@ -2202,7 +2202,8 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, struct net *net,
> >
> > retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd,
> > GFP_KERNEL);
> > - BUG_ON(retval < 0);
> > + if (WARN_ON_ONCE(retval < 0))
> > + return ERR_PTR(-EMSGSIZE);
>
> Hi, sunichi. Thanks for the patch! Though I don't think this is the right
> solution. Instead of just failing the request, we should allocate appropriate
> amount of memory for it instead. The fact that the array is sort of unbounded
> today is also a problem.
>
> So, what I'd suggest is, let's limit the number of upcall pids for a vport with
> the number of CPUs as it is done for the upcall_pids array on the datapath level.
> This will give us some reasonable upper value as there is no point for the
> application to have more handlers than there are CPUs, and the existing userspace
> never does that, so the limit should be safe. Next we can create a new function
> ovs_vport_cmd_msg_size() similar to the existing ovs_dp_cmd_msg_size() that would
> calculate and allocate the appropriate message size, so the allocation is always
> correct.
Thanks for the detailed review, Ilya!
Agreed on all points. For v2 patch submit later, would:
1. Add ovs_vport_cmd_msg_size() to calculate the correct allocation size upfront,
eliminating the EMSGSIZE path entirely.
2. Fix the memory leak on the error path.
> P.S.: This patch also needs a Fixes tag and should be targeted for the 'net'
> tree, i.e. have [PATCH net] as a subject prefix. Also, IIRC, kernel requires
> a full name in the sign-off tag.
>
> AI review also points out a memory leak in case we just return without freeing
> and potentially leaving half- or even fully configured port while returning an
> error to the userspace.
>
> Best regards, Ilya Maximets.
>
Respectfully, sunichi.
^ permalink raw reply
* [PATCH net 3/3] selftests/tc-testing: add tests for cls_fw and cls_flow on shared blocks
From: Xiang Mei @ 2026-03-27 6:13 UTC (permalink / raw)
To: netdev; +Cc: jhs, jiri, davem, edumazet, kuba, horms, shuah, bestswngs,
Xiang Mei
In-Reply-To: <20260327061300.3066844-1-xmei5@asu.edu>
Regression tests for the shared-block NULL derefs fixed in the previous
two patches:
- fw: attach an empty fw filter to a shared block and send traffic.
- flow: create a flow filter on a shared block without a baseclass.
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
.../tc-testing/tc-tests/infra/filter.json | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/infra/filter.json b/tools/testing/selftests/tc-testing/tc-tests/infra/filter.json
index 8d10042b489b..b749087d1691 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/infra/filter.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/infra/filter.json
@@ -22,5 +22,50 @@
"teardown": [
"$TC qdisc del dev $DUMMY root handle 1: htb default 1"
]
+ },
+ {
+ "id": "b7e3",
+ "name": "Empty fw filter on shared block - no NULL deref in fw_classify",
+ "category": [
+ "filter",
+ "fw"
+ ],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 ingress_block 1 clsact",
+ "$TC filter add block 1 protocol ip prio 1 fw"
+ ],
+ "cmdUnderTest": "$IP addr add 10.10.10.1/24 dev $DEV1 && ping -I$DEV1 -c1 -W1 10.10.10.1 || true",
+ "expExitCode": "0",
+ "verifyCmd": "$TC qdisc show dev $DEV1",
+ "matchPattern": "clsact",
+ "matchCount": "1",
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact"
+ ]
+ },
+ {
+ "id": "c8f4",
+ "name": "Flow filter on shared block without baseclass - no NULL deref in flow_change",
+ "category": [
+ "filter",
+ "flow"
+ ],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 ingress_block 1 clsact"
+ ],
+ "cmdUnderTest": "$TC filter add block 1 protocol ip prio 1 handle 1 flow map key dst",
+ "expExitCode": "2",
+ "verifyCmd": "$TC filter show block 1",
+ "matchPattern": "flow",
+ "matchCount": "0",
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact"
+ ]
}
]
--
2.43.0
^ permalink raw reply related
* [PATCH net 2/3] net/sched: cls_flow: fix NULL pointer dereference on shared blocks
From: Xiang Mei @ 2026-03-27 6:12 UTC (permalink / raw)
To: netdev; +Cc: jhs, jiri, davem, edumazet, kuba, horms, shuah, bestswngs,
Xiang Mei
In-Reply-To: <20260327061300.3066844-1-xmei5@asu.edu>
flow_change() calls tcf_block_q() and dereferences q->handle to derive
a default baseclass. Shared blocks leave block->q NULL, causing a NULL
deref when a flow filter without a fully qualified baseclass is created
on a shared block.
Check tcf_block_shared() before accessing block->q and return -EINVAL
for shared blocks. This avoids the null-deref shown below:
=======================================================================
KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
RIP: 0010:flow_change (net/sched/cls_flow.c:508)
Call Trace:
tc_new_tfilter (net/sched/cls_api.c:2432)
rtnetlink_rcv_msg (net/core/rtnetlink.c:6980)
[...]
=======================================================================
Fixes: 1abf272022cf ("net: sched: tcindex, fw, flow: use tcf_block_q helper to get struct Qdisc")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
net/sched/cls_flow.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 339c664beff6..26077681c9b6 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -503,8 +503,13 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
}
if (TC_H_MAJ(baseclass) == 0) {
- struct Qdisc *q = tcf_block_q(tp->chain->block);
+ struct tcf_block *block = tp->chain->block;
+ struct Qdisc *q;
+ if (tcf_block_shared(block))
+ goto err2;
+
+ q = tcf_block_q(block);
baseclass = TC_H_MAKE(q->handle, baseclass);
}
if (TC_H_MIN(baseclass) == 0)
--
2.43.0
^ permalink raw reply related
* [PATCH net 1/3] net/sched: cls_fw: fix NULL pointer dereference on shared blocks
From: Xiang Mei @ 2026-03-27 6:12 UTC (permalink / raw)
To: netdev; +Cc: jhs, jiri, davem, edumazet, kuba, horms, shuah, bestswngs,
Xiang Mei
The old-method path in fw_classify() calls tcf_block_q() and
dereferences q->handle. Shared blocks leave block->q NULL, causing a
NULL deref when an empty cls_fw filter is attached to a shared block
and a packet with a nonzero major skb mark is classified.
Check tcf_block_shared() before accessing block->q and return -1 (no
match) for shared blocks, consistent with cls_u32's tc_u_common_ptr().
The fixed null-ptr-deref calling stack:
KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
RIP: 0010:fw_classify (net/sched/cls_fw.c:81)
Call Trace:
tcf_classify (./include/net/tc_wrapper.h:197 net/sched/cls_api.c:1764 net/sched/cls_api.c:1860)
tc_run (net/core/dev.c:4401)
__dev_queue_xmit (net/core/dev.c:4535 net/core/dev.c:4790)
Fixes: 1abf272022cf ("net: sched: tcindex, fw, flow: use tcf_block_q helper to get struct Qdisc")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
net/sched/cls_fw.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index be81c108179d..caf17ab3be87 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -74,9 +74,14 @@ TC_INDIRECT_SCOPE int fw_classify(struct sk_buff *skb,
}
}
} else {
- struct Qdisc *q = tcf_block_q(tp->chain->block);
+ struct tcf_block *block = tp->chain->block;
+ struct Qdisc *q;
+
+ if (tcf_block_shared(block))
+ return -1;
/* Old method: classify the packet using its skb mark. */
+ q = tcf_block_q(block);
if (id && (TC_H_MAJ(id) == 0 ||
!(TC_H_MAJ(id ^ q->handle)))) {
res->classid = id;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next 1/4] dt-bindings: net: dsa: add MT7628 ESW
From: Joris Vaisvila @ 2026-03-27 6:00 UTC (permalink / raw)
To: Daniel Golle
Cc: netdev, horms, pabeni, kuba, edumazet, davem, olteanv,
Andrew Lunn, devicetree, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
In-Reply-To: <acW9G8vrMz89Enss@makrotopia.org>
Hi Daniel, thanks for the feedback
On Thu, Mar 26, 2026 at 11:11:23PM +0000, Daniel Golle wrote:
> > [...]
> > + port@6 {
> > + reg = <6>;
> > + ethernet = <ðernet>;
> > + phy-mode = "rgmii";
>
> Is this actually RGMII internally? Or some unknown internal way to
> wire the switch CPU port to the CPU MAC? In this case, "internal"
> should be used here as well.
I don't know how to find this out for sure.
In the MT7628 doc (https://vonger.cn/upload/MT7628_Full.pdf) port 6 is
refered to as RGMII port 1 (RGMII port 0 being the non-existent port 5),
but there are no clock registers to be seen.
In RT3050 docs there are RGMII clock registers for port 5, but nothing
for port 6, so maybe the CPU port is really using some mystery internal
connection and only uses "RGMII" as a way to say it's a Gigabit port?
On the hardware I'm testing on, it works fine with the port set to
"internal" or "rgmii". Would it make more sense to set "internal" then?
Thanks,
Joris
^ permalink raw reply
* [PATCH net-next 2/2] net: lan743x: add support for RMII interface
From: Thangaraj Samynathan @ 2026-03-27 5:40 UTC (permalink / raw)
To: bryan.whitehead, UNGLinuxDriver, andrew+netdev, davem, edumazet,
kuba, pabeni, linux, Raju.Lakkaraju, maxime.chevallier
Cc: netdev, linux-kernel
In-Reply-To: <20260327054008.79294-1-thangaraj.s@microchip.com>
Enable RMII interface in the lan743x driver for PHY and MAC
configuration.
- Select RMII interface in lan743x_phy_interface_select().
- Update phylink supported_interfaces and MAC capabilities.
- Enable RMII via RMII_CTL in lan743x_hardware_init().
- Define RMII_CTL register and enable bit in lan743x_main.h.
Signed-off-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
---
drivers/net/ethernet/microchip/lan743x_main.c | 18 ++++++++++++++++++
drivers/net/ethernet/microchip/lan743x_main.h | 3 +++
2 files changed, 21 insertions(+)
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index b7b1584d867d..86d35810460f 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -1370,6 +1370,8 @@ static void lan743x_phy_interface_select(struct lan743x_adapter *adapter)
if (adapter->is_pci11x1x && adapter->is_sgmii_en)
adapter->phy_interface = PHY_INTERFACE_MODE_SGMII;
+ else if (adapter->is_pci11x1x && adapter->is_rmii_en)
+ adapter->phy_interface = PHY_INTERFACE_MODE_RMII;
else if (id_rev == ID_REV_ID_LAN7430_)
adapter->phy_interface = PHY_INTERFACE_MODE_GMII;
else if ((id_rev == ID_REV_ID_LAN7431_) && (data & MAC_CR_MII_EN_))
@@ -3158,6 +3160,13 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
__set_bit(PHY_INTERFACE_MODE_MII,
adapter->phylink_config.supported_interfaces);
break;
+ case PHY_INTERFACE_MODE_RMII:
+ __set_bit(PHY_INTERFACE_MODE_RMII,
+ adapter->phylink_config.supported_interfaces);
+ adapter->phylink_config.mac_capabilities &= ~MAC_1000FD;
+ adapter->phylink_config.lpi_capabilities = 0;
+ break;
+
default:
phy_interface_set_rgmii(adapter->phylink_config.supported_interfaces);
}
@@ -3165,6 +3174,9 @@ static int lan743x_phylink_create(struct lan743x_adapter *adapter)
memcpy(adapter->phylink_config.lpi_interfaces,
adapter->phylink_config.supported_interfaces,
sizeof(adapter->phylink_config.lpi_interfaces));
+ if (adapter->phy_interface == PHY_INTERFACE_MODE_RMII)
+ __clear_bit(PHY_INTERFACE_MODE_RMII,
+ adapter->phylink_config.lpi_interfaces);
pl = phylink_create(&adapter->phylink_config, NULL,
adapter->phy_interface, &lan743x_phylink_mac_ops);
@@ -3509,6 +3521,7 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
{
struct lan743x_tx *tx;
u32 sgmii_ctl;
+ u32 rmii_ctl;
int index;
int ret;
@@ -3530,6 +3543,11 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
sgmii_ctl |= SGMII_CTL_SGMII_POWER_DN_;
}
lan743x_csr_write(adapter, SGMII_CTL, sgmii_ctl);
+ if (adapter->is_rmii_en) {
+ rmii_ctl = lan743x_csr_read(adapter, RMII_CTL);
+ rmii_ctl |= RMII_CTL_RMII_ENABLE_;
+ lan743x_csr_write(adapter, RMII_CTL, rmii_ctl);
+ }
} else {
adapter->max_tx_channels = LAN743X_MAX_TX_CHANNELS;
adapter->used_tx_channels = LAN743X_USED_TX_CHANNELS;
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index 1d7d37456553..03f3727ed8f7 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -324,6 +324,9 @@
#define MAC_WUCSR2_IPV6_TCPSYN_RCD_ BIT(5)
#define MAC_WUCSR2_IPV4_TCPSYN_RCD_ BIT(4)
+#define RMII_CTL (0x710)
+#define RMII_CTL_RMII_ENABLE_ BIT(0)
+
#define SGMII_ACC (0x720)
#define SGMII_ACC_SGMII_BZY_ BIT(31)
#define SGMII_ACC_SGMII_WR_ BIT(30)
--
2.34.1
^ permalink raw reply related
* [PATCH net-next 1/2] net: lan743x: add RMII strap status detection for PCI11x1x
From: Thangaraj Samynathan @ 2026-03-27 5:40 UTC (permalink / raw)
To: bryan.whitehead, UNGLinuxDriver, andrew+netdev, davem, edumazet,
kuba, pabeni, linux, Raju.Lakkaraju, maxime.chevallier
Cc: netdev, linux-kernel
In-Reply-To: <20260327054008.79294-1-thangaraj.s@microchip.com>
Extend pci11x1x_strap_get_status() to read the RMII strap bits from
the STRAP_READ register. The is_rmii_en flag is initialized to
false and updated based on the hardware strap only if SGMII is not
already enabled. This ensures correct interface identification during
adapter initialization.
Signed-off-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
---
drivers/net/ethernet/microchip/lan743x_main.c | 7 +++++++
drivers/net/ethernet/microchip/lan743x_main.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index b4cabde6625a..b7b1584d867d 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -42,6 +42,7 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
u32 strap;
int ret;
+ adapter->is_rmii_en = false;
/* Timeout = 100 (i.e. 1 sec (10 msce * 100)) */
ret = lan743x_hs_syslock_acquire(adapter, 100);
if (ret < 0) {
@@ -73,6 +74,12 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
adapter->is_sgmii_en = false;
}
}
+
+ if (!adapter->is_sgmii_en && strap & STRAP_READ_USE_RMII_EN_) {
+ if (strap & STRAP_READ_RMII_EN_)
+ adapter->is_rmii_en = true;
+ }
+
netif_dbg(adapter, drv, adapter->netdev,
"SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
}
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index 160d94a7cee6..1d7d37456553 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -36,7 +36,9 @@
#define FPGA_SGMII_OP BIT(24)
#define STRAP_READ (0x0C)
+#define STRAP_READ_USE_RMII_EN_ BIT(23)
#define STRAP_READ_USE_SGMII_EN_ BIT(22)
+#define STRAP_READ_RMII_EN_ BIT(7)
#define STRAP_READ_SGMII_EN_ BIT(6)
#define STRAP_READ_SGMII_REFCLK_ BIT(5)
#define STRAP_READ_SGMII_2_5G_ BIT(4)
@@ -1071,6 +1073,7 @@ struct lan743x_adapter {
struct lan743x_rx rx[LAN743X_USED_RX_CHANNELS];
bool is_pci11x1x;
bool is_sgmii_en;
+ bool is_rmii_en;
/* protect ethernet syslock */
spinlock_t eth_syslock_spinlock;
bool eth_syslock_en;
--
2.34.1
^ permalink raw reply related
* [PATCH net-next 0/2] Add RMII Interface Support
From: Thangaraj Samynathan @ 2026-03-27 5:40 UTC (permalink / raw)
To: bryan.whitehead, UNGLinuxDriver, andrew+netdev, davem, edumazet,
kuba, pabeni, linux, Raju.Lakkaraju, maxime.chevallier
Cc: netdev, linux-kernel
patch 1 reads strap register to check if RMII enabled
pacth 2 adds RMII interface support to the driver
Thangaraj Samynathan (2):
net: lan743x: add RMII strap status detection for PCI11x1x
net: lan743x: add support for RMII interface
drivers/net/ethernet/microchip/lan743x_main.c | 25 +++++++++++++++++++
drivers/net/ethernet/microchip/lan743x_main.h | 6 +++++
2 files changed, 31 insertions(+)
--
2.34.1
^ permalink raw reply
* Re: [PATCH net v2] net: qrtr: fix endian handling of confirm_rx field
From: Manivannan Sadhasivam @ 2026-03-27 5:07 UTC (permalink / raw)
To: Alexander Wilhelm
Cc: Simon Horman, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Bjorn Andersson, linux-arm-msm, netdev, linux-kernel
In-Reply-To: <acTV86xU6piDih4B@FUE-ALEWI-WINX>
On Thu, Mar 26, 2026 at 07:45:07AM +0100, Alexander Wilhelm wrote:
> On Wed, Mar 25, 2026 at 06:07:37PM +0000, Simon Horman wrote:
> > On Tue, Mar 24, 2026 at 01:28:32PM +0530, Manivannan Sadhasivam wrote:
> > > On Tue, Mar 24, 2026 at 08:50:33AM +0100, Alexander Wilhelm wrote:
> > > > Convert confirm_rx to little endian when enqueueing and convert it back on
> > > > receive. This fixes control flow on big endian hosts, little endian is
> > > > unaffected.
> > > >
> > > > On transmit, store confirm_rx as __le32 using cpu_to_le32(). On receive,
> > > > apply le32_to_cpu() before using the value. !! ensures the value is 0 or 1
> > > > in native endianness, so the conversion isn’t strictly required here, but
> > > > it is kept for consistency and clarity.
> > > >
> > > > Fixes: 5fdeb0d372ab ("net: qrtr: Implement outgoing flow control")
> > > > Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> > >
> > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> >
> > Sorry if this contradicts my review of v1.
> >
> > But as this isn't strictly necessary let's target net-next
> > and drop the Fixes tag.
>
> Sure, I will rebase onto the latest `net-next` repository and send out `v3`,
> without a "Fixes" tag.
>
FWIW: Adding Fixes tag doesn't mean that the patch should be queued for -rcS.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH net-next v2 1/3] gve: skip error logging for retryable AdminQ commands
From: Li Xiasong @ 2026-03-27 5:06 UTC (permalink / raw)
To: Jordan Rhee
Cc: Harshitha Ramamurthy, joshwash, andrew+netdev, davem, edumazet,
kuba, pabeni, richardcochran, willemb, nktgrg, jfraker, ziweixiao,
maolson, thostet, jefrogers, alok.a.tiwari, yyd, linux-kernel,
netdev, yuehaibing, zhangchangzhong, weiyongjun1
In-Reply-To: <CA+mzVtsXm1M4G_qiuEZ4A_ttJk4NL-DP-Ceo=nFPWRNUSn6RBQ@mail.gmail.com>
Hi, Jordan
On 3/27/2026 11:56 AM, Jordan Rhee wrote:
> Hi Li, thank you very much for the review. The intent is only to skip
> logging for *retryable* commands that return -EAGAIN. If a
> non-retryable command fails, we do want to log, even if it returns
> -EAGAIN.
> Jordan
>
Thanks for the explanation! I totally misread the condition - I confused
opcode with err because they both start with GVE_ADMINQ_.
The || makes perfect sense now. Sorry for the noise!
Best regards,
Li Xiasong
>
> On Thu, Mar 26, 2026 at 7:28 PM Li Xiasong <lixiasong1@huawei.com> wrote:
>>
>> Hi,
>>
>> On 3/27/2026 6:45 AM, Harshitha Ramamurthy wrote:
>>> 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.
>>>
>>> Reviewed-by: Joshua Washington <joshwash@google.com>
>>> Signed-off-by: Jordan Rhee <jordanrhee@google.com>
>>> Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
>>> ---
>>> drivers/net/ethernet/google/gve/gve_adminq.c | 26 +++++++++++++++-----
>>> 1 file changed, 20 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
>>> index 08587bf40ed4..c7834614c5f0 100644
>>> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
>>> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
>>> @@ -416,11 +416,6 @@ 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;
>>> @@ -455,6 +450,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 +487,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)
>>
>> In gve_adminq_kick_and_wait(), the condition is:
>>
>> if (!gve_adminq_is_retryable(opcode) || err != -EAGAIN)
>> dev_err_ratelimited(...);
>>
>> Based on the commit log, the goal is to skip logging when the result is
>> -EAGAIN for transient conditions. However, when gve_adminq_is_retryable()
>> returns false (e.g., GVE_ADMINQ_COMMAND_ERROR_ABORTED), even if err is
>> -EAGAIN, the condition evaluates to true and the error would still be logged.
>>
>> Would it be more appropriate to use && instead of || here?
>>
>> if (!gve_adminq_is_retryable(opcode) && err != -EAGAIN)
>>
>> I may be missing something, so please let me know if I've misunderstood.
>>
>>> + 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;
>>
>> Best regards,
>> Li Xiasong
^ permalink raw reply
* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Eric Woudstra @ 2026-03-27 4:41 UTC (permalink / raw)
To: Lucien.Jheng, andrew, hkallweit1, linux, davem, edumazet, kuba,
pabeni, netdev, linux-kernel, bjorn
Cc: frank-w, daniel, lucien.jheng
In-Reply-To: <20260326153518.8387-1-lucienzx159@gmail.com>
On 3/26/26 4:35 PM, Lucien.Jheng wrote:
> AN8811HB needs a MCU soft-reset cycle before firmware loading begins.
> Assert the MCU (hold it in reset) and immediately deassert (release)
> via a dedicated PBUS register pair (0x5cf9f8 / 0x5cf9fc), accessed
> through the PHY-addr+8 MDIO bus node rather than the BUCKPBUS indirect
> path. This clears the MCU state before the firmware loading sequence
> starts.
>
> Add __air_pbus_reg_write() as a low-level helper for this access, then
> implement an8811hb_mcu_assert() / _deassert() on top of it. Wire both
> into an8811hb_load_firmware() and en8811h_restart_mcu() so every
> firmware load or MCU restart on AN8811HB correctly sequences the reset
> control registers.
>
> Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB support")
> Signed-off-by: Lucien Jheng <lucienzx159@gmail.com>
> ---
> Changes in v2:
> - Rewrite commit message: The previous wording was ambiguous,
> as it suggested the MCU remains asserted during the entire
> firmware loading process, rather than a quick reset cycle
> before it starts.
> Clarify that assert and deassert is an immediate soft-reset
> cycle to clear MCU state before firmware loading begins.
> - Add Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB
> support").
> - Change phydev_info() to phydev_dbg() in an8811hb_mcu_assert() and
> an8811hb_mcu_deassert() to avoid noise during normal boot.
>
> drivers/net/phy/air_en8811h.c | 105 ++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
> index 29ae73e65caa..01fce1b93618 100644
> --- a/drivers/net/phy/air_en8811h.c
> +++ b/drivers/net/phy/air_en8811h.c
> @@ -170,6 +170,16 @@
> #define AN8811HB_CLK_DRV_CKO_LDPWD BIT(13)
> #define AN8811HB_CLK_DRV_CKO_LPPWD BIT(14)
>
> +#define AN8811HB_MCU_SW_RST 0x5cf9f8
> +#define AN8811HB_MCU_SW_RST_HOLD BIT(16)
> +#define AN8811HB_MCU_SW_RST_RUN (BIT(16) | BIT(0))
> +#define AN8811HB_MCU_SW_START 0x5cf9fc
> +#define AN8811HB_MCU_SW_START_EN BIT(16)
> +
> +/* MII register constants for PBUS access (PHY addr + 8) */
> +#define AIR_PBUS_ADDR_HIGH 0x1c
> +#define AIR_PBUS_DATA_HIGH 0x10
> +
> /* Led definitions */
> #define EN8811H_LED_COUNT 3
>
> @@ -254,6 +264,36 @@ static int air_phy_write_page(struct phy_device *phydev, int page)
> return __phy_write(phydev, AIR_EXT_PAGE_ACCESS, page);
> }
>
> +static int __air_pbus_reg_write(struct phy_device *phydev,
> + u32 pbus_reg, u32 pbus_data)
> +{
> + struct mii_bus *bus = phydev->mdio.bus;
> + int pbus_addr = phydev->mdio.addr + 8;
> + int ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_EXT_PAGE_ACCESS,
> + upper_16_bits(pbus_reg));
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_ADDR_HIGH,
> + (pbus_reg & GENMASK(15, 6)) >> 6);
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, (pbus_reg & GENMASK(5, 2)) >> 2,
> + lower_16_bits(pbus_data));
> + if (ret < 0)
> + return ret;
> +
> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_DATA_HIGH,
> + upper_16_bits(pbus_data));
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
Maybe you can use mutex_lock() and mutex_unlock() here also, like so:
static int __air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
u32 pbus_data)
{
int ret;
ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
(u16)(pbus_address >> 6));
if (ret < 0)
return ret;
ret = __mdiobus_write(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf,
(u16)pbus_data);
if (ret < 0)
return ret;
ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH,
(u16)(pbus_data >> 16));
if (ret < 0)
return ret;
return 0;
}
static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
u32 pbus_data)
{
int ret;
mutex_lock(&mdio->bus->mdio_lock);
ret = __air_pbus_reg_write(mdio, pbus_address, pbus_data);
if (ret < 0)
dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
pbus_address, ret);
mutex_unlock(&mdio->bus->mdio_lock);
return ret;
}
static int __air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
u32 *pbus_data)
{
int ret, pbus_data_low;
ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
(u16)(pbus_address >> 6));
if (ret < 0)
return ret;
ret = __mdiobus_read(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf);
if (ret < 0)
return ret;
pbus_data_low = ret;
ret = __mdiobus_read(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH);
if (ret < 0)
return ret;
*pbus_data = (ret << 16) + pbus_data_low;
return 0;
}
static int air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
u32 *pbus_data)
{
int ret;
mutex_lock(&mdio->bus->mdio_lock);
ret = __air_pbus_reg_read(mdio, pbus_address, pbus_data);
if (ret < 0)
dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
pbus_address, ret);
mutex_unlock(&mdio->bus->mdio_lock);
return ret;
}
With:
#define AIR_PBUS_DATA_HIGH 0x10
> static int __air_buckpbus_reg_write(struct phy_device *phydev,
> u32 pbus_address, u32 pbus_data)
> {
> @@ -570,10 +610,65 @@ static int an8811hb_load_file(struct phy_device *phydev, const char *name,
> return ret;
> }
>
> +static int an8811hb_mcu_assert(struct phy_device *phydev)
> +{
> + int ret;
> +
> + phy_lock_mdio_bus(phydev);
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
> + AN8811HB_MCU_SW_RST_HOLD);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START, 0);
> + if (ret < 0)
> + goto unlock;
> +
> + msleep(50);
> + phydev_dbg(phydev, "MCU asserted\n");
> +
> +unlock:
> + phy_unlock_mdio_bus(phydev);
> + return ret;
> +}
> +
> +static int an8811hb_mcu_deassert(struct phy_device *phydev)
> +{
> + int ret;
> +
> + phy_lock_mdio_bus(phydev);
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START,
> + AN8811HB_MCU_SW_START_EN);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
> + AN8811HB_MCU_SW_RST_RUN);
> + if (ret < 0)
> + goto unlock;
> +
> + msleep(50);
> + phydev_dbg(phydev, "MCU deasserted\n");
> +
> +unlock:
> + phy_unlock_mdio_bus(phydev);
> + return ret;
> +}
> +
> static int an8811hb_load_firmware(struct phy_device *phydev)
> {
> int ret;
>
> + ret = an8811hb_mcu_assert(phydev);
> + if (ret < 0)
> + return ret;
> +
> + ret = an8811hb_mcu_deassert(phydev);
> + if (ret < 0)
> + return ret;
> +
> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
> EN8811H_FW_CTRL_1_START);
> if (ret < 0)
> @@ -662,6 +757,16 @@ static int en8811h_restart_mcu(struct phy_device *phydev)
> {
> int ret;
>
> + if (phy_id_compare_model(phydev->phy_id, AN8811HB_PHY_ID)) {
> + ret = an8811hb_mcu_assert(phydev);
> + if (ret < 0)
> + return ret;
> +
> + ret = an8811hb_mcu_deassert(phydev);
> + if (ret < 0)
> + return ret;
> + }
> +
> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
> EN8811H_FW_CTRL_1_START);
> if (ret < 0)
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH net-next] net: add sysctl to toggle napi_consume_skb() alien skb defer
From: Stanislav Fomichev @ 2026-03-27 4:13 UTC (permalink / raw)
To: Jason Xing; +Cc: davem, edumazet, kuba, pabeni, horms, netdev
In-Reply-To: <20260326144249.97213-1-kerneljasonxing@gmail.com>
On 03/26, Jason Xing wrote:
> Commit e20dfbad8aab ("net: fix napi_consume_skb() with alien skbs")
> defers freeing of alien SKBs (alloc_cpu != current cpu) via
> skb_attempt_defer_free() on the TX completion path to reduce cross-NUMA
> SLUB spinlock contention to improve multi-queue UDP workloads.
>
> However, this unconditionally impacts the napi_skb_cache fast recycle
> path for single-flow / few-flow workloads (e.g. AF_XDP benchmarks[1]):
> when the TX completion NAPI CPU differs from the SKB allocation CPU,
> SKBs are deferred instead of being returned to the local napi_skb_cache,
> forcing RX allocations back to the slow slab path.
>
> The existing net.core.skb_defer_max=0 could disable this, but it is a
> global switch that also disables the defer mechanism in TCP/UDP/MPTCP
> recvmsg paths, losing its positive SLUB locality benefits there. AF_XDP
> can co-exist with other protocols. That's the reason why I gave up
> reusing skb_defer_disable_key. Besides, if the defer path is disabled,
> that means TCP/UDP/MPTCP in process path will trigger directly freeing
> skb with enabling/disabling bottom half(in kfree_skb_napi_cache())
> which could affect others. So my thinking is not to touch this path.
>
> Add a dedicated sysctl net.core.napi_consume_skb_defer backed by a
> static key to selectively control the alien skb defer feature. Let
> users decide which is the best fit for their own requirements.
For a non-zc path adding a userspace knob feels like too much. And there
is zero documentation for users to decide which mode to use.
^ permalink raw reply
* [PATCH v2 net-next] tcp: use __jhash_final() in inet6_ehashfn()
From: Eric Dumazet @ 2026-03-27 4:06 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Neal Cardwell, Kuniyuki Iwashima, netdev,
eric.dumazet, Eric Dumazet
I misread jhash2() implementation.
Last round should use __jhash_final() instead of __jhash_mix().
Using __jhash_mix() here leaves entropy distributed across a, b, and c,
which might lead to incomplete diffusion of the faddr and fport bits
into the bucket index. Replacing this last __jhash_mix() with
__jhash_final() provides the correct avalanche properties
for the returned value in c.
$ scripts/bloat-o-meter -t vmlinux.0 vmlinux
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-4 (-4)
Function old new delta
inet6_ehashfn 306 302 -4
Total: Before=25155089, After=25155085, chg -0.00%
Fixes: 854587e69ef3 ("tcp: improve inet6_ehashfn() entropy")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v2: also update the comment (Jakub)
net/ipv6/inet6_hashtables.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 109fbf620ad7cde54300b937db58ada0d5c80664..b111b51d69fc9b77ac6ea2576c1b553c74bb139d 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -42,7 +42,7 @@ u32 inet6_ehashfn(const struct net *net,
* Hash laddr + faddr + lport/fport + net_hash_mix.
* Notes:
* We combine laddr[0] (high order 32 bits of local address)
- * with net_hash_mix() to avoid using __jhash_final(a, b, c).
+ * with net_hash_mix() to hash a multiple of 3 words.
*
* We do not include JHASH_INITVAL + 36 contribution
* to initial values of a, b, c.
@@ -63,7 +63,7 @@ u32 inet6_ehashfn(const struct net *net,
a += (__force u32)faddr->s6_addr32[2];
b += (__force u32)faddr->s6_addr32[3];
c += (__force u32)fport;
- __jhash_mix(a, b, c);
+ __jhash_final(a, b, c);
/* Note: We need to add @lport instead of fully hashing it.
* See commits 9544d60a2605 ("inet: change lport contribution
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* Re: [net-next] tcp: use __jhash_final() in inet6_ehashfn()
From: Eric Dumazet @ 2026-03-27 4:05 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, pabeni, kuniyu, horms, netdev, eric.dumazet, ncardwell
In-Reply-To: <20260324015011.858344-1-kuba@kernel.org>
On Mon, Mar 23, 2026 at 6:50 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> tcp: use __jhash_final() in inet6_ehashfn()
>
> This commit fixes a bug in inet6_ehashfn() where the last round of
> jhash incorrectly used __jhash_mix() instead of __jhash_final(),
> which left entropy distributed incompletely and didn't provide proper
> avalanche properties.
>
> > diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
> > index 109fbf620ad7..0a781cba4085 100644
> > --- a/net/ipv6/inet6_hashtables.c
> > +++ b/net/ipv6/inet6_hashtables.c
> > @@ -63,7 +63,7 @@ u32 inet6_ehashfn(const struct net *net,
> > a += (__force u32)faddr->s6_addr32[2];
> > b += (__force u32)faddr->s6_addr32[3];
> > c += (__force u32)fport;
> > - __jhash_mix(a, b, c);
> > + __jhash_final(a, b, c);
> >
> > /* Note: We need to add @lport instead of fully hashing it.
> > * See commits 9544d60a2605 ("inet: change lport contribution
>
> This isn't a bug, but the comment earlier in the function needs updating.
> The comment currently says:
>
> /*
> * Please look at jhash() implementation for reference.
> * Hash laddr + faddr + lport/fport + net_hash_mix.
> * Notes:
> * We combine laddr[0] (high order 32 bits of local address)
> * with net_hash_mix() to avoid using __jhash_final(a, b, c).
> *
>
> The line "to avoid using __jhash_final(a, b, c)" is now misleading since
> the code was just changed to use __jhash_final() in the last round. Should
> this comment be updated to explain that we combine laddr[0] with
> net_hash_mix() to avoid an extra __jhash_mix() round, and that the final
> round correctly uses __jhash_final() for proper avalanche properties?
Ah well, comments are stale after one week.
^ permalink raw reply
* RE: [EXTERNAL] Re: [PATCH net-next v2] net: mana: Set default number of queues to 16
From: Long Li @ 2026-03-27 4:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Konstantin Taranov, David S . Miller, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Jason Gunthorpe, Leon Romanovsky, Haiyang Zhang,
KY Srinivasan, Wei Liu, Dexuan Cui, Simon Horman,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20260326201841.3b7e5b78@kernel.org>
> On Mon, 23 Mar 2026 12:49:25 -0700 Long Li wrote:
> > Set the default number of queues per vPort to MANA_DEF_NUM_QUEUES
> > (16), as 16 queues can achieve optimal throughput for typical
> > workloads. The actual number of queues may be lower if it exceeds the
> > hardware reported limit. Users can increase the number of queues up to
> > max_queues via ethtool if needed.
>
> Sorry we are a bit backlogged I didn't spot this in time (read: I'm planning to
> revert this unless proper explanation is provided)
>
> Could you explain why not use netif_get_num_default_rss_queues() ?
> Having local driver innovations is a major PITA for users who deal with
> heterogeneous envs.
Hi Jakub,
We considered netif_get_num_default_rss_queues() but chose a fixed default based on our performance testing. On Azure VMs, typical
workloads plateau at around 16 queues - adding more queues beyond that doesn't improve throughput but increases memory usage and
interrupt overhead.
netif_get_num_default_rss_queues() would return 32-64 on large VMs (64-128 vCPUs), which wastes resources without benefit.
That said, I agree that completely ignoring the core-based heuristic isn't ideal for consistency. One option is to use
netif_get_num_default_rss_queues() but clamp it to a maximum of MANA_DEF_NUM_QUEUES (16), so small VMs still get enough queues and
large VMs don't over-allocate. Something like:
apc->num_queues = min(netif_get_num_default_rss_queues(), MANA_DEF_NUM_QUEUES);
apc->num_queues = min(apc->num_queues, gc->max_num_queues);
For reference, it seems mlx4 does something similar - it caps at DEF_RX_RINGS (16) regardless of core count.
Do you want me to send a v2?
Thanks,
Long
^ permalink raw reply
* Re: [PATCH net-next v1] tcp: Fix inconsistent indenting warning
From: patchwork-bot+netdevbpf @ 2026-03-27 4:00 UTC (permalink / raw)
To: Jiayuan Chen
Cc: netdev, lkp, edumazet, ncardwell, kuniyu, davem, kuba, pabeni,
horms, linux-kernel
In-Reply-To: <20260325071854.805-1-jiayuan.chen@linux.dev>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 25 Mar 2026 15:18:54 +0800 you wrote:
> Suppress such warning reported by test robot:
> include/net/tcp.h:1449 tcp_ca_event() warn: inconsistent indenting
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202603251430.gQ3VuiKV-lkp@intel.com/
> Fixes: d1e59a469737 ("tcp: add cwnd_event_tx_start to tcp_congestion_ops")
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>
> [...]
Here is the summary with links:
- [net-next,v1] tcp: Fix inconsistent indenting warning
https://git.kernel.org/netdev/net-next/c/552994294fe2
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net-next v2 1/3] gve: skip error logging for retryable AdminQ commands
From: Jordan Rhee @ 2026-03-27 3:56 UTC (permalink / raw)
To: Li Xiasong
Cc: Harshitha Ramamurthy, joshwash, andrew+netdev, davem, edumazet,
kuba, pabeni, richardcochran, willemb, nktgrg, jfraker, ziweixiao,
maolson, thostet, jefrogers, alok.a.tiwari, yyd, linux-kernel,
netdev, yuehaibing, zhangchangzhong, weiyongjun1
In-Reply-To: <d5312a24-68d5-49e3-9787-9a01c8606d9a@huawei.com>
Hi Li, thank you very much for the review. The intent is only to skip
logging for *retryable* commands that return -EAGAIN. If a
non-retryable command fails, we do want to log, even if it returns
-EAGAIN.
Jordan
On Thu, Mar 26, 2026 at 7:28 PM Li Xiasong <lixiasong1@huawei.com> wrote:
>
> Hi,
>
> On 3/27/2026 6:45 AM, Harshitha Ramamurthy wrote:
> > 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.
> >
> > Reviewed-by: Joshua Washington <joshwash@google.com>
> > Signed-off-by: Jordan Rhee <jordanrhee@google.com>
> > Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> > ---
> > drivers/net/ethernet/google/gve/gve_adminq.c | 26 +++++++++++++++-----
> > 1 file changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> > index 08587bf40ed4..c7834614c5f0 100644
> > --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> > +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> > @@ -416,11 +416,6 @@ 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;
> > @@ -455,6 +450,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 +487,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)
>
> In gve_adminq_kick_and_wait(), the condition is:
>
> if (!gve_adminq_is_retryable(opcode) || err != -EAGAIN)
> dev_err_ratelimited(...);
>
> Based on the commit log, the goal is to skip logging when the result is
> -EAGAIN for transient conditions. However, when gve_adminq_is_retryable()
> returns false (e.g., GVE_ADMINQ_COMMAND_ERROR_ABORTED), even if err is
> -EAGAIN, the condition evaluates to true and the error would still be logged.
>
> Would it be more appropriate to use && instead of || here?
>
> if (!gve_adminq_is_retryable(opcode) && err != -EAGAIN)
>
> I may be missing something, so please let me know if I've misunderstood.
>
> > + 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;
>
> Best regards,
> Li Xiasong
^ 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