On 1/14/2019 4:20 PM, Gregory Rose wrote:
On 1/14/2019 3:22 PM, Yi-Hung Wei wrote:
On Thu, Jan 10, 2019 at 2:09 PM Greg Rose <[email protected]> wrote:
Upstream commit 648700f76b03 ("inet: frags: use rhashtables...")
changed
how ipv6 fragmentation is implemented. This patch was backported to
the upstream stable 4.9.x kernel starting at 4.9.135.
This patch creates the compatibility layer changes required to both
compile and also operate correctly with ipv6 fragmentation on these
kernels. Check if the inet_frags 'rnd' field is present to key on
whether the upstream patch is present. Also update Travis to the
latest 4.9 kernel release so that this patch is compile tested.
Passes Travis:
https://travis-ci.org/gvrose8192/ovs-experimental/builds/478033409
Cc: William Tu <[email protected]>
Cc: Yi-Hung Wei <[email protected]>
Cc: Yifeng Sun <[email protected]>
Signed-off-by: Greg Rose <[email protected]>
---
Thanks Greg for the patch. In general, it looks good to me, I only
have two small questions as below. I tested it on 4.10.17 stable
kernel from linux-stable tree. The compilation is passed, and the
relevant IPv6 fragmentation system traffic tests are passed.
@@ -614,10 +658,12 @@ void ovs_netns_frags6_init(struct net *net)
void ovs_netns_frags6_exit(struct net *net)
{
+#ifdef HAVE_INET_FRAGS_RND
struct netns_frags *frags;
frags = get_netns_frags6_from_net(net);
inet_frags_exit_net(frags, &nf_frags);
+#endif
}
Don't we need to do inet_frags_exit_net() if HAVE_INET_FRAGS_RND is
false?
From ./net/ipv6/netfilter/nf_conntrack_reasm.c in the linux-stable
branch 4.10.y, it looks like inet_frags_exit_net() is still be used.
In 4.9.135 it was causing a panic. I added that at the time to
prevent the panic but perhaps there's a better
solution I should be looking for.
Yi-hung,
I looked more closely at this in the stable tree and for 4.10.y
HAVE_INET_FRAGS_RND will be defined and
this function will work as you expect. So unless I'm missing something
I feel confident that this is the correct
thing to do here.
.....
-#ifdef HAVE_INET_FRAGS_WITH_FRAGS_WORK
+#if defined(HAVE_INET_FRAGS_WITH_FRAGS_WORK) ||
!defined(HAVE_INET_FRAGS_RND)
nf_frags.frags_cache_name = nf_frags_cache_name;
#endif
Not sure if this changed is needed? It seems to me that it depends on
HAVE_INET_FRAGS_WITH_FRAGS_WORK but not on HAVE_INET_FRAGS_RND?
It turns out after I had more of a chance to look at this that
HAVE_INET_FRAGS_WITH_FRAGS_WORK and
HAVE_INET_FRAGS_RND are both keying off the same upstream patch I
mentioned in the commit message
(648700f76b03). There's no need for the redundancy. Nice catch!
I'm going to fix this by getting rid of HAVE_INET_FRAGS_WITH_FRAGS_WORK
because that is sort of
unwieldy and only used in a few spots. We can simplify acinclude.m4 a
bit and work with a single define
for the compilation switch.
A v2 patch will be forthcoming. Thanks for the review!
- Greg
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
