[ovs-dev] [PATCH v2 00/16] datapath: NAT backports

[Jarno Rajahalme]

> On Jun 17, 2016, at 10:47 AM, Jesse Gross <jesse at kernel.org <mailto:jesse at kernel.org>> wrote:
> 
> On Tue, Jun 14, 2016 at 3:25 PM, Jarno Rajahalme <jarno at ovn.org <mailto:jarno at 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.

  Jarno

> --- /home/jgross/openvswitch/datapath/conntrack.c 2016-06-17
> 10:25:09.967245333 -0700
> +++ net/openvswitch/conntrack.c 2016-05-18 12:18:50.597639085 -0700
> @@ -357,14 +351,9 @@
> static int handle_fragments(struct net *net, struct sw_flow_key *key,
>     u16 zone, struct sk_buff *skb)
> [...]
> - if (!skb->dev) {
> - OVS_NLERR(true, "%s: skb has no dev; dropping", __func__);
> - return -EINVAL;
> - }
> -
> 
> @@ -745,7 +734,7 @@
>  /* Repeat if requested, see nf_iterate(). */
>  do {
>  err = nf_conntrack_in(net, info->family,
> -      NF_INET_FORWARD, skb);
> +      NF_INET_PRE_ROUTING, skb);
>  } while (err == NF_REPEAT);
> 
>  if (err != NF_ACCEPT)
> @@ -1353,7 +1342,7 @@
>  if (ct_info->helper)
>  module_put(ct_info->helper->me);
>  if (ct_info->ct)
> - nf_ct_tmpl_free(ct_info->ct);
> + nf_ct_put(ct_info->ct);
> }