On Fri, Sep 9, 2016 at 2:37 PM, Joe Stringer <j...@ovn.org> wrote: > On 8 September 2016 at 08:49, Jesse Gross <je...@kernel.org> wrote: >> On Wed, Sep 7, 2016 at 5:18 PM, Joe Stringer <j...@ovn.org> wrote: >>> On 1 September 2016 at 18:08, Jesse Gross <je...@kernel.org> wrote: >>>> On Thu, Sep 1, 2016 at 5:01 PM, Joe Stringer <j...@ovn.org> wrote: >>>>> The upstream code uses NF_INET_PRE_ROUTING hook for the nf_conntrack_in() >>>>> call, which does deeper (eg l4proto) validation. It was previously >>>>> thought that using the NF_INET_ROUTING hook for this function on older >>>>> kernels would trigger kernel panics due to a dependency on the >>>>> unpopulated skb->dev, however during recent testing on a variety of >>>>> platforms (Centos7.[12], Ubuntu 1[46].04, Fedora23) using the latest >>>>> distribution kernels and the OVS kernel module testsuite, no such kernel >>>>> panics were observed. Therefore it appears to be safe to bring this in >>>>> line with upstream without any other workarounds. >>>>> >>>>> Reported-by: Jesse Gross <je...@kernel.org> >>>>> Signed-off-by: Joe Stringer <j...@ovn.org> >>>> >>>> If you are confident that it doesn't cause problems on older kernels, >>>> the change looks obviously correct to me relative to upstream. >>> >>> Unfortunately I don't have concrete details of the original issue, so I >>> can't say this with strong confidence. >>> >>> I don't think it was ever a problem upstream, (ie 4.3+), so we /could/ >>> keep it as NF_INET_FORWARD on kernels older than that.. >> >> I think if you've tested it on the major distribution kernels then >> it's unlikely we'll see problems in practice. It's probably not worth >> retaining the old symbol based on an issue that we don't even know the >> details of, so I would just go ahead with this patch. > > That sounds reasonable. If down the line we rediscover this issue, we > can consider whether to add back a workaround for it. > > Applied to master. > > Any opinions on backporting to branch-2.[56]?
I would be more inclined to err on the side of caution for the release branches. This doesn't fix any major bugs - I know this does change behavior a bit but in my mind the most important part of the change is to make things consistent with upstream. As a result, I would probably keep this on master only. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
