Message ID | 1567101350-27905-2-git-send-email-gvrose8192@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,branch-2.8,1/2] datapath: Properly set L4 keys on "later" IP fragments | expand |
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 <gvrose8192@gmail.com> Lines checked: 58, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
> On Aug 29, 2019, at 10:55 AM, Greg Rose <gvrose8192@gmail.com> 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(&key->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(&key->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
On 8/29/2019 3:25 PM, Justin Pettit wrote: >> On Aug 29, 2019, at 10:55 AM, Greg Rose <gvrose8192@gmail.com> 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(&key->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(&key->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
> On Aug 29, 2019, at 6:46 PM, Gregory Rose <gvrose8192@gmail.com> wrote: > > > On 8/29/2019 3:25 PM, Justin Pettit wrote: >>> On Aug 29, 2019, at 10:55 AM, Greg Rose <gvrose8192@gmail.com> 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(&key->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(&key->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? --Justin
On 8/29/2019 10:44 PM, Justin Pettit wrote: >> On Aug 29, 2019, at 6:46 PM, Gregory Rose <gvrose8192@gmail.com> wrote: >> >> >> On 8/29/2019 3:25 PM, Justin Pettit wrote: >>>> On Aug 29, 2019, at 10:55 AM, Greg Rose <gvrose8192@gmail.com> 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(&key->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(&key->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
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(&key->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(&key->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;