On 2019/06/12 6:57, Govindarajulu Varadarajan (gvaradar) wrote:
@On Tue, 2019-06-11 at 13:34 +0900, Toshiaki Makita wrote:
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 <gvara...@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.
Makes sense to me. If rx_handler is registered to the device, pkt should be sent
untagged to rx_handler.
I would like to get some clarification on few more cases before I change the
code. In the setup:
br0
|
vlan 100 |
|(802.1AD) |
| |
+--------------------+
| eth0 |
+--------------------+
Case 1: [802.1P vlan0] [IP]
Current behaviour: pkt is sent to br0 with priority tag. i.e we should not
remove
the 802.1P tag.
This patch: removes the 802.1P tag and br0 receives untagged packet. This is
wrong.
Expected behaviour: Should be same as current behaviour.
Case 2: [802.1AD vlan 100] [IP]
Current behaviour: pkt is sent to vlan 100 device.
This patch: same as current behaviour.
Expected behaviour: same as current behaviour
Case 3: [802.1P vlan 0] [802.1AD vlan 100] [IP]
Current behaviour: Pkt is sent to br0 rx_handler. This happens because
vlan_do_receive() returns false (vlan 0 device is not present). Stack does not
go
through inner headers.
This patch: pkt is sent to vlan 100 device. Because vlan_do_receive() strips
vlan
0 header and finds vlan 100 device.
Expected behaviour: ?
IMO: Pkt should be sent to vlan 100 device because 802.1P should be treated as
priority tag and not as vlan tagged pkt. Since vlan 100 device is present, it
should be sent to vlan 100 device.
Maybe yes, maybe no. There is no standard about that. Your opinion is consistent
behavior between untagged and priority-tagged. OTOH, it changes existing
behavior.
We basically try to keep existing behavior even if the behavior looks odd in
some
way in order not to break existing users. So I would choose the other option,
send
packets to br0.
Case 4: [802.1AD vlan 200] [802.1AD vlan 100] [IP]
Current behaviour: Pkt is sent to br0 since vlan 200 device is not found.
This patch: same as current behaviour.
Expected behaviour: Same as current behaviour.
Is my understanding correct?
Agree except case 3.
Toshiaki Makita
