diff mbox

[ovs-dev,PATCHv2,14/20] datapath: Add conntrack action

Message ID 20151211082350.GA14773@vergenet.net
State Not Applicable
Headers show

Commit Message

Simon Horman Dec. 11, 2015, 8:23 a.m. UTC 
Hi Joe,

On Wed, Dec 02, 2015 at 11:53:50PM -0800, Joe Stringer wrote:

[snip]

> diff --git a/datapath/actions.c b/datapath/actions.c
> index 0397bfe3dba4..0625d7e01176 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c

[snip]

> @@ -37,6 +39,7 @@
>  #include <net/sctp/checksum.h>
>  
>  #include "datapath.h"
> +#include "conntrack.h"
>  #include "gso.h"
>  #include "vlan.h"
>  #include "vport.h"
> @@ -53,6 +56,20 @@ struct deferred_action {
>  	struct sw_flow_key pkt_key;
>  };
>  
> +#define MAX_L2_LEN	(VLAN_ETH_HLEN + 3 * MPLS_HLEN)
> +struct ovs_frag_data {
> +	unsigned long dst;
> +	struct vport *vport;
> +	struct ovs_skb_cb cb;
> +	__be16 inner_protocol;
> +	__u16 vlan_tci;
> +	__be16 vlan_proto;
> +	unsigned int l2_len;
> +	u8 l2_data[MAX_L2_LEN];
> +};
> +
> +static DEFINE_PER_CPU(struct ovs_frag_data, ovs_frag_data_storage);
> +
>  #define DEFERRED_ACTION_FIFO_SIZE 10
>  struct action_fifo {
>  	int head;
> @@ -606,14 +623,157 @@ static int set_sctp(struct sk_buff *skb, struct sw_flow_key *flow_key,
>  	return 0;
>  }
>  
> -static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port)

> +#if LINUX_VERSION_CODE > KERNEL_VERSION(3,9,0)

[snip]

> +static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru,
> +			 __be16 ethertype)
> +{
> +	if (skb_network_offset(skb) > MAX_L2_LEN) {
> +		OVS_NLERR(1, "L2 header too long to fragment");
> +		return;
> +	}
> +
> +	if (ethertype == htons(ETH_P_IP)) {
> +		struct dst_entry ovs_dst;
> +		unsigned long orig_dst;
> +
> +		prepare_frag(vport, skb);
> +		dst_init(&ovs_dst, &ovs_dst_ops, NULL, 1,
> +			 DST_OBSOLETE_NONE, DST_NOCOUNT);
> +		ovs_dst.dev = vport->dev;
> +
> +		orig_dst = (unsigned long) skb_dst(skb);
> +		skb_dst_set_noref(skb, &ovs_dst);
> +		IPCB(skb)->frag_max_size = mru;
> +
> +		ip_do_fragment(skb->sk, skb, ovs_vport_output);

It seems that this code is compiled for kernel versions > 3.9
but that a backport of ip_do_fragment is only provided for
kernels >= 3.10.

I'm unsure of the intention but perhaps the code above should
not be compiled for when building against 3.9.

With the following building succeeds against v3.9.11.

Comments

Joe Stringer Dec. 15, 2015, 7:27 p.m. UTC | #1 
On 11 December 2015 at 00:23, Simon Horman <simon.horman@netronome.com> wrote:
> [snip]
>
>> +static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru,
>> +                      __be16 ethertype)
>> +{
>> +     if (skb_network_offset(skb) > MAX_L2_LEN) {
>> +             OVS_NLERR(1, "L2 header too long to fragment");
>> +             return;
>> +     }
>> +
>> +     if (ethertype == htons(ETH_P_IP)) {
>> +             struct dst_entry ovs_dst;
>> +             unsigned long orig_dst;
>> +
>> +             prepare_frag(vport, skb);
>> +             dst_init(&ovs_dst, &ovs_dst_ops, NULL, 1,
>> +                      DST_OBSOLETE_NONE, DST_NOCOUNT);
>> +             ovs_dst.dev = vport->dev;
>> +
>> +             orig_dst = (unsigned long) skb_dst(skb);
>> +             skb_dst_set_noref(skb, &ovs_dst);
>> +             IPCB(skb)->frag_max_size = mru;
>> +
>> +             ip_do_fragment(skb->sk, skb, ovs_vport_output);
>
> It seems that this code is compiled for kernel versions > 3.9
> but that a backport of ip_do_fragment is only provided for
> kernels >= 3.10.
>
> I'm unsure of the intention but perhaps the code above should
> not be compiled for when building against 3.9.
>
> With the following building succeeds against v3.9.11.
>
> [snip]

Thanks for the report. I found a few other minor inconsistencies, and
fixed them too in the below patch:
http://openvswitch.org/pipermail/dev/2015-December/063442.html

Travis seems OK with it:
https://travis-ci.org/joestringer/openvswitch/builds/97055675

While I think that some (most?) of the requirements may be available
on v3.9, there is no way that we will be able to test it as thoroughly
as v3.10 backport and I'm not aware of any particular distributions
that are standardising on that kernel version, so IMHO it's better to
use v3.10 as the lower bound.
Simon Horman Dec. 16, 2015, 12:12 a.m. UTC | #2 
2015/12/16 4:28 "Joe Stringer" <joe@ovn.org>:
>
> On 11 December 2015 at 00:23, Simon Horman <simon.horman@netronome.com>
wrote:
> > [snip]
> >
> >> +static void ovs_fragment(struct vport *vport, struct sk_buff *skb,
u16 mru,
> >> +                      __be16 ethertype)
> >> +{
> >> +     if (skb_network_offset(skb) > MAX_L2_LEN) {
> >> +             OVS_NLERR(1, "L2 header too long to fragment");
> >> +             return;
> >> +     }
> >> +
> >> +     if (ethertype == htons(ETH_P_IP)) {
> >> +             struct dst_entry ovs_dst;
> >> +             unsigned long orig_dst;
> >> +
> >> +             prepare_frag(vport, skb);
> >> +             dst_init(&ovs_dst, &ovs_dst_ops, NULL, 1,
> >> +                      DST_OBSOLETE_NONE, DST_NOCOUNT);
> >> +             ovs_dst.dev = vport->dev;
> >> +
> >> +             orig_dst = (unsigned long) skb_dst(skb);
> >> +             skb_dst_set_noref(skb, &ovs_dst);
> >> +             IPCB(skb)->frag_max_size = mru;
> >> +
> >> +             ip_do_fragment(skb->sk, skb, ovs_vport_output);
> >
> > It seems that this code is compiled for kernel versions > 3.9
> > but that a backport of ip_do_fragment is only provided for
> > kernels >= 3.10.
> >
> > I'm unsure of the intention but perhaps the code above should
> > not be compiled for when building against 3.9.
> >
> > With the following building succeeds against v3.9.11.
> >
> > [snip]
>
> Thanks for the report. I found a few other minor inconsistencies, and
> fixed them too in the below patch:
> http://openvswitch.org/pipermail/dev/2015-December/063442.html
>
> Travis seems OK with it:
> https://travis-ci.org/joestringer/openvswitch/builds/97055675
>
> While I think that some (most?) of the requirements may be available
> on v3.9, there is no way that we will be able to test it as thoroughly
> as v3.10 backport and I'm not aware of any particular distributions
> that are standardising on that kernel version, so IMHO it's better to
> use v3.10 as the lower bound.

Thanks Joe,

I'll do some testing and see if anything pops up.

Regarding features for v3.9: I for have no special requirements there at
this time and I agree with the approach you are taking.
diff mbox

Patch

diff --git a/datapath/actions.c b/datapath/actions.c
index 0f1f4bc06ba1..ffa97e3e0231 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -68,7 +68,7 @@  struct ovs_frag_data {
 	u8 l2_data[MAX_L2_LEN];
 };
 
-#if LINUX_VERSION_CODE > KERNEL_VERSION(3,9,0)
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0)
 static DEFINE_PER_CPU(struct ovs_frag_data, ovs_frag_data_storage);
 #endif
 
@@ -625,7 +625,7 @@  static int set_sctp(struct sk_buff *skb, struct sw_flow_key *flow_key,
 	return 0;
 }
 
-#if LINUX_VERSION_CODE > KERNEL_VERSION(3,9,0)
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0)
 static int ovs_vport_output(OVS_VPORT_OUTPUT_PARAMS)
 {
 	struct ovs_frag_data *data = get_pcpu_ptr(ovs_frag_data_storage);
@@ -740,7 +740,7 @@  static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru,
 err:
 	kfree_skb(skb);
 }
-#else /* <= 3.9 */
+#else /* < 3.10 */
 static void ovs_fragment(struct vport *vport, struct sk_buff *skb, u16 mru,
 			 __be16 ethertype)
 {