netdev - Re: [PATCH net] stmmac: fix reception of 802.1ad Ethernet tagged frames

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f56f7073-46ac-9f9e-f8c0-deaa23ab3aa9@lab.ntt.co.jp>
Date:   Tue, 8 May 2018 16:34:41 +0900
From:   Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To:     Elad Nachman <eladv6@...il.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH net] stmmac: fix reception of 802.1ad Ethernet tagged
 frames

On 2018/05/08 16:11, Elad Nachman wrote:
> Currently running:
> ip link add link eth0 eth0.100 type vlan proto 802.1ad id 100
> 
> On eth0=stmmac succeeds, but the end result is that the vlan device gets proto 802.1q instead of proto 802.1ad and drops the received packet. Without the patch packets gets dropped for a seemingly "correct" 802.1ad ip link configuration.
> 
> If NETIF_F_HW_VLAN_STAG_RX is a requirement for the driver for supporting 802.1ad protocols then the Linux kernel should return error when user-space requests to create a vlan device with proto 802.1ad for physical devices which lacks NETIF_F_HW_VLAN_STAG_RX, which is not currently the case.

No. You can create 802.1ad devices without HW_VLAN_STAG_RX, but you
should not strip 802.1ad tag in driver without HW_VLAN_STAG_RX.
__netif_receive_skb_core should handle them if the device does not have
HW_VLAN_STAG_RX.

> skb_vlan_untag() does nothing if __vlan_hwaccel_put_tag() was already called before (in the driver). The only possible alternative is to completely remove stmmac_rx_vlan() from the stmmac code and let skb_vlan_untag() handles things in a generic way.

You cannot remove an already added feature in the driver.
Alternatively you can skip stripping vlan if vlan_proto is not 8021Q.
Something like this.

	if ((dev->features & NETIF_F_HW_VLAN_CTAG_RX) ==
	    NETIF_F_HW_VLAN_CTAG_RX &&
	    !__vlan_get_tag(skb, &vlanid)) {
		veth = (struct vlan_ethhdr *)skb->data;
		vlan_proto = veth->h_vlan_proto;
		if (vlan_proto == htons(ETH_P_8021Q)) {
			/* pop the vlan tag */
			memmove(skb->data + VLAN_HLEN, veth, ETH_ALEN * 2);
			skb_pull(skb, VLAN_HLEN);
			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlanid);
		}
 	}
 }

-- 
Toshiaki Makita