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: <87y4zhsdzi.wl%atzm@stratosphere.co.jp>
Date:	Mon, 07 Apr 2014 18:09:05 +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 Sat, 05 Apr 2014 00:25:50 +0900,
Toshiaki Makita wrote:
> 
> On Thu, 2014-04-03 at 18:42 +0900, Atzm Watanabe wrote:
> > 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.
> 
> To resolve the problem, we have to take the same policy as surrounding
> switches. We seem to need another ability to set native vlan for
> untagged frames.
> 
> > 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?
> 
> I think we can add another feature to specify inner vlan protocol.

IMHO, still we cannot avoid incorrect delivery even if vxlan only
supports single outermost tag with user specified protocol because it
must learn both S-VID and C-VID (and MAC) to support 8021ad, otherwise
there is a possibility of this problem...  For example, when a
service (a S-tagged) uses same MAC on different VLANs (C-tagged), the
problem will be caused if vxlan forwards the frame only based on S-VID
and MAC.

Ok, so, I'll try to fix this problem by supporting double tagged VLAN.
(but this may take a while, sorry.
 if another fine patch was posted, please give priority to it.)

Thanks a lot!
--
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