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 5B8DCC79FBB for ; Thu, 10 Sep 2026 23:46:08 +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:MIME-Version: Content-Transfer-Encoding:Content-Type:References:In-Reply-To:Message-ID:Date :Cc:To:From:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=hIXlAmZhvd51LswZ7HvfeI0d4Z0o2ZTt1SJi9IxuaTo=; b=yJVS1DIS/rz0os+WAHFiYa2otS X58Xn1uUAhqGkMVC2+/l5fUUkT4wV+U3lUpL9bM41HYg6ct+jvz9qteatzf+p/dRBltjkD5vqh6v2 Ry5O6lXomZP8krMzBTvXhtGmpo7lRVkIzk+S4cedVjPp0LjPVmfV7aZVE2lp/ndMgvJ6RoKrBFYbc lvNcTQgs+XMo1s3Vn85gLINphdqpEUkt1qS7uxnhcgdDO6Y9LqY+n6Q5VBr4QfouhNI9WZJtWzFzG ouBvG6YFManXvcXk6iM1x8ZAJM10Jq8zYpjYKcoJ2pZbGzEZ7xEsW3HxpTr3W4fmHUvYdYL3GCImE btaZ8+xg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1x4oSv-0000000FZHq-2YH6; Thu, 10 Sep 2026 23:46:01 +0000 Received: from sea.source.kernel.org ([2600:3c0a:e001:78e:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1x4oSt-0000000FZHJ-3Nwc for linux-arm-kernel@lists.infradead.org; Thu, 10 Sep 2026 23:45:59 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by sea.source.kernel.org (Postfix) with ESMTP id 0D3F74017E; Thu, 10 Sep 2026 23:45:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E4C581F000FF; Thu, 10 Sep 2026 23:45:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1789083958; bh=hIXlAmZhvd51LswZ7HvfeI0d4Z0o2ZTt1SJi9IxuaTo=; h=Subject:From:To:Cc:Date:In-Reply-To:References; b=O4XxoVV8d7plkRZt1vihRfv/5KDKkZrqzVEZcDLggkRlggcTw7AJBl9IkZvNzS0P2 0Eu4mUqlfAscLhvMbOY55IVRVaLqSssZt+3xQxJNFM9sLBHN0V/raW8ODWDdrUjXd2 m7aXcfRTshjM6tNHGd7+wOGiREsWUJaXkrrPQ8r2zH5706Q4OmD61jM39vF6q9ezDV 2+jLl2Xa5U0BMxxHRxgjT2eyMZxJ2MLwgQ2dFzDeoFYlwRuH4q5YK4xceCIXC7aXnt qzyT2lGmPvJJPfvliuCCV55qCxyXwlkPKEE2iFCfsiZ0b0lnYqZkwbIUoNNlnuG3XQ k2OdacV15jCuw== Subject: Re: [PATCH net v4 1/7] net: stmmac: Remove VLAN perfect matching dead code From: netdev-bot+sashiko@kernel.org To: ovidiu.panait.rb@renesas.com Cc: maxime.chevallier@bootlin.com, recv.jo@gmail.com, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, shuah@kernel.org, joabreu@synopsys.com, yi.fang.gan@intel.com, jun.ann.lai@intel.com, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kselftest@vger.kernel.org Date: Thu, 10 Sep 2026 23:45:57 +0000 Message-ID: <178908395746.219967.12509019591003417803@kernel.org> In-Reply-To: <20260908164309.59282-2-ovidiu.panait.rb@renesas.com> References: <20260908164309.59282-2-ovidiu.panait.rb@renesas.com> X-sashiko-severity: Medium Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit MIME-Version: 1.0 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 Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 1 · Low: 1 - [Medium] Incomplete removal: the two in-tree self-tests whose only purpose was to exercise the deleted VLAN "perfect matching" fallback… - [Low] The commit carries `Fixes: c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if HASH is not available")` while its own… -- net: stmmac: Remove VLAN perfect matching dead code This removes the "perfect matching" fallback that stmmac_vlan_update() used when priv->dma_cap.vlhash was clear, along with the perfect_match argument of the ->update_vlan_hash callback and the corresponding branches in vlan_update_hash() and dwxgmac2_update_vlan_hash(). No Link: tags were present in the commit header. > stmmac_vlan_update() falls back to "perfect matching" when the VLAN hash > filter is unavailable (!priv->dma_cap.vlhash). This fallback has been > unreachable in normal operation since its introduction in > commit c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if > HASH is not available") because the NETIF_F_HW_VLAN_{CTAG,STAG}_FILTER > features are advertised only when priv->dma_cap.vlhash is true. > > Fixes: c7ab0b8088d7 ("net: stmmac: Fallback to VLAN Perfect filtering if HASH is not available") [Severity: Low] Is the Fixes: tag intended here, given the message states the removed code "has been unreachable in normal operation since its introduction"? The unreachability claim does check out: NETIF_F_HW_VLAN_CTAG_FILTER and NETIF_F_HW_VLAN_STAG_FILTER are only set when priv->dma_cap.vlhash is true in stmmac_hw_init(): if (priv->dma_cap.vlhash) { ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER; ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER; } and the VLAN core only calls ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid for devices advertising hardware filtering (vlan_hw_filter_capable() in net/8021q/vlan_core.c). So no user-visible misbehaviour appears to be corrected, while the patch does change the ->update_vlan_hash signature in hwif.h, stmmac_main.c and stmmac_vlan.c. Would it be better to drop the Fixes: tag and target net-next, or alternatively describe a concretely reachable failure in the message? > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 24656b35350b..24d64cce1d87 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -6816,29 +6816,18 @@ static u32 stmmac_vid_crc32_le(__le16 vid_le) > static int stmmac_vlan_update(struct stmmac_priv *priv, bool is_double) > { > u32 crc, hash = 0; > - u16 pmatch = 0; > - int count = 0; > u16 vid = 0; > > for_each_set_bit(vid, priv->active_vlans, VLAN_N_VID) { > __le16 vid_le = cpu_to_le16(vid); > crc = bitrev32(~stmmac_vid_crc32_le(vid_le)) >> 28; > hash |= (1 << crc); > - count++; > - } > - > - if (!priv->dma_cap.vlhash) { > - if (count > 2) /* VID = 0 always passes filter */ > - return -EOPNOTSUPP; > - > - pmatch = vid; > - hash = 0; > } [Severity: Medium] Should the two self-tests that exist only to exercise this fallback be removed in the same patch? After this hunk, stmmac_vlan_update() no longer reads priv->dma_cap.vlhash, but stmmac_selftests.c still overrides the capability around the ordinary test bodies: stmmac_test_vlanfilt_perfect(): int ret, prev_cap = priv->dma_cap.vlhash; if (!(priv->dev->features & NETIF_F_HW_VLAN_CTAG_FILTER)) return -EOPNOTSUPP; priv->dma_cap.vlhash = 0; ret = __stmmac_test_vlanfilt(priv); priv->dma_cap.vlhash = prev_cap; stmmac_test_dvlanfilt_perfect() does the same around __stmmac_test_dvlanfilt(). Since nothing under test inspects dma_cap.vlhash any more, do these two wrappers now behave exactly like stmmac_test_vlanfilt() and stmmac_test_dvlanfilt()? That would make the "VLAN Filtering (perf)" and "Double VLAN Filter (perf)" entries in stmmac_selftests[] report PASS from ethtool -t for a filtering mode the driver no longer implements, and they could never fail for the behaviour they name. Verified that both wrappers are still unchanged at the end of the series (bb70e68e6ebc), so this does not look like a later cleanup either. > @@ -184,20 +184,6 @@ static void vlan_update_hash(struct mac_device_info *hw, u32 hash, > } > > writel(value, ioaddr + VLAN_TAG); > - } else if (perfect_match) { > - u32 value = VLAN_ETV; > - > - if (is_double) { > - value |= VLAN_EDVLP; > - value |= VLAN_ESVL; > - value |= VLAN_DOVLTC; > - } else { > - value &= ~VLAN_EDVLP; > - value &= ~VLAN_ESVL; > - value &= ~VLAN_DOVLTC; > - } > - > - writel(value | perfect_match, ioaddr + VLAN_TAG); > } else { > value &= ~(VLAN_VTHM | VLAN_ETV); > value &= ~(VLAN_EDVLP | VLAN_ESVL); With only the hash and no-hash branches left in vlan_update_hash() and in dwxgmac2_update_vlan_hash(), a nonzero hash always enables hash matching, so the "perf" self-tests end up programming the plain hash filter. Would dropping those two test functions and their stmmac_selftests[] entries (or re-pointing them at the vlan_add_hw_rx_fltr() path) be the right follow-up inside this patch? [ ... remaining hunks snipped ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908164309.59282-1-ovidiu.panait.rb%40renesas.com