On Sun, Jul 17, 2016 at 9:50 PM, Simon Horman <simon.hor...@netronome.com> wrote: > [CC Jiri Benc for portion regarding GRE] > > Hi Pravin, > > On Fri, Jul 15, 2016 at 02:07:37PM -0700, pravin shelar wrote: >> On Wed, Jul 13, 2016 at 12:31 AM, Simon Horman >> <simon.hor...@netronome.com> wrote: >> > Hi Pravin, >> > >> > On Thu, Jul 07, 2016 at 01:54:15PM -0700, pravin shelar wrote: >> >> On Wed, Jul 6, 2016 at 10:59 AM, Simon Horman >> >> <simon.hor...@netronome.com> wrote: >> > >> > ... >> >> > >> >> > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c >> >> > index 0ea128eeeab2..86f2cfb19de3 100644 >> >> > --- a/net/openvswitch/flow.c >> >> > +++ b/net/openvswitch/flow.c >> >> ... >> >> >> >> > @@ -723,9 +729,17 @@ int ovs_flow_key_extract(const struct >> >> > ip_tunnel_info *tun_info, >> >> > key->phy.skb_mark = skb->mark; >> >> > ovs_ct_fill_key(skb, key); >> >> > key->ovs_flow_hash = 0; >> >> > + key->phy.is_layer3 = skb->mac_len == 0; >> >> >> >> I do not think mac_len can be used. mac_header needs to be checked. >> >> ... >> > >> > Yes, indeed. The update to use skb_mac_header_was_set() here accidently >> > slipped into the following patch, sorry about that. >> > >> > With that change in place I believe that this patch is internally >> > consistent because mac_header and mac_len are set correctly by the >> > call to key_extract() which is called by ovs_flow_key_extract() just >> > after where the excerpt above ends. >> > >> > That said, I do think that it is possible to rely on skb_mac_header_was_set >> > throughout the datapath, including action processing etc... I have provided >> > an incremental patch - which I created on top of this entire series - at >> > the end of this email. If you prefer that approach I am happy to take it, >> > though I do feel that using mac_len leads to slightly cleaner code. Let me >> > know what you think. >> > >> >> >> I am not sure if you can use only mac_len to detect L3 packet. This >> does not work with MPLS packets, mac_len is used to account MPLS >> headers pushed on skb. Therefore in case of a MPLS header on L3 >> packet, mac_len would be non zero and we have to look at either >> mac_header or some other metadata like is_layer3 flag from key to >> check for L3 packet. > > At least within OvS mac_len does not include the length of the MPLS label > stack. Rather, the MPLS label stack length is the difference between the > end of (mac_header + mac_len) and network_header. > > So I think that the scheme does work as mac_len is 0 if there is no L2 > header regardless of if an MPLS label stack is present or not. >
I was thinking in overall networking stack rather than just ovs datapath. I think we should have consistent method of detecting L3 packet. As commented in previous mail it could be achieved using skb-protocol and device type. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
