RE: [PATCH net v6 2/5] net: stmmac: Stop toggling the EDVLP bit

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 * High: 0 (1) * Medium: 1 * Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The COE benefit that the commit message gives as its reason
  for this change never shows up in this tree.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is still
  there at the end of the series for XGMAC 2.10.

[...]

[Severity: High]
This problem was not introduced by this patch. It is still present on
XGMAC 2.10 cores at the end of the series, though.

Registering an 802.1ad VID follows this call chain, with is_double set:

stmmac_vlan_rx_add_vid()->stmmac_vlan_update()-

quoted

dwxgmac2_update_vlan_hash()

That sets ESVL and DOVLTC. With hw_vlan_en, which is the default on XGMAC,
vlan_set_hw_mode() also programs EVLS=STRIP_ALL and EVLRXS. The MAC then
strips 0x88A8 outer tags and writes the TCI into RDES0.

dwxgmac2_wrback_get_rx_vlan_valid() accepts the S-tag packet types:

	return et_lt >= XGMAC_ET_LT_VLAN_STAG &&
	       et_lt <= XGMAC_ET_LT_DVLAN_STAG_CTAG;

However, vlan_rx_hw() always reports the stripped tag as a C-tag:

drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c:vlan_rx_hw() {
	if (hw->desc->get_rx_vlan_valid(rx_desc)) {
		u16 vid = hw->desc->get_rx_vlan_tci(rx_desc);

		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
	}
}

Won't S-tagged frames then go to an 802.1Q upper device with the same VID,
or be dropped, and never reach the 802.1ad upper device? The software path
in stmmac_rx_vlan() does tell ETH_P_8021AD apart, so the two paths
disagree.

The last patch in the series, "net: stmmac: Disable S-Tag processing on
dwmac4", stops advertising STAG_RX/STAG_FILTER only on dwmac4. Its commit
message says the XGMAC tag type can be derived from ET_LT. Yet
vlan_rx_hw()
is unchanged, and XGMAC still advertises the S-tag features in
__stmmac_dvr_probe():

	if (priv->plat->core_type == DWMAC_CORE_XGMAC)
		ndev->features |= NETIF_F_HW_VLAN_STAG_RX;

On XGMAC, should vlan_rx_hw() use ET_LT to report ETH_P_8021AD, or should
XGMAC also stop advertising the S-tag features?

I tried to address this in the initial version, but since I do not have
access to a board using XGMAC, the fix was only compile tested:
https://lore.kernel.org/all/20260729095110.164157-6-ovidiu.panait.rb@renesas.com/ (local)

However, Nazim tested it and noticed that it doesn't fix the issue on
on dwxgmac2, so I dropped the patch from the series.

I think someone with access to a dwxgmac2 platform should be continuing the
investigation.

Note that the other "High" finding from Sashiko is referring to the same issue. 

Thanks,
Ovidiu

S-tag recognition appears to be controlled by ESVL/DOVLTC, not EDVLP.
Setting EDVLP unconditionally here does not seem to make this worse.

[ ... ]

--
Sashiko AI review * https://netdev-
ai.bots.linux.dev/sashiko/#/patchset/20260928203441.34876-1-
ovidiu.panait.rb%40renesas.com