Re: [PATCH RESEND net] net: handle 802.1P vlan 0 packets properly

[Toshiaki Makita <toshiaki.makita1@gmail.com>]

On 2019/06/11 3:31, Govindarajulu Varadarajan wrote:
> When stack receives pkt: [802.1P vlan 0][802.1AD vlan 100][IPv4],
> vlan_do_receive() returns false if it does not find vlan_dev. Later
> __netif_receive_skb_core() fails to find packet type handler for
> skb->protocol 801.1AD and drops the packet.
> 
> 801.1P header with vlan id 0 should be handled as untagged packets.
> This patch fixes it by checking if vlan_id is 0 and processes next vlan
> header.
> 
> Signed-off-by: Govindarajulu Varadarajan <gvaradar@cisco.com>
> ---
>   net/8021q/vlan_core.c | 24 +++++++++++++++++++++---
>   1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index a313165e7a67..0cde54c02c3f 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -9,14 +9,32 @@
>   bool vlan_do_receive(struct sk_buff **skbp)
>   {
>   	struct sk_buff *skb = *skbp;
> -	__be16 vlan_proto = skb->vlan_proto;
> -	u16 vlan_id = skb_vlan_tag_get_id(skb);
> +	__be16 vlan_proto;
> +	u16 vlan_id;
>   	struct net_device *vlan_dev;
>   	struct vlan_pcpu_stats *rx_stats;
>   
> +again:
> +	vlan_proto = skb->vlan_proto;
> +	vlan_id = skb_vlan_tag_get_id(skb);
>   	vlan_dev = vlan_find_dev(skb->dev, vlan_proto, vlan_id);
> -	if (!vlan_dev)
> +	if (!vlan_dev) {
> +		/* Incase of 802.1P header with vlan id 0, continue if
> +		 * vlan_dev is not found.
> +		 */
> +		if (unlikely(!vlan_id)) {
> +			__vlan_hwaccel_clear_tag(skb);

Looks like this changes existing behavior. Priority-tagged packets will be untagged
before bridge, etc. I think priority-tagged packets should be forwarded as priority-tagged
(iff bridge is not vlan-aware), not untagged.

> +			if (skb->protocol == cpu_to_be16(ETH_P_8021Q) ||
> +			    skb->protocol == cpu_to_be16(ETH_P_8021AD)) {
> +				skb = skb_vlan_untag(skb);
> +				*skbp = skb;
> +				if (unlikely(!skb))
> +					return false;
> +				goto again;
> +			}
> +		}
>   		return false;
> +	}
>   
>   	skb = *skbp = skb_share_check(skb, GFP_ATOMIC);
>   	if (unlikely(!skb))
> 

Toshiaki Makita