On Mon, May 20, 2013 at 2:33 PM, Pravin B Shelar <pshe...@nicira.com> wrote: > Following patch restructures ovs tunneling and gre vport > implementation to make ovs tunneling more in sync with > upstream kernel tunneling. Doing this tunneling code is > simplified as most of protocol processing on send and > recv is pushed to kernel tunneling. For external ovs > module the code is moved to kernel compatibility code. > > Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
A couple more comments (just on the non-compat portions): > diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c > index add17d9..f551976 100644 > --- a/datapath/vport-gre.c > +++ b/datapath/vport-gre.c > -static __be32 be64_get_high32(__be64 x) > +static __be16 ovs_tnl_flags_to_gre_flags(u16 tun_flags) [...] > + __be16 flags = 0; > + if (tun_flags & OVS_TNL_F_CSUM) > + flags |= TUNNEL_CSUM; > + if (tun_flags & OVS_TNL_F_KEY) > + flags |= TUNNEL_KEY; > + return flags; > } Is there any way that we can unify the tunnel flags values to avoid needing to do this conversion? I know the existing flags are based on the GRE values, so maybe we should push the conversion down there. > /* Called with rcu_read_lock and BH disabled. */ > -static int gre_rcv(struct sk_buff *skb) > +static int gre_rcv(struct sk_buff *skb, > + const struct tnl_ptk_info *tpi) [...] > + hdr_len = _parse_header(tpi, &tnl_flags, &key); I would probably start the function name with a double underscore, just to be more consistent. > ovs_net = net_generic(dev_net(skb->dev), ovs_net_id); > - if (is_gre64) > + if (tpi->flags & TUNNEL_SEQ) I think the GRE64 check is not quite the same as before - it previously required both the key and the sequence number to be set, which seems more ideal to me. > vport = rcu_dereference(ovs_net->vport_net.gre64_vport); > else > vport = rcu_dereference(ovs_net->vport_net.gre_vport); If neither of these vports are set, then should we return the packet back to the handler instead of accepting and freeing it? If so, then we might also want to move the check earlier in the function since it doesn't depend on the previous results. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
