[ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition in VXLAN

2016-04-15 Thread Paul Boca 
Instance ID flag must be set to 1 in case of valid VXLAN id

Signed-off-by: Paul-Daniel Boca 
---
 datapath-windows/ovsext/Vxlan.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c
index b89c032..20214cb 100644
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -304,10 +304,8 @@ OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
 vxlanHdr->locallyReplicate = 0;
 vxlanHdr->flags2 = 0;
 vxlanHdr->reserved1 = 0;
-if (tunKey->flags | OVS_TNL_F_KEY) {
-vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
-vxlanHdr->instanceID = 1;
-}
+vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
+vxlanHdr->instanceID = 1;
 vxlanHdr->reserved2 = 0;
 }
 return STATUS_SUCCESS;
-- 
2.7.2.windows.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition in VXLAN

2016-04-15 Thread Nithin Raju 
Paul,
If you are sure that tunKey->tunnelId will always be set, it would be a
good idea to mark .optional as FALSE in definition of
nlFlowTunnelKeyPolicy.

-- Nithin

-Original Message-
From: dev  on behalf of Paul Boca

Date: Friday, April 15, 2016 at 8:04 AM
To: "dev@openvswitch.org" 
Subject: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always
true    condition in VXLAN

>Instance ID flag must be set to 1 in case of valid VXLAN id
>
>Signed-off-by: Paul-Daniel Boca 
>---
> datapath-windows/ovsext/Vxlan.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Vxlan.c
>b/datapath-windows/ovsext/Vxlan.c
>index b89c032..20214cb 100644
>--- a/datapath-windows/ovsext/Vxlan.c
>+++ b/datapath-windows/ovsext/Vxlan.c
>@@ -304,10 +304,8 @@ OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
> vxlanHdr->locallyReplicate = 0;
> vxlanHdr->flags2 = 0;
> vxlanHdr->reserved1 = 0;
>-if (tunKey->flags | OVS_TNL_F_KEY) {
>-vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
>-vxlanHdr->instanceID = 1;
>-}
>+vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
>+vxlanHdr->instanceID = 1;
> vxlanHdr->reserved2 = 0;
> }
> return STATUS_SUCCESS;
>-- 
>2.7.2.windows.1
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pN
>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=L790khoAaFh_SQ9evsnFC1zxBU5uiJ
>5XMRpbbvbPiII&s=GJyKbM5YwP_YPJNP_puR1CGbF_kf0nNHuTBdb6rvDGQ&e= 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition in VXLAN

2016-04-15 Thread Jesse Gross 
On Fri, Apr 15, 2016 at 10:27 AM, Nithin Raju  wrote:
> Paul,
> If you are sure that tunKey->tunnelId will always be set, it would be a
> good idea to mark .optional as FALSE in definition of
> nlFlowTunnelKeyPolicy.

For tunnels in general, the key won't always be present. (GRE is an
example of where the key is optional in the protocol itself and this
is reflected in what would be received from netlink.)

In the specific case of VXLAN, though, it is true that the instance ID
bit must always be set.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition in VXLAN

2016-04-15 Thread Nithin Raju 
-Original Message-
From: Jesse Gross 
Date: Friday, April 15, 2016 at 10:40 AM
To: Nithin Raju 
Cc: Paul Boca , "dev@openvswitch.org"

Subject: Re: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true
condition in VXLAN

>On Fri, Apr 15, 2016 at 10:27 AM, Nithin Raju  wrote:
>> Paul,
>> If you are sure that tunKey->tunnelId will always be set, it would be a
>> good idea to mark .optional as FALSE in definition of
>> nlFlowTunnelKeyPolicy.
>
>For tunnels in general, the key won't always be present. (GRE is an
>example of where the key is optional in the protocol itself and this
>is reflected in what would be received from netlink.)
>
>In the specific case of VXLAN, though, it is true that the instance ID
>bit must always be set.

Thanks for the input.

That calls for more checks while adding the port and perhaps an ASSERT
during VXLAN encap to make sure there¹s a valid Tunnel ID.

-- Nithin

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition in VXLAN

2016-04-15 Thread Jesse Gross 
On Fri, Apr 15, 2016 at 10:56 AM, Nithin Raju  wrote:
> -Original Message-
> From: Jesse Gross 
> Date: Friday, April 15, 2016 at 10:40 AM
> To: Nithin Raju 
> Cc: Paul Boca , "dev@openvswitch.org"
> 
> Subject: Re: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true
> condition in VXLAN
>
>>On Fri, Apr 15, 2016 at 10:27 AM, Nithin Raju  wrote:
>>> Paul,
>>> If you are sure that tunKey->tunnelId will always be set, it would be a
>>> good idea to mark .optional as FALSE in definition of
>>> nlFlowTunnelKeyPolicy.
>>
>>For tunnels in general, the key won't always be present. (GRE is an
>>example of where the key is optional in the protocol itself and this
>>is reflected in what would be received from netlink.)
>>
>>In the specific case of VXLAN, though, it is true that the instance ID
>>bit must always be set.
>
> Thanks for the input.
>
> That calls for more checks while adding the port and perhaps an ASSERT
> during VXLAN encap to make sure there¹s a valid Tunnel ID.

I think it's a bit heavyweight to have an assertion here - the kernel
should be able to deal with userspace that is buggy or malicious in a
graceful manner. If userspace doesn't provide a value for the tunnel
ID for protocols that require one, then it seems like the cleanest
thing to do is just fill in zero. (This is what Linux does.) I believe
that's what the original patch does, so it seems fine to me as is.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition in VXLAN

2016-04-18 Thread Sorin Vinturis 
Acked-by: Sorin Vinturis 

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Paul Boca
Sent: Friday, 15 April, 2016 18:05
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition 
in VXLAN

Instance ID flag must be set to 1 in case of valid VXLAN id

Signed-off-by: Paul-Daniel Boca 
---
 datapath-windows/ovsext/Vxlan.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c 
index b89c032..20214cb 100644
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -304,10 +304,8 @@ OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
 vxlanHdr->locallyReplicate = 0;
 vxlanHdr->flags2 = 0;
 vxlanHdr->reserved1 = 0;
-if (tunKey->flags | OVS_TNL_F_KEY) {
-vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
-vxlanHdr->instanceID = 1;
-}
+vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
+vxlanHdr->instanceID = 1;
 vxlanHdr->reserved2 = 0;
 }
 return STATUS_SUCCESS;
--
2.7.2.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