diff mbox

[ovs-dev] datapath: Check for sock argument to v6ops->fragment.

Message ID 1458589414-41954-1-git-send-email-jesse@kernel.org
State Accepted
Headers show

Commit Message

Jesse Gross March 21, 2016, 7:43 p.m. UTC
Ubuntu 3.13.0-83-generic has backported a patch that adds an intermediate
version of the v6ops->fragment function that doesn't seem to ever been
part of a released upstream kernel. This version is missing the sock
argument to the fragment function.

Since we already have a backported version of the function from a newer
kernel, this simply ignores the version that Ubuntu is now making available
and continues to use the OVS version, similar to what it was doing before.

Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com>
Reported-by: Aaron Rosen <aaronorosen@gmail.com>
Reported-by: Russell Bryant <russell@ovn.org>
Signed-off-by: Jesse Gross <jesse@kernel.org>
---
 acinclude.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Russell Bryant March 21, 2016, 7:49 p.m. UTC | #1
On Mon, Mar 21, 2016 at 12:43 PM, Jesse Gross <jesse@kernel.org> wrote:

> Ubuntu 3.13.0-83-generic has backported a patch that adds an intermediate
> version of the v6ops->fragment function that doesn't seem to ever been
> part of a released upstream kernel. This version is missing the sock
> argument to the fragment function.
>
> Since we already have a backported version of the function from a newer
> kernel, this simply ignores the version that Ubuntu is now making available
> and continues to use the OVS version, similar to what it was doing before.
>
> Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com>
> Reported-by: Aaron Rosen <aaronorosen@gmail.com>
> Reported-by: Russell Bryant <russell@ovn.org>
> Signed-off-by: Jesse Gross <jesse@kernel.org>
>

I verified that this patch resolves the issue for me.  Thank you!

Acked-by: Russell Bryant <russell@ovn.org>
Jesse Gross March 21, 2016, 7:59 p.m. UTC | #2
On Mon, Mar 21, 2016 at 12:49 PM, Russell Bryant <russell@ovn.org> wrote:
> On Mon, Mar 21, 2016 at 12:43 PM, Jesse Gross <jesse@kernel.org> wrote:
>>
>> Ubuntu 3.13.0-83-generic has backported a patch that adds an intermediate
>> version of the v6ops->fragment function that doesn't seem to ever been
>> part of a released upstream kernel. This version is missing the sock
>> argument to the fragment function.
>>
>> Since we already have a backported version of the function from a newer
>> kernel, this simply ignores the version that Ubuntu is now making
>> available
>> and continues to use the OVS version, similar to what it was doing before.
>>
>> Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com>
>> Reported-by: Aaron Rosen <aaronorosen@gmail.com>
>> Reported-by: Russell Bryant <russell@ovn.org>
>> Signed-off-by: Jesse Gross <jesse@kernel.org>
>
>
> I verified that this patch resolves the issue for me.  Thank you!
>
> Acked-by: Russell Bryant <russell@ovn.org>

Thanks, that was fast! I applied this patch to master and branch-2.5

In terms of the OpenStack CI infrastructure, is that sufficient to
resolve the issue? i.e. Are all cases where this matters is it using
OVS off the branches directly (presumably for OVN) or are there other
situations where this will cause problems?
Russell Bryant March 21, 2016, 8:07 p.m. UTC | #3
On Mon, Mar 21, 2016 at 12:59 PM, Jesse Gross <jesse@kernel.org> wrote:

> On Mon, Mar 21, 2016 at 12:49 PM, Russell Bryant <russell@ovn.org> wrote:
> > On Mon, Mar 21, 2016 at 12:43 PM, Jesse Gross <jesse@kernel.org> wrote:
> >>
> >> Ubuntu 3.13.0-83-generic has backported a patch that adds an
> intermediate
> >> version of the v6ops->fragment function that doesn't seem to ever been
> >> part of a released upstream kernel. This version is missing the sock
> >> argument to the fragment function.
> >>
> >> Since we already have a backported version of the function from a newer
> >> kernel, this simply ignores the version that Ubuntu is now making
> >> available
> >> and continues to use the OVS version, similar to what it was doing
> before.
> >>
> >> Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com>
> >> Reported-by: Aaron Rosen <aaronorosen@gmail.com>
> >> Reported-by: Russell Bryant <russell@ovn.org>
> >> Signed-off-by: Jesse Gross <jesse@kernel.org>
> >
> >
> > I verified that this patch resolves the issue for me.  Thank you!
> >
> > Acked-by: Russell Bryant <russell@ovn.org>
>
> Thanks, that was fast! I applied this patch to master and branch-2.5
>
> In terms of the OpenStack CI infrastructure, is that sufficient to
> resolve the issue? i.e. Are all cases where this matters is it using
> OVS off the branches directly (presumably for OVN) or are there other
> situations where this will cause problems?
>

Good question.

For OVN, we're using master directly, so this is sufficient.

I know of one other OpenStack CI job that compiles OVS and uses the v2.5.0
tag.  I just checked a run of that job and for whatever reason, it wasn't
using the newer ubuntu kernel yet.  If/when it does, it will be easy to
update it to get this commit on branch-2.5.  I copied a few people to make
sure they're aware of the issue.
Jesse Gross March 21, 2016, 8:20 p.m. UTC | #4
On Mon, Mar 21, 2016 at 1:07 PM, Russell Bryant <russell@ovn.org> wrote:
>
>
> On Mon, Mar 21, 2016 at 12:59 PM, Jesse Gross <jesse@kernel.org> wrote:
>>
>> On Mon, Mar 21, 2016 at 12:49 PM, Russell Bryant <russell@ovn.org> wrote:
>> > On Mon, Mar 21, 2016 at 12:43 PM, Jesse Gross <jesse@kernel.org> wrote:
>> >>
>> >> Ubuntu 3.13.0-83-generic has backported a patch that adds an
>> >> intermediate
>> >> version of the v6ops->fragment function that doesn't seem to ever been
>> >> part of a released upstream kernel. This version is missing the sock
>> >> argument to the fragment function.
>> >>
>> >> Since we already have a backported version of the function from a newer
>> >> kernel, this simply ignores the version that Ubuntu is now making
>> >> available
>> >> and continues to use the OVS version, similar to what it was doing
>> >> before.
>> >>
>> >> Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com>
>> >> Reported-by: Aaron Rosen <aaronorosen@gmail.com>
>> >> Reported-by: Russell Bryant <russell@ovn.org>
>> >> Signed-off-by: Jesse Gross <jesse@kernel.org>
>> >
>> >
>> > I verified that this patch resolves the issue for me.  Thank you!
>> >
>> > Acked-by: Russell Bryant <russell@ovn.org>
>>
>> Thanks, that was fast! I applied this patch to master and branch-2.5
>>
>> In terms of the OpenStack CI infrastructure, is that sufficient to
>> resolve the issue? i.e. Are all cases where this matters is it using
>> OVS off the branches directly (presumably for OVN) or are there other
>> situations where this will cause problems?
>
>
> Good question.
>
> For OVN, we're using master directly, so this is sufficient.
>
> I know of one other OpenStack CI job that compiles OVS and uses the v2.5.0
> tag.  I just checked a run of that job and for whatever reason, it wasn't
> using the newer ubuntu kernel yet.  If/when it does, it will be easy to
> update it to get this commit on branch-2.5.  I copied a few people to make
> sure they're aware of the issue.

Sounds good - thanks for looking into it.
Jakub Libosvar March 21, 2016, 9:39 p.m. UTC | #5
On 21/03/16 21:07, Russell Bryant wrote:
>
>
> On Mon, Mar 21, 2016 at 12:59 PM, Jesse Gross <jesse@kernel.org
> <mailto:jesse@kernel.org>> wrote:
>
>     On Mon, Mar 21, 2016 at 12:49 PM, Russell Bryant <russell@ovn.org
>     <mailto:russell@ovn.org>> wrote:
>     > On Mon, Mar 21, 2016 at 12:43 PM, Jesse Gross <jesse@kernel.org <mailto:jesse@kernel.org>> wrote:
>     >>
>     >> Ubuntu 3.13.0-83-generic has backported a patch that adds an intermediate
>     >> version of the v6ops->fragment function that doesn't seem to ever been
>     >> part of a released upstream kernel. This version is missing the sock
>     >> argument to the fragment function.
>     >>
>     >> Since we already have a backported version of the function from a newer
>     >> kernel, this simply ignores the version that Ubuntu is now making
>     >> available
>     >> and continues to use the OVS version, similar to what it was doing before.
>     >>
>     >> Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com <mailto:zoltan.balogh@ericsson.com>>
>     >> Reported-by: Aaron Rosen <aaronorosen@gmail.com <mailto:aaronorosen@gmail.com>>
>     >> Reported-by: Russell Bryant <russell@ovn.org <mailto:russell@ovn.org>>
>     >> Signed-off-by: Jesse Gross <jesse@kernel.org <mailto:jesse@kernel.org>>
>     >
>     >
>     > I verified that this patch resolves the issue for me.  Thank you!
>     >
>     > Acked-by: Russell Bryant <russell@ovn.org <mailto:russell@ovn.org>>
>
>     Thanks, that was fast! I applied this patch to master and branch-2.5
>
>     In terms of the OpenStack CI infrastructure, is that sufficient to
>     resolve the issue? i.e. Are all cases where this matters is it using
>     OVS off the branches directly (presumably for OVN) or are there other
>     situations where this will cause problems?
>
>
> Good question.
>
> For OVN, we're using master directly, so this is sufficient.
>
> I know of one other OpenStack CI job that compiles OVS and uses the
> v2.5.0 tag.  I just checked a run of that job and for whatever reason,
> it wasn't using the newer ubuntu kernel yet.  If/when it does, it will
> be easy to update it to get this commit on branch-2.5.  I copied a few
> people to make sure they're aware of the issue.

Thank for notice. We do use newer kernel of ubuntu in upstream but we 
still haven't switched to v2.5.0 tag because we hit this issue with 
fragment function arguments [1]. Luckily, we were too slow to merge this 
before the switch from devstack-trusty. :)

Kuba

[1] https://review.openstack.org/#/c/286106/
>
> --
> Russell Bryant
Russell Bryant March 21, 2016, 11:17 p.m. UTC | #6
On Monday, March 21, 2016, Jakub Libosvar <jlibosva@redhat.com> wrote:

> On 21/03/16 21:07, Russell Bryant wrote:
>
>>
>>
>> On Mon, Mar 21, 2016 at 12:59 PM, Jesse Gross <jesse@kernel.org
>> <mailto:jesse@kernel.org>> wrote:
>>
>>     On Mon, Mar 21, 2016 at 12:49 PM, Russell Bryant <russell@ovn.org
>>     <mailto:russell@ovn.org>> wrote:
>>     > On Mon, Mar 21, 2016 at 12:43 PM, Jesse Gross <jesse@kernel.org
>> <mailto:jesse@kernel.org>> wrote:
>>     >>
>>     >> Ubuntu 3.13.0-83-generic has backported a patch that adds an
>> intermediate
>>     >> version of the v6ops->fragment function that doesn't seem to ever
>> been
>>     >> part of a released upstream kernel. This version is missing the
>> sock
>>     >> argument to the fragment function.
>>     >>
>>     >> Since we already have a backported version of the function from a
>> newer
>>     >> kernel, this simply ignores the version that Ubuntu is now making
>>     >> available
>>     >> and continues to use the OVS version, similar to what it was doing
>> before.
>>     >>
>>     >> Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com <mailto:
>> zoltan.balogh@ericsson.com>>
>>     >> Reported-by: Aaron Rosen <aaronorosen@gmail.com <mailto:
>> aaronorosen@gmail.com>>
>>     >> Reported-by: Russell Bryant <russell@ovn.org <mailto:
>> russell@ovn.org>>
>>     >> Signed-off-by: Jesse Gross <jesse@kernel.org <mailto:
>> jesse@kernel.org>>
>>     >
>>     >
>>     > I verified that this patch resolves the issue for me.  Thank you!
>>     >
>>     > Acked-by: Russell Bryant <russell@ovn.org <mailto:russell@ovn.org>>
>>
>>     Thanks, that was fast! I applied this patch to master and branch-2.5
>>
>>     In terms of the OpenStack CI infrastructure, is that sufficient to
>>     resolve the issue? i.e. Are all cases where this matters is it using
>>     OVS off the branches directly (presumably for OVN) or are there other
>>     situations where this will cause problems?
>>
>>
>> Good question.
>>
>> For OVN, we're using master directly, so this is sufficient.
>>
>> I know of one other OpenStack CI job that compiles OVS and uses the
>> v2.5.0 tag.  I just checked a run of that job and for whatever reason,
>> it wasn't using the newer ubuntu kernel yet.  If/when it does, it will
>> be easy to update it to get this commit on branch-2.5.  I copied a few
>> people to make sure they're aware of the issue.
>>
>
> Thank for notice. We do use newer kernel of ubuntu in upstream but we
> still haven't switched to v2.5.0 tag because we hit this issue with
> fragment function arguments [1]. Luckily, we were too slow to merge this
> before the switch from devstack-trusty. :)
>
> Kuba
>
> [1] https://review.openstack.org/#/c/286106/


Oh, so you did hit it! I hadn't noticed.

It should work now if you use HEAD of branch-2.5.
diff mbox

Patch

diff --git a/acinclude.m4 b/acinclude.m4
index 74f0494..f345c31 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -393,7 +393,7 @@  AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_hookfn.*nf_hook_ops],
                   [OVS_DEFINE([HAVE_NF_HOOKFN_ARG_OPS])])
   OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netfilter_ipv6.h], [nf_ipv6_ops],
-                        [fragment], [OVS_DEFINE([HAVE_NF_IPV6_OPS_FRAGMENT])])
+                        [fragment.*sock], [OVS_DEFINE([HAVE_NF_IPV6_OPS_FRAGMENT])])
 
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack.h],
                   [tmpl_alloc.*conntrack_zone],