• Vladimir Oltean's avatar
    net: ethernet: mtk_eth_soc: fix DSA TX tag hwaccel for switch port 0 · 1a3245fe
    Vladimir Oltean authored
    Arınç reports that on his MT7621AT Unielec U7621-06 board and MT7623NI
    Bananapi BPI-R2, packets received by the CPU over mt7530 switch port 0
    (of which this driver acts as the DSA master) are not processed
    correctly by software. More precisely, they arrive without a DSA tag
    (in packet or in the hwaccel area - skb_metadata_dst()), so DSA cannot
    demux them towards the switch's interface for port 0. Traffic from other
    ports receives a skb_metadata_dst() with the correct port and is demuxed
    properly.
    
    Looking at mtk_poll_rx(), it becomes apparent that this driver uses the
    skb vlan hwaccel area:
    
    	union {
    		u32		vlan_all;
    		struct {
    			__be16	vlan_proto;
    			__u16	vlan_tci;
    		};
    	};
    
    as a temporary storage for the VLAN hwaccel tag, or the DSA hwaccel tag.
    If this is a DSA master it's a DSA hwaccel tag, and finally clears up
    the skb VLAN hwaccel header.
    
    I'm guessing that the problem is the (mis)use of API.
    skb_vlan_tag_present() looks like this:
    
     #define skb_vlan_tag_present(__skb)	(!!(__skb)->vlan_all)
    
    So if both vlan_proto and vlan_tci are zeroes, skb_vlan_tag_present()
    returns precisely false. I don't know for sure what is the format of the
    DSA hwaccel tag, but I surely know that lowermost 3 bits of vlan_proto
    are 0 when receiving from port 0:
    
    	unsigned int port = vlan_proto & GENMASK(2, 0);
    
    If the RX descriptor has no other bits set to non-zero values in
    RX_DMA_VTAG, then the call to __vlan_hwaccel_put_tag() will not, in
    fact, make the subsequent skb_vlan_tag_present() return true, because
    it's implemented like this:
    
    static inline void __vlan_hwaccel_put_tag(struct sk_buff *skb,
    					  __be16 vlan_proto, u16 vlan_tci)
    {
    	skb->vlan_proto = vlan_proto;
    	skb->vlan_tci = vlan_tci;
    }
    
    What we need to do to fix this problem (assuming this is the problem) is
    to stop using skb->vlan_all as temporary storage for driver affairs, and
    just create some local variables that serve the same purpose, but
    hopefully better. Instead of calling skb_vlan_tag_present(), let's look
    at a boolean has_hwaccel_tag which we set to true when the RX DMA
    descriptors have something. Disambiguate based on netdev_uses_dsa()
    whether this is a VLAN or DSA hwaccel tag, and only call
    __vlan_hwaccel_put_tag() if we're certain it's a VLAN tag.
    
    Arınç confirms that the treatment works, so this validates the
    assumption.
    
    Link: https://lore.kernel.org/netdev/704f3a72-fc9e-714a-db54-272e17612637@arinc9.com/
    Fixes: 2d7605a7
    
     ("net: ethernet: mtk_eth_soc: enable hardware DSA untagging")
    Reported-by: default avatarArınç ÜNAL <arinc.unal@arinc9.com>
    Tested-by: default avatarArınç ÜNAL <arinc.unal@arinc9.com>
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Reviewed-by: default avatarFelix Fietkau <nbd@nbd.name>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    1a3245fe
mtk_eth_soc.c 115 KB