netdev - Re: [PATCH net-next v2] vxlan: fix handling of the inner 8021Q tagged frame

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: <8738huu4ul.wl%atzm@stratosphere.co.jp>
Date:	Thu, 03 Apr 2014 18:42:10 +0900
From:	Atzm Watanabe <atzm@...atosphere.co.jp>
To:	Toshiaki Makita <toshiaki.makita1@...il.com>
Cc:	Toshiaki Makita <makita.toshiaki@....ntt.co.jp>,
	netdev@...r.kernel.org,
	Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH net-next v2] vxlan: fix handling of the inner 8021Q tagged frame

At Thu, 03 Apr 2014 01:16:36 +0900,
Toshiaki Makita wrote:
> > At Wed, 02 Apr 2014 17:30:55 +0900,
> > Toshiaki Makita wrote:
> > > 
> > > (2014/04/01 23:27), Atzm Watanabe wrote:
> > > > Currently the implementation can forward the 8021Q tagged frame,
> > > > but the FDB cannot learn the VID.
> > > > So there is a possibility of forwarding the frame to wrong VTEP,
> > > > when same LLADDR exists on different VLANs.
> > > > 
> > > > This patch supports only single tagged frame, so the outermost
> > > > tag will be used when handling the 8021AD Q-in-Q frame.
> > > > 
> > > > v2: Fix probably unsafe operation on the struct vxlan_key.
> > > >     The outermost tag will be used when handling the 8021AD
> > > >     Q-in-Q frame.  Based on Stephen Hemminger's comments.
> > > > 
> > > > Signed-off-by: Atzm Watanabe <atzm@...atosphere.co.jp>
> > > ...
> > > > @@ -1215,8 +1257,18 @@ static void vxlan_rcv(struct vxlan_sock *vs,
> > > >  #endif
> > > >  	}
> > > >  
> > > > +	ether_addr_copy(key.eth_addr, eth_hdr(skb)->h_source);
> > > > +	switch (ntohs(eth_hdr(skb)->h_proto)) {
> > > > +	case ETH_P_8021Q:
> > > > +	case ETH_P_8021AD:
> > > > +		key.vlan_id = ntohs(vlan_eth_hdr(skb)->h_vlan_TCI) & VLAN_VID_MASK;
> > > > +		break;
> > > 
> > > It seems that we can't segregate skbs tagged by same vlan id but
> > > different vlan protocols.
> > 
> > Yes, but I believe it is better than all vlan protocols are ignored.
> > 
> > Of course it still has problems when multiple protocols (0x8100,
> > 0x88a8, 0x9100, ...) are mixed in a network, but I want to fix
> > a problem in case of single tagged, at the beginning.
> 
> What I'm worried about is the use of the native vlan.
> For example, if we are using C-vlan/S-vlan combination and use native
> vlan for a certain S-vlan id.
> In this case, we may see both double C/S-tagged frames and single
> C-tagged frames, and we may treat C-vid as S-vid for single tagged
> case...
> This causes incorrect delivery of frames.

Thank you for telling me details.
Yes, indeed.  This case means that single tagged frames and double
tagged frames are mixing in a network, just for the vtep.


> Maybe we can explicitly set the vlan protocol to be focused on by user
> space, but this is just a suggestion.

Thank you for the suggestion.
Hmm...  for example, if userspace set the protocol to 8021q and vxlan
receives 8021ad or untagged frame, how vxlan should handle them?
Perhaps the vid should be treated as 0, or perhaps the frame should be
dropped.
Also I'm a bit worried this ABI perhaps may become a fetter for the
compatibility when we want to support all of stacked vlan protocols in
the future...
What do you think?


> If you are starting by this implementation, it's maybe better to note
> this equating two protocols and the possible problem (in changelog or
> somewhere).

Ok, agreed.


Thank you!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html