From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C93101E1024; Sat, 31 Jan 2026 20:41:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769892082; cv=none; b=SGSWWqE8tnalFYWa1iRNqPzopx/Sjh3cT85GLhVgZew2g8KN4u8zEAnF6OBufndvgbyozyLslPm7wLkRr7RneIhuS1Vuch1oK8ANltSkHDBzvKPfeTDcgKwWo50/7mVZ7UK6QIpWPFIoXjJBAlApBPgGke9MU9o05gvN/DmMjyQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769892082; c=relaxed/simple; bh=0L9KHIRomV4Jf/gGMn8eAxBc1vQow6EnmR9i9GB45X8=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=smoXNIgiq0UeF+8z2Jx/2IXhTnVLgyzu4E1PVDvod6ulUwgYEAXA/ElPFH3Q3g/x39kYO0i3I9vtwXXNT4ac98BhKD6e9+ViEpegfNqiow45OvBEeU6hijV//MqSMTxnlyEv/Vi4zyjvLDXDqrnZ33Q6U0xTU40/rfFT2BAUcxg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=s9wfntQV; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="s9wfntQV" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BC0D0C4CEF1; Sat, 31 Jan 2026 20:41:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769892082; bh=0L9KHIRomV4Jf/gGMn8eAxBc1vQow6EnmR9i9GB45X8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=s9wfntQV4G/S6o0H1SS0WjOXHPLfqyfakQr52mFSxxf9YnVifL+OnZlm+RBTK/Lo3 tTef/j3HOyWcyLM2/32Rcj7B/BsaeKwRcVid1BJ8E/xVwXbWcOJGoJQxtLc7JMq1Nb S++LeiHWawD8rI9dv383XRRVJySmDjzZdic2qF5xGAjMQIQINcqgIYt40F7LHphoI+ nP+czU+BscUZITDR/KpIDCycZNDeIWewB9wQxA2Kp+LwryBfdap8SAxHBkrzrixl+b NyJnQBbna9/YeEwYiz6SIgK1tXJteoDseT92oU7fb57uQcroVjRL55kIqMFIdyKxM6 NJSSbtOYJ2GGg== Date: Sat, 31 Jan 2026 12:41:20 -0800 From: Jakub Kicinski To: Kevin Hao Cc: netdev@vger.kernel.org, stable@vger.kernel.org, Siddharth Vadapalli , Roger Quadros , Andrew Lunn , "David S. Miller" , Eric Dumazet , Paolo Abeni , Vladimir Oltean , Kuniyuki Iwashima , linux-omap@vger.kernel.org Subject: Re: [PATCH net v4] net: cpsw_new: Execute ndo_set_rx_mode callback in a work queue Message-ID: <20260131124120.744bd931@kernel.org> In-Reply-To: <20260130-bbb-v4-1-2bd000a15c34@gmail.com> References: <20260130-bbb-v4-1-2bd000a15c34@gmail.com> Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Fri, 30 Jan 2026 13:34:07 +0800 Kevin Hao wrote: > --- a/drivers/net/ethernet/ti/cpsw_new.c > +++ b/drivers/net/ethernet/ti/cpsw_new.c What's your plan for fixing drivers/net/ethernet/ti/cpsw.c ? My preference would be to post both of the fixes at once, I think this version is quite close, just a couple of nit picks below.. > @@ -248,15 +248,25 @@ static int cpsw_purge_all_mc(struct net_device *ndev, const u8 *addr, int num) > return 0; > } > > -static void cpsw_ndo_set_rx_mode(struct net_device *ndev) > +static void cpsw_ndo_set_rx_mode_work(struct work_struct *work) > { > - struct cpsw_priv *priv = netdev_priv(ndev); > + struct cpsw_priv *priv = container_of(work, struct cpsw_priv, rx_mode_work); > struct cpsw_common *cpsw = priv->cpsw; > + struct net_device *ndev = priv->ndev; > > + rtnl_lock(); > + if (!netif_running(ndev)) { > + rtnl_unlock(); > + return; since the "undo" logic is getting complex you should use a goto. Replace the unlock and the return; here with: goto unlock_rtnl; > + } > + > + netif_addr_lock_bh(ndev); > if (ndev->flags & IFF_PROMISC) { > /* Enable promiscuous mode */ > cpsw_set_promiscious(ndev, true); > cpsw_ale_set_allmulti(cpsw->ale, IFF_ALLMULTI, priv->emac_port); > + netif_addr_unlock_bh(ndev); > + rtnl_unlock(); goto unlock_addr; > return; > } > > @@ -270,6 +280,15 @@ static void cpsw_ndo_set_rx_mode(struct net_device *ndev) > /* add/remove mcast address either for real netdev or for vlan */ > __hw_addr_ref_sync_dev(&ndev->mc, ndev, cpsw_add_mc_addr, > cpsw_del_mc_addr); And place a labels here: unlock_addr: > + netif_addr_unlock_bh(ndev); unlock_rtnl: > + rtnl_unlock(); > +} > for (i = 0; i < cpsw->data.slaves; i++) { > - if (!cpsw->slaves[i].ndev) > + ndev = cpsw->slaves[i].ndev; > + if (!ndev) > continue; > > - unregister_netdev(cpsw->slaves[i].ndev); > + priv = netdev_priv(ndev); > + disable_work_sync(&priv->rx_mode_work); > + unregister_netdev(ndev); I understand that this is safe but I think that more logical ordering would be to shut things down _after_ object is unregistered. -- pw-bot: cr