netdev - Re: [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port support

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]

Date:	Wed, 18 May 2016 11:18:16 +0900
From:	Simon Horman <simon.horman@...ronome.com>
To:	Jiri Benc <jbenc@...hat.com>
Cc:	pravin shelar <pshelar@....org>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	ovs dev <dev@...nvswitch.org>,
	Lorand Jakab <lojakab@...co.com>,
	Thomas Morin <thomas.morin@...nge.com>
Subject: Re: [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port
 support

On Tue, May 17, 2016 at 04:43:20PM +0200, Jiri Benc wrote:
> On Thu, 12 May 2016 07:46:52 +0900, Simon Horman wrote:
> > If we can live with a bogus skb->mac_len value that is sufficient for
> > ovs_flow_key_extract.() and set correctly by key_extract() (which happens
> > anyway) we could do something like this:
> > 
> > 	} else if (vport->dev->type == ARPHRD_NONE) {
> > 		if (skb->protocol == htons(ETH_P_TEB))
> > 			/* Ignores presence of VLAN but is sufficient for
> > 			 * ovs_flow_key_extract() which then calls key_extract()
> > 			 * which calculates skb->mac_len correctly. */
> > 			skb->mac_len = ETH_HLEN; /* Ignores presence of VLAN */
> > 		else
> > 			skb->mac_len = 0;
> > 	}
> > 
> > 
> > But perhaps I have missed the point somehow.
> 
> You did not, I was more thinking aloud. But you're right, it doesn't
> make much sense.
> 
> So, wouldn't it be actually more correct and in line with patch 2 to
> call eth_type_trans() here?

Yes, that seems reasonable.

> Possibly even followed by skb_vlan_untag
> (not needed. But it might make things easier to have the first vlan tag
> always reside inside skb->vlan_tci and treat that as an invariant in
> the whole ovs kernel code. It'll need to be done in more spots than
> just this one, though, and is probably matter of a separate patchset).

That also seems reasonable. But as it seems more invasive and is not
strictly necessary I'd rather handle that update separately.