On Tue, Mar 25, 2014 at 3:36 PM, Pravin Shelar <pshe...@nicira.com> wrote:

> On Fri, Mar 21, 2014 at 10:41 AM, Kyle Mestery
> <mest...@noironetworks.com> wrote:
> > Add support for building the in-tree kernel datapath for Linux kernel
> 3.13.
> > There were some changes in the netlink area which required adding new
> > compatibility code for this layer.
> >
> > Signed-off-by: Kyle Mestery <mest...@noironetworks.com>
> > ---
> >  FAQ                                           |  2 +-
> >  acinclude.m4                                  |  4 +--
> >  datapath/datapath.c                           | 27 +++++++++++-----
> >  datapath/datapath.h                           |  1 +
> >  datapath/dp_notify.c                          | 11 +++----
> >  datapath/linux/compat/genetlink-openvswitch.c | 20 +++++++++---
> >  datapath/linux/compat/include/net/genetlink.h | 45
> +++++++++++++++++++++++++--
> >  datapath/linux/compat/include/net/ip.h        |  4 +++
> >  datapath/linux/compat/utils.c                 |  3 ++
> >  datapath/vport-lisp.c                         |  7 +++--
> >  datapath/vport-vxlan.c                        |  3 +-
> >  11 files changed, 100 insertions(+), 27 deletions(-)
> >
> > diff --git a/FAQ b/FAQ
> > index a54bbf9..040cdf7 100644
> > --- a/FAQ
> > +++ b/FAQ
> > @@ -148,7 +148,7 @@ A: The following table lists the Linux kernel
> versions against which the
> >         1.10.x     2.6.18 to 3.8
> >         1.11.x     2.6.18 to 3.8
> >         2.0.x      2.6.32 to 3.10
> > -       2.1.x      2.6.32 to 3.12
> > +       2.1.x      2.6.32 to 3.13
> >
> >     Open vSwitch userspace should also work with the Linux kernel module
> >     built into Linux 3.3 and later.
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 8f41e33..bd8714c 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [
> >      AC_MSG_RESULT([$kversion])
> >
> >      if test "$version" -ge 3; then
> > -       if test "$version" = 3 && test "$patchlevel" -le 12; then
> > +       if test "$version" = 3 && test "$patchlevel" -le 13; then
> >            : # Linux 3.x
> >         else
> > -         AC_ERROR([Linux kernel in $KBUILD is version $kversion, but
> version newer than 3.12.x is not supported])
> > +         AC_ERROR([Linux kernel in $KBUILD is version $kversion, but
> version newer than 3.13.x is not supported])
> >         fi
> >      else
> >         if test "$version" -le 1 || test "$patchlevel" -le 5 || test
> "$sublevel" -le 31; then
> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> > index f7c3391..b4f09b7 100644
> > --- a/datapath/datapath.c
> > +++ b/datapath/datapath.c
> > @@ -64,11 +64,13 @@
> >
> >  int ovs_net_id __read_mostly;
> >
> > +static struct genl_family dp_packet_genl_family;
> > +
> >  static void ovs_notify(struct sk_buff *skb, struct genl_info *info,
> >                        struct genl_multicast_group *grp)
> >  {
> > -       genl_notify(skb, genl_info_net(info), info->snd_portid,
> > -                   grp->id, info->nlhdr, GFP_KERNEL);
> > +       genl_notify(&dp_packet_genl_family, skb, genl_info_net(info),
> > +                   info->snd_portid, 0, info->nlhdr, GFP_KERNEL);
> >  }
> >
> >  /**
> > @@ -883,8 +885,8 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff
> *skb, struct genl_info *info)
> >         if (!IS_ERR(reply))
> >                 ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
> >         else
> > -               netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
> > -                               ovs_dp_flow_multicast_group.id,
> PTR_ERR(reply));
> > +               genl_set_err(&dp_flow_genl_family, sock_net(skb->sk), 0,
> > +                            0, PTR_ERR(reply));
> >         return 0;
> >
> >  err_flow_free:
> > @@ -1365,8 +1367,8 @@ static int ovs_dp_cmd_set(struct sk_buff *skb,
> struct genl_info *info)
> >         reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
> >         if (IS_ERR(reply)) {
> >                 err = PTR_ERR(reply);
> > -               netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
> > -                               ovs_dp_datapath_multicast_group.id,
> err);
> > +               genl_set_err(&dp_datapath_genl_family,
> sock_net(skb->sk), 0,
> > +                            0, err);
> >                 err = 0;
> >                 goto unlock;
> >         }
> > @@ -1463,7 +1465,7 @@ static const struct nla_policy
> vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
> >         [OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
> >  };
> >
> > -static struct genl_family dp_vport_genl_family = {
> > +struct genl_family dp_vport_genl_family = {
> >         .id = GENL_ID_GENERATE,
> >         .hdrsize = sizeof(struct ovs_header),
> >         .name = OVS_VPORT_FAMILY,
> > @@ -1862,6 +1864,7 @@ static int dp_register_genl(void)
> >         for (i = 0; i < ARRAY_SIZE(dp_genl_families); i++) {
> >                 const struct genl_family_and_ops *f =
> &dp_genl_families[i];
> >
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0)
> >                 err = genl_register_family_with_ops(f->family, f->ops,
> >                                                     f->n_ops);
> >                 if (err)
> > @@ -1873,6 +1876,16 @@ static int dp_register_genl(void)
> >                         if (err)
> >                                 goto error;
> >                 }
> > +#else
> > +               f->family->ops = f->ops;
> > +               f->family->n_ops = f->n_ops;
> > +               f->family->mcgrps = f->group;
> > +               f->family->n_mcgrps = f->group ? 1 : 0;
> This is not generic, We can add number of groups to struct
> genl_family_and_ops.
>
> Will clean this up.


> > +               err = genl_register_family(f->family);
> > +               if (err)
> > +                       goto error;
> > +               n_registered++;
> > +#endif
>
> #ifdef can be avoided by defining compat genl_register_family() for
> older kernel.
>
> Yes, I'll make this generic and compatible.


> >         }
> >
> >         return 0;
> > diff --git a/datapath/datapath.h b/datapath/datapath.h
> > index d81e05c..40e0f90 100644
> > --- a/datapath/datapath.h
> > +++ b/datapath/datapath.h
> > @@ -184,6 +184,7 @@ static inline struct vport *ovs_vport_ovsl(const
> struct datapath *dp, int port_n
> >  }
> >
> >  extern struct notifier_block ovs_dp_device_notifier;
> > +extern struct genl_family dp_vport_genl_family;
> >  extern struct genl_multicast_group ovs_dp_vport_multicast_group;
> >
> >  void ovs_dp_process_received_packet(struct vport *, struct sk_buff *);
> > diff --git a/datapath/dp_notify.c b/datapath/dp_notify.c
> > index 0b22d0c..e96b242 100644
> > --- a/datapath/dp_notify.c
> > +++ b/datapath/dp_notify.c
> > @@ -35,15 +35,14 @@ static void dp_detach_port_notify(struct vport
> *vport)
> >                                           OVS_VPORT_CMD_DEL);
> >         ovs_dp_detach_port(vport);
> >         if (IS_ERR(notify)) {
> > -               netlink_set_err(ovs_dp_get_net(dp)->genl_sock, 0,
> > -                               ovs_dp_vport_multicast_group.id,
> > -                               PTR_ERR(notify));
> > +               genl_set_err(&dp_vport_genl_family, ovs_dp_get_net(dp),
> 0,
> > +                            0, PTR_ERR(notify));
> >                 return;
> >         }
> >
> > -       genlmsg_multicast_netns(ovs_dp_get_net(dp), notify, 0,
> > -                               ovs_dp_vport_multicast_group.id,
> > -                               GFP_KERNEL);
> > +       genlmsg_multicast_netns(&dp_vport_genl_family,
> > +                               ovs_dp_get_net(dp), notify, 0,
> > +                               0, GFP_KERNEL);
> >  }
> >
> >  void ovs_dp_notify_wq(struct work_struct *work)
> > diff --git a/datapath/linux/compat/genetlink-openvswitch.c
> b/datapath/linux/compat/genetlink-openvswitch.c
> > index 359f916..b987aff 100644
> > --- a/datapath/linux/compat/genetlink-openvswitch.c
> > +++ b/datapath/linux/compat/genetlink-openvswitch.c
> > @@ -1,17 +1,27 @@
> >  #include <net/genetlink.h>
> >  #include <linux/version.h>
> >
> > -/* This is analogous to rtnl_notify() but uses genl_sock instead of
> rtnl.
> > - *
> > - * This is not (yet) in any upstream kernel. */
> > -void genl_notify(struct sk_buff *skb, struct net *net, u32 portid, u32
> group,
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0)
> > +void genl_notify(struct genl_family *family,
> > +                struct sk_buff *skb, struct net *net, u32 portid, u32
> group,
> >                  struct nlmsghdr *nlh, gfp_t flags)
> >  {
> >         struct sock *sk = net->genl_sock;
> >         int report = 0;
> > +       struct genl_multicast_group *grp;
> > +       int i = 0;
> >
> >         if (nlh)
> >                 report = nlmsg_report(nlh);
> >
> > -       nlmsg_notify(sk, skb, portid, group, report, flags);
> > +       list_for_each_entry(grp, &family->mcast_groups, list) {
> > +               if (group == 0)
> > +                       break;
> > +               i++;
> > +       }
> > +
> > +       if (WARN_ON_ONCE(grp == NULL))
> > +               return;
> > +       nlmsg_notify(sk, skb, portid, grp->id, report, flags);
> >  }
> > +#endif /* kernel version < 3.13.0 */
> This is available from 3.3, I think we missed it. We can just start
> using it from 3.3 kernel.
>
> OK.


> > diff --git a/datapath/linux/compat/include/net/genetlink.h
> b/datapath/linux/compat/include/net/genetlink.h
> > index 09ee23b..6f09f84 100644
> > --- a/datapath/linux/compat/include/net/genetlink.h
> > +++ b/datapath/linux/compat/include/net/genetlink.h
> > @@ -17,8 +17,49 @@
> >  #define portid pid
> >  #endif
> >
> > -extern void genl_notify(struct sk_buff *skb, struct net *net, u32
> portid,
> > -                       u32 group, struct nlmsghdr *nlh, gfp_t flags);
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0)
> > +#define genl_notify rpl__genl_notify
> > +void genl_notify(struct genl_family *family,
> > +                struct sk_buff *skb, struct net *net, u32 portid, u32
> group,
> > +                struct nlmsghdr *nlh, gfp_t flags);
> > +
> > +#define genl_set_err rpl__genl_set_err
> > +static inline int genl_set_err(struct genl_family *family, struct net
> *net,
> > +                              u32 portid, u32 group, int code)
> > +{
> > +       struct genl_multicast_group *grp;
> > +       int i = 0;
> > +
> > +       list_for_each_entry(grp, &family->mcast_groups, list) {
> > +               if (group == 0)
> > +                       break;
> > +               i++;
> > +       }
> > +
> All compat function added works only if group is zero. Why are you
> adding this restriction ?
>
> It wasn't clear to me if older kernels allowed registering more than a
single group or not, I'll revisit this and your comment below as well.

Thanks,
Kyle


> > +       if (WARN_ON_ONCE(grp == NULL))
> > +               return -EINVAL;
> > +       return netlink_set_err(net->genl_sock, portid, grp->id, code);
> > +}
> we can just call netlink_set_err().
> > +
> > +#define genlmsg_multicast_netns rpl__genlmsg_multicast_netns
> > +static inline int genlmsg_multicast_netns(struct genl_family *family,
> > +                                         struct net *net, struct
> sk_buff *skb,
> > +                                         u32 portid, unsigned int
> group, gfp_t flags)
> > +{
> > +       struct genl_multicast_group *grp;
> > +       int i = 0;
> > +
> > +       list_for_each_entry(grp, &family->mcast_groups, list) {
> > +               if (group == 0)
> > +                       break;
> > +               i++;
> > +       }
> > +
> > +       if (WARN_ON_ONCE(grp == NULL))
> > +               return -EINVAL;
> > +       return nlmsg_multicast(net->genl_sock, skb, portid, grp->id,
> flags);
> > +}
> same as above, I am not sure what is purpose of the loop over family
> mc-groups.
>
> > +#endif
> >
> >  #if LINUX_VERSION_CODE < KERNEL_VERSION(3,14,0)
> >  static inline struct sk_buff *genlmsg_new_unicast(size_t payload,
> > diff --git a/datapath/linux/compat/include/net/ip.h
> b/datapath/linux/compat/include/net/ip.h
> > index 4193d32..a82d4ca 100644
> > --- a/datapath/linux/compat/include/net/ip.h
> > +++ b/datapath/linux/compat/include/net/ip.h
> > @@ -12,4 +12,8 @@ static inline bool ip_is_fragment(const struct iphdr
> *iph)
> >  }
> >  #endif
> >
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0)
> > +#define inet_get_local_port_range(a, b, c) inet_get_local_port_range(b,
> c)
> > +#endif
> > +
> >  #endif
> > diff --git a/datapath/linux/compat/utils.c
> b/datapath/linux/compat/utils.c
> > index dc4df2a..9404e20 100644
> > --- a/datapath/linux/compat/utils.c
> > +++ b/datapath/linux/compat/utils.c
> > @@ -6,6 +6,7 @@
> >  #include <linux/mm.h>
> >  #include <linux/net.h>
> >  #include <net/checksum.h>
> > +#include <net/ip.h>
> >  #include <linux/string.h>
> >  #include <linux/types.h>
> >  #include <linux/percpu.h>
> > @@ -38,6 +39,7 @@ void inet_proto_csum_replace16(__sum16 *sum, struct
> sk_buff *skb,
> >  }
> >  #endif
> >
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0)
> >  bool __net_get_random_once(void *buf, int nbytes, bool *done,
> >                            atomic_t *done_key)
> >  {
> > @@ -58,3 +60,4 @@ bool __net_get_random_once(void *buf, int nbytes, bool
> *done,
> >
> >         return true;
> >  }
> > +#endif
> > diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c
> > index e33cffe..8e3ff69 100644
> > --- a/datapath/vport-lisp.c
> > +++ b/datapath/vport-lisp.c
> > @@ -163,7 +163,7 @@ static __be64 instance_id_to_tunnel_id(__u8 *iid)
> >  /* Compute source UDP port for outgoing packet.
> >   * Currently we use the flow hash.
> >   */
> > -static u16 get_src_port(struct sk_buff *skb)
> > +static u16 get_src_port(struct net *net, struct sk_buff *skb)
> >  {
> >         u32 hash = skb_get_rxhash(skb);
> >         unsigned int range;
> > @@ -177,7 +177,7 @@ static u16 get_src_port(struct sk_buff *skb)
> >                             sizeof(*pkt_key) / sizeof(u32), 0);
> >         }
> >
> > -       inet_get_local_port_range(&low, &high);
> > +       inet_get_local_port_range(net, &low, &high);
> >         range = (high - low) + 1;
> >         return (((u64) hash * range) >> 32) + low;
> >  }
> > @@ -185,13 +185,14 @@ static u16 get_src_port(struct sk_buff *skb)
> >  static void lisp_build_header(const struct vport *vport,
> >                               struct sk_buff *skb)
> >  {
> > +       struct net *net = ovs_dp_get_net(vport->dp);
> >         struct lisp_port *lisp_port = lisp_vport(vport);
> >         struct udphdr *udph = udp_hdr(skb);
> >         struct lisphdr *lisph = (struct lisphdr *)(udph + 1);
> >         const struct ovs_key_ipv4_tunnel *tun_key = OVS_CB(skb)->tun_key;
> >
> >         udph->dest = lisp_port->dst_port;
> > -       udph->source = htons(get_src_port(skb));
> > +       udph->source = htons(get_src_port(net, skb));
> >         udph->check = 0;
> >         udph->len = htons(skb->len - skb_transport_offset(skb));
> >
> > diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
> > index d264785..cc9477d 100644
> > --- a/datapath/vport-vxlan.c
> > +++ b/datapath/vport-vxlan.c
> > @@ -139,6 +139,7 @@ error:
> >
> >  static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
> >  {
> > +       struct net *net = ovs_dp_get_net(vport->dp);
> >         struct vxlan_port *vxlan_port = vxlan_vport(vport);
> >         __be16 dst_port = inet_sport(vxlan_port->vs->sock->sk);
> >         struct rtable *rt;
> > @@ -172,7 +173,7 @@ static int vxlan_tnl_send(struct vport *vport,
> struct sk_buff *skb)
> >
> >         skb->local_df = 1;
> >
> > -       inet_get_local_port_range(&port_min, &port_max);
> > +       inet_get_local_port_range(net, &port_min, &port_max);
> >         src_port = vxlan_src_port(port_min, port_max, skb);
> >
> >         err = vxlan_xmit_skb(vxlan_port->vs, rt, skb,
> > --
> > 1.8.5.3
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to