From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4A217CD98C5 for ; Sat, 13 Jun 2026 10:04:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=AyPgplNXCBS/dtReoSHAFEqQvmD72NfRUURLDpSPEOI=; b=4jWb36LeHVHl/Zq9hc++9CJOfP QP70JrVhJfTm1sE0CIbvRV4OTpHmJNQ6THov/e5oWRSxkMN+ihs68CMC+Pw62Q7TQFR3Yei/EP33J q3VYO8kh+JhI3A6y6aOdcm3sUJU7PQ856nZkfaWT3HA8d7QxyCUyNOmN2XoS7T53+ztYCno7Lyy8S g7by+5/bgY5ils15WkahGjQtiX4HodLePmnJgsc6qLu1eEDclC+RIaNT6Ez45MZyP5y8S9uq4Hlr7 ujJZGd0w0lx4VfAQ4x2JG6RqwR9XI3K7EL9n49dc0ojKdkacXfoRQrK4+yYx6uYCdi8Gq6mil6FgC tZ9hL27g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wYLDy-0000000CA3U-091v; Sat, 13 Jun 2026 10:04:22 +0000 Received: from tor.source.kernel.org ([172.105.4.254]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1wYLDx-0000000CA3J-12hU; Sat, 13 Jun 2026 10:04:21 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by tor.source.kernel.org (Postfix) with ESMTP id AE01E60018; Sat, 13 Jun 2026 10:04:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D88C81F000E9; Sat, 13 Jun 2026 10:04:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1781345060; bh=AyPgplNXCBS/dtReoSHAFEqQvmD72NfRUURLDpSPEOI=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=lFEmyH7Gjwzi1tLTY2WewA95xMpfSwXpoSYyH4z1mPK1SkEwcd/OBdfJPeNBkVFTv Vqb2jYtWq4XhNyRLggNHzQ8q+I0PRzoPwBOnACd7Ssbn0s3b9bEccY6z6rGIGA52Z6 hNiF9Ocf2w6s4HUlrjxvzWRGtQXBSGA3hOH0faQ7wmWyQCXtqB9nLd5nPzur1Ch/nP ycw87bxVHNogVDx7XoPK+OuMI92tZVG6XO9u0P8U9dizKIp0e4y6SsiQRXaLGaMFOO ij8/klMNZEsSmcXzqCBHEwQenryVCk5/Cg2H1xSDnZou+5Ej1jLdj7YaCf/fb15+Dt b8H2A3ZvKdNjA== Date: Sat, 13 Jun 2026 12:04:16 +0200 From: Lorenzo Bianconi To: Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni Cc: linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, netdev@vger.kernel.org, Madhur Agrawal , Alexander Lobakin Subject: Re: [PATCH net-next v5 3/3] net: airoha: defer GDM3/GDM4 WAN mode and GDM2 loopback to QoS offload Message-ID: References: <20260611-airoha-ethtool-priv_flags-v5-0-c11de08486d1@kernel.org> <20260611-airoha-ethtool-priv_flags-v5-3-c11de08486d1@kernel.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="D4O7rKOkeJ+Rx7Jv" Content-Disposition: inline In-Reply-To: <20260611-airoha-ethtool-priv_flags-v5-3-c11de08486d1@kernel.org> X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --D4O7rKOkeJ+Rx7Jv Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Commenting on sashiko's report: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260611-airoha-ethtool= -priv_flags-v5-0-c11de08486d1%40kernel.org [...] > +static int airoha_enable_qos_for_gdm34(struct net_device *netdev, > + struct netlink_ext_ack *extack) > +{ > + struct airoha_gdm_dev *wan_dev, *dev =3D netdev_priv(netdev); > + struct airoha_gdm_port *port =3D dev->port; > + struct airoha_eth *eth =3D dev->eth; > + int err =3D -EBUSY; > + > + if (port->id !=3D AIROHA_GDM3_IDX && > + port->id !=3D AIROHA_GDM4_IDX) { > + /* HW QoS is always supported by GDM1 and GDM2 */ > + return 0; > + } > + > + if (!airoha_is_lan_gdm_dev(dev)) /* Already enabled */ > + return 0; > + > + mutex_lock(&flow_offload_mutex); > + > + wan_dev =3D airoha_get_wan_gdm_dev(eth); > + if (wan_dev) { > + if ((wan_dev->flags & AIROHA_PRIV_F_QOS) || > + wan_dev->port->id =3D=3D AIROHA_GDM2_IDX) { > + NL_SET_ERR_MSG_MOD(extack, > + "QoS configured for WAN device"); > + goto error_unlock; > + } > + airoha_disable_qos_for_gdm34(netdev_from_priv(wan_dev)); > + } > + > + dev->flags |=3D AIROHA_PRIV_F_WAN; > + airoha_dev_set_qdma(dev); > + err =3D airoha_enable_gdm2_loopback(dev); > + if (err) > + goto error_disable_wan; > + > + err =3D airoha_set_macaddr(dev, netdev->dev_addr); > + if (err) > + goto error_disable_loopback; > + > + if (netif_running(netdev)) { > + u32 pse_port; > + > + pse_port =3D airoha_ppe_is_enabled(eth, 1) ? FE_PSE_PORT_PPE2 > + : FE_PSE_PORT_PPE1; > + airoha_set_gdm_port_fwd_cfg(eth, REG_GDM_FWD_CFG(port->id), > + pse_port); > + } > + > + mutex_unlock(&flow_offload_mutex); > + > + return 0; > + > +error_disable_loopback: > + airoha_disable_gdm2_loopback(dev); > +error_disable_wan: > + dev->flags &=3D ~AIROHA_PRIV_F_WAN; > + airoha_dev_set_qdma(dev); > +error_unlock: > + mutex_unlock(&flow_offload_mutex); > + > + return err; > +} - The error_disable_loopback / error_disable_wan / error_unlock paths only revert the requesting dev. The earlier airoha_disable_qos_for_gdm34(wan_d= ev) demotion is never undone. Could a TC_HTB_CREATE that returns an error to userspace then leave the system with no WAN GDM3/GDM4 device, with the previously-working sibling silently flipped to LAN, its QDMA migrated to QDMA0, GDM2 loopback torn down, and its forwarding rewritten to PPE1? Should the failure paths re-promote wan_dev (re-call airoha_enable_qos_fo= r_gdm34 or an equivalent restore helper on it) before unlocking and returning? - This is the same item reported by sashiko-gemini in [0]. In my previous reply I have explained why I do not think it worths to re-promote the original interface in case of failure in airoha_enable_qos_for_gdm34(). Regards, Lorenzo [0] https://sashiko.dev/#/patchset/20260611-airoha-ethtool-priv_flags-v5-0-= c11de08486d1%40kernel.org > + > static int airoha_tc_htb_destroy(struct net_device *netdev) > { > struct airoha_gdm_dev *dev =3D netdev_priv(netdev); > @@ -3038,6 +3205,8 @@ static int airoha_tc_htb_destroy(struct net_device = *netdev) > for_each_set_bit(q, dev->qos_sq_bmap, AIROHA_NUM_QOS_CHANNELS) > airoha_tc_remove_htb_queue(netdev, q); > =20 > + dev->flags &=3D ~AIROHA_PRIV_F_QOS; > + > return 0; > } > =20 > @@ -3057,24 +3226,33 @@ static int airoha_tc_get_htb_get_leaf_queue(struc= t net_device *netdev, > return 0; > } > =20 > -static int airoha_tc_setup_qdisc_htb(struct net_device *dev, > +static int airoha_tc_setup_qdisc_htb(struct net_device *netdev, > struct tc_htb_qopt_offload *opt) > { > switch (opt->command) { > - case TC_HTB_CREATE: > + case TC_HTB_CREATE: { > + struct airoha_gdm_dev *dev =3D netdev_priv(netdev); > + int err; > + > + err =3D airoha_enable_qos_for_gdm34(netdev, opt->extack); > + if (err) > + return err; > + > + dev->flags |=3D AIROHA_PRIV_F_QOS; > break; > + } > case TC_HTB_DESTROY: > - return airoha_tc_htb_destroy(dev); > + return airoha_tc_htb_destroy(netdev); > case TC_HTB_NODE_MODIFY: > - return airoha_tc_htb_modify_queue(dev, opt); > + return airoha_tc_htb_modify_queue(netdev, opt); > case TC_HTB_LEAF_ALLOC_QUEUE: > - return airoha_tc_htb_alloc_leaf_queue(dev, opt); > + return airoha_tc_htb_alloc_leaf_queue(netdev, opt); > case TC_HTB_LEAF_DEL: > case TC_HTB_LEAF_DEL_LAST: > case TC_HTB_LEAF_DEL_LAST_FORCE: > - return airoha_tc_htb_delete_leaf_queue(dev, opt); > + return airoha_tc_htb_delete_leaf_queue(netdev, opt); > case TC_HTB_LEAF_QUERY_QUEUE: > - return airoha_tc_get_htb_get_leaf_queue(dev, opt); > + return airoha_tc_get_htb_get_leaf_queue(netdev, opt); > default: > return -EOPNOTSUPP; > } > diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ether= net/airoha/airoha_eth.h > index 24fd8dcf7fca..d1390ffcea7c 100644 > --- a/drivers/net/ethernet/airoha/airoha_eth.h > +++ b/drivers/net/ethernet/airoha/airoha_eth.h > @@ -540,11 +540,12 @@ struct airoha_qdma { > =20 > enum airoha_priv_flags { > AIROHA_PRIV_F_WAN =3D BIT(0), > + AIROHA_PRIV_F_QOS =3D BIT(1), > }; > =20 > struct airoha_gdm_dev { > + struct airoha_qdma __rcu *qdma; > struct airoha_gdm_port *port; > - struct airoha_qdma *qdma; > struct airoha_eth *eth; > =20 > DECLARE_BITMAP(qos_sq_bmap, AIROHA_NUM_QOS_CHANNELS); > @@ -676,6 +677,16 @@ int airoha_get_fe_port(struct airoha_gdm_dev *dev); > bool airoha_is_valid_gdm_dev(struct airoha_eth *eth, > struct airoha_gdm_dev *dev); > =20 > +extern struct mutex flow_offload_mutex; > + > +static inline struct airoha_qdma * > +airoha_qdma_deref(struct airoha_gdm_dev *dev) > +{ > + return rcu_dereference_protected(dev->qdma, > + lockdep_rtnl_is_held() || > + lockdep_is_held(&flow_offload_mutex)); > +} > + > void airoha_ppe_set_cpu_port(struct airoha_gdm_dev *dev, u8 ppe_id, u8 f= port); > bool airoha_ppe_is_enabled(struct airoha_eth *eth, int index); > void airoha_ppe_check_skb(struct airoha_ppe_dev *dev, struct sk_buff *sk= b, > diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ether= net/airoha/airoha_ppe.c > index 91bcc55a6ac6..1d1b1a57d795 100644 > --- a/drivers/net/ethernet/airoha/airoha_ppe.c > +++ b/drivers/net/ethernet/airoha/airoha_ppe.c > @@ -15,7 +15,10 @@ > #include "airoha_regs.h" > #include "airoha_eth.h" > =20 > -static DEFINE_MUTEX(flow_offload_mutex); > +/* Serialize airoha_gdm_dev flags, QDMA pointer and PPE CPU port > + * configuration. > + */ > +DEFINE_MUTEX(flow_offload_mutex); > static DEFINE_SPINLOCK(ppe_lock); > =20 > static const struct rhashtable_params airoha_flow_table_params =3D { > @@ -86,8 +89,8 @@ static u32 airoha_ppe_get_timestamp(struct airoha_ppe *= ppe) > =20 > void airoha_ppe_set_cpu_port(struct airoha_gdm_dev *dev, u8 ppe_id, u8 f= port) > { > - struct airoha_qdma *qdma =3D dev->qdma; > - struct airoha_eth *eth =3D qdma->eth; > + struct airoha_qdma *qdma =3D airoha_qdma_deref(dev); > + struct airoha_eth *eth =3D dev->eth; > u8 qdma_id =3D qdma - ð->qdma[0]; > u32 fe_cpu_port; > =20 > diff --git a/drivers/net/ethernet/airoha/airoha_regs.h b/drivers/net/ethe= rnet/airoha/airoha_regs.h > index 436f3c8779c1..4e17dfbcf2b8 100644 > --- a/drivers/net/ethernet/airoha/airoha_regs.h > +++ b/drivers/net/ethernet/airoha/airoha_regs.h > @@ -376,6 +376,7 @@ > =20 > #define REG_SRC_PORT_FC_MAP6 0x2298 > #define FC_ID_OF_SRC_PORT_MASK(_n) GENMASK(4 + ((_n) << 3), ((_n) << 3)) > +#define FC_MAP6_DEF_VALUE 0x1b1a1918 > =20 > #define REG_CDM5_RX_OQ1_DROP_CNT 0x29d4 > =20 >=20 > --=20 > 2.54.0 >=20 --D4O7rKOkeJ+Rx7Jv Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCai0rHgAKCRA6cBh0uS2t rIUoAP4pZrm2UQU8qtzgsF0pQQuPaoB3Q5R0siU/rjRMb5PkgwEAmSLKBZg1XeBq C1qnxWUBxlg29kh8vqeqgW8n2q2DZAY= =ei15 -----END PGP SIGNATURE----- --D4O7rKOkeJ+Rx7Jv--