diff mbox

[ovs-dev,4/4,v2] datapath-windows: remove extract flow in OvsDoRecirc()

Message ID 1463505322-3008-4-git-send-email-nithin@vmware.com
State Accepted
Headers show

Commit Message

Nithin Raju May 17, 2016, 5:15 p.m. UTC 
It is not necessary to do a flow extract in OvsDoRecirc().
In fact, doing it would overwrite the tunnel key within
'key'. So, let's remove the call.

Signed-off-by: Nithin Raju <nithin@vmware.com>
Signed-off-by: Sairam Venugopal <vsairam@vmware.com>
Co-Authored-by: Sairam Venugopal <vsairam@vmware.com>
---
 datapath-windows/ovsext/Actions.c | 9 ---------
 1 file changed, 9 deletions(-)

Comments

Sairam Venugopal May 17, 2016, 6:05 p.m. UTC | #1 
Acked-by: Sairam Venugopal <vsairam@vmware.com>


On 5/17/16, 10:15 AM, "Nithin Raju" <nithin@vmware.com> wrote:

>It is not necessary to do a flow extract in OvsDoRecirc().
>In fact, doing it would overwrite the tunnel key within
>'key'. So, let's remove the call.
>
>Signed-off-by: Nithin Raju <nithin@vmware.com>
>Signed-off-by: Sairam Venugopal <vsairam@vmware.com>
>Co-Authored-by: Sairam Venugopal <vsairam@vmware.com>
>---
> datapath-windows/ovsext/Actions.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Actions.c
>b/datapath-windows/ovsext/Actions.c
>index 5ad29ee..4edf7d0 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -1989,15 +1989,6 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
>                  
>NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl),
>                          completionList, layers, TRUE);
> 
>-    status = OvsExtractFlow(ovsFwdCtx.curNbl, ovsFwdCtx.srcVportNo, key,
>-                            &ovsFwdCtx.layers, NULL);
>-    if (status != NDIS_STATUS_SUCCESS) {
>-        OvsCompleteNBLForwardingCtx(&ovsFwdCtx,
>-            L"OVS-Dropped due to extract flow failure");
>-        ovsActionStats.failedFlowMiss++;
>-        return NDIS_STATUS_FAILURE;
>-    }
>-
>     flow = OvsLookupFlow(&ovsFwdCtx.switchContext->datapath, key, &hash,
>FALSE);
>     if (flow) {
>         UINT32 level = OvsDeferredActionsLevelGet();
>-- 
>2.7.1.windows.1
>
Paul Boca May 23, 2016, 6:02 a.m. UTC | #2 
Looks good to me.

Acked-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com>


> -----Original Message-----

> From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Sairam

> Venugopal

> Sent: Tuesday, May 17, 2016 9:06 PM

> To: Nithin Raju; dev@openvswitch.org

> Subject: Re: [ovs-dev] [PATCH 4/4 v2] datapath-windows: remove extract flow

> in OvsDoRecirc()

> 

> Acked-by: Sairam Venugopal <vsairam@vmware.com>

> 

> 

> On 5/17/16, 10:15 AM, "Nithin Raju" <nithin@vmware.com> wrote:

> 

> >It is not necessary to do a flow extract in OvsDoRecirc().

> >In fact, doing it would overwrite the tunnel key within

> >'key'. So, let's remove the call.

> >

> >Signed-off-by: Nithin Raju <nithin@vmware.com>

> >Signed-off-by: Sairam Venugopal <vsairam@vmware.com>

> >Co-Authored-by: Sairam Venugopal <vsairam@vmware.com>

> >---

> > datapath-windows/ovsext/Actions.c | 9 ---------

> > 1 file changed, 9 deletions(-)

> >

> >diff --git a/datapath-windows/ovsext/Actions.c

> >b/datapath-windows/ovsext/Actions.c

> >index 5ad29ee..4edf7d0 100644

> >--- a/datapath-windows/ovsext/Actions.c

> >+++ b/datapath-windows/ovsext/Actions.c

> >@@ -1989,15 +1989,6 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT

> switchContext,

> >

> >NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl),

> >                          completionList, layers, TRUE);

> >

> >-    status = OvsExtractFlow(ovsFwdCtx.curNbl, ovsFwdCtx.srcVportNo, key,

> >-                            &ovsFwdCtx.layers, NULL);

> >-    if (status != NDIS_STATUS_SUCCESS) {

> >-        OvsCompleteNBLForwardingCtx(&ovsFwdCtx,

> >-            L"OVS-Dropped due to extract flow failure");

> >-        ovsActionStats.failedFlowMiss++;

> >-        return NDIS_STATUS_FAILURE;

> >-    }

> >-

> >     flow = OvsLookupFlow(&ovsFwdCtx.switchContext->datapath, key, &hash,

> >FALSE);

> >     if (flow) {

> >         UINT32 level = OvsDeferredActionsLevelGet();

> >--

> >2.7.1.windows.1

> >

> 

> _______________________________________________

> dev mailing list

> dev@openvswitch.org

> http://openvswitch.org/mailman/listinfo/dev
Joe Stringer May 25, 2016, 5:35 p.m. UTC | #3 
Thanks all, applied.

On 22 May 2016 at 23:02, Paul Boca <pboca@cloudbasesolutions.com> wrote:
> Looks good to me.
>
> Acked-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Sairam
>> Venugopal
>> Sent: Tuesday, May 17, 2016 9:06 PM
>> To: Nithin Raju; dev@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH 4/4 v2] datapath-windows: remove extract flow
>> in OvsDoRecirc()
>>
>> Acked-by: Sairam Venugopal <vsairam@vmware.com>
>>
>>
>> On 5/17/16, 10:15 AM, "Nithin Raju" <nithin@vmware.com> wrote:
>>
>> >It is not necessary to do a flow extract in OvsDoRecirc().
>> >In fact, doing it would overwrite the tunnel key within
>> >'key'. So, let's remove the call.
>> >
>> >Signed-off-by: Nithin Raju <nithin@vmware.com>
>> >Signed-off-by: Sairam Venugopal <vsairam@vmware.com>
>> >Co-Authored-by: Sairam Venugopal <vsairam@vmware.com>
>> >---
>> > datapath-windows/ovsext/Actions.c | 9 ---------
>> > 1 file changed, 9 deletions(-)
>> >
>> >diff --git a/datapath-windows/ovsext/Actions.c
>> >b/datapath-windows/ovsext/Actions.c
>> >index 5ad29ee..4edf7d0 100644
>> >--- a/datapath-windows/ovsext/Actions.c
>> >+++ b/datapath-windows/ovsext/Actions.c
>> >@@ -1989,15 +1989,6 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT
>> switchContext,
>> >
>> >NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl),
>> >                          completionList, layers, TRUE);
>> >
>> >-    status = OvsExtractFlow(ovsFwdCtx.curNbl, ovsFwdCtx.srcVportNo, key,
>> >-                            &ovsFwdCtx.layers, NULL);
>> >-    if (status != NDIS_STATUS_SUCCESS) {
>> >-        OvsCompleteNBLForwardingCtx(&ovsFwdCtx,
>> >-            L"OVS-Dropped due to extract flow failure");
>> >-        ovsActionStats.failedFlowMiss++;
>> >-        return NDIS_STATUS_FAILURE;
>> >-    }
>> >-
>> >     flow = OvsLookupFlow(&ovsFwdCtx.switchContext->datapath, key, &hash,
>> >FALSE);
>> >     if (flow) {
>> >         UINT32 level = OvsDeferredActionsLevelGet();
>> >--
>> >2.7.1.windows.1
>> >
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Alin Serdean May 25, 2016, 5:38 p.m. UTC | #4 
We need to update the "layers" after an action. Think about the following scenario:
Mpls_pop,recirc
We do not update the "layers" after mpls_pop so we will have the initial values.
That is why we did another flow extract just to update the layers.

Alin.

> -----Mesaj original-----

> De la: dev [mailto:dev-bounces@openvswitch.org] În numele Nithin Raju

> Trimis: Tuesday, May 17, 2016 8:15 PM

> Către: dev@openvswitch.org

> Subiect: [ovs-dev] [PATCH 4/4 v2] datapath-windows: remove extract flow in

> OvsDoRecirc()

> 

> It is not necessary to do a flow extract in OvsDoRecirc().

> In fact, doing it would overwrite the tunnel key within 'key'. So, let's remove

> the call.

> 

> Signed-off-by: Nithin Raju <nithin@vmware.com>

> Signed-off-by: Sairam Venugopal <vsairam@vmware.com>

> Co-Authored-by: Sairam Venugopal <vsairam@vmware.com>

> ---

>  datapath-windows/ovsext/Actions.c | 9 ---------

>  1 file changed, 9 deletions(-)

> 

> diff --git a/datapath-windows/ovsext/Actions.c b/datapath-

> windows/ovsext/Actions.c

> index 5ad29ee..4edf7d0 100644

> --- a/datapath-windows/ovsext/Actions.c

> +++ b/datapath-windows/ovsext/Actions.c

> @@ -1989,15 +1989,6 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT

> switchContext,

>                           NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl),

>                           completionList, layers, TRUE);

> 

> -    status = OvsExtractFlow(ovsFwdCtx.curNbl, ovsFwdCtx.srcVportNo, key,

> -                            &ovsFwdCtx.layers, NULL);

> -    if (status != NDIS_STATUS_SUCCESS) {

> -        OvsCompleteNBLForwardingCtx(&ovsFwdCtx,

> -            L"OVS-Dropped due to extract flow failure");

> -        ovsActionStats.failedFlowMiss++;

> -        return NDIS_STATUS_FAILURE;

> -    }

> -

>      flow = OvsLookupFlow(&ovsFwdCtx.switchContext->datapath, key,

> &hash, FALSE);

>      if (flow) {

>          UINT32 level = OvsDeferredActionsLevelGet();

> --

> 2.7.1.windows.1

> 

> _______________________________________________

> dev mailing list

> dev@openvswitch.org

> http://openvswitch.org/mailman/listinfo/dev
diff mbox

Patch

diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index 5ad29ee..4edf7d0 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1989,15 +1989,6 @@  OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
                          NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl),
                          completionList, layers, TRUE);
 
-    status = OvsExtractFlow(ovsFwdCtx.curNbl, ovsFwdCtx.srcVportNo, key,
-                            &ovsFwdCtx.layers, NULL);
-    if (status != NDIS_STATUS_SUCCESS) {
-        OvsCompleteNBLForwardingCtx(&ovsFwdCtx,
-            L"OVS-Dropped due to extract flow failure");
-        ovsActionStats.failedFlowMiss++;
-        return NDIS_STATUS_FAILURE;
-    }
-
     flow = OvsLookupFlow(&ovsFwdCtx.switchContext->datapath, key, &hash, FALSE);
     if (flow) {
         UINT32 level = OvsDeferredActionsLevelGet();