diff mbox

[ovs-dev] datapath-windows: Fix various Geneve bugs

Message ID 1468466479-23000-1-git-send-email-linyi@vmware.com
State Accepted
Delegated to: Guru Shetty
Headers show

Commit Message

Yin Lin July 14, 2016, 3:21 a.m. UTC 
Signed-off-by: Yin Lin <linyi@vmware.com>
---
 datapath-windows/ovsext/Flow.c  | 2 +-
 datapath-windows/ovsext/Vport.c | 3 ++-
 datapath-windows/ovsext/Vport.h | 5 +++++
 3 files changed, 8 insertions(+), 2 deletions(-)

Comments

Nithin Raju July 14, 2016, 5:28 a.m. UTC | #1 
-----Original Message-----
From: dev <dev-bounces@openvswitch.org> on behalf of Yin Lin
<linyi@vmware.com>
Date: Wednesday, July 13, 2016 at 8:21 PM
To: "dev@openvswitch.org" <dev@openvswitch.org>
Cc: Yin Lin <linyi@vmware.com>
Subject: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs

>Signed-off-by: Yin Lin <linyi@vmware.com>

Acked-by: Nithin Raju <nithin@vmware.com>
Alin Serdean July 21, 2016, 1:29 p.m. UTC | #2 
Hi Yin,

Thanks for the patch one. Just one small question from me inlined.

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

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

> Trimis: Thursday, July 14, 2016 6:21 AM

> Către: dev@openvswitch.org

> Cc: Yin Lin <linyi@vmware.com>

> Subiect: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs

> 

> Signed-off-by: Yin Lin <linyi@vmware.com>

> ---

>  datapath-windows/ovsext/Flow.c  | 2 +-

>  datapath-windows/ovsext/Vport.c | 3 ++-  datapath-

> windows/ovsext/Vport.h | 5 +++++

>  3 files changed, 8 insertions(+), 2 deletions(-)

> -                if (vport->ovsType != OVS_VPORT_TYPE_VXLAN) {

> +                if (vport->ovsType != OVS_VPORT_TYPE_GENEVE &&

[Alin Gabriel Serdean: ] Shouldn't it be a || ("vport->ovsType != OVS_VPORT_TYPE_GENEVE && ||") ?
> +                    vport->ovsType != OVS_VPORT_TYPE_VXLAN) {

>                      continue;

>                  }

>                  break;

> diff --git a/datapath-windows/ovsext/Vport.h b/datapath-

> windows/ovsext/Vport.h index f0a9acd..1f4968e 100644

> --- a/datapath-windows/ovsext/Vport.h

> +++ b/datapath-windows/ovsext/Vport.h

> @@ -21,6 +21,7 @@
Alin Serdean July 21, 2016, 2 p.m. UTC | #3 
> -----Mesaj original-----

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

> Trimis: Thursday, July 21, 2016 4:30 PM

> Către: Yin Lin <linyi@vmware.com>; dev@openvswitch.org

> Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs

> 

> Hi Yin,

> 

> Thanks for the patch one. Just one small question from me inlined.

> 

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

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

> > Trimis: Thursday, July 14, 2016 6:21 AM

> > Către: dev@openvswitch.org

> > Cc: Yin Lin <linyi@vmware.com>

> > Subiect: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs

> >

> > Signed-off-by: Yin Lin <linyi@vmware.com>

> > ---

> >  datapath-windows/ovsext/Flow.c  | 2 +-

> > datapath-windows/ovsext/Vport.c | 3 ++-  datapath-

> > windows/ovsext/Vport.h | 5 +++++

> >  3 files changed, 8 insertions(+), 2 deletions(-)

> > -                if (vport->ovsType != OVS_VPORT_TYPE_VXLAN) {

> > +                if (vport->ovsType != OVS_VPORT_TYPE_GENEVE &&

> [Alin Gabriel Serdean: ] Shouldn't it be a || ("vport->ovsType !=

> OVS_VPORT_TYPE_GENEVE ||") ?

[Alin Gabriel Serdean: ] Small edit "vport->ovsType != OVS_VPORT_TYPE_GENEVE ||"
> > +                    vport->ovsType != OVS_VPORT_TYPE_VXLAN) {

> >                      continue;

> >                  }

> >                  break;

> > diff --git a/datapath-windows/ovsext/Vport.h b/datapath-

> > windows/ovsext/Vport.h index f0a9acd..1f4968e 100644

> > --- a/datapath-windows/ovsext/Vport.h

> > +++ b/datapath-windows/ovsext/Vport.h

> > @@ -21,6 +21,7 @@

> 

> _______________________________________________

> dev mailing list

> dev@openvswitch.org

> http://openvswitch.org/mailman/listinfo/dev
Yin Lin July 21, 2016, 5:02 p.m. UTC | #4 
Hi Alin,

I know this can be confusing but after a second thought I think mine is
correct. Your suggestion is equivalent to (abbreviated):
!(port == geneve && port == vxlan)
which is always TRUE because port can not be both type at the same time.

Thanks,
Yin

On Thu, Jul 21, 2016 at 7:00 AM, Alin Serdean <
aserdean@cloudbasesolutions.com> wrote:

>
>
> > -----Mesaj original-----
> > De la: dev [mailto:dev-bounces@openvswitch.org] În numele Alin Serdean
> > Trimis: Thursday, July 21, 2016 4:30 PM
> > Către: Yin Lin <linyi@vmware.com>; dev@openvswitch.org
> > Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs
> >
> > Hi Yin,
> >
> > Thanks for the patch one. Just one small question from me inlined.
> >
> > > -----Mesaj original-----
> > > De la: dev [mailto:dev-bounces@openvswitch.org] În numele Yin Lin
> > > Trimis: Thursday, July 14, 2016 6:21 AM
> > > Către: dev@openvswitch.org
> > > Cc: Yin Lin <linyi@vmware.com>
> > > Subiect: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs
> > >
> > > Signed-off-by: Yin Lin <linyi@vmware.com>
> > > ---
> > >  datapath-windows/ovsext/Flow.c  | 2 +-
> > > datapath-windows/ovsext/Vport.c | 3 ++-  datapath-
> > > windows/ovsext/Vport.h | 5 +++++
> > >  3 files changed, 8 insertions(+), 2 deletions(-)
> > > -                if (vport->ovsType != OVS_VPORT_TYPE_VXLAN) {
> > > +                if (vport->ovsType != OVS_VPORT_TYPE_GENEVE &&
> > [Alin Gabriel Serdean: ] Shouldn't it be a || ("vport->ovsType !=
> > OVS_VPORT_TYPE_GENEVE ||") ?
> [Alin Gabriel Serdean: ] Small edit "vport->ovsType !=
> OVS_VPORT_TYPE_GENEVE ||"
> > > +                    vport->ovsType != OVS_VPORT_TYPE_VXLAN) {
> > >                      continue;
> > >                  }
> > >                  break;
> > > diff --git a/datapath-windows/ovsext/Vport.h b/datapath-
> > > windows/ovsext/Vport.h index f0a9acd..1f4968e 100644
> > > --- a/datapath-windows/ovsext/Vport.h
> > > +++ b/datapath-windows/ovsext/Vport.h
> > > @@ -21,6 +21,7 @@
> >
> > _______________________________________________
> > 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 July 21, 2016, 5:41 p.m. UTC | #5 
Hi Yin,

Thanks for the response, you are correct. I lost from overview what we are testing for ☺.

Acked-by: Alin Gabriel Serdean aserdean@cloudbasesolutions.com<mailto:aserdean@cloudbasesolutions.com>


De la: Yin Lin [mailto:yinlin10@gmail.com]
Trimis: Thursday, July 21, 2016 8:02 PM
Către: Alin Serdean <aserdean@cloudbasesolutions.com>
Cc: Yin Lin <linyi@vmware.com>; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs

Hi Alin,

I know this can be confusing but after a second thought I think mine is correct. Your suggestion is equivalent to (abbreviated):
!(port == geneve && port == vxlan)
which is always TRUE because port can not be both type at the same time.

Thanks,
Yin

On Thu, Jul 21, 2016 at 7:00 AM, Alin Serdean <aserdean@cloudbasesolutions.com<mailto:aserdean@cloudbasesolutions.com>> wrote:


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

> De la: dev [mailto:dev-bounces@openvswitch.org<mailto:dev-bounces@openvswitch.org>] În numele Alin Serdean

> Trimis: Thursday, July 21, 2016 4:30 PM

> Către: Yin Lin <linyi@vmware.com<mailto:linyi@vmware.com>>; dev@openvswitch.org<mailto:dev@openvswitch.org>

> Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs

>

> Hi Yin,

>

> Thanks for the patch one. Just one small question from me inlined.

>

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

> > De la: dev [mailto:dev-bounces@openvswitch.org<mailto:dev-bounces@openvswitch.org>] În numele Yin Lin

> > Trimis: Thursday, July 14, 2016 6:21 AM

> > Către: dev@openvswitch.org<mailto:dev@openvswitch.org>

> > Cc: Yin Lin <linyi@vmware.com<mailto:linyi@vmware.com>>

> > Subiect: [ovs-dev] [PATCH] datapath-windows: Fix various Geneve bugs

> >

> > Signed-off-by: Yin Lin <linyi@vmware.com<mailto:linyi@vmware.com>>

> > ---

> >  datapath-windows/ovsext/Flow.c  | 2 +-

> > datapath-windows/ovsext/Vport.c | 3 ++-  datapath-

> > windows/ovsext/Vport.h | 5 +++++

> >  3 files changed, 8 insertions(+), 2 deletions(-)

> > -                if (vport->ovsType != OVS_VPORT_TYPE_VXLAN) {

> > +                if (vport->ovsType != OVS_VPORT_TYPE_GENEVE &&

> [Alin Gabriel Serdean: ] Shouldn't it be a || ("vport->ovsType !=

> OVS_VPORT_TYPE_GENEVE ||") ?

[Alin Gabriel Serdean: ] Small edit "vport->ovsType != OVS_VPORT_TYPE_GENEVE ||"
> > +                    vport->ovsType != OVS_VPORT_TYPE_VXLAN) {

> >                      continue;

> >                  }

> >                  break;

> > diff --git a/datapath-windows/ovsext/Vport.h b/datapath-

> > windows/ovsext/Vport.h index f0a9acd..1f4968e 100644

> > --- a/datapath-windows/ovsext/Vport.h

> > +++ b/datapath-windows/ovsext/Vport.h

> > @@ -21,6 +21,7 @@

>

> _______________________________________________

> dev mailing list

> dev@openvswitch.org<mailto:dev@openvswitch.org>

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

_______________________________________________
dev mailing list
dev@openvswitch.org<mailto:dev@openvswitch.org>
http://openvswitch.org/mailman/listinfo/dev
Gurucharan Shetty July 21, 2016, 6:28 p.m. UTC | #6 
On 13 July 2016 at 20:21, Yin Lin <linyi@vmware.com> wrote:

> Signed-off-by: Yin Lin <linyi@vmware.com>
>
Thank you, applied.


> ---
>  datapath-windows/ovsext/Flow.c  | 2 +-
>  datapath-windows/ovsext/Vport.c | 3 ++-
>  datapath-windows/ovsext/Vport.h | 5 +++++
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Flow.c
> b/datapath-windows/ovsext/Flow.c
> index bc0bb37..7a57f96 100644
> --- a/datapath-windows/ovsext/Flow.c
> +++ b/datapath-windows/ovsext/Flow.c
> @@ -1683,7 +1683,7 @@ OvsTunnelAttrToGeneveOptions(PNL_ATTR attr,
>          option = (GeneveOptionHdr *)((UINT8 *)option + len);
>          optLen -= len;
>      }
> -    memcpy(TunnelKeyGetOptions(tunKey), option, optLen);
> +    memcpy(TunnelKeyGetOptions(tunKey), NlAttrData(attr),
> tunKey->tunOptLen);
>      if (isCritical) {
>          tunKey->flags |= OVS_TNL_F_CRT_OPT;
>      }
> diff --git a/datapath-windows/ovsext/Vport.c
> b/datapath-windows/ovsext/Vport.c
> index 1462453..22741db 100644
> --- a/datapath-windows/ovsext/Vport.c
> +++ b/datapath-windows/ovsext/Vport.c
> @@ -724,7 +724,8 @@
> OvsFindTunnelVportByDstPortAndNWProto(POVS_SWITCH_CONTEXT switchContext,
>          if (GetPortFromPriv(vport) == dstPort) {
>              switch (nwProto) {
>              case IPPROTO_UDP:
> -                if (vport->ovsType != OVS_VPORT_TYPE_VXLAN) {
> +                if (vport->ovsType != OVS_VPORT_TYPE_GENEVE &&
> +                    vport->ovsType != OVS_VPORT_TYPE_VXLAN) {
>                      continue;
>                  }
>                  break;
> diff --git a/datapath-windows/ovsext/Vport.h
> b/datapath-windows/ovsext/Vport.h
> index f0a9acd..1f4968e 100644
> --- a/datapath-windows/ovsext/Vport.h
> +++ b/datapath-windows/ovsext/Vport.h
> @@ -21,6 +21,7 @@
>  #include "Stt.h"
>  #include "Switch.h"
>  #include "VxLan.h"
> +#include "Geneve.h"
>
>  #define OVS_MAX_DPPORTS             MAXUINT16
>  #define OVS_DPPORT_NUMBER_INVALID   OVS_MAX_DPPORTS
> @@ -183,6 +184,7 @@ static __inline BOOLEAN
>  OvsIsTunnelVportType(OVS_VPORT_TYPE ovsType)
>  {
>      return ovsType == OVS_VPORT_TYPE_VXLAN ||
> +           ovsType == OVS_VPORT_TYPE_GENEVE ||
>             ovsType == OVS_VPORT_TYPE_STT ||
>             ovsType == OVS_VPORT_TYPE_GRE;
>  }
> @@ -270,6 +272,9 @@ GetPortFromPriv(POVS_VPORT_ENTRY vport)
>      case OVS_VPORT_TYPE_VXLAN:
>          dstPort = ((POVS_VXLAN_VPORT)vportPriv)->dstPort;
>          break;
> +    case OVS_VPORT_TYPE_GENEVE:
> +        dstPort = ((POVS_GENEVE_VPORT) vportPriv)->dstPort;
> +        break;
>      default:
>          ASSERT(! "Port is not a tunnel port");
>      }
> --
> 2.8.0.windows.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
diff mbox

Patch

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index bc0bb37..7a57f96 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -1683,7 +1683,7 @@  OvsTunnelAttrToGeneveOptions(PNL_ATTR attr,
         option = (GeneveOptionHdr *)((UINT8 *)option + len);
         optLen -= len;
     }
-    memcpy(TunnelKeyGetOptions(tunKey), option, optLen);
+    memcpy(TunnelKeyGetOptions(tunKey), NlAttrData(attr), tunKey->tunOptLen);
     if (isCritical) {
         tunKey->flags |= OVS_TNL_F_CRT_OPT;
     }
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 1462453..22741db 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -724,7 +724,8 @@  OvsFindTunnelVportByDstPortAndNWProto(POVS_SWITCH_CONTEXT switchContext,
         if (GetPortFromPriv(vport) == dstPort) {
             switch (nwProto) {
             case IPPROTO_UDP:
-                if (vport->ovsType != OVS_VPORT_TYPE_VXLAN) {
+                if (vport->ovsType != OVS_VPORT_TYPE_GENEVE &&
+                    vport->ovsType != OVS_VPORT_TYPE_VXLAN) {
                     continue;
                 }
                 break;
diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h
index f0a9acd..1f4968e 100644
--- a/datapath-windows/ovsext/Vport.h
+++ b/datapath-windows/ovsext/Vport.h
@@ -21,6 +21,7 @@ 
 #include "Stt.h"
 #include "Switch.h"
 #include "VxLan.h"
+#include "Geneve.h"
 
 #define OVS_MAX_DPPORTS             MAXUINT16
 #define OVS_DPPORT_NUMBER_INVALID   OVS_MAX_DPPORTS
@@ -183,6 +184,7 @@  static __inline BOOLEAN
 OvsIsTunnelVportType(OVS_VPORT_TYPE ovsType)
 {
     return ovsType == OVS_VPORT_TYPE_VXLAN ||
+           ovsType == OVS_VPORT_TYPE_GENEVE ||
            ovsType == OVS_VPORT_TYPE_STT ||
            ovsType == OVS_VPORT_TYPE_GRE;
 }
@@ -270,6 +272,9 @@  GetPortFromPriv(POVS_VPORT_ENTRY vport)
     case OVS_VPORT_TYPE_VXLAN:
         dstPort = ((POVS_VXLAN_VPORT)vportPriv)->dstPort;
         break;
+    case OVS_VPORT_TYPE_GENEVE:
+        dstPort = ((POVS_GENEVE_VPORT) vportPriv)->dstPort;
+        break;
     default:
         ASSERT(! "Port is not a tunnel port");
     }