On 18 June 2016 at 03:20, Jarno Rajahalme <ja...@ovn.org> wrote: > >> On Jun 17, 2016, at 10:47 AM, Jesse Gross <je...@kernel.org >> <mailto:je...@kernel.org>> wrote: >> >> On Tue, Jun 14, 2016 at 3:25 PM, Jarno Rajahalme <ja...@ovn.org >> <mailto:ja...@ovn.org>> wrote: >>> This series adds the conntrack NAT integration upstreamed in Linux 4.6 >>> to the OVS tree kernel module. Main code is the same as upstream >>> net-next, backports are provided for Linux kernels 3.10 - 4.6. Code >>> compiles on each Linux version on this range, except for Linux 4.4 - >>> 4.6, which fail to compile due to reasons unrelated to NAT and/or >>> conntrack. >>> >>> The backports are tested on linux-stable versions 4.3 and 4.1, and >>> Ubuntu 14.04 with kernels 3.16.0-71-generic and 3.19.0-59-generic. >>> >>> While testing I observed kernel crashes from 'expiry' tests in >>> tests/system-traffic.at <http://system-traffic.at/>. I was able to >>> reproduce these crashes on OVS >>> master with various Linux kernel versions, and did not experience any >>> crashes when running only the NAT test cases with the backports in >>> this series. This tells me that the problem is not related to the NAT >>> backports. >>> >>> The patch that adds GCC 5 support for older kernels was used for >>> compile-only testing. >> >> I don't have any further comments on this series besides the ones I >> already responded to - the rest of the patches look like pretty >> straightforward backports. >> > > I'll do a v3 addressing your comments later today. > >> I diff'ed current net-next against OVS master plus this patch series >> for conntrack.c. Mostly it was the same but I noticed a few things >> that didn't appear to be related to backporting necessities. Are we >> missing some patches? This is what I got (I edited the diff to remove >> irrelevant stuff): >> > > I guess Joe could have an idea about these. I recall him mentioning that the > NF_INET_PRE_ROUTING change was a backport to avoid dependency on skb->dev in > some cases, but I don't know the detail. I guess this change could be done > upstream as well, but maybe it is not relevant for the current upstream > kernel.
Just to follow up on the NF_INET_PRE_ROUTING change, I think the upstream approach is probably more correct - it would do l4proto checksum validation and such. However, like Jarno mentioned, on older kernels this path has a dependency on skb->dev that leads to a kernel panic. I'm considering shifting this change into the compat code so the code is the same as upstream, with an override in compat that checks for the kernel version and uses NF_INET_FORWARD where there is an skb->dev dependency to avoid the crash. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
