[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20141121202933.2970b7ea@griffin>
Date: Fri, 21 Nov 2014 20:29:33 +0100
From: Jiri Benc <jbenc@...hat.com>
To: Pravin Shelar <pshelar@...ira.com>
Cc: Jiri Pirko <jiri@...nulli.us>, netdev <netdev@...r.kernel.org>,
David Miller <davem@...emloft.net>,
Jamal Hadi Salim <jhs@...atatu.com>,
Tom Herbert <therbert@...gle.com>,
Eric Dumazet <edumazet@...gle.com>,
Willem de Bruijn <willemb@...gle.com>,
Daniel Borkmann <dborkman@...hat.com>, mst@...hat.com,
fw@...len.de, Paul.Durrant@...rix.com, Thomas Graf <tgraf@...g.ch>,
Cong Wang <cwang@...pensource.com>
Subject: Re: [patch net-next v4 8/9] net: move vlan pop/push functions into
common code
On Fri, 21 Nov 2014 10:41:27 -0800, Pravin Shelar wrote:
> There is bug in the openvswitch code where vlan-pop does mac-length
> reset rather than subtracting VLAN_HLEN from mac-len. MPLS code
> depends on this. Now this patch moves the bug to common code. I am
> fine with this patch and I can send fix to change code vlan-pop code
> to adjust mac_len later on.
> But I am worried about the comment made by Jiri Benc on earlier
> version of this patch where is mentioned that subtracting VLAN_HLEN
> can break common case for vlan-pop. In that case we can not change
> this common function. Jiri Benc, Can you point me to the code, so the
> we can work on solution to fix this issue.
In such case, a skb that enters the __pop_vlan_tci function looks like
this:
+--------+--------+-------------+-----+---------+-------
| dstmac | srcmac | ETH_P_8021Q | tci | ETH_P_x | data
+--------+--------+-------------+-----+---------+-------
^ ^
mac_header network_header
skb->protocol = ETH_P_8021Q
skb->mac_len = 14
After the function finishes, it's supposed to look like this:
+--------+--------+---------+-------
| dstmac | srcmac | ETH_P_x | data
+--------+--------+---------+-------
^ ^
mac_header network_header
skb->protocol = ETH_P_x
skb->mac_len = 14
Blindly decrementing mac_len wouldn't work here, you'd end up with
mac_len = 10. You'd either need to ensure this case doesn't happen (it
does with openvswitch currently) or detect this case.
Jiri
--
Jiri Benc
--
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
Powered by blists - more mailing lists