> -----Original Message-----
> From: Stephen Suryaputra <ssuryae...@gmail.com>
> Sent: Monday, June 10, 2019 4:09 PM
> To: David Miller <da...@davemloft.net>
> Cc: Govindarajulu Varadarajan (gvaradar) <gvara...@cisco.com>; Christian
> Benvenuti (benve) <be...@cisco.com>; netdev@vger.kernel.org;
> govind.vara...@gmail.com
> Subject: Re: [PATCH net] net: handle 802.1P vlan 0 packets properly
>
> On Mon, Jun 10, 2019 at 02:28:10PM -0700, David Miller wrote:
> > From: Govindarajulu Varadarajan <gvara...@cisco.com>
> > Date: Mon, 10 Jun 2019 07:27:02 -0700
> >
> > > 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 <gvara...@cisco.com>
> >
> > Under Linux we absolutely do not decapsulate the VLAN protocol unless
> > a VLAN device is configured on that interface.
>
> VLAN ID 0 is treated as if the VLAN protocol isn't there. It is used so that
> the
> 802.1 priority bits can be encoded and acted upon.
David,
if we assume that the kernel is supposed to deal properly with .1p tagged
frames, regardless
of what the next header is (802.{1Q,1AD} or something else), I think the case
this patch was
trying to address (that is 1Q+1AD) is not handled properly in the case of
priority tagged frames
when the (1Q) vlan is untagged and therefore 1Q is only used to carry 1p.
[1P vid=0][1AD].
Here is a simplified summary of how the kernel is dealing with priority frames
right now, based on
- what the next protocol is
and
- whether a vlan device exists or not for the outer (1Q) header.
PS:
'vid' below refers to the vlan id in the 1Q header (not the 1AD header)
Case 1: vid != 0 , no nested 1Q/1AD - OK
Case 2: vid != 0 , nested 1Q/1AD - OK
Case 3: vid = 0 , no nested 1Q/1AD - OK
Case 4: vid = 0 , nested 1Q/1AD <--- The patch addresses this case
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Case 1: vid != 0 , no nested 1Q/1AD
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Packet looks like this:
[802.1Q vid = V ! = 0] [Next header: Anything but 802.{1Q,1AD}]
Case 1a: Vlan device present (*)
__netif_receive_skb_core()
+-> skb->protocol != ETH_P_802{1Q,1AD} <--- therefore no call to
skb_vlan_untag()
+-> vlan_do_receive()
+-> vlan_dev = vlan_find_dev(...) <--- vlan device found (*)
+-> skb->dev = vlan_dev
+-> Deliver skb to vlan device
Case 1b: Vlan device NOT present (**)
__netif_receive_skb_core()
+-> skb->protocol != ETH_P_802{1Q,1AD} <--- therefore no call to
skb_vlan_untag()
+-> vlan_do_receive()
+-> vlan_dev = vlan_find_dev(...) <--- vlan device NOT found
(**)
+-> if (skb_vlan_tag_present(skb)) <--- TRUE
if (skb_vlan_tag_get_id(skb)) <--- TRUE (***)
PACKET_OTHERHOST
__vlan_hwaccel_clear_tag(skb)
+-> Deliver pkt to next layer protocol
If we take the example of IPv4, ip_rcv_core() will drop the pkt
because of
PACKET_OTHERHOST.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Case 2: vid != 0 , nested 1Q/1AD
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Packet looks like this:
[802.1Q vid = V ! = 0] [Next header: 802.{1Q,1AD}]
Case 2a: Vlan device present
+-> if (skb->protocol == ETH_P_8021AD) <--- TRUE
+-> skb_vlan_untag(skb)
+-> if unlikely(skb_vlan_tag_present(skb)) <--- TRUE
return skb
+-> if (skb_vlan_tag_present(skb)) <--- TRUE (again)
if (vlan_do_receive(&skb)) <--- TRUE
+-> vlan_dev = vlan_find_dev(...) <--- vlan device
found
+-> skb->dev = vlan_dev
goto another_round <-- At the following round
the packet will be delivered to the next layer proto
Case 2b: Vlan device NOT present
__netif_receive_skb_core()
+-> if (skb->protocol == ETH_P_8021AD) <--- TRUE
+-> skb_vlan_untag(skb)
+-> If (unlikely(skb_vlan_tag_present(skb)) <--- TRUE (packet
is .1p tagged, vid=0)
return skb <--- packet is returned as is, 1AD
header still inline
+-> if (skb_vlan_tag_present(skb)) <--- TRUE (again)
if (vlan_do_receive(&skb)) <--- FALSE
+-> vlan_dev = vlan_find_dev(...) <--- vlan device
NOT found
The 2nd part is the same as 1b:
+-> if (skb_vlan_tag_present(skb)) <--- TRUE
if (skb_vlan_tag_get_id(skb)) <--- TRUE (***)
PACKET_OTHERHOST
__vlan_hwaccel_clear_tag(skb)
+-> Deliver pkt to next layer protocol
If we take the example of IPv4, ip_rcv_core() will drop the pkt
because of
PACKET_OTHERHOST.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Case 3: vid = 0 , no nested 1Q/1AD
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Packet looks like this:
[802.1Q vid = 0][Next header: Anything but 802.{1Q,1AD}]
Case 3a: vlan (0) device present
Same as case 1a: packet delivered to vlan device.
Case 3b: vlan (0) device not present
This is similar to case 1b above, with the difference that condition
(***) is false
and therefore pkt type is NOT set to PACKET_OTHERHOST, which translates
to
".1p / vid==0 header ignored and packet NOT accepted".
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Case 4: vid = 0 , nested 1Q/1AD
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Packet looks like this:
[802.1Q vid == 0][Next header: 802.{1Q,1AD}]
Case 4a: vlan (0) device present
This is equivalent to the cases 1a/2a/3a above: the packet is going to
be delivered to the vlan (0) device.
Case 4b: vlan (0) device not present.
THIS IS THE CASE THE PATCH TRIED TO ADDRESS.
__netif_receive_skb_core()
+-> if (skb->protocol == ETH_P_8021AD) <--- TRUE
+-> skb_vlan_untag(skb)
+-> If (unlikely(skb_vlan_tag_present(skb)) <--- TRUE (packet
is .1p tagged, vid=0)
return skb <--- packet is returned as is, 1AD
header still inline
+-> Since there is no 1AD proto handler, packet will be dropped (but it
should not)
Thanks
/Chris
