On Thu, Mar 27, 2014 at 12:09 PM, Pravin Shelar <pshe...@nicira.com> wrote:

> On Wed, Mar 26, 2014 at 4:32 PM, Kyle Mestery <mest...@noironetworks.com>
> wrote:
> > 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.
> >>
> >> > +               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.
> >>
> > So, after spending some time looking at this particular comment today, it
> > turns out this
> > is harder than I thought. I had to essentially move "struct
> > genl_family_and_ops" into a
> > header file, which would deviate datapath/datapath.c siginificantly from
> > upstream. The
> > #ifdef isn't pretty, but I can't think of another way around this
> particular
> > issue. Did you
> > have any thoughts here Pravin? If not, I'll re-submit the patch with your
> > other issues
> > taken care and the #ifdef still here.
> >
>
> All information is passed to genl_register_family() so I think we can
> make it wotk.
> We can define compat function for genl_register_family ) along with
> compat struct genl_family, we can pass compat-genl_family to
> compat-genl_register_family(). compat-genl_register_family() can call
> native (older)  genl_register_family_with_ops() with its (older)
> struct genl_family.
>
> Let me know if this works.
>

I think this will work, I just coded it up and will send a v3 out soon.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to