Re: [ovs-dev] [PATCH branch-2.8 2/2] datapath: Clear the L4 portion of the key for "later" fragments

2019-08-30 Thread Gregory Rose


On 8/29/2019 10:44 PM, Justin Pettit wrote:

On Aug 29, 2019, at 6:46 PM, Gregory Rose  wrote:


On 8/29/2019 3:25 PM, Justin Pettit wrote:

On Aug 29, 2019, at 10:55 AM, Greg Rose  wrote:

diff --git a/datapath/flow.c b/datapath/flow.c
index 083288f..92fc6ac 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -529,6 +529,7 @@ static int key_extract_l3l4(struct sk_buff *skb, struct 
sw_flow_key *key)
offset = nh->frag_off & htons(IP_OFFSET);
if (offset) {
key->ip.frag = OVS_FRAG_TYPE_LATER;
+memset(>tp, 0, sizeof(key->tp));
return 0;
}
if (nh->frag_off & htons(IP_MF) ||
@@ -647,8 +648,11 @@ static int key_extract_l3l4(struct sk_buff *skb, struct 
sw_flow_key *key)
return error;
}

-if (key->ip.frag == OVS_FRAG_TYPE_LATER)
+if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
+memset(>tp, 0, sizeof(key->tp));
return 0;
+}
+#ifdef HAVE_SKB_GSO_UDP

My system's kernel is too new to be built against an OVS this old, but I 
noticed this patch for OVS versions 2.5 through 2.9 introduce this #ifdef 
without a corresponding #endif.  Was this intentional?  Does it even build?

--Justin



Conflict resolutions error - I'll probably have to resend branches 2.8 through 
2.5.

Thanks.  Can you please at least compile-check them before sending out v2?



I'll have to wait for a travis check to finish - I can't seem to get any 
branches older than 2.9 to compile on my
own system anymore due to some Python issues.  So I'll just let travis 
do the work.


Sorry about that, trying to do too many things at once...

- Greg

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH branch-2.8 2/2] datapath: Clear the L4 portion of the key for "later" fragments

2019-08-29 Thread Gregory Rose



On 8/29/2019 3:25 PM, Justin Pettit wrote:

On Aug 29, 2019, at 10:55 AM, Greg Rose  wrote:

diff --git a/datapath/flow.c b/datapath/flow.c
index 083288f..92fc6ac 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -529,6 +529,7 @@ static int key_extract_l3l4(struct sk_buff *skb, struct 
sw_flow_key *key)
offset = nh->frag_off & htons(IP_OFFSET);
if (offset) {
key->ip.frag = OVS_FRAG_TYPE_LATER;
+   memset(>tp, 0, sizeof(key->tp));
return 0;
}
if (nh->frag_off & htons(IP_MF) ||
@@ -647,8 +648,11 @@ static int key_extract_l3l4(struct sk_buff *skb, struct 
sw_flow_key *key)
return error;
}

-   if (key->ip.frag == OVS_FRAG_TYPE_LATER)
+   if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
+   memset(>tp, 0, sizeof(key->tp));
return 0;
+   }
+#ifdef HAVE_SKB_GSO_UDP

My system's kernel is too new to be built against an OVS this old, but I 
noticed this patch for OVS versions 2.5 through 2.9 introduce this #ifdef 
without a corresponding #endif.  Was this intentional?  Does it even build?

--Justin


Conflict resolutions error - I'll probably have to resend branches 2.8 
through 2.5.


- Greg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH branch-2.8 2/2] datapath: Clear the L4 portion of the key for "later" fragments

2019-08-29 Thread Justin Pettit


> On Aug 29, 2019, at 10:55 AM, Greg Rose  wrote:
> 
> diff --git a/datapath/flow.c b/datapath/flow.c
> index 083288f..92fc6ac 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
> @@ -529,6 +529,7 @@ static int key_extract_l3l4(struct sk_buff *skb, struct 
> sw_flow_key *key)
>   offset = nh->frag_off & htons(IP_OFFSET);
>   if (offset) {
>   key->ip.frag = OVS_FRAG_TYPE_LATER;
> + memset(>tp, 0, sizeof(key->tp));
>   return 0;
>   }
>   if (nh->frag_off & htons(IP_MF) ||
> @@ -647,8 +648,11 @@ static int key_extract_l3l4(struct sk_buff *skb, struct 
> sw_flow_key *key)
>   return error;
>   }
> 
> - if (key->ip.frag == OVS_FRAG_TYPE_LATER)
> + if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
> + memset(>tp, 0, sizeof(key->tp));
>   return 0;
> + }
> +#ifdef HAVE_SKB_GSO_UDP

My system's kernel is too new to be built against an OVS this old, but I 
noticed this patch for OVS versions 2.5 through 2.9 introduce this #ifdef 
without a corresponding #endif.  Was this intentional?  Does it even build?

--Justin


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH branch-2.8 2/2] datapath: Clear the L4 portion of the key for "later" fragments

2019-08-29 Thread 0-day Robot
Bleep bloop.  Greetings Greg Rose, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 58, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH branch-2.8 2/2] datapath: Clear the L4 portion of the key for "later" fragments

2019-08-29 Thread Greg Rose
From: Justin Pettit 

Upstream commit:
commit 0754b4e8cdf3eec6e4122e79af26ed9bab20f8f8
Author: Justin Pettit 
Date:   Tue Aug 27 07:58:10 2019 -0700

openvswitch: Clear the L4 portion of the key for "later" fragments.

Only the first fragment in a datagram contains the L4 headers.  When the
Open vSwitch module parses a packet, it always sets the IP protocol
field in the key, but can only set the L4 fields on the first fragment.
The original behavior would not clear the L4 portion of the key, so
garbage values would be sent in the key for "later" fragments.  This
patch clears the L4 fields in that circumstance to prevent sending those
garbage values as part of the upcall.

Signed-off-by: Justin Pettit 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Signed-off-by: Greg Rose 
Signed-off-by: Justin Pettit 
---
 datapath/flow.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index 083288f..92fc6ac 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -529,6 +529,7 @@ static int key_extract_l3l4(struct sk_buff *skb, struct 
sw_flow_key *key)
offset = nh->frag_off & htons(IP_OFFSET);
if (offset) {
key->ip.frag = OVS_FRAG_TYPE_LATER;
+   memset(>tp, 0, sizeof(key->tp));
return 0;
}
if (nh->frag_off & htons(IP_MF) ||
@@ -647,8 +648,11 @@ static int key_extract_l3l4(struct sk_buff *skb, struct 
sw_flow_key *key)
return error;
}
 
-   if (key->ip.frag == OVS_FRAG_TYPE_LATER)
+   if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
+   memset(>tp, 0, sizeof(key->tp));
return 0;
+   }
+#ifdef HAVE_SKB_GSO_UDP
if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
key->ip.frag = OVS_FRAG_TYPE_FIRST;
 
-- 
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev