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:	Tue, 17 May 2016 16:43:20 +0200
From:	Jiri Benc <jbenc@...hat.com>
To:	Simon Horman <simon.horman@...ronome.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 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? 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).

 Jiri