Re: [ovs-dev] [PATCH] ofp-actions: Report an error if there are too many actions to parse.

[aserdean]

From: Alin-Gabriel Serdean 

>Not a very important fix, but fuzzer times out trying to test parsing
>of a huge number of actions.  Fixing that by reporting an error as
>soon as ofpacts oversized.
>
>It would be great to use ofpacts_oversized() function instead of manual
>size checking, but ofpacts->header here always points to the last
>pushed action, so the value that ofpacts_oversized() would check is
>always small.
>
>Adding a unit test for this, plus the extra test for too deep nesting.
>
>Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20254
>Signed-off-by: Ilya Maximets 
>
Acked-by: Alin-Gabriel Serdean 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Python: Fix Idl.run change_seqno update

[aserdean]

From: Alin Gabriel Serdean 

>Fix an issue where Idl.run() returned False even if there was a change.
>If Idl.run() reads multiple messages from the database server, some
>may constitute changes and some may not. Changed the way change_seqno
>is reset: if a message is not a change, reset change_seqno only to the
>value before reading this message, not to the value before reading the
>first message.
>This will fix the return value in a scenario where some message was a
>change and the last one wasn't. The new change_seqno will now be the
>value after handling the message with the last change.
>
>Signed-off-by: Bodo Petermann 
>---
>
Acked-by: Alin Gabriel Serdean 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 4/9] ct-dpif, dpif-netlink: Add conntrack timeout policy support

[aserdean]

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> On Behalf Of Justin Pettit
> Sent: Tuesday, August 6, 2019 8:28 AM
> To: Yi-Hung Wei 
> Cc: ovs dev ; Alin Gabriel Serdean
> 
> Subject: Re: [ovs-dev] [PATCH v2 4/9] ct-dpif, dpif-netlink: Add conntrack
> timeout policy support
> 
> 
> > On Aug 1, 2019, at 3:07 PM, Yi-Hung Wei  wrote:
> >
> > This patch also adds missing symbols in the windows datapath so that
> > the build on windows can pass.
> 
> Alin, can you do a quick sanity-check on the Windows part?  Thanks.
[Alin] Sorry for the delay. I replied here:
https://patchwork.ozlabs.org/patch/1145968/#2236957

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

Re: [ovs-dev] [PATCH v3 4/9] ct-dpif, dpif-netlink: Add conntrack timeout policy support

[aserdean]

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> On Behalf Of Yi-Hung Wei
> Sent: Tuesday, August 13, 2019 3:52 AM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH v3 4/9] ct-dpif, dpif-netlink: Add conntrack
> timeout policy support
> 
> This patch first defines the dpif interface for a datapath to support
> adding, deleting, getting and dumping conntrack timeout policy.
> The timeout policy is identified by a 4 bytes unsigned integer in
> datapath, and it currently support timeout for TCP, UDP, and ICMP
> protocols.
> 
> Moreover, this patch provides the implementation for Linux kernel
> datapath in dpif-netlink.
> 
> In Linux kernel, the timeout policy is maintained per L3/L4 protocol,
> and it is identified by 32 bytes null terminated string.  On the other
> hand, in vswitchd, the timeout policy is a generic one that consists of
> all the supported L4 protocols.  Therefore, one of the main task in
> dpif-netlink is to break down the generic timeout policy into 6
> sub policies (ipv4 tcp, udp, icmp, and ipv6 tcp, udp, icmp),
> and push down the configuration using the netlink API in
> netlink-conntrack.c.
> 
> This patch also adds missing symbols in the windows datapath so
> that the build on windows can pass.
> 
> Appveyor CI:
> * https://ci.appveyor.com/project/YiHungWei/ovs/builds/26387754
> 
> Signed-off-by: Yi-Hung Wei 
> ---
>  Documentation/faq/releases.rst |   3 +-
>  datapath-windows/include/OvsDpInterfaceCtExt.h | 114 +
>  datapath-windows/ovsext/Netlink/NetlinkProto.h |   8 +-
>  include/windows/automake.mk|   1 +
>  .../windows/linux/netfilter/nfnetlink_cttimeout.h  |   0
>  lib/ct-dpif.c  | 104 +
>  lib/ct-dpif.h  |  56 +++
>  lib/dpif-netdev.c  |   6 +
>  lib/dpif-netlink.c | 469
+
>  lib/dpif-netlink.h |   1 -
>  lib/dpif-provider.h|  44 ++
>  lib/netlink-conntrack.c| 308 ++
>  lib/netlink-conntrack.h|  27 +-
>  lib/netlink-protocol.h |   8 +-
>  14 files changed, 1142 insertions(+), 7 deletions(-)
>  create mode 100644 include/windows/linux/netfilter/nfnetlink_cttimeout.h
> 
[Alin] This is not an actual review.

I'm okay with the Windows changes.

I also tested the series and things look good.

Do you mind folding in the following:
diff --git a/datapath-windows/include/OvsDpInterfaceCtExt.h
b/datapath-windows/include/OvsDpInterfaceCtExt.h
index 4379855bb..3379f0a25 100644
--- a/datapath-windows/include/OvsDpInterfaceCtExt.h
+++ b/datapath-windows/include/OvsDpInterfaceCtExt.h
@@ -421,7 +421,7 @@ struct nf_ct_tcp_flags {
 UINT8 mask;
 };

-/* File: nfnetlink_cttimeout.h */
+/* File: nfnetlink_cttimeout.h. XXX: the following are not implemented */
 enum ctnl_timeout_msg_types {
 IPCTNL_MSG_TIMEOUT_NEW,
 IPCTNL_MSG_TIMEOUT_GET,


Acked-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH] [windows][wmi] Switch from internal port to all ports defined

[aserdean]

Thanks! Applied on master and backported back until branch-2.10.

 

Alin.

 

From: Anand Kumar  
Sent: Thursday, August 1, 2019 5:41 AM
To: Alin Serdean ; d...@openvswitch.org
Cc: Danting Liu 
Subject: Re: [ovs-dev] [PATCH] [windows][wmi] Switch from internal port to all 
ports defined

 

Acked-by: Anand Kumar mailto:kumaran...@vmware.com> >

 

Thanks,

Anand Kumar

 

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

Re: [ovs-dev] [PATCH] AUTHORS: Add Dumitru Ceara.

[aserdean]

Subject: [ovs-dev] [PATCH] AUTHORS: Add Dumitru Ceara.

From: Numan Siddique 

Signed-off-by: Numan Siddique 
---
 AUTHORS.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index ee2a3d4e9..d0d9e9ab7 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -118,6 +118,7 @@ Dongdong   dongdo...@huawei.com
 Dongjundo...@dtdream.com
 Duan Jiong djduanji...@gmail.com
 Duffie Cooley
+Dumitru Ceara  dce...@redhat.com
 Dustin Lundquist   dus...@null-ptr.net
 Ed Maste   ema...@freebsd.org
 Ed Swierk  eswi...@skyportsystems.com
-- 

Applied on master!

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

Re: [ovs-dev] [PATCH v2] datapath-windows: Add Win10Analyze target

[aserdean]

Applied on master.

-Original Message-
From: ovs-dev-boun...@openvswitch.org  On 
Behalf Of Anand Kumar via dev
Sent: Wednesday, May 1, 2019 8:04 PM
To: Alin Gabriel Serdean ; d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Add Win10Analyze target

Thanks for the adding windows 10 analyze target.

Acked-by: Anand Kumar 

Regards,
Anand Kumar

On 4/3/19, 10:48 AM, "Alin Gabriel Serdean"  wrote:

This patch adds a new target called `Win10Analyze` to the driver solution.

It enables us to trigger static analysis over the Win10 target.

Since the location of the ruleset of drivers is somewhat random
starting from 1803:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.osr.com%2Fblog%2F2018%2F05%2F21%2Fwdk-1803-ca%2F&data=02%7C01%7Ckumaranand%40vmware.com%7C2580925eae8c4b9a4e4608d6b85c8b60%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636899104959730588&sdata=%2FLesq2PWVRbSB55zyEQbRTrsL2W07s6on8SGlhGTg0w%3D&reserved=0

Commit the ruleset inside our repository. This is the same ruleset used for
8,8.1 and 10.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/Package/package.VcxProj  | 21 -
 datapath-windows/Package/package.VcxProj.user |  5 +-
 datapath-windows/automake.mk  |  2 +
 .../misc/DriverRecommendedRules.ruleset   | 86 +++
 datapath-windows/ovsext.sln   |  5 ++
 datapath-windows/ovsext/ovsext.vcxproj| 39 -
 datapath-windows/ovsext/ovsext.vcxproj.user   |  5 +-
 7 files changed, 159 insertions(+), 4 deletions(-)
 create mode 100644 datapath-windows/misc/DriverRecommendedRules.ruleset

diff --git a/datapath-windows/Package/package.VcxProj 
b/datapath-windows/Package/package.VcxProj
index de747eed2..5939a8816 100644
--- a/datapath-windows/Package/package.VcxProj
+++ b/datapath-windows/Package/package.VcxProj
@@ -9,6 +9,10 @@
   Win10 Release
   x64
 
+
+  Win10Analyze
+  x64
+
 
   Win8.1 Debug
   x64
@@ -71,6 +75,13 @@
 
WindowsKernelModeDriver$(PlatformToolsetVer)
 Desktop
   
+  
+
+true
+10.0
+
WindowsKernelModeDriver$(PlatformToolsetVer)
+Desktop
+  
   
 Windows8
 true
@@ -113,6 +124,9 @@
   
 true
   
+  
+true
+  
   
 DbgengKernelDebugger
 False
@@ -148,6 +162,11 @@
   true
 
   
+  
+
+  true
+
+  
   
 
   true
@@ -185,4 +204,4 @@
   
   
   
-
+
\ No newline at end of file
diff --git a/datapath-windows/Package/package.VcxProj.user 
b/datapath-windows/Package/package.VcxProj.user
index 6231d93f7..5b0c53539 100644
--- a/datapath-windows/Package/package.VcxProj.user
+++ b/datapath-windows/Package/package.VcxProj.user
@@ -15,6 +15,9 @@
   
 TestSign
   
+  
+TestSign
+  
   
 TestSign
   
@@ -24,4 +27,4 @@
   
 TestSign
   
-
+
\ No newline at end of file
diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 3820041f6..b8cf5dd95 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -3,6 +3,7 @@ EXTRA_DIST += \
datapath-windows/Package/package.VcxProj.user \
datapath-windows/include/OvsDpInterfaceExt.h \
datapath-windows/include/OvsDpInterfaceCtExt.h \
+   datapath-windows/misc/DriverRecommendedRules.ruleset \
datapath-windows/misc/OVS.psm1 \
datapath-windows/misc/install.cmd \
datapath-windows/misc/uninstall.cmd \
@@ -86,5 +87,6 @@ EXTRA_DIST += \
datapath-windows/ovsext/resource.h
 
 datapath_windows_analyze: all
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln 
/target:Build /property:Configuration="Win10Analyze"
MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln 
/target:Build /property:Configuration="Win8.1Analyze"
MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln 
/target:Build /property:Configuration="Win8Analyze"
diff --git a/datapath-windows/misc/DriverRecommendedRules.ruleset 
b/datapath-windows/misc/DriverRecommendedRules.ruleset
new file mode 100644
index 0..0faae599c
--- /dev/null
+++ b/datapath-windows/misc/DriverRecommendedRules.ruleset
@@ -0,0 +1,86 @@
+
+
+  
+  
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ 

Re: [ovs-dev] [PATCH v2] datapath-windows: Address memory allocation issues for OVS_BUFFER_CONTEXT

[aserdean]

-Original Message-
From: ovs-dev-boun...@openvswitch.org  On
Behalf Of Anand Kumar via dev
Sent: Thursday, March 21, 2019 1:55 AM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH v2] datapath-windows: Address memory allocation
issues for OVS_BUFFER_CONTEXT

With current implementation, when nbl pool is allocated, context size is
specified as 64 bytes, while the OVS_BUFFER_CONTEXT size is only 32 bytes.
Since context size is never changed, additional memory is not required.

This patch makes it simpler to allocate memory for OVS_BUFFER_CONTEXT so
that it is always aligned to MEMORY_ALLOCATION_ALIGNMENT.
This is acheived by updating "value" field in the context structure, so that
number of elements in array is always a multiple of
MEMORY_ALLOCATION_ALIGNMENT.

Also change the DEFAULT_CONTEXT_SIZE to accomodate OVS_BUFFER_CONTEXT size.

Signed-off-by: Anand Kumar 
---
 
Thanks a lot for implementing this!

Acked-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH v2] python: Fix package requirements with old setuptools

[aserdean]

Thanks for the patch, applied on master!

-Original Message-
From: ovs-dev-boun...@openvswitch.org  On
Behalf Of aserd...@ovn.org
Sent: Friday, March 22, 2019 4:26 PM
To: 'Timothy Redaelli' ; d...@openvswitch.org
Cc: Lucian Petrut 
Subject: Re: [ovs-dev] [PATCH v2] python: Fix package requirements with old
setuptools

Acked-by: Alin Gabriel Serdean 

-Original Message-
From: Timothy Redaelli 
Sent: Friday, March 22, 2019 4:02 PM
To: d...@openvswitch.org
Cc: Alin Serdean ; Lucian Petrut

Subject: [PATCH v2] python: Fix package requirements with old setuptools

Commit 00fcc832d598 ("Update Python package requirements") added a PEP 508
environment marker to install pywin32 on Windows systems.

This requires a new setuptools version (>= 20.5), but (at least)
RHEL/CentOS7 and Debian Jessie are using an older version of setuptools and
so python extension failed to build.

This commit adds "extras_require" instead of the PEP 508 environment markers
in order to have the conditional dependency of pywin32, but by remaining
compatible with the old setuptools versions.

CC: Alin Gabriel Serdean 
CC: Lucian Petrut 
Fixes: 00fcc832d598 ("Update Python package requirements")
Signed-off-by: Timothy Redaelli 
---
v1 -> v2:
- Use the more compatible PEP 496 environment marker "sys_platform"
  instead of "platform_system".
---
 python/setup.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/setup.py b/python/setup.py index 14814ca4a..b7252800c
100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -82,8 +82,8 @@ setup_args = dict(
 ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
   libraries=['openvswitch'])],
 cmdclass={'build_ext': try_build_ext},
-install_requires=['sortedcontainers',
-  'pywin32>=1.0;platform_system=="Windows"'],
+install_requires=['sortedcontainers'],
+extras_require={':sys_platform == "win32"': ['pywin32 >= 1.0']},
 )
 
 try:
--
2.20.1


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

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

Re: [ovs-dev] [PATCH v2] python: Fix package requirements with old setuptools

[aserdean]

Acked-by: Alin Gabriel Serdean 

-Original Message-
From: Timothy Redaelli  
Sent: Friday, March 22, 2019 4:02 PM
To: d...@openvswitch.org
Cc: Alin Serdean ; Lucian Petrut

Subject: [PATCH v2] python: Fix package requirements with old setuptools

Commit 00fcc832d598 ("Update Python package requirements") added a PEP 508
environment marker to install pywin32 on Windows systems.

This requires a new setuptools version (>= 20.5), but (at least)
RHEL/CentOS7 and Debian Jessie are using an older version of setuptools and
so python extension failed to build.

This commit adds "extras_require" instead of the PEP 508 environment markers
in order to have the conditional dependency of pywin32, but by remaining
compatible with the old setuptools versions.

CC: Alin Gabriel Serdean 
CC: Lucian Petrut 
Fixes: 00fcc832d598 ("Update Python package requirements")
Signed-off-by: Timothy Redaelli 
---
v1 -> v2:
- Use the more compatible PEP 496 environment marker "sys_platform"
  instead of "platform_system".
---
 python/setup.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/setup.py b/python/setup.py index 14814ca4a..b7252800c
100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -82,8 +82,8 @@ setup_args = dict(
 ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
   libraries=['openvswitch'])],
 cmdclass={'build_ext': try_build_ext},
-install_requires=['sortedcontainers',
-  'pywin32>=1.0;platform_system=="Windows"'],
+install_requires=['sortedcontainers'],
+extras_require={':sys_platform == "win32"': ['pywin32 >= 1.0']},
 )
 
 try:
--
2.20.1


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

Re: [ovs-dev] [PATCH v2] checkpatch: Normalize exit code for Windows

[aserdean]

> > because of:
> >  ./checkpatch.at:32: exit code was 0, expected 255
> >
> > This patch introduces a positive constant for the default exit code.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> > Acked-by: Ben Pfaff 
> > Acked-by: Aaron Conole 
> 
> I'm pretty sure that this is just for exit on failure, so probably
EXIT_FAILURE
> (or similar) would be a better name.
[Alin Serdean] Updated here: https://patchwork.ozlabs.org/patch/1056056/
> 
> I'm not sure why we're using -1 (or 255).  Most OVS utilities, and most
Unix-
> like software in general, use exit status 1 on failure.
[Alin Serdean] TBH I just followed the exit code from:
https://github.com/openvswitch/ovs/blob/master/tests/checkpatch.at#L33
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] datapath-windows: Guard vport usage in user.c

[aserdean]

Applied on master and branch-2.11

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Sairam Venugopal via dev
> Trimis: Tuesday, March 12, 2019 8:20 PM
> Către: Alin Gabriel Serdean ; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Guard vport usage in
> user.c
> 
> Acked-by: Sairam Venugopal 
> 
> On 2/27/19, 6:10 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin
> Gabriel Serdean"  aserd...@ovn.org> wrote:
> 
> When using a vport we need to guard its usage with the dispatch lock.
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  datapath-windows/ovsext/User.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/User.c b/datapath-
> windows/ovsext/User.c
> index b43d7cc04..ed1fcbea8 100644
> --- a/datapath-windows/ovsext/User.c
> +++ b/datapath-windows/ovsext/User.c
> @@ -452,14 +452,6 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
>  }
> 
>  fwdDetail = NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(pNbl);
> -vport = OvsFindVportByPortNo(gOvsSwitchContext, execute->inPort);
> -if (vport) {
> -fwdDetail->SourcePortId = vport->portId;
> -fwdDetail->SourceNicIndex = vport->nicIndex;
> -} else {
> -fwdDetail->SourcePortId = NDIS_SWITCH_DEFAULT_PORT_ID;
> -fwdDetail->SourceNicIndex = 0;
> -}
>  // XXX: Figure out if any of the other members of fwdDetail need to 
> be
> set.
> 
>  status = OvsGetFlowMetadata(&key, execute->keyAttrs);
> @@ -502,6 +494,14 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
> 
>  if (ndisStatus == NDIS_STATUS_SUCCESS) {
>  NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock,
> &lockState, 0);
> +vport = OvsFindVportByPortNo(gOvsSwitchContext, execute-
> >inPort);
> +if (vport) {
> +fwdDetail->SourcePortId = vport->portId;
> +fwdDetail->SourceNicIndex = vport->nicIndex;
> +} else {
> +fwdDetail->SourcePortId = NDIS_SWITCH_DEFAULT_PORT_ID;
> +fwdDetail->SourceNicIndex = 0;
> +}
>  ndisStatus = OvsActionsExecute(gOvsSwitchContext, NULL, pNbl,
> vport ? vport->portNo :
> OVS_DPPORT_NUMBER_INVALID,
> --
> 2.16.1.windows.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> 
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.o
> penvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=02%7C01%7Cvsairam%40vmware.com%7C0bd42693441d43f1
> 2bfa08d69cbd589d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C6
> 36868734400091268&sdata=ShTAK8Qyi4hL3EG8PgLzbIxzSMril0w%2BHEE
> 8VO6qWlU%3D&reserved=0
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] datapath-windows: Add annotations to find vport functions

[aserdean]

Applied on master and branch-2.11.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Sairam Venugopal via dev
> Trimis: Tuesday, March 12, 2019 8:20 PM
> Către: Alin Gabriel Serdean ; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Add annotations to find
> vport functions
> 
> Acked-by: Sairam Venugopal 
> 
> On 2/27/19, 9:34 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin
> Gabriel Serdean"  aserd...@ovn.org> wrote:
> 
> Add annotations to find vport functions to check if the dispatch lock is
> held.
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  datapath-windows/ovsext/Vport.c | 7 ++-
>  datapath-windows/ovsext/Vport.h | 5 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-
> windows/ovsext/Vport.c
> index e08cb90ce..f79324d28 100644
> --- a/datapath-windows/ovsext/Vport.c
> +++ b/datapath-windows/ovsext/Vport.c
> @@ -135,7 +135,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
> switchContext,
>   * Lookup by port name to see if this port with this name had been
> added
>   * (and deleted) previously.
>   */
> -vport = OvsFindVportByHvNameW(gOvsSwitchContext,
> +vport = OvsFindVportByHvNameW(switchContext,
>portParam->PortFriendlyName.String,
>portParam->PortFriendlyName.Length);
>  if (vport && vport->isAbsentOnHv == FALSE) {
> @@ -693,6 +693,7 @@ done:
>  /*
>   * OVS Vport related functionality.
>   */
> +_Use_decl_annotations_
>  POVS_VPORT_ENTRY
>  OvsFindVportByPortNo(POVS_SWITCH_CONTEXT switchContext,
>   UINT32 portNo)
> @@ -787,6 +788,7 @@
> OvsFindTunnelVportByPortType(POVS_SWITCH_CONTEXT switchContext,
>  return NULL;
>  }
> 
> +_Use_decl_annotations_
>  POVS_VPORT_ENTRY
>  OvsFindVportByOvsName(POVS_SWITCH_CONTEXT switchContext,
>PSTR name)
> @@ -810,6 +812,7 @@
> OvsFindVportByOvsName(POVS_SWITCH_CONTEXT switchContext,
>  }
> 
>  /* OvsFindVportByHvName: "name" is assumed to be null-terminated */
> +_Use_decl_annotations_
>  POVS_VPORT_ENTRY
>  OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT switchContext,
>PWSTR wsName, SIZE_T wstrSize)
> @@ -862,6 +865,7 @@ Cleanup:
>  return vport;
>  }
> 
> +_Use_decl_annotations_
>  POVS_VPORT_ENTRY
>  OvsFindVportByHvNameA(POVS_SWITCH_CONTEXT switchContext,
>PSTR name)
> @@ -884,6 +888,7 @@
> OvsFindVportByHvNameA(POVS_SWITCH_CONTEXT switchContext,
>  return vport;
>  }
> 
> +_Use_decl_annotations_
>  POVS_VPORT_ENTRY
>  OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT
> switchContext,
>  NDIS_SWITCH_PORT_ID portId,
> diff --git a/datapath-windows/ovsext/Vport.h b/datapath-
> windows/ovsext/Vport.h
> index 7d88f86fb..32cbf8bcc 100644
> --- a/datapath-windows/ovsext/Vport.h
> +++ b/datapath-windows/ovsext/Vport.h
> @@ -122,15 +122,20 @@ typedef struct _OVS_VPORT_ENTRY {
> 
>  struct _OVS_SWITCH_CONTEXT;
> 
> +_Requires_lock_held_(switchContext->dispatchLock)
>  POVS_VPORT_ENTRY OvsFindVportByPortNo(POVS_SWITCH_CONTEXT
> switchContext,
>UINT32 portNo);
>  /* "name" is null-terminated */
> +_Requires_lock_held_(switchContext->dispatchLock)
>  POVS_VPORT_ENTRY OvsFindVportByOvsName(POVS_SWITCH_CONTEXT
> switchContext,
> PSTR name);
> +_Requires_lock_held_(switchContext->dispatchLock)
>  POVS_VPORT_ENTRY OvsFindVportByHvNameA(POVS_SWITCH_CONTEXT
> switchContext,
> PSTR name);
> +_Requires_lock_held_(switchContext->dispatchLock)
>  POVS_VPORT_ENTRY
> OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT switchContext,
> PWSTR wsName, SIZE_T wstrSize);
> +_Requires_lock_held_(switchContext->dispatchLock)
>  POVS_VPORT_ENTRY
> OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT
> switchContext,
>   NDIS_SWITCH_PORT_ID 
> portId,
>   NDIS_SWITCH_NIC_INDEX 
> index);
> --
> 2.16.1.windows.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> 
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.o
> penvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=02%7C01%7Cvsairam%40vmware.com%7C5ae778841e0345fb
> af6608d69cd9d189%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C6
> 36868856729356401&sdata=WxRHDA%2BKmMnZQ4N3J9ydzc4252vguQ
> PCNRuXlug48V8%3D&reserved=0
> 

Re: [ovs-dev] [PATCH] faq: Update features supported on Hyper-V

[aserdean]

Thanks for the patch.

Applied on master

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Sairam Venugopal via dev
> Trimis: Tuesday, March 12, 2019 8:21 PM
> Către: Anand Kumar ; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] faq: Update features supported on Hyper-V
> 
> Acked-by: Sairam Venugopal 
> 
> On 3/11/19, 1:48 PM, "ovs-dev-boun...@openvswitch.org on behalf of
> Anand Kumar via dev"  ovs-dev@openvswitch.org> wrote:
> 
> These features were added a while back, so updating
> the documentation.
> 
> Signed-off-by: Anand Kumar 
> ---
>  Documentation/faq/releases.rst | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/faq/releases.rst
> b/Documentation/faq/releases.rst
> index fd45efd..cd5aad1 100644
> --- a/Documentation/faq/releases.rst
> +++ b/Documentation/faq/releases.rst
> @@ -110,8 +110,8 @@ Q: Are all features available with all datapaths?
>  == == ==
> = ===
>  Connection tracking 4.3YES  YES  
> YES
>  Conntrack Fragment Reass.   4.3YES  YES  
> YES
> -NAT 4.6YES  YES  
> NO
> -Conntrack zone limit4.18   YES  NO   
> NO
> +NAT 4.6YES  YES  
> YES
> +Conntrack zone limit4.18   YES  NO   
> YES
>  Tunnel - LISP   NO YES  NO   
> NO
>  Tunnel - STTNO YES  NO   
> YES
>  Tunnel - GRE3.11   YES  YES  
> YES
> @@ -125,7 +125,7 @@ Q: Are all features available with all datapaths?
>  QoS - Policing  YESYES  YES  
> NO
>  QoS - Shaping   YESYES  NO   
> NO
>  sFlow   YESYES  YES  
> NO
> -IPFIX   3.10   YES  YES  
> NO
> +IPFIX   3.10   YES  YES  
> YES
>  Set action  YESYES  YES
> PARTIAL
>  NIC Bonding YESYES  YES  
> YES
>  Multiple VTEPs  YESYES  YES  
> YES
> --
> 2.9.3.windows.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> 
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.
> openvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=02%7C01%7Cvsairam%40vmware.com%7C5f1fa9a3b2ec42de
> 9a2b08d6a662d7ca%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C6
> 36879341325458846&sdata=2%2F8XzsQyWcqAaX2kWRtIYweJY6AYf9pq7
> e3CJ6OYLGk%3D&reserved=0
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH v2] datapath-windows: Fix potential deadlock in event subscription

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele aserd...@ovn.org
> Trimis: Wednesday, February 27, 2019 3:45 PM
> Către: 'Sairam Venugopal' ; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH v2] datapath-windows: Fix potential deadlock
> in event subscription
> 
> >
> > Move the EventQueue lock acquisition after the dispatchLock to prevent
> > a potential deadlock in port creation pipeline. There could be a case
> > where
> a
> > port event could try to take up the Dispatch Lock before the Event
> > Queue lock and the subscription queue event could take up the event
> > queue lock before the dispatch lock.
> >
> > Found while testing with Driver Verifier enabled.
> >
> > Signed-off-by: Sairam Venugopal 
> > ---
> >  datapath-windows/ovsext/Event.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> [Alin Serdean] Thanks for fixing this. I'm attaching the call stack dump
just for
> reference:
> =
> Thread 0: TERMINATED took locks in the following order:
> 
> Lock A = d58387490fa0 (+0xd58387490fa0) - Type 'Spinlock'.
> 
> Stack:   f80046e75f39
> NDIS!NdisAcquireRWLockWrite+0x0039
>  f80046eeeb8a
> NDIS!ndisVerifyNdisAcquireRWLockWrite+0x006a
>  f800488cd513 OVSExt!HvDeletePort+0x0063
> [c:\\ovs\datapath-windows\ovsext\vport.c @ 305]
>  f800488c6294 OVSExt!OvsProcessSetOid+0x01f0
> [c:\\ovs\datapath-windows\ovsext\oid.c @ 267]
>  f800488c5808 OVSExt!OvsExtOidRequest+0x00c8
> [c:\\ovs\datapath-windows\ovsext\oid.c @ 397]
>  f80046e7c86d
> NDIS!ndisFDoOidRequestInternal+0x028d
>  f80238704118
> nt!KeExpandKernelStackAndCalloutInternal+0x0078
>  f8023870408d
> nt!KeExpandKernelStackAndCalloutEx+0x001d
> 
> Lock B = f800488da4c0
> (OVSExt!eventQueueLockArr+0x)
> - Type 'Spinlock'.
> 
> Stack:   f800488b9ea3 OVSExt!OvsPostVportEvent+0x0073
> [c:\\ovs\datapath-windows\ovsext\event.c @ 166]
>  f800488cd5ce OVSExt!HvDeletePort+0x011e
> [c:\\ovs\datapath-windows\ovsext\vport.c @ 323]
>  f800488c6294 OVSExt!OvsProcessSetOid+0x01f0
> [c:\\ovs\datapath-windows\ovsext\oid.c @ 267]
>  f800488c5808 OVSExt!OvsExtOidRequest+0x00c8
> [c:\\ovs\datapath-windows\ovsext\oid.c @ 397]
>  f80046e7c86d
> NDIS!ndisFDoOidRequestInternal+0x028d
>  f80238704118
> nt!KeExpandKernelStackAndCalloutInternal+0x0078
>  f8023870408d
> nt!KeExpandKernelStackAndCalloutEx+0x001d
>  f80046e7bba1 NDIS!ndisQueueOidRequest+0x0311
> 
> =
> Thread 1: d5838bca1080 took locks in the following order:
> 
> Lock B = f800488da4c0
> (OVSExt!eventQueueLockArr+0x)
> - Type 'Spinlock'.
> 
> Stack:   f800488ba400
> OVSExt!OvsSubscribeEventIoctl+0x00d8
> [c:\\ovs\datapath-windows\ovsext\event.c @ 248]
>  f800488c3ed0
> OVSExt!OvsSubscribeEventCmdHandler+0x0104
> [c:\\ovs\datapath-windows\ovsext\datapath.c @ 1329]
>  f800488c44f2
> OVSExt!InvokeNetlinkCmdHandler+0x00a2
> [c:\\ovs\datapath-windows\ovsext\datapath.c @ 1100]
>  f800488de334 OVSExt!OvsDeviceControl+0x0308
> [c:\\ovs\datapath-windows\ovsext\datapath.c @ 966]
>  f80046f23451 NDIS!ndisDummyIrpHandler+0x0091
>  f80046a6c38d
> VerifierExt!xdv_IRP_MJ_DEVICE_CONTROL_wrapper+0x009d
>  f80238f63a14
nt!ViGenericDispatchHandler+0x0040
>  f80238f63996 nt!ViGenericDeviceControl+0x0016
> 
> Lock A = d58387490fa0 (+0xd58387490fa0) - Type 'Spinlock'.
> 
> Stack:   << Current stack trace - use kb to display it >>
> 
> Same as on the other patch please try to respin the patch because the
author
> seems wrong.
> 
> Acked-by: Alin Gabriel Serdean 
> 

[Alin Serdean] Applied on master and branch-2.11

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

Re: [ovs-dev] [PATCH] datapath-windows: Guard vport usage in user.c

[aserdean]

> Do we need to keep the dispatchLock for reading vport structures? 
[Alin Serdean] We need to make sure that port is not deleted since we use
it for processing.
>If that's
> the case, then we will need to fix it in other areas of the code too.
[Alin Serdean] I sent out https://patchwork.ozlabs.org/patch/1049043/ so we
can easily see where we need to change it. 
> It would
> be better to move the locking inside the relevant vport.c functions instead of
> taking out global ones.
[Alin Serdean] That would be ideal, but unfortunately if the vport is used
afterwards it needs to be guarded.
> 
> Thanks,
> Sairam
> 
> On 2/27/19, 6:10 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin
> Gabriel Serdean"  aserd...@ovn.org> wrote:
> 
> When using a vport we need to guard its usage with the dispatch lock.
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---

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

Re: [ovs-dev] [PATCH v2] datapath-windows: Fix potential deadlock in event subscription

[aserdean]

> 
> Move the EventQueue lock acquisition after the dispatchLock to prevent a
> potential deadlock in port creation pipeline. There could be a case where
a
> port event could try to take up the Dispatch Lock before the Event Queue
> lock and the subscription queue event could take up the event queue lock
> before the dispatch lock.
> 
> Found while testing with Driver Verifier enabled.
> 
> Signed-off-by: Sairam Venugopal 
> ---
>  datapath-windows/ovsext/Event.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
[Alin Serdean] Thanks for fixing this. I'm attaching the call stack dump
just
for reference:
=
Thread 0: TERMINATED took locks in the following order:

Lock A = d58387490fa0 (+0xd58387490fa0) - Type 'Spinlock'.

Stack:   f80046e75f39 NDIS!NdisAcquireRWLockWrite+0x0039
 f80046eeeb8a
NDIS!ndisVerifyNdisAcquireRWLockWrite+0x006a
 f800488cd513 OVSExt!HvDeletePort+0x0063
[c:\\ovs\datapath-windows\ovsext\vport.c @ 305]
 f800488c6294 OVSExt!OvsProcessSetOid+0x01f0
[c:\\ovs\datapath-windows\ovsext\oid.c @ 267]
 f800488c5808 OVSExt!OvsExtOidRequest+0x00c8
[c:\\ovs\datapath-windows\ovsext\oid.c @ 397]
 f80046e7c86d
NDIS!ndisFDoOidRequestInternal+0x028d
 f80238704118
nt!KeExpandKernelStackAndCalloutInternal+0x0078
 f8023870408d
nt!KeExpandKernelStackAndCalloutEx+0x001d

Lock B = f800488da4c0 (OVSExt!eventQueueLockArr+0x)
- Type 'Spinlock'.

Stack:   f800488b9ea3 OVSExt!OvsPostVportEvent+0x0073
[c:\\ovs\datapath-windows\ovsext\event.c @ 166]
 f800488cd5ce OVSExt!HvDeletePort+0x011e
[c:\\ovs\datapath-windows\ovsext\vport.c @ 323]
 f800488c6294 OVSExt!OvsProcessSetOid+0x01f0
[c:\\ovs\datapath-windows\ovsext\oid.c @ 267]
 f800488c5808 OVSExt!OvsExtOidRequest+0x00c8
[c:\\ovs\datapath-windows\ovsext\oid.c @ 397]
 f80046e7c86d
NDIS!ndisFDoOidRequestInternal+0x028d
 f80238704118
nt!KeExpandKernelStackAndCalloutInternal+0x0078
 f8023870408d
nt!KeExpandKernelStackAndCalloutEx+0x001d
 f80046e7bba1 NDIS!ndisQueueOidRequest+0x0311

=
Thread 1: d5838bca1080 took locks in the following order:

Lock B = f800488da4c0 (OVSExt!eventQueueLockArr+0x)
- Type 'Spinlock'.

Stack:   f800488ba400
OVSExt!OvsSubscribeEventIoctl+0x00d8
[c:\\ovs\datapath-windows\ovsext\event.c @ 248]
 f800488c3ed0
OVSExt!OvsSubscribeEventCmdHandler+0x0104
[c:\\ovs\datapath-windows\ovsext\datapath.c @ 1329]
 f800488c44f2
OVSExt!InvokeNetlinkCmdHandler+0x00a2
[c:\\ovs\datapath-windows\ovsext\datapath.c @ 1100]
 f800488de334 OVSExt!OvsDeviceControl+0x0308
[c:\\ovs\datapath-windows\ovsext\datapath.c @ 966]
 f80046f23451 NDIS!ndisDummyIrpHandler+0x0091
 f80046a6c38d
VerifierExt!xdv_IRP_MJ_DEVICE_CONTROL_wrapper+0x009d
 f80238f63a14 nt!ViGenericDispatchHandler+0x0040
 f80238f63996 nt!ViGenericDeviceControl+0x0016

Lock A = d58387490fa0 (+0xd58387490fa0) - Type 'Spinlock'.

Stack:   << Current stack trace - use kb to display it >>

Same as on the other patch please try to respin the patch because the author
seems wrong.

Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH] datapath-windows: Fix race condition during port creation

[aserdean]

> 
> Hold the dispatch lock until port-add operations are completed.
> 
> Found by inspection.
> 
> Signed-off-by: Sairam Venugopal 
[Alin Serdean] Thanks a lot for the patch!

For some reason the author seems to be ovs-dev@openvswitch.org,
can you try to respin the patch?

Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH] checkpatch: Normalize exit code for Windows

[aserdean]

> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Aaron Conole
> Trimis: Tuesday, February 5, 2019 10:39 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] checkpatch: Normalize exit code for Windows
> 
> Alin Gabriel Serdean  writes:
> 
> > Using python `sys.exit(-1)` on Windows produces mixed results.
> > Let's take the following results from different shells:
> > CMD
> >>python -c "import sys; sys.exit(-1)" & echo %errorlevel%
> > 1
> > MSYS
> > $ python -c "import sys; sys.exit(-1)" && echo $?
> > 0
> > WSL
> > $ python -c "import sys; sys.exit(-1)"; echo $?
> > 255
> >
> > this results in the following tests to fail:
> > checkpatch
> >
> >  10: checkpatch - sign-offs  FAILED
(checkpatch.at:32)
> >  11: checkpatch - parenthesized constructs   FAILED
(checkpatch.at:32)
> >  12: checkpatch - parenthesized constructs - for FAILED
> (checkpatch.at:32)
> >  13: checkpatch - comments   FAILED
(checkpatch.at:32)
> >
> > because of:
> >  ./checkpatch.at:32: exit code was 0, expected 255
> >
> > This patch introduces a positive constant for the default exit code.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> > ---
> 
> Acked-by: Aaron Conole 
> 
> With one little fixup:
> 
> 1. either we should eliminate the result argument to
>ovs_checkpatch_print_result
> 
> 2. or the test for the result < 0 needs to change to if result:
> 
> Otherwise, when an error is introduced, the exit code will be propagated,
> but the printout will be confusing:
> 
>   03:30:50 aconole {(0a8c1ec04...)} ~/git/ovs$ git commit -m "dpdk: This
is a
> test
>   Just testing
>   "
>   ERROR: C99 style comment
>   #10 FILE: lib/dpdk.c:17:
>   // BREAK!!
> 
> Lines checked: 15, no obvious problems found
> 
> :-)
> 
[Alin Serdean] Thanks for the feedback. I sent an incremental:
https://patchwork.ozlabs.org/patch/1039912/ . Do you mind taking another
look?

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

Re: [ovs-dev] [PATCH] Fix test 'testing ovn -- IP packet buffering' on Windows

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Wednesday, January 23, 2019 5:33 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] Fix test 'testing ovn -- IP packet
buffering' on
> Windows
> 
> On Wed, Jan 23, 2019 at 12:41:10PM +0200, Alin Gabriel Serdean wrote:
> > The test fails on Windows because of:
> > <--cut-->
> > ovn-nbctl: sw0: invalid network address: 2001;1\64
> > ovn-nbctl: sw1: invalid network address: 2002;1\64 <--cut-->
> >
> > This is due to the fact msys converts '::1' into ';1'.
> >
> > Use IPv6 long form instead of its short variant.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Acked-by: Ben Pfaff 
> ___

Thanks applied on master and branch-2.11

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

Re: [ovs-dev] [PATCH] datapath-windows: Add support for 'OVS_KEY_ATTR_ENCAP' key attribute.

[aserdean]

> > -Mesaj original-
> > De la: ovs-dev-boun...@openvswitch.org  > boun...@openvswitch.org> În numele Anand Kumar
> > Trimis: Friday, January 11, 2019 2:45 AM
> > Către: d...@openvswitch.org
> > Subiect: [ovs-dev] [PATCH] datapath-windows: Add support for
> > 'OVS_KEY_ATTR_ENCAP' key attribute.
> >
> > Add a new structure in l2 header to accomodate vlan header, based of
> > commit "d7efce7beff25052bd9083419200e1a47f0d6066
> > datapath: 802.1AD Flow handling, actions, vlan parsing, netlink
> attributes"
> >
> > Also reset vlan header in flow key, after deleting vlan tag from nbl
> >
> > With this change a sample vlan flow would look like,
> >
>
eth(src=0a:ea:8a:24:03:86,dst=0a:cd:fa:4d:15:5c),in_port(3),eth_type(0x8100
> > ),
> >
> vlan(vid=2239,pcp=0),encap(eth_type(0x0800),ipv4(src=13.12.11.149,dst=13.
> > 12.11.107,
> > proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0))
> >
> > Signed-off-by: Anand Kumar 
> > ---
> >  datapath-windows/ovsext/Actions.c|   3 +
> >  datapath-windows/ovsext/DpInternal.h |  12 +++-
> >  datapath-windows/ovsext/Flow.c   | 126
> > +++
> >  datapath-windows/ovsext/User.c   |  19 ++
> >  4 files changed, 144 insertions(+), 16 deletions(-)
> >
> [Alin Serdean]
> 
> Thanks for the patch!
> Acked-by: Alin Gabriel Serdean 
> 
Applied on master and branch-2.11

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

Re: [ovs-dev] [PATCH] datapath-windows: Add support for 'OVS_KEY_ATTR_ENCAP' key attribute.

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Anand Kumar
> Trimis: Friday, January 11, 2019 2:45 AM
> Către: d...@openvswitch.org
> Subiect: [ovs-dev] [PATCH] datapath-windows: Add support for
> 'OVS_KEY_ATTR_ENCAP' key attribute.
> 
> Add a new structure in l2 header to accomodate vlan header, based of
> commit "d7efce7beff25052bd9083419200e1a47f0d6066
> datapath: 802.1AD Flow handling, actions, vlan parsing, netlink
attributes"
> 
> Also reset vlan header in flow key, after deleting vlan tag from nbl
> 
> With this change a sample vlan flow would look like,
>
eth(src=0a:ea:8a:24:03:86,dst=0a:cd:fa:4d:15:5c),in_port(3),eth_type(0x8100
> ),
> vlan(vid=2239,pcp=0),encap(eth_type(0x0800),ipv4(src=13.12.11.149,dst=13.
> 12.11.107,
> proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0))
> 
> Signed-off-by: Anand Kumar 
> ---
>  datapath-windows/ovsext/Actions.c|   3 +
>  datapath-windows/ovsext/DpInternal.h |  12 +++-
>  datapath-windows/ovsext/Flow.c   | 126
> +++
>  datapath-windows/ovsext/User.c   |  19 ++
>  4 files changed, 144 insertions(+), 16 deletions(-)
> 
[Alin Serdean] 

Thanks for the patch!
Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH] datapath-windows: Use layers info to extract IP header in IpFragment

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Anand Kumar
> Trimis: Friday, November 9, 2018 9:09 PM
> Către: d...@openvswitch.org
> Subiect: [ovs-dev] [PATCH] datapath-windows: Use layers info to extract IP
> header in IpFragment
> 
> - Rely on layers l3Offset field to get offset of IP header.
> - Aslo fix passing 'newNbl' to IP fragment which is not required.
> - Fixed including a header file twice.
> 
> Signed-off-by: Anand Kumar 
> ---
>  datapath-windows/ovsext/BufferMgmt.c | 16 --  datapath-
> windows/ovsext/Conntrack.c  | 11 +++  datapath-
> windows/ovsext/Conntrack.h  |  1 -  datapath-
> windows/ovsext/IpFragment.c | 62 
>  datapath-windows/ovsext/IpFragment.h |  4 +--
>  5 files changed, 41 insertions(+), 53 deletions(-)
> 
Thanks a lot for the change!

Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [RFC] cirrus: Add Cirrus CI integration for FreeBSD build

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ilya Maximets
> Trimis: Wednesday, December 12, 2018 6:59 PM
> Către: aserd...@ovn.org; 'Ben Pfaff' 
> Cc: ovs-dev@openvswitch.org
> Subiect: Re: [ovs-dev] [RFC] cirrus: Add Cirrus CI integration for FreeBSD
build
> 
> On 12.12.2018 19:50, aserd...@ovn.org wrote:
> >> -Mesaj original-
> >> De la: ovs-dev-boun...@openvswitch.org  >> boun...@openvswitch.org> În numele Ilya Maximets
> >> Trimis: Wednesday, December 12, 2018 6:36 PM
> >> Către: Ben Pfaff 
> >> Cc: ovs-dev@openvswitch.org; Alin Gabriel Serdean
> 
> >> Subiect: Re: [ovs-dev] [RFC] cirrus: Add Cirrus CI integration for
> >> FreeBSD
> > build
> >>
> >> On 12.12.2018 19:24, Ben Pfaff wrote:
> >>> On Wed, Dec 12, 2018 at 07:00:28PM +0300, Ilya Maximets wrote:
>  On 12.12.2018 0:06, Ben Pfaff wrote:
> > On Tue, Dec 11, 2018 at 05:34:17PM +0300, Ilya Maximets wrote:
> >> CirrusCI [1] is free for open-sorce projects and provides similar
> >> to TravisCI interfaces. One significant difference is ability to
> >> run tasks on FreeBSD instances.
> >>
> >> This patch adds simple configuration file to test OVS build on
> >> two FreeBSD releases with gcc and clang.
> >> Unit tests are commented out because they are broken for now.
> >>
> >> To enable the automated checks Cirrus CI application from GitHub
> >> Marketplace should be installed. See details in Quick Start guide
> > [2].
> >>
> >> [1] https://cirrus-ci.org
> >> [2] https://cirrus-ci.org/guide/quick-start/
> >>
> >> Signed-off-by: Ilya Maximets 
> >> ---
> >>
> >> This could be useful to test FreeBSD build automatically to early
> >> detect issues. CirrusCI looks suitable for that purpose and it's
> >> the only public CI system that I managed to find that supports
> >> FreeBSD images. Sending as RFC to start discussion.
> >>
> >> Few more differentiation points with Travis. Cirrus allows to
> >> configure number of CPUs and the amount of required memory. It
> >> also supports Linux, OSX and Windows containers. So, maybe, if
> >> we'll like it, we could drop Travis and AppVeyor and use single
> >> CI system for all the tests.
> >>
> >> I'm not advertising and not trying to convince to replace our
> >> current CI systems. I found CirrusCI just few days ago by a mail
> >> [3] in qemu-devel list. I just think that it's the only option
> >> for now to test BSD builds with public CI. And it worked fine for
> >> me with below configuration.
> >>
> >> Thoughts ?
> >>
> >> As a note: I just spotted that TravisCI adds windows support.
> >> Maybe we could use it in the future for windows builds.
> >>
> >> [3]
> >> https://lists.nongnu.org/archive/html/qemu-devel/2018-
> >> 12/msg00935.h
> >> tml
> >
> > Good idea.  It's hard for me to see what harm this would cause, so
> > I enabled cirrus-ci at github and applied this to master.
> >
> > The big weakness with appveyor is that its limits are too low to
> > run the whole OVS testsuite.  I wonder whether cirrus-ci has
> > better limits for this purpose.
> 
>  As far as I know, the upper limit for resources is 8CPU + 24GB RAM.
>  Default timeout for tasks is 60 minutes and it, probably, could be
>  extended using special config in .yml file.
> >>>
> >>> I think that the issue with appveyor is the time limit of 60 minutes.
> >>> If cirrus-ci can allow for a longer build time, it would probably
> >>> make a big difference.
> >>
> >> There is a field 'timeout_in' in .cirrus.yml that could be used to
> >> extend
> > the
> >> time limit:
> >> https://cirrus-ci.org/faq/#instance-timed-out
> >>
> >> We could try it out.
> >>
> >> It'll be better if someone with "OVS on Windows" experience could
> >> look at this.
> >
> > Sorry for the delay.
> >
> > I'll play a bit with it tonight and send out updates.
> 
> Cool. Thanks.
> 
> I just wanted to mention that even without extending the timeout we can
> just split the test suite into separate tasks like this:
> 
>   env:
> matrix:
>   TESTSUITEFLAGS: -1000
>   TESTSUITEFLAGS: 1001-2000
>   TESTSUITEFLAGS: 2001-
> 
>   check_script:
> - make -j8 check TESTSUITEFLAGS="-j8 $TESTSUITEFLAGS"
> 
> And each of three tasks will have its own 60 minutes limit.
> This could help if the testsuite is the main time consuming issue.

Thanks a lot for the pointers!


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

Re: [ovs-dev] [RFC] cirrus: Add Cirrus CI integration for FreeBSD build

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ilya Maximets
> Trimis: Wednesday, December 12, 2018 6:36 PM
> Către: Ben Pfaff 
> Cc: ovs-dev@openvswitch.org; Alin Gabriel Serdean 
> Subiect: Re: [ovs-dev] [RFC] cirrus: Add Cirrus CI integration for FreeBSD
build
> 
> On 12.12.2018 19:24, Ben Pfaff wrote:
> > On Wed, Dec 12, 2018 at 07:00:28PM +0300, Ilya Maximets wrote:
> >> On 12.12.2018 0:06, Ben Pfaff wrote:
> >>> On Tue, Dec 11, 2018 at 05:34:17PM +0300, Ilya Maximets wrote:
>  CirrusCI [1] is free for open-sorce projects and provides similar
>  to TravisCI interfaces. One significant difference is ability to
>  run tasks on FreeBSD instances.
> 
>  This patch adds simple configuration file to test OVS build on two
>  FreeBSD releases with gcc and clang.
>  Unit tests are commented out because they are broken for now.
> 
>  To enable the automated checks Cirrus CI application from GitHub
>  Marketplace should be installed. See details in Quick Start guide
[2].
> 
>  [1] https://cirrus-ci.org
>  [2] https://cirrus-ci.org/guide/quick-start/
> 
>  Signed-off-by: Ilya Maximets 
>  ---
> 
>  This could be useful to test FreeBSD build automatically to early
>  detect issues. CirrusCI looks suitable for that purpose and it's
>  the only public CI system that I managed to find that supports
>  FreeBSD images. Sending as RFC to start discussion.
> 
>  Few more differentiation points with Travis. Cirrus allows to
>  configure number of CPUs and the amount of required memory. It also
>  supports Linux, OSX and Windows containers. So, maybe, if we'll
>  like it, we could drop Travis and AppVeyor and use single CI system
>  for all the tests.
> 
>  I'm not advertising and not trying to convince to replace our
>  current CI systems. I found CirrusCI just few days ago by a mail
>  [3] in qemu-devel list. I just think that it's the only option for
>  now to test BSD builds with public CI. And it worked fine for me
>  with below configuration.
> 
>  Thoughts ?
> 
>  As a note: I just spotted that TravisCI adds windows support.
>  Maybe we could use it in the future for windows builds.
> 
>  [3]
>  https://lists.nongnu.org/archive/html/qemu-devel/2018-
> 12/msg00935.h
>  tml
> >>>
> >>> Good idea.  It's hard for me to see what harm this would cause, so I
> >>> enabled cirrus-ci at github and applied this to master.
> >>>
> >>> The big weakness with appveyor is that its limits are too low to run
> >>> the whole OVS testsuite.  I wonder whether cirrus-ci has better
> >>> limits for this purpose.
> >>
> >> As far as I know, the upper limit for resources is 8CPU + 24GB RAM.
> >> Default timeout for tasks is 60 minutes and it, probably, could be
> >> extended using special config in .yml file.
> >
> > I think that the issue with appveyor is the time limit of 60 minutes.
> > If cirrus-ci can allow for a longer build time, it would probably make
> > a big difference.
> 
> There is a field 'timeout_in' in .cirrus.yml that could be used to extend
the
> time limit:
> https://cirrus-ci.org/faq/#instance-timed-out
> 
> We could try it out.
> 
> It'll be better if someone with "OVS on Windows" experience could look at
> this.

Sorry for the delay.

I'll play a bit with it tonight and send out updates.

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

Re: [ovs-dev] [PATCH] pcap: Fix reading regular old Ethernet pcap files.

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, November 15, 2018 7:39 PM
> Către: d...@openvswitch.org
> Cc: Ben Pfaff ; Alin Gabriel Serdean 
> Subiect: [ovs-dev] [PATCH] pcap: Fix reading regular old Ethernet pcap
files.
> 
> This broke the unit tests.
> 
> Fixes: 597177a283da ("pcap-file: Add support for Linux SLL formatted PCAP
> files.")
> Reported-by: Alin Gabriel Serdean 
> Signed-off-by: Ben Pfaff 
> ---
Acked-by: Alin Gabriel Serdean 
Tested-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH] dpif-netlink: Fix error behavior in dpif_netlink_port_add__().

[aserdean]

Acked-by: Alin Gabriel Serdean 


> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, November 15, 2018 7:08 PM
> Către: d...@openvswitch.org
> Cc: Ben Pfaff 
> Subiect: [ovs-dev] [PATCH] dpif-netlink: Fix error behavior in
> dpif_netlink_port_add__().
> 
> Until now, the code here would report an error to its caller as success.
> This fixes the problem.
> 
> Found by inspection.
> 
> Signed-off-by: Ben Pfaff 
> ---
>  lib/dpif-netlink.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index
> f2011f22e548..7f6a6cc59592 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -716,7 +716,8 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif,
> const char *name,
>  int error = 0;
> 
>  if (dpif->handlers) {
> -if (nl_sock_create(NETLINK_GENERIC, &socksp)) {
> +error = nl_sock_create(NETLINK_GENERIC, &socksp);
> +if (error) {
>  return error;
>  }
>  }
> --
> 2.16.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] datapath-windows: Fix invalid reference in Buffermgmt.c

[aserdean]

Thanks for the fix!

Acked-by: Alin Gabriel Serdean 

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Sairam Venugopal
> Trimis: Wednesday, November 14, 2018 10:07 PM
> Către: d...@openvswitch.org
> Subiect: [ovs-dev] [PATCH] datapath-windows: Fix invalid reference in
> Buffermgmt.c
> 
> OVS_BUFFER_CONTEXT gets cleared as part of
> NdisFreeNetBufferListContext function call. This causes an invalid
reference
> error.
> 
> Found while testing with driver verifier enabled.
> 
> Signed-off-by: Sairam Venugopal 
> ---
>  datapath-windows/ovsext/BufferMgmt.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/BufferMgmt.c b/datapath-
> windows/ovsext/BufferMgmt.c
> index 448cd76..da5c04a 100644
> --- a/datapath-windows/ovsext/BufferMgmt.c
> +++ b/datapath-windows/ovsext/BufferMgmt.c

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

Re: [ovs-dev] [PATCH] Tests: Fix testing bridge - add port after stopping controller on Windows

[aserdean]

Applied on master, branch-2.10, branch-2.9.

--
Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele aserd...@ovn.org
> Trimis: Thursday, November 15, 2018 3:23 PM
> Către: 'Sairam Venugopal' ; 'Alin Gabriel Serdean'
> ; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] Tests: Fix testing bridge - add port after
> stopping controller on Windows
> 
> I have opened an issue for it: https://github.com/openvswitch/ovs-
> issues/issues/165
> 
> It's not test specific unfortunately. The file is put up for unlink on exit, 
> but I'm
> guessing someone still has an opened handle at that point.
> 
> I'm applying this patch for the moment and following up with the actual fix
> for the userspace.
> 
> Thanks for the ack.
> 
> Alin.
> 
> > -Mesaj original-
> > De la: ovs-dev-boun...@openvswitch.org  > boun...@openvswitch.org> În numele Sairam Venugopal
> > Trimis: Wednesday, November 14, 2018 10:19 PM
> > Către: Alin Gabriel Serdean ; d...@openvswitch.org
> > Subiect: Re: [ovs-dev] [PATCH] Tests: Fix testing bridge - add port
> > after stopping controller on Windows
> >
> > Thanks for fixing this. The files normally get deleted when the agent
> > closes. Is this a regression or just test related? Ack'ing the fix.
> >
> > Acked-by: Sairam Venugopal 
> >
> > On 11/14/18, 7:31 AM, "ovs-dev-boun...@openvswitch.org on behalf of
> > Alin Gabriel Serdean"  > aserd...@ovn.org> wrote:
> >
> > On Windows the file which is used for the named pipe connection
> > (`punix:file`)
> > is not deleted when the process is closed.
> >
> > Try to delete the `controller` file and fail if we can't (on Windows you
> can't
> > delete a file if there still an opened handle to it).
> >
> > Also add a check to see if the `ovs-testcontroller` was successfully
> started.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> > ---
> >  tests/bridge.at | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/bridge.at b/tests/bridge.at
> > index ee398bdb1..b94afc194 100644
> > --- a/tests/bridge.at
> > +++ b/tests/bridge.at
> > @@ -84,7 +84,7 @@ AT_SETUP([bridge - add port after stopping
> > controller])
> >  OVS_VSWITCHD_START
> >
> >  dnl Start ovs-testcontroller
> > -ovs-testcontroller --detach punix:controller --pidfile=ovs-
> > testcontroller.pid
> > +AT_CHECK([ovs-testcontroller --detach punix:controller
> > --pidfile], [0],
> > [ignore])
> >  OVS_WAIT_UNTIL([test -e controller])
> >
> >  AT_CHECK([ovs-vsctl set-controller br0 unix:controller])
> > @@ -93,6 +93,9 @@ AT_CHECK([ovs-appctl -t ovs-vswitchd version],
> > [0],
> > [ignore])
> >
> >  # Now kill the ovs-testcontroller
> >  kill `cat ovs-testcontroller.pid`
> > +if test "$IS_WIN32" = "yes"; then
> > +AT_CHECK([rm controller], [0], [ignore])
> > +fi
> >  OVS_WAIT_UNTIL([! test -e controller])
> >  AT_CHECK([ovs-vsctl --no-wait add-port br0 p2 -- set Interface p2
> > type=internal], [0], [ignore])
> >  AT_CHECK([ovs-appctl -t ovs-vswitchd version], [0], [ignore])
> > --
> > 2.16.1.windows.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> >
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.
> > o
> > penvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> >
> dev&data=02%7C01%7Cvsairam%40vmware.com%7C074d45f6e70d4431
> >
> f7a708d64a4639a6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C6
> >
> 36778062866824077&sdata=EPht5myv2MmYwaY49hSYegAfewIVjkbvNC
> > Tf5YBsYJM%3D&reserved=0
> >
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] Tests: Fix testing bridge - add port after stopping controller on Windows

[aserdean]

I have opened an issue for it: 
https://github.com/openvswitch/ovs-issues/issues/165

It's not test specific unfortunately. The file is put up for unlink on exit, 
but I'm guessing
someone still has an opened handle at that point.

I'm applying this patch for the moment and following up with the actual fix for 
the
userspace.

Thanks for the ack.

Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Sairam Venugopal
> Trimis: Wednesday, November 14, 2018 10:19 PM
> Către: Alin Gabriel Serdean ; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] Tests: Fix testing bridge - add port after
> stopping controller on Windows
> 
> Thanks for fixing this. The files normally get deleted when the agent closes. 
> Is
> this a regression or just test related? Ack'ing the fix.
> 
> Acked-by: Sairam Venugopal 
> 
> On 11/14/18, 7:31 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin
> Gabriel Serdean"  aserd...@ovn.org> wrote:
> 
> On Windows the file which is used for the named pipe connection
> (`punix:file`)
> is not deleted when the process is closed.
> 
> Try to delete the `controller` file and fail if we can't (on Windows you 
> can't
> delete a file if there still an opened handle to it).
> 
> Also add a check to see if the `ovs-testcontroller` was successfully 
> started.
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  tests/bridge.at | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/bridge.at b/tests/bridge.at
> index ee398bdb1..b94afc194 100644
> --- a/tests/bridge.at
> +++ b/tests/bridge.at
> @@ -84,7 +84,7 @@ AT_SETUP([bridge - add port after stopping
> controller])
>  OVS_VSWITCHD_START
> 
>  dnl Start ovs-testcontroller
> -ovs-testcontroller --detach punix:controller --pidfile=ovs-
> testcontroller.pid
> +AT_CHECK([ovs-testcontroller --detach punix:controller --pidfile], [0],
> [ignore])
>  OVS_WAIT_UNTIL([test -e controller])
> 
>  AT_CHECK([ovs-vsctl set-controller br0 unix:controller])
> @@ -93,6 +93,9 @@ AT_CHECK([ovs-appctl -t ovs-vswitchd version], [0],
> [ignore])
> 
>  # Now kill the ovs-testcontroller
>  kill `cat ovs-testcontroller.pid`
> +if test "$IS_WIN32" = "yes"; then
> +AT_CHECK([rm controller], [0], [ignore])
> +fi
>  OVS_WAIT_UNTIL([! test -e controller])
>  AT_CHECK([ovs-vsctl --no-wait add-port br0 p2 -- set Interface p2
> type=internal], [0], [ignore])
>  AT_CHECK([ovs-appctl -t ovs-vswitchd version], [0], [ignore])
> --
> 2.16.1.windows.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> 
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.o
> penvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=02%7C01%7Cvsairam%40vmware.com%7C074d45f6e70d4431
> f7a708d64a4639a6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C6
> 36778062866824077&sdata=EPht5myv2MmYwaY49hSYegAfewIVjkbvNC
> Tf5YBsYJM%3D&reserved=0
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] [RFC][windows]: Remap VPORT socket pool

[aserdean]

Answers inlined.

> Subiect: Re: [ovs-dev] [PATCH] [RFC][windows]: Remap VPORT socket pool
> 
> Hi Alin,
> 
> Thanks for the patch. Shashank is giving it a run. How do we prevent this
> issue in the future?
[Alin Serdean] Thanks for trying it out, and I'm sorry I haven't caught this 
issue
in time.

The best way would be to have a CI and run at least some sanity
checks on the windows datapath when it is enabled.
I can work on getting some
tests from the: `make check-kernel` and `make check-kmod` to run on Windows
and we can reuse most of the logic.
Another useful CI would be to run the unit tests on Windows as well.
> 
> For starters, I think we need to consolidate the usage under nl_* functions to
> ensure it's not doing something differently in vport_*.
[Alin Serdean] I agree. We should also spend some cycles switching to IOCPs
IMO.
> 
> We need to look into investing in the test framework for catching basic
> Windows failures like these.
[Alin Serdean] We can reuse the test framework that is used on Linux as I 
pointed
above the biggest issue IMO is where and how we run it.
> 
> Thanks,
> Sairam
> 
> On 11/12/18, 8:51 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin
> Gabriel Serdean"  aserd...@ovn.org> wrote:
> 
> Fixes:
> 
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
> .com%2Fopenvswitch%2Fovs-
> issues%2Fissues%2F164&data=02%7C01%7Cvsairam%40vmware.com%
> 7C76dd26cc93e145b3a12008d648bf0d08%7Cb39138ca3cee4b4aa4d6cd83d9dd
> 62f0%7C1%7C0%7C636776382753818056&sdata=E%2FWSjXq%2BMfQls
> GG9Fw59FblJxGTjOXYVFpNUKJtyp0k%3D&reserved=0
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  lib/dpif-netlink.c | 58
> +-
>  1 file changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 69c145cc3..5994c3445 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -246,6 +246,38 @@ static int dpif_netlink_port_query__(const struct
> dpif_netlink *dpif,
>   odp_port_t port_no, const char 
> *port_name,
>   struct dpif_port *dpif_port);
> 
> +
> +static struct nl_sock *
> +vport_create_socksp_windows(struct dpif_netlink *dpif, int *error)
> +OVS_REQ_WRLOCK(dpif->upcall_lock)
> +{
> +struct nl_sock *socksp;
> +size_t i;
> +/* Pick netlink sockets to use in a round-robin fashion from each
> + * handler's pool of sockets. */
> +struct dpif_handler *handler = &dpif->handlers[0];
> +struct dpif_windows_vport_sock *sock_pool = handler-
> >vport_sock_pool;
> +size_t index = handler->last_used_pool_idx;
> +/* A pool of sockets is allocated when the handler is initialized. */
> +if (sock_pool == NULL) {
> +*error = EINVAL;
> +return NULL;
> +}
> +ovs_assert(index < VPORT_SOCK_POOL_SIZE);
> +socksp = sock_pool[index].nl_sock;
> +ovs_assert(socksp);
> +index = (index == VPORT_SOCK_POOL_SIZE - 1) ? 0 : index + 1;
> +handler->last_used_pool_idx = index;
> +*error = 0;
> +return socksp;
> +}
> +static void
> +vport_del_socksp_windows(struct dpif_netlink *dpif, struct nl_sock
> *socksp)
> +{
> +socksp = NULL;
> +}
> +
> +
>  static struct dpif_netlink *
>  dpif_netlink_cast(const struct dpif *dpif)
>  {
> @@ -716,7 +748,12 @@ dpif_netlink_port_add__(struct dpif_netlink
> *dpif, const char *name,
>  int error = 0;
> 
>  if (dpif->handlers) {
> -if (nl_sock_create(NETLINK_GENERIC, &socksp)) {
> +#ifdef _WIN32
> +socksp = vport_create_socksp_windows(dpif, &error);
> +#else
> +error = nl_sock_create(NETLINK_GENERIC, &socksp);
> +#endif
> +if (!socksp) {
>  return error;
>  }
>  }
> @@ -748,7 +785,11 @@ dpif_netlink_port_add__(struct dpif_netlink
> *dpif, const char *name,
>dpif_name(&dpif->dpif), *port_nop);
>  }
> 
> +#ifdef _WIN32
> +vport_del_socksp_windows(dpif, socksp);
> +#else
>  nl_sock_destroy(socksp);
> +#endif
>  goto exit;
>  }
> 
> @@ -763,7 +804,11 @@ dpif_netlink_port_add__(struct dpif_netlink
> *dpif, const char *name,
>  request.dp_ifindex = dpif->dp_ifindex;
>  request.port_no = *port_nop;
>  dpif_netlink_vport_transact(&request, NULL, NULL);
> +#ifdef _WIN32
> +vport_del_socksp_windows(dpif, socksp);
> +#else
>  nl_sock_destroy(socksp);
> +#endif
>  goto exit;
>  }
> 
> @@ -2249,15 +2294,26 @@ dpif_netlink_refresh_channels(struct
> dpif_netlink *dpif, uint32_t n_handlers)
>  || !vport_get_pid(dpif, port_no, &u

Re: [ovs-dev] [RFC PATCH v2] datapath-windows: Remove IP neighbor entries when internal adapter is down.

[aserdean]

Thanks for the patch Anand! Applied on master.

Alin

> >
> > Remove the IP neighboring entries when adapter is down, so that when
> > 'OVS_IPHELPER_INSTANCE' is deleted, no stale entries are present
> >
> > Also fix accessing iphelper instance without acquiring the lock.
> >
> > Signed-off-by: Anand Kumar 
> > ---
> Thanks for incorporating the comments:
> Acked-by: Alin Gabriel Serdean 
> Tested-by: Alin Gabriel Serdean 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [RFC PATCH v2] datapath-windows: Remove IP neighbor entries when internal adapter is down.

[aserdean]

> 
> Remove the IP neighboring entries when adapter is down, so that when
> 'OVS_IPHELPER_INSTANCE' is deleted, no stale entries are present
> 
> Also fix accessing iphelper instance without acquiring the lock.
> 
> Signed-off-by: Anand Kumar 
> ---
Thanks for incorporating the comments:
Acked-by: Alin Gabriel Serdean 
Tested-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH] datapath-windows: Fix payload length calculation in Conntrack.h

[aserdean]

Thanks! I applied this on master and branch-2.10.

Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Anand Kumar
> Trimis: Thursday, September 20, 2018 5:30 AM
> Către: Alin Gabriel Serdean ; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Fix payload length
> calculation in Conntrack.h
> 
> Hi Alin,
> 
> Thanks for fixing this,
> Acked-by: Anand Kumar 
> 
> Regards,
> Anand Kumar
> 
> On 9/19/18, 3:37 PM, "ovs-dev-boun...@openvswitch.org on behalf of Alin
> Gabriel Serdean"  aserd...@ovn.org> wrote:
> 
> The payload calculation in OvsGetTcpHeader is wrong:
> `ntohs(ipHdr->tot_len) - expr` instead of `ntohs((ipHdr->tot_len) -
expr)`.
> 
> We already have a macro for that calculation defined in NetProto.h so
use
> it.
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  datapath-windows/ovsext/Conntrack.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-
> windows/ovsext/Conntrack.h
> index d4152b33a..044fb436c 100644
> --- a/datapath-windows/ovsext/Conntrack.h
> +++ b/datapath-windows/ovsext/Conntrack.h
> @@ -175,8 +175,7 @@ OvsGetTcpHeader(PNET_BUFFER_LIST nbl,
>  tcp = (TCPHdr *)((PCHAR)ipHdr + ipHdr->ihl * 4);
>  if (tcp->doff * 4 >= sizeof *tcp) {
>  NdisMoveMemory(dest, tcp, sizeof(TCPHdr));
> -*tcpPayloadLen = ntohs((ipHdr->tot_len) - (ipHdr->ihl * 4) -
> -   (TCP_HDR_LEN(tcp)));
> +*tcpPayloadLen = TCP_DATA_LENGTH(ipHdr, tcp);
>  return storage;
>  }
> 
> --
> 2.16.1.windows.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> 
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.o
> penvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=02%7C01%7Ckumaranand%40vmware.com%7C30dc4304c493
> 4da3166a08d61e807a8f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0
> %7C636729934502072772&sdata=FWmdwckcC%2B5ymgR0Ryt8iBU0YHA
> c6%2BRFPoFz2w66vlc%3D&reserved=0
> 
> 
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH v4] datapath-windows: Add support to configure ct zone limits

[aserdean]

I applied the patch on master and branch-2.10.

Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Alin Gabriel Serdean
> Trimis: Thursday, September 20, 2018 1:23 AM
> Către: Anand Kumar 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH v4] datapath-windows: Add support to
> configure ct zone limits
> 
> 
> 
> > On 19 Sep 2018, at 21:39, Anand Kumar 
> wrote:
> >
> > This patch implements limiting conntrack entries per zone using dpctl
> > commands.
> >
> > Example:
> > ovs-appctl dpctl/ct-set-limits default=5 zone=1,limit=2 zone=1,limit=3
> > ovs-appctl dpct/ct-del-limits zone=4 ovs-appctl dpct/ct-get-limits
> > zone=1,2,3
> >
> > - Also update the netlink-socket.c to support netlink family
> > 'OVS_WIN_NL_CTLIMIT_FAMILY_ID' for conntrack zone limit.
> >
> > Signed-off-by: Anand Kumar 
> > ---
> > v3->v4:
> >  - Fix macro defnition
> > v2->v3:
> >  - Change loop index variable from UINT16 to UINT32
> > v1->v2:
> >  - Use spinlock to guard against multiple access.
> >  - Use Interlock api to update zone counters.
> >  - Address review comments.
> > ---
> > datapath-windows/include/OvsDpInterfaceExt.h |   1 +
> > datapath-windows/ovsext/Conntrack.c  | 167
> ++-
> > datapath-windows/ovsext/Conntrack.h  |  12 ++
> > datapath-windows/ovsext/Datapath.c   |  34 +-
> > lib/netlink-socket.c |   5 +
> > 5 files changed, 216 insertions(+), 3 deletions(-)
> >
> 
> Thanks a lot for the patch! I tested it a bit and things look ok.
> 
> Acked-by: Alin Gabriel Serdean  >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH 1/2] windows: Add set_deatch function to daemon-windows.c

[aserdean]

> On Tue, 21 Aug 2018 at 11:19, Ben Pfaff  wrote:
> 
> > On Tue, Aug 21, 2018 at 05:57:47PM +0300, Alin Gabriel Serdean wrote:
> > > The daemon-windows file is missing a `set_detach` routine, so add it.
> > >
> > > This will be useful in the long run.
> > >
> > > Signed-off-by: Alin Gabriel Serdean 
> >
> > Thank you for maintaining the Windows port.  We could not do it
> > without you.
> >
> > There is a misspelling in the subject: s/deatch/detach/.
Sorry for the typo 😊. I'll send a V2 
> >
> > I don't entirely understand the implications of this patch.  Guru, I
> > believe that you were involved in the design of --service.  Do you
> > have any comments?
> >
> I do not see a need for this. But I don't see a harm either. I suppose Alin
> forsees a need for this in the future. I am ok with a merge.
> 
I think this will be make the code a bit simpler for both type of developers.
If someone uses get_detach (either for  `--service` or `--detach`) they know
about the users intention.
i.e.:
https://github.com/openvswitch/ovs/blob/master/ovn/utilities/ovn-nbctl.c#L183
> >
> > For myself:
> > Acked-by: Ben Pfaff 
> >
> > Thanks,
> >
> > Ben.
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] tunnel, tests: Sort flow output in ERSPAN v1/v2 metadata

[aserdean]

> 
> On Tue, Aug 21, 2018 at 11:01 AM Ben Pfaff  wrote:
> >
> > On Tue, Aug 21, 2018 at 05:18:06PM +0300, Alin Gabriel Serdean wrote:
> > > Sort the flows by input port in the test:
> > > tunnel.at:512: testing tunnel - ERSPAN v1/v2 metadata ...
> > >
> > > This tests fails on Windows due to the hash function sensitivity
described
> > > in:
> > > https://mail.openvswitch.org/pipermail/ovs-dev/2018-
> August/350685.html
> > >
> > > Signed-off-by: Alin Gabriel Serdean 
> >
> > Works here.
> >
> > Acked-by: Ben Pfaff 
> Thanks for the fix
> Acked-by: William Tu 
> 
Thanks both! I applied it on master and branch-2.10

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

Re: [ovs-dev] [PATCH] ofctl: Fixup compare_flows function

[aserdean]

Can you also provide a Signed-off-by pls?

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Tuesday, August 7, 2018 12:40 AM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] ofctl: Fixup compare_flows function
> 
> On Tue, Aug 07, 2018 at 12:34:45AM +0300, Alin Gabriel Serdean wrote:
> > In the case there was no sorting criteria the flows on Windows were
> > being rearranged because it was always returning zero.
> >
> > Also check if there we need sorting to save a few cycles.
> >
> > CC: Ben Pfaff 
> > Co-authored-by: Ben Pfaff 
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Acked-by: Ben Pfaff 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] meta-flow: Make mf_vl_mff_mf_from_nxm_header() require a valid field.

[aserdean]

> 
> All the users of mf_vl_mff_mf_from_nxm_header() expect it to always
> obtain a valid field or to report an error.  In practice, it did not
report an error
> in the case where the field was unknown (although it did report an error
in
> some other cases).  This commit fixes the problem.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9652
> Signed-off-by: Ben Pfaff 
> ---
>  lib/meta-flow.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c index
> fd8f3c6d85ea..b6d9e92b691d 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -3519,7 +3519,9 @@ mf_vl_mff_mf_from_nxm_header(uint32_t
> header,
>   uint64_t *tlv_bitmap)  {
>  *field = mf_from_nxm_header(header, vl_mff_map);
> -if (mf_vl_mff_invalid(*field, vl_mff_map)) {
> +if (!*field) {
> +return OFPERR_OFPBAC_BAD_SET_TYPE;
> +} else if (mf_vl_mff_invalid(*field, vl_mff_map)) {
>  return OFPERR_NXFMFC_INVALID_TLV_FIELD;
>  }
> 
> --
> 2.16.1
> 

Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH] pcap-file: Fix formatting of log message.

[aserdean]

> Signed-off-by: Ben Pfaff 
> ---
>  lib/pcap-file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/pcap-file.c b/lib/pcap-file.c index
981d56d48830..ea5cfa3b274e
> 100644
> --- a/lib/pcap-file.c
> +++ b/lib/pcap-file.c
> @@ -163,7 +163,7 @@ ovs_pcap_read(FILE *file, struct dp_packet **bufp,
> long long int *when)
>  if (swap) {
>  len = uint32_byteswap(len);
>  if (len > 0x) {
> -VLOG_WARN("bad packet length %"PRIuSIZE" or %"PRIu32
> +VLOG_WARN("bad packet length %"PRIuSIZE" or %"PRIu32" "
>"reading pcap file",
>len, uint32_byteswap(len));
>  return EPROTO;
> --
> 2.16.1

Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH 3/4] unixctl: Make path to unixctl_server socket available to the client.

[aserdean]

> Signed-off-by: Ben Pfaff 
> ---
<---B<-->
>  if (error) {
> -ovs_error(error, "could not initialize control socket %s",
punix_path);
[Alin Serdean] Changing the log message will make the following test fail:
130. daemon.at:148: testing daemon --detach startup errors ...
./daemon.at:150: ovsdb-tool create db
$abs_top_srcdir/vswitchd/vswitch.ovsschema
stderr:
stdout:
./daemon.at:150: ovsdb-tool transact db \
'["Open_vSwitch",
  {"op": "insert",
   "table": "Open_vSwitch",
   "row": {}}]'
stderr:
stdout:
[{"uuid":["uuid","017d6f26-bd86-40bf-8886-230ea05c26a0"]}]
./daemon.at:151: ovsdb-server --detach --no-chdir --pidfile
--unixctl=nonexistent/unixctl db
stderr:
2018-08-03T14:04:58Z|1|fatal_signal|WARN|could not unlink
"/home/ubuntu/ovs/tests/testsuite.dir/0130/nonexistent/unixctl" (No such
file or directory)
2018-08-03T14:04:58Z|2|stream_unix|ERR|/home/ubuntu/ovs/tests/testsuite.
dir/0130/nonexistent/unixctl: binding failed: No such file or directory
ovsdb-server: /home/ubuntu/ovs/tests/testsuite.dir/0130/nonexistent/unixctl:
could not initialize control socket (No such file or directory)
./daemon.at:152: grep 'ovsdb-server: could not initialize control socket'
stderr
stdout:
./daemon.at:152: exit code was 1, expected 0
pid:
sed: can't read pid: No such file or directory
130. daemon.at:148:  FAILED (daemon.at:152)
> -goto exit;
> +ovs_error(error, "%s: could not initialize control socket",
abs_path);
> +free(abs_path);
> +return error;
>  }
> 
<---B<-->

Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH 2/4] unixctl: Use absolute paths on Windows too.

[aserdean]

> 
> When this case was adapted for Windows, asb_file_name() simply didn't
> work at all there.  Now, it should work OK, and it seems like the right
thing to
> do, and it makes the code more straightforward too.
> 
> CC: Alin Gabriel Serdean 
> Signed-off-by: Ben Pfaff 

Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH] Add opterr and optopt to Windows headers

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Tuesday, July 31, 2018 7:37 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] Add opterr and optopt to Windows headers
> 
> On Tue, Jul 31, 2018 at 03:08:13PM +0300, Alin Gabriel Serdean wrote:
> > Until now we only had optind defined in the header.
> >
> > Since we are using the BSD getopt variant add opterr and optopt.
> >
> > Fixes: current Windows build failures.
> 
> Usually Fixes is used like this:
> 
> CC: Jakub Sitnicki 
> Fixes: 3ec06ea9c668 ("ovn-nbctl: Initial support for daemon mode.")
> 
> I use the following script, installed under the name git-fixes, to
generate it:
> 
> #! /bin/sh
> git --no-pager log -1 --pretty=format:"CC: %an <%ae>%nFixes: %h
> (\"%s\")%n" --abbrev=12 "$@"
> 
Thanks or the script Ben. I updated the commit message and sent out a v2.

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

Re: [ovs-dev] build: Add gitattribute file to build-aux

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Aaron Conole
> Trimis: Wednesday, July 18, 2018 6:02 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] build: Add gitattribute file to build-aux
> 
> Hi Alin,
> 
> 0-day Robot  writes:
> 
> > Bleep bloop.  Greetings Alin Gabriel Serdean, 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.
> >
> >
> 
> I suggest folding in something like the following (since I don't think it
makes
> sense to worry about distributing gitattributes files):
> 
Thanks for the suggestion Aaron. I sent out an incremental:
https://patchwork.ozlabs.org/patch/946511/
Do you mind looking over it and adding a Signed-off?

Thanks,
Alin.

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

Re: [ovs-dev] [PATCH] dpif-netlink: Add meter support.

[aserdean]

> 
> From: Andy Zhou 
> 
> To work with kernel datapath that supports meter.
> 
> Signed-off-by: Andy Zhou 
> Co-authored-by: Justin Pettit 
> Signed-off-by: Justin Pettit 
> ---
>  Documentation/faq/releases.rst |   1 +
>  lib/dpif-netlink.c | 302 ++---
>  2 files changed, 283 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/faq/releases.rst
> b/Documentation/faq/releases.rst index fab93b1888a6..d5f14e384155
> 100644
[Alin Serdean] Small nit: shouldn't it be v3?

Acked-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH v5 0/3] Optimize conntrack performance

[aserdean]

> This patch series is primarily to refactor conntrack code for better
throughput
> with conntrack.
> 
> With this patch series TCP throughput with conntrack increased by ~50%.
> 
> Anand Kumar (3):
>   datapath-windows: Use spinlock instead of RW lock for ct entry
>   datapath-windows: Implement locking in conntrack NAT.
>   datapath-windows: Compute ct hash based on 5-tuple and zone
> 
>  datapath-windows/ovsext/Conntrack-ftp.c |   4 +-
>  datapath-windows/ovsext/Conntrack-nat.c |  35 ++-
>  datapath-windows/ovsext/Conntrack-related.c |  21 +-
>  datapath-windows/ovsext/Conntrack-tcp.c |  15 +-
>  datapath-windows/ovsext/Conntrack.c | 469
+
> ---
>  datapath-windows/ovsext/Conntrack.h |  40 ++-
>  datapath-windows/ovsext/Util.h  |  18 ++
>  7 files changed, 311 insertions(+), 291 deletions(-)
> 
> --

Thanks for the series Anand, I applied it on master.

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

Re: [ovs-dev] [PATCH v4 3/3] datapath-windows: Compute ct hash based on 5-tuple and zone

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Anand Kumar
> Trimis: Tuesday, June 19, 2018 8:33 PM
> Către: d...@openvswitch.org
> Subiect: [ovs-dev] [PATCH v4 3/3] datapath-windows: Compute ct hash
> based on 5-tuple and zone
> 
> Conntrack 5-tuple consists of src address, dst address, src port, dst port
and
> protocol which will be unique to a ct session.
> Use this information along with zone to compute hash.
> 
> Also re-factor conntrack code related to parsing netlink attributes.
> 
> Testing:
> Verified loading/unloading the driver with driver verified enabled.
> Ran TCP/UDP and ICMP traffic.
> 
> Signed-off-by: Anand Kumar 
> ---
> v1->v2: Updated commit message to include testing done.
> v2->v3: No change
> v3->v4: No change
> ---
>  datapath-windows/ovsext/Conntrack.c | 228 ++-
> -

Acked-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH v4 1/3] datapath-windows: Use spinlock instead of RW lock for ct entry

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Anand Kumar
> Trimis: Tuesday, June 19, 2018 8:33 PM
> Către: d...@openvswitch.org
> Subiect: [ovs-dev] [PATCH v4 1/3] datapath-windows: Use spinlock instead
of
> RW lock for ct entry
> 
> This patch mainly changes a ndis RW lock for conntrack entry to a spinlock
> along with some minor refactor in conntrack. Using spinlock instead of RW
> lock as RW locks causes performance hits when acquired/released multiple
> times.
> 
> - Use NdisInterlockedXX wrapper api's instead of InterlockedXX.
> - Update 'ctTotalRelatedEntries' using interlocked functions.
> - Move conntrack lock out of NAT module.
> 
> Testing:
> Verified loading/unloading the driver with driver verified enabled.
> Ran TCP/UDP and ICMP traffic.
> 
> Signed-off-by: Anand Kumar 
> ---
> v1->v2: Calculate the dispatch level only in cases where the locks are
> v1->being acquired
>   multiple times within a given context and minor style change.
> v2->v3: Fix kernel crash while executing cleanup thread in
> v2->conntrack-related
> v3->v4: Fix a bug found through code analysis
> ---

Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH] Embrace anonymous unions.

[aserdean]

> > On May 24, 2018, at 10:32 AM, Ben Pfaff  wrote:
> >
> > Several OVS structs contain embedded named unions, like this:
> >
> > struct {
> >...
> >union {
> >...
> >} u;
> > };
> >
> > C11 standardized a feature that many compilers already implemented
> > anyway, where an embedded union may be unnamed, like this:
> >
> > struct {
> >...
> >union {
> >...
> >};
> > };
> >
> > This is more convenient because it allows the programmer to omit "u."
> > in many places.  OVS already used this feature in several places.
> > This commit embraces it in several others.
> >
> > Signed-off-by: Ben Pfaff 
> 
> I didn't read through it carefully, but I support the change, and it's the
sort of
> thing that normally breaks in obvious ways.
> 
> Acked-by: Justin Pettit 
> 
> --Justin
> 
> 
FYI I compiled on MSVC and ran the unit test and it was fine.

Tested-by: Alin Gabriel Serdean 
Acked-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH] ovn.at: Fix IPv6 periodic RA test on Windows

[aserdean]

Apparently I needed the '\n'. I retested with tip of master on Ubuntu and
Windows and it was fine.

I applied it on master.

Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele aserd...@ovn.org
> Trimis: Tuesday, January 23, 2018 11:02 PM
> Către: 'Ben Pfaff' ; 'Alin Gabriel Serdean'

> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] ovn.at: Fix IPv6 periodic RA test on
Windows
> 
> Thanks for the review!
> 


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

Re: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only when extension is enabled

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Shashank Ram
> Trimis: Wednesday, April 4, 2018 8:31 PM
> Către: Alin Gabriel Serdean ; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] RFC datapath-windows: Create com device
> only when extension is enabled
> 
> 
> 
> 
> 
> 
> From: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> on behalf of Alin Gabriel Serdean
> 
> Sent: Tuesday, April 3, 2018 8:08 AM
> To: d...@openvswitch.org
> Cc: Alin Gabriel Serdean
> Subject: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only
> whenextension is enabled
> 
> Until now the communication device between the kernel and userspace
> is created when the network driver is installed.
> 
> Since we only do processing if the Hyper-V vSwitch extension is enabled by
> the user, it makes more sense to move the device creation only when that is
> true.
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  datapath-windows/ovsext/Datapath.c |  2 +-
>  datapath-windows/ovsext/Driver.c   | 10 --
>  datapath-windows/ovsext/Switch.c   |  7 +++
>  3 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-
> windows/ovsext/Datapath.c
> index 34ef2b40a..264cfd3a0 100644
> --- a/datapath-windows/ovsext/Datapath.c
> +++ b/datapath-windows/ovsext/Datapath.c
> @@ -352,7 +352,7 @@ _Dispatch_type_(IRP_MJ_DEVICE_CONTROL)
>  DRIVER_DISPATCH OvsDeviceControl;
> 
>  #ifdef ALLOC_PRAGMA
> -#pragma alloc_text(INIT, OvsCreateDeviceObject)
> +#pragma alloc_text(PAGE, OvsCreateDeviceObject)
>  #pragma alloc_text(PAGE, OvsOpenCloseDevice)
>  #pragma alloc_text(PAGE, OvsCleanupDevice)
>  #pragma alloc_text(PAGE, OvsDeviceControl)
> diff --git a/datapath-windows/ovsext/Driver.c b/datapath-
> windows/ovsext/Driver.c
> index 50c9614e4..39e50c34b 100644
> --- a/datapath-windows/ovsext/Driver.c
> +++ b/datapath-windows/ovsext/Driver.c
> @@ -152,14 +152,6 @@ DriverEntry(PDRIVER_OBJECT driverObject,
>  goto cleanup;
>  }
> 
> -/* Create the communication channel for userspace. */
> -status = OvsCreateDeviceObject(gOvsExtDriverHandle);
> -if (status != NDIS_STATUS_SUCCESS) {
> -NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
> -gOvsExtDriverHandle = NULL;
> -goto cleanup;
> -}
> -
>  cleanup:
>  if (status != NDIS_STATUS_SUCCESS){
>  OvsCleanup();
> @@ -179,8 +171,6 @@ OvsExtUnload(struct _DRIVER_OBJECT *driverObject)
>  {
>  UNREFERENCED_PARAMETER(driverObject);
> 
> -OvsDeleteDeviceObject();
> -
>  NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
> 
>  /* Release driver associated data structures. */
> diff --git a/datapath-windows/ovsext/Switch.c b/datapath-
> windows/ovsext/Switch.c
> index 1ac4fa77c..81b897ffe 100644
> --- a/datapath-windows/ovsext/Switch.c
> +++ b/datapath-windows/ovsext/Switch.c
> @@ -73,6 +73,12 @@ OvsExtAttach(NDIS_HANDLE ndisFilterHandle,
>  NDIS_STATUS status = NDIS_STATUS_FAILURE;
>  NDIS_FILTER_ATTRIBUTES ovsExtAttributes;
>  POVS_SWITCH_CONTEXT switchContext = NULL;
> +/* Create the communication channel for userspace. */
> +status = OvsCreateDeviceObject(gOvsExtDriverHandle);
> +if (status != NDIS_STATUS_SUCCESS) {
> +status = NDIS_STATUS_INVALID_PARAMETER;
> 
> Why change the status? It would be better to retain the status to whatever is
> returned by OvsCreateDeviceObject()
> 
[Alin Serdean] Ooops. Copy paste error 😊. I should also log the error using 
'OVS_LOG_TRACE'.
Does the change fix the issue you encountered?

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

Re: [ovs-dev] [PATCH] tests: Skip tests that need "normal" diff if not available.

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Monday, April 2, 2018 8:15 PM
> Către: d...@openvswitch.org
> Cc: Ben Pfaff ; Stuart Cardall 
> Subiect: [ovs-dev] [PATCH] tests: Skip tests that need "normal" diff if
not
> available.
> 
> busybox diff does not support "normal" diff format, only the unified
format.
> A few OVS tests rely on "normal" format, so those would fail.
> This commit avoids the problem by skipping tests that require "normal"
> format if it is not available.
> 
> Reported-by: Stuart Cardall 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-
> March/046460.html
> Signed-off-by: Ben Pfaff 
> ---
>  tests/atlocal.in  | 10 ++
>  tests/ovsdb-server.at |  5 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/tests/atlocal.in b/tests/atlocal.in index
> 55f9333eee08..2721a588cb90 100644
> --- a/tests/atlocal.in
> +++ b/tests/atlocal.in
> @@ -170,6 +170,16 @@ find_command tcpdump
> 
>  CURL_OPT="-g -v --max-time 1 --retry 2 --retry-delay 1 --connect-timeout
1"
> 
> +# Determine whether "diff" supports "normal" diffs.  (busybox diff does
> +not.) diff () {
> +busybox diff "$@"
> +}
> +if echo xyzzy | diff /dev/null - | grep '^>' >/dev/null; then
> +DIFF_SUPPORTS_NORMAL_FORMAT=yes
> +else
> +DIFF_SUPPORTS_NORMAL_FORMAT=no
> +fi
> +
>  # Turn off proxies.
>  unset http_proxy
>  unset https_proxy
> diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at index
> 2781a14004eb..cbbccab5e5db 100644
> --- a/tests/ovsdb-server.at
> +++ b/tests/ovsdb-server.at
> @@ -1455,6 +1455,7 @@ AT_BANNER([OVSDB -- ovsdb-server replication
> table-exclusion])  m4_define([OVSDB_CHECK_REPLICATION],
> [AT_SETUP([$1])
> AT_KEYWORDS([ovsdb server tcp replication table-exclusion])
> +   AT_SKIP_IF([test $DIFF_SUPPORTS_NORMAL_FORMAT = no])
> $2 > schema
> AT_CHECK([ovsdb-tool create db1 schema], [0], [stdout], [ignore])
> AT_CHECK([ovsdb-tool create db2 schema], [0], [stdout], [ignore]) @@ -
> 1532,6 +1533,8 @@ AT_CLEANUP  #ovsdb-server/set-sync-exclude-tables
> command
>  AT_SETUP([ovsdb-server/set-sync-exclude-tables])
>  AT_KEYWORDS([ovsdb server replication set-exclude-tables])
> +AT_SKIP_IF([test $DIFF_SUPPORTS_NORMAL_FORMAT = no])
> +
>  replication_schema > schema
>  AT_CHECK([ovsdb-tool create db1 schema], [0], [stdout], [ignore])
> AT_CHECK([ovsdb-tool create db2 schema], [0], [stdout], [ignore]) @@ -
> 1619,6 +1622,8 @@ AT_CLEANUP  #ovsdb-server/disconnect-active-server
> command
>  AT_SETUP([ovsdb-server/disconnect-active-server])
>  AT_KEYWORDS([ovsdb server replication disconnect-active-server])
> +AT_SKIP_IF([test $DIFF_SUPPORTS_NORMAL_FORMAT = no])
> +
>  replication_schema > schema
>  AT_CHECK([ovsdb-tool create db1 schema], [0], [stdout], [ignore])
> AT_CHECK([ovsdb-tool create db2 schema], [0], [stdout], [ignore])
> --
> 2.16.1
> 
Acked-by: Alin Gabriel Serdean 


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

Re: [ovs-dev] [PATCH] netlink-socket: don't check for namespace id under Windows

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Flavio Leitner
> Trimis: Tuesday, April 3, 2018 2:48 AM
> Către: Aaron Conole 
> Cc: d...@openvswitch.org; Alin Gabriel Serdean 
> Subiect: Re: [ovs-dev] [PATCH] netlink-socket: don't check for namespace id
> under Windows
> 
> On Mon, Apr 02, 2018 at 06:31:05PM -0400, Aaron Conole wrote:
> > The namespace identification is only relevant under Linux systems.
> > Even if it weren't, this code is targetted to Linux, and the Windows
> > control message structure doesn't look the same.
> >
> > When building on Windows platform, the following error occurs:
> >
> >   lib/netlink-socket.c(750) : error C2039: 'Control' : is not a member of
> 'msghdr'
> >
> > Fixes: a86bd14 ("netlink: provide network namespace id from a msg.")
> > Cc: Flavio Leitner 
> > Signed-off-by: Aaron Conole 
> > ---
> 
> Acked-by: Flavio Leitner 
> 
> Thanks Aaron!
> fbl
> 
Thanks for trying to fix this Aaron and Flavio!
I sent a patch yesterday on the same topic and Ben saw we missed the 
initialization part: 
https://mail.openvswitch.org/pipermail/ovs-dev/2018-April/345677.html 
Again, really thanks for trying to fix it 😊

Alin.

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

Re: [ovs-dev] [PATCH] netlink-socket: Fix broken build on Windows

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Monday, April 2, 2018 7:35 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] netlink-socket: Fix broken build on Windows
> 
> On Mon, Apr 02, 2018 at 02:24:36PM +0300, Alin Gabriel Serdean wrote:
> > Skip network namespace id check on windows since we lack support and
> > integration for their equivalent at the moment.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> I find myself wondering whether this should be:
> 
> diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c index
> f3cce9314ebb..5234ca314008 100644
> --- a/lib/netlink-socket.c
> +++ b/lib/netlink-socket.c
> @@ -747,6 +747,7 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf
> *buf, int *nsid, bool wait)
>   * namespace (no id). Latest kernels return a valid ID only if
>   * available or nothing. */
>  netnsid_set_local(nsid);
> +#ifndef _WIN32
>  cmsg = CMSG_FIRSTHDR(&msg);
>  while (cmsg != NULL) {
>  if (cmsg->cmsg_level == SOL_NETLINK @@ -771,6 +772,7 @@
> nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, int *nsid, bool
wait)
> 
>  cmsg = CMSG_NXTHDR(&msg, cmsg);
>  }
> +#endif
>  }
> 
>  log_nlmsg(__func__, 0, buf->data, buf->size, sock->protocol);
> 
> so that *nsid gets initialized if nonnull.
> 
> Once you decided either way:
> 
> Acked-by: Ben Pfaff 
Thanks for the suggestion Ben. You are right, I didn't notice the
initialization part.

I folded in the comments and applied it on master.

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

Re: [ovs-dev] [PATCH] v2, python-windows: Fix unicode python tests on Windows

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Saturday, March 31, 2018 10:46 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] v2, python-windows: Fix unicode python
> tests on Windows
> 
> On Wed, Mar 28, 2018 at 12:11:18PM +0300, Alin Gabriel Serdean wrote:
> > This patch changes the default filesystem encodings to the values used
> > before python3.6 to ensure compatibility with older versions.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> > Co-authored-by: Alin Balutoiu 
> > ---
> > v2: update commit message
> 
> I don't really understand this change, but if it fixes a problem in the
tests
> then I'm all in favor of it.
> 
> If it is not difficult, then I would suggest adding a little more
information to
> the commit message, such as an example of what goes wrong without it.
> 
> Acked-by: Ben Pfaff 
> ___
I'll drop this change and improve the commit message in the next revision.
Thanks for the review!

Alin.



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

Re: [ovs-dev] [PATCH] Windows: Fix broken build caused by a bad file extension

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Saturday, March 31, 2018 10:47 PM
> Către: Alin Gabriel Serdean 
> Cc: ovs-dev@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] Windows: Fix broken build caused by a bad
> file extension
> 
> On Tue, Mar 27, 2018 at 08:29:49PM +0300, Alin Gabriel Serdean wrote:
> > The compiler (cl) complains:
> > `ovsdb/ovsdb-server.c(689) : fatal error C1083:
> > Cannot open include file: 'ovsdb/_server.ovsschema.inc':
> >No such file or directory`
> > (https://ci.appveyor.com/project/blp/ovs/build/1.0.4079#L2586)
> >
> > Generated compiler objects have the extension `.obj` on Windows.
> >
> > This patch switches to `$(OBJEXT)` instead, so the schema will be
> generated.
> >
> > Signed-off-by: Alin Gabriel Serdean aserd...@ovn.org
> 
> Oops!  Thanks for the fix.
> 
> Acked-by: Ben Pfaff 
No worries, it happens. Thanks for the review, I applied it on master!

Alin.

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

Re: [ovs-dev] [PATCH v3] ovsdb-client: Set binary mode when doing backup/restore

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Wednesday, March 14, 2018 8:11 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH v3] ovsdb-client: Set binary mode when doing
> backup/restore
> 
> On Mon, Mar 12, 2018 at 03:17:42PM +0200, Alin Gabriel Serdean wrote:
> > Add some needed consistency on Windows for STD_IN/OUT file
> descriptors
> > when doing backup and restore.
> >
> > Reported-at:https://mail.openvswitch.org/pipermail/ovs-dev/2018-Januar
> > y/343518.html
> > Suggested-by: Ben Pfaff 
> > Co-authored-by: Ben Pfaff 
> > Signed-off-by: Ben Pfaff 
> > Signed-off-by: Alin Gabriel Serdean 
> > ---
> >  v3: Incorporate comments suggested by Ben.
> >  v2: Fix copy paste error.
> 
> I'd move the { to a line of its own here:
> 
> > +static void
> > +set_binary_mode(FILE *stream OVS_UNUSED) {
> 
> Acked-by: Ben Pfaff 
Ooops. I folded in the change and applied on master. Ty!


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

Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client backup test on Win

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 8:55 PM
> Către: aserd...@ovn.org
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client backup test
> on Win
> 
> On Thu, Mar 08, 2018 at 08:49:07PM +0200, aserd...@ovn.org wrote:
> >
> >
> > > -Mesaj original-
> > > De la: ovs-dev-boun...@openvswitch.org  > > boun...@openvswitch.org> În numele Ben Pfaff
> > > Trimis: Thursday, March 8, 2018 8:45 PM
> > > Către: aserd...@ovn.org
> > > Cc: d...@openvswitch.org
> > > Subiect: Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client
> > > backup test on Win
> > >
> > > On Thu, Mar 08, 2018 at 08:33:20PM +0200, aserd...@ovn.org wrote:
> > > > It's been a while 😊.
> > > >
> > > > I sent a patch as you suggested:
> > > https://patchwork.ozlabs.org/patch/883250/ .
> > > > Mind taking a look?
> > >
> > > I reviewed it now.
> > That was quick!
> >
> 
> You caught me at a good time.  Now I'm AFK for a bit...
[Alin Serdean] Can you take a look over the next incremental: 
https://patchwork.ozlabs.org/patch/884591/ when you have time?

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

Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client backup test on Win

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 8:45 PM
> Către: aserd...@ovn.org
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client backup test
> on Win
> 
> On Thu, Mar 08, 2018 at 08:33:20PM +0200, aserd...@ovn.org wrote:
> > It's been a while 😊.
> >
> > I sent a patch as you suggested:
> https://patchwork.ozlabs.org/patch/883250/ .
> > Mind taking a look?
> 
> I reviewed it now.
That was quick!

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

Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client backup test on Win

[aserdean]

It's been a while 😊.

I sent a patch as you suggested: https://patchwork.ozlabs.org/patch/883250/ .
Mind taking a look?

Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele aserd...@ovn.org
> Trimis: Tuesday, January 23, 2018 11:01 PM
> Către: 'Ben Pfaff' ; 'Alin Gabriel Serdean'
> 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client backup test
> on Win
> 
> (facepalm). Thanks for pointing me to the right direction. I will look into 
> it.
> 
> Thanks,
> Alin.
> 
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Tuesday, January 23, 2018 2:24 AM
> > To: Alin Gabriel Serdean 
> > Cc: d...@openvswitch.org
> > Subject: Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client
> > backup
> test
> > on Win
> >
> > On Sun, Jan 14, 2018 at 08:26:47PM +0200, Alin Gabriel Serdean wrote:
> > > The test:
> > > 1948. ovsdb-client.at:15: testing ovsdb-client backup and restore
> > > fails on Windows with:
> > > --- /dev/null   2018-01-14 20:09:57 +0200
> > > +++ /c/_2018/january/14/ovs/tests/testsuite.dir/at-groups/1948/stder
> > > +++ r
> > > @@ -0,0 +1,3 @@
> > > +ovsdb-server: ovsdb error: backup: unexpected file format
> > > +ovsdb-server: Failed to read from child (The pipe has been ended.
> > > +)
> > > ./ovsdb-client.at:111: exit code was 1, expected 0
> > >
> > > The root cause is that when redirecting output defaults to the
> > > Windows line
> > > endings(CRLF):
> > > $ file db
> > > db: ASCII text, with very long lines $ file backup
> > > backup: ASCII text, with very long lines, with CRLF line terminators
> > >
> > > Add a `dos2unix` command to convert to the line endings expected by
> > > ovsdb-server.
> > >
> > > Signed-off-by: Alin Gabriel Serdean 
> >
> > Thank you for the fix.
> >
> > Hmm, maybe this is a bug in "ovsdb-client backup".  Maybe it should do
> > something like:
> >
> > #ifdef _WIN32
> > fflush(stdout);
> > _setmode(STDOUT_FILENO, _O_BINARY); #endif
> >
> > and then the test wouldn't have to change at all?  What do you think?
> >
> > I don't know whether anything similar is needed for stdin for
> "ovsdb-client
> > restore".
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] *** SPAM *** [PATCH] [RFC] build-windows: Add pthread to windows build

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 2:12 AM
> Către: aserd...@ovn.org
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] *** SPAM *** [PATCH] [RFC] build-windows: Add
> pthread to windows build
> 
> On Thu, Mar 08, 2018 at 01:20:43AM +0200, aserd...@ovn.org wrote:
> > If we need the files in the pthread library, why not check in the files?
> > It seems more straightforward than checking in the tarball, especially
if we
> need to maintain it ourselves (which seems likely if it is truly
abandoned).
> >
> > What is the license for the pthread library?
> > [Alin Serdean]
> > Since we don't need to change any of the files yet, I thought the
> > tarball (either in the repo or if the user points to a tarball) would
> > be cleaner for the moment.
> >
> > I'm okay with checking in the files.
> >
> > License is LGPL
> > (https://www.sourceware.org/pthreads-win32/copying.html),
> > I don't know if that would cause any issues.
> 
> I think it is OK, as long as we are dynamically linking against it.

Dropping this change and will send a different patch set.

Alin.

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

Re: [ovs-dev] [PATCH] tests-windows: Add OpenSSL directory to autotest path

[aserdean]

Thanks for the review! Applied on master.

Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 3:09 AM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests-windows: Add OpenSSL directory to
> autotest path
> 
> On Tue, Mar 06, 2018 at 01:01:00PM +0200, Alin Gabriel Serdean wrote:
> > Running OpenSSL unit tests without it already being included in
> > library path revealed a problem: the AUTOTEST_PATH does not include it.
> >
> > This patch adds a new variable `SSL_DIR` which will be added in
> AUTOTEST_PATH.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Acked-by: Ben Pfaff 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] ovs-bugtool: Fix ambiguous variable flake8 warnings

[aserdean]

No worries!

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 3:25 AM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] ovs-bugtool: Fix ambiguous variable flake8
> warnings
> 
> On Thu, Mar 08, 2018 at 03:18:46AM +0200, Alin Gabriel Serdean wrote:
> > Flake8 complains:
> > utilities/bugtool/ovs-bugtool.in:761:5: E741 ambiguous variable name 'l'
> > utilities/bugtool/ovs-bugtool.in:1162:5: E741 ambiguous variable name
'l'
> > Makefile:6193: recipe for target 'flake8-check' failed
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> I applied this to master.
> 
> (Afterward I realized that I should have let you do that.  Sorry!)
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 3:07 AM
> Către: aserd...@ovn.org
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is
> hanging
> 
> On Thu, Mar 08, 2018 at 02:22:45AM +0200, aserd...@ovn.org wrote:
> > > -Mesaj original-
> > > De la: ovs-dev-boun...@openvswitch.org  > > boun...@openvswitch.org> În numele Ben Pfaff
> > > Trimis: Thursday, March 8, 2018 2:11 AM
> > > Către: aserd...@ovn.org
> > > Cc: d...@openvswitch.org
> > > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test
> > > which is hanging
> > >
> > > On Thu, Mar 08, 2018 at 01:45:51AM +0200, aserd...@ovn.org wrote:
> > > > > -Mesaj original-
> > > > > De la: ovs-dev-boun...@openvswitch.org  > > > > boun...@openvswitch.org> În numele Ben Pfaff
> > > > > Trimis: Wednesday, March 7, 2018 11:20 PM
> > > > > Către: Alin Gabriel Serdean 
> > > > > Cc: d...@openvswitch.org
> > > > > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test
> > > > > which is hanging
> > > > >
> > > > > On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean
> wrote:
> > > > > > The test:
> > > > > > `1827. ovsdb-server.at:490: testing SSL db: implementation
> > > > > > ...` is hanging on Windows because the returned in the case
> > > > > > the client failed to connect is "Unknown error" vs the normal
> "Protocol error".
> > > > > >
> > > > > > Update the test to accommodate for this.
> > > > > >
> > > > > > Signed-off-by: Alin Gabriel Serdean 
> > > > >
> > > > > Thanks for finding and fixing the error.
> > > > >
> > > > > I think that there is an underlying problem here: the code is
> > > > > bad at error handling in general.  It's supposed to kill the
> > > > > ovsdb-server if anything fails, but it didn't do it right: it
> > > > > killed $(cat pid) but the pidfile is actually in ovsdb-
> > > > > server.pid.  This meant that the issue showed up as a hang instead of
> a test failure, which seems bad.
> > > > >
> > > > > Also, I'm kind of inclined to just disregard the particular
> > > > > error message.  It might save time later.  (I guess there's a
> > > > > risk that the connection fails happens for some other reason
> > > > > that should fail the test
> > > > > though.)
> > > > >
> > > > > Anyway, I sent a pair of patches that implement my suggestions:
> > > > > https://patchwork.ozlabs.org/patch/882796/
> > > > > https://patchwork.ozlabs.org/patch/882798/
> > > > >
> > > > > What are your thoughts?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Ben.
> > > > [Alin Serdean] I'm dropping this change and acking yours.
> > > > They are much cleaner and better 😊.
> > >
> > > Thanks!  I applied both of these fixes to master.
> > >
> > > Do you want me to backport them to branch-2.9 (or earlier)?  I have
> > > not done that yet.
> >
> > branch-2.9 also please.
> >
> 
> Done.
Thank you!

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

Re: [ovs-dev] [PATCH] tests-windows: Make flow extractor work on Windows

[aserdean]

Thanks for the quick review.
I removed the import and changed a bit the message.

Applied on master.
Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Wednesday, March 7, 2018 11:07 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests-windows: Make flow extractor work on
> Windows
> 
> Applying this commit adds a file named
> "c/_2018/march/06/ovs/tests/testsuite.dir/at-groups/42/stderr" to the
> repo!  I think that the failure needs to be indented a bit to un-confuse
"git
> am".
> 
> When I fix that up, I get the following failure from flake8:
> ../tests/flowgen.py:17:1: F401 'os' imported but unused
> 
> Assuming you fix up those two little things:
> Acked-by: Ben Pfaff 
> 
> Thanks!
> 
> On Tue, Mar 06, 2018 at 05:23:56PM +0200, Alin Gabriel Serdean wrote:
> > 42. library.at:3: testing flow extractor ...
> > ./library.at:4: $PYTHON $srcdir/flowgen.py >/dev/null 3>flows 4>pcap
> > --- /dev/null   2018-03-06 15:59:49 +0200
> > +++ /c/_2018/march/06/ovs/tests/testsuite.dir/at-groups/42/stderr
> 2018-03-06 15:59:49 +0200
> > @@ -0,0 +1,4 @@
> > +Traceback (most recent call last):
> > +  File "../.././flowgen.py", line 204, in 
> > +flows = os.fdopen(3, 'wb')
> > +OSError: [Errno 9] Bad file descriptor
> > ./library.at:4: exit code was 1, expected 0 42. library.at:3:  FAILED
> > (library.at:4)
> >
> > Unfortunately 3/4/5/.. FDs don't work on Windows. Switch to a filename
> > open instead of opening the FDs.
> >
> > Signed-off-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH v2] datapath-windows: Do not drop Ip fragments less than MIN_FRAGMENT_SIZE

[aserdean]

Applied on master!

Thanks,
Alin.

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Anand Kumar
> Trimis: Thursday, March 8, 2018 2:16 AM
> Către: aserd...@ovn.org; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH v2] datapath-windows: Do not drop Ip
> fragments less than MIN_FRAGMENT_SIZE
> 
> Hi Alin,
> 
> Thanks for reviewing the patch.
> Can we get this patch applied?
> 
> Thanks,
> Anand Kumar
> 

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

Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 2:11 AM
> Către: aserd...@ovn.org
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is
> hanging
> 
> On Thu, Mar 08, 2018 at 01:45:51AM +0200, aserd...@ovn.org wrote:
> > > -Mesaj original-
> > > De la: ovs-dev-boun...@openvswitch.org  > > boun...@openvswitch.org> În numele Ben Pfaff
> > > Trimis: Wednesday, March 7, 2018 11:20 PM
> > > Către: Alin Gabriel Serdean 
> > > Cc: d...@openvswitch.org
> > > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test
> > > which is hanging
> > >
> > > On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean wrote:
> > > > The test:
> > > > `1827. ovsdb-server.at:490: testing SSL db: implementation ...` is
> > > > hanging on Windows because the returned in the case the client
> > > > failed to connect is "Unknown error" vs the normal "Protocol error".
> > > >
> > > > Update the test to accommodate for this.
> > > >
> > > > Signed-off-by: Alin Gabriel Serdean 
> > >
> > > Thanks for finding and fixing the error.
> > >
> > > I think that there is an underlying problem here: the code is bad at
> > > error handling in general.  It's supposed to kill the ovsdb-server
> > > if anything fails, but it didn't do it right: it killed $(cat pid)
> > > but the pidfile is actually in ovsdb- server.pid.  This meant that
> > > the issue showed up as a hang instead of a test failure, which seems bad.
> > >
> > > Also, I'm kind of inclined to just disregard the particular error
> > > message.  It might save time later.  (I guess there's a risk that
> > > the connection fails happens for some other reason that should fail
> > > the test
> > > though.)
> > >
> > > Anyway, I sent a pair of patches that implement my suggestions:
> > > https://patchwork.ozlabs.org/patch/882796/
> > > https://patchwork.ozlabs.org/patch/882798/
> > >
> > > What are your thoughts?
> > >
> > > Thanks,
> > >
> > > Ben.
> > [Alin Serdean] I'm dropping this change and acking yours.
> > They are much cleaner and better 😊.
> 
> Thanks!  I applied both of these fixes to master.
> 
> Do you want me to backport them to branch-2.9 (or earlier)?  I have not done
> that yet.

branch-2.9 also please.

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

Re: [ovs-dev] [PATCH 2/2] ovsdb-server: Don't be picky about particular error in test.

[aserdean]

Acked-by: Alin Gabriel Serdean 

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Wednesday, March 7, 2018 11:19 PM
> Către: d...@openvswitch.org
> Cc: Ben Pfaff ; Alin Gabriel Serdean 
> Subiect: [ovs-dev] [PATCH 2/2] ovsdb-server: Don't be picky about
particular
> error in test.
> 
> On Windows this test reports "Unknown error" instead of "Protocol error",
> so disregard the particular error message.
> 
> Reported-by: Alin Gabriel Serdean 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-
> March/344951.html
> Signed-off-by: Ben Pfaff 
> ---
>  tests/ovsdb-server.at | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 


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

Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

[aserdean]

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Wednesday, March 7, 2018 11:20 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is
> hanging
> 
> On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean wrote:
> > The test:
> > `1827. ovsdb-server.at:490: testing SSL db: implementation ...` is
> > hanging on Windows because the returned in the case the client failed
> > to connect is "Unknown error" vs the normal "Protocol error".
> >
> > Update the test to accommodate for this.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Thanks for finding and fixing the error.
> 
> I think that there is an underlying problem here: the code is bad at error
> handling in general.  It's supposed to kill the ovsdb-server if anything 
> fails,
> but it didn't do it right: it killed $(cat pid) but the pidfile is actually 
> in ovsdb-
> server.pid.  This meant that the issue showed up as a hang instead of a test
> failure, which seems bad.
> 
> Also, I'm kind of inclined to just disregard the particular error message.  It
> might save time later.  (I guess there's a risk that the connection fails
> happens for some other reason that should fail the test
> though.)
> 
> Anyway, I sent a pair of patches that implement my suggestions:
> https://patchwork.ozlabs.org/patch/882796/
> https://patchwork.ozlabs.org/patch/882798/
> 
> What are your thoughts?
> 
> Thanks,
> 
> Ben.
[Alin Serdean] I'm dropping this change and acking yours.
They are much cleaner and better 😊.

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

Re: [ovs-dev] [PATCH 1/2] tests: Fix hang when "SSL db: implementation" test failed.

[aserdean]

Acked-by: Alin Gabriel Serdean 

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Wednesday, March 7, 2018 11:19 PM
> Către: d...@openvswitch.org
> Cc: Ben Pfaff ; Alin Gabriel Serdean 
> Subiect: [ovs-dev] [PATCH 1/2] tests: Fix hang when "SSL db:
> implementation" test failed.
> 
> The tests were killing $(cat pid) on failure but needed to kill $(cat
ovsdb-
> server.pid).
> 
> Reported-by: Alin Gabriel Serdean 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-
> March/344951.html
> Signed-off-by: Ben Pfaff 
> ---

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

Re: [ovs-dev] *** SPAM *** [PATCH] [RFC] build-windows: Add pthread to windows build

[aserdean]

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org  În 
numele Ben Pfaff
Trimis: Wednesday, March 7, 2018 10:53 PM
Către: Alin Gabriel Serdean 
Cc: d...@openvswitch.org
Subiect: Re: [ovs-dev] *** SPAM *** [PATCH] [RFC] build-windows: Add pthread to 
windows build

On Mon, Mar 05, 2018 at 02:40:01PM +0200, Alin Gabriel Serdean wrote:
> Until know we asked users to download a precompiled library of 
> `pthreads` as part of the build chain.
> 
> A big problem with this approach is that the precompiled version of 
> pthreads is built using VS2010 and we must redistribute that runtime 
> to install targets as well.
> 
> The library seems to be abandoned and no longer maintained. I tried to 
> email the authors several times and unfortunately, I haven’t received a reply.
> In the end we might end up maintaining the project under the folder `windows`.
> 
> For now, however we just add the source tarball of the pthread library 
> to the `windows` folder.  We simplify the process of installing and 
> building by compiling pthread library shipped in the `windows` folder 
> during the build step.
> We still allow `--with-pthread=` argument during configure step. If 
> that argument is not specified we fall back to the shipped tarball.
> 
> Signed-off-by: Alin Gabriel Serdean 

If we need the files in the pthread library, why not check in the files?
It seems more straightforward than checking in the tarball, especially if we 
need to maintain it ourselves (which seems likely if it is truly abandoned).

What is the license for the pthread library?
[Alin Serdean]
Since we don't need to change any of the files yet,
I thought the tarball (either in the repo or if the user points to a tarball)
would be cleaner for the moment.

I'm okay with checking in the files.

License is LGPL (https://www.sourceware.org/pthreads-win32/copying.html),
I don't know if that would cause any issues.


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

Re: [ovs-dev] [PATCH v2] datapath-windows: Do not drop Ip fragments less than MIN_FRAGMENT_SIZE

[aserdean]

Acked-by: Alin Gabriel Serdean 

Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org  În
numele Anand Kumar
Trimis: Wednesday, March 7, 2018 1:48 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v2] datapath-windows: Do not drop Ip fragments
less than MIN_FRAGMENT_SIZE

Previously ipfragment module would drop any fragments less than
MIN_FRAGMENT_SIZE (400 bytes), which was added to safeguard against the
vulnerability CVE-2000-0305. This check is incorrect, since minimum size of
the Ipfragment is 68 bytes (i.e. max length of Ip Header + 8 bytes of
L4 header). So Ip fragments less than MIN_FRAGMENT_SIZE (400 bytes) is not
guranted to be malformed or illegal.

To guard against security vulnerability CVE-2000-0305, for a given ip
datagram, ipfragments should be dropped only when number of smallest
fragments recieved reaches a certain threshold.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/IpFragment.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/datapath-windows/ovsext/IpFragment.c
b/datapath-windows/ovsext/IpFragment.c
index 3d5277a..d59d7cf 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -25,10 +25,10 @@
 #undef OVS_DBG_MOD
 #endif
 #define OVS_DBG_MOD OVS_DBG_IPFRAG
-/* Based on MIN_FRAGMENT_SIZE.*/
-#define MAX_FRAGMENTS 164
+
 #define MIN_FRAGMENT_SIZE 400
 #define MAX_IPDATAGRAM_SIZE 65535
+#define MAX_FRAGMENTS MAX_IPDATAGRAM_SIZE/MIN_FRAGMENT_SIZE + 1
 
 /* Function declarations */
 static KSTART_ROUTINE OvsIpFragmentEntryCleaner; @@ -275,10 +275,7 @@
OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext,
 offset = ntohs(ipHdr->frag_off) & IP_OFFSET;
 offset <<= 3;
 flags = ntohs(ipHdr->frag_off) & IP_MF;
-/* Only the last fragment can be of smaller size.*/
-if (flags && ntohs(ipHdr->tot_len) < MIN_FRAGMENT_SIZE) {
-return NDIS_STATUS_INVALID_LENGTH;
-}
+
 /*Copy fragment specific fields. */
 fragKey.protocol = ipHdr->protocol;
 fragKey.id = ipHdr->id;
--
2.9.3.windows.1

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

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

Re: [ovs-dev] [PATCH] [RFC] build-windows: Add pthread to windows build

[aserdean]

Adding Ben to thread also.

The footprint isn't that bad compares to the MSI images:
alin@DESKTOP-IUDEFLM /c/_2018/march/temp/ovs/windows
$ ls -lh ovs-windows-installer/images/
total 582K
-rw-r--r-- 1 alin Administrators 132K Mar  6 20:00 bannrbmp.bmp
-rw-r--r-- 1 alin Administrators 450K Mar  6 20:00 dlgbmp.bmp

alin@DESKTOP-IUDEFLM /c/_2018/march/temp/ovs/windows
$ ls -lh pthreads-w32-2-9-1-release.tar.gz
-rw-r--r-- 1 alin Administrators 361K Mar  6 20:37 
pthreads-w32-2-9-1-release.tar.gz

It makes it easier for people which are building on systems that
don't have an internet connections.

Another idea would be to have `--with-pthread-tar=` and point it
to the tarball.

Which one do you prefer?

Thanks,
Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org  În 
numele Shashank Ram
Trimis: Tuesday, March 6, 2018 8:17 PM
Către: Alin Gabriel Serdean 
Cc: d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] [RFC] build-windows: Add pthread to windows build

Hi Alin, please lets not checkin a binary file into the repo. It will make
the repo size bigger and some git operations slower. Also it will be a pain
to erase it from the history if it needs to be removed.

Thanks,
Shashank

On Mon, Mar 5, 2018 at 4:40 AM, Alin Gabriel Serdean 
wrote:

> Until know we asked users to download a precompiled library of `pthreads`
> as
> part of the build chain.
>
> A big problem with this approach is that the precompiled version of
> pthreads
> is built using VS2010 and we must redistribute that runtime to install
> targets
> as well.
>
> The library seems to be abandoned and no longer maintained. I tried to
> email
> the authors several times and unfortunately, I haven’t received a reply.
> In the end we might end up maintaining the project under the folder
> `windows`.
>
> For now, however we just add the source tarball of the pthread library to
> the
> `windows` folder.  We simplify the process of installing and building by
> compiling pthread library shipped in the `windows` folder during the build
> step.
> We still allow `--with-pthread=` argument during configure step. If that
> argument is not specified we fall back to the shipped tarball.
>
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  Makefile.am   |  11 +++
>  m4/openvswitch.m4 |  17 -
>  windows/.gitignore|   3 +++
>  windows/automake.mk   |   3 ++-
>  windows/pthreads-w32-2-9-1-release.tar.gz | Bin 0 -> 369537 bytes
>  5 files changed, 32 insertions(+), 2 deletions(-)
>  create mode 100644 windows/pthreads-w32-2-9-1-release.tar.gz
>

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

Re: [ovs-dev] [PATCH] datapath-windows: Fix static analysis in Stt.c

[aserdean]

Thanks. Applied on master.

Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org  În 
numele Sairam Venugopal
Trimis: Tuesday, March 6, 2018 7:56 PM
Către: Alin Gabriel Serdean ; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Fix static analysis in Stt.c

Didn't realize this patch wasn't acked. Please push this in.

Acked-by: Sairam Venugopal 

Thanks,
Sairam

On 2/8/18, 9:31 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin Gabriel 
Serdean"  wrote:

The WDK 10 static analysis complains:
stt.c(427): warning C30030: Warning: Allocating executable memory via
specifying a MM_PAGE_PRIORITY type without a bitwise OR with
MdlMappingNoExecute.

Signed-off-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH] datapath-windows: Do not drop Ip fragments less than MIN_FRAGMENT_SIZE

[aserdean]

I guess you can also remove the define
(https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/IpFr
agment.c#L30)
since it is not used anywhere else.

Thanks,
Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org  În
numele Anand Kumar
Trimis: Tuesday, March 6, 2018 1:21 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Do not drop Ip fragments less
than MIN_FRAGMENT_SIZE

Previously ipfragment module would drop any fragments less than
MIN_FRAGMENT_SIZE (400 bytes), which was added to safeguard against the
vulnerability CVE-2000-0305. This check is incorrect, since minimum size of
the Ipfragment is 68 bytes (i.e. max length of Ip Header + 8 bytes of
L4 header). So Ip fragments less than MIN_FRAGMENT_SIZE (400 bytes) is not
guranted to be malformed or illegal.

To guard against security vulnerability CVE-2000-0305, for a given ip
datagram, ipfragments should be dropped only when number of smallest
fragments recieved reaches a certain threshold.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/IpFragment.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/IpFragment.c
b/datapath-windows/ovsext/IpFragment.c
index 3d5277a..da9d33a 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -275,10 +275,7 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT
switchContext,
 offset = ntohs(ipHdr->frag_off) & IP_OFFSET;
 offset <<= 3;
 flags = ntohs(ipHdr->frag_off) & IP_MF;
-/* Only the last fragment can be of smaller size.*/
-if (flags && ntohs(ipHdr->tot_len) < MIN_FRAGMENT_SIZE) {
-return NDIS_STATUS_INVALID_LENGTH;
-}
+
 /*Copy fragment specific fields. */
 fragKey.protocol = ipHdr->protocol;
 fragKey.id = ipHdr->id;
--
2.9.3.windows.1

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

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

Re: [ovs-dev] [PATCH] AppVeyor: Add Win10 compilation to the build

[aserdean]

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org  În
numele Shashank Ram
Trimis: Monday, March 5, 2018 6:44 AM
Către: Alin Gabriel Serdean 
Cc: d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] AppVeyor: Add Win10 compilation to the build

On Fri, Mar 2, 2018 at 3:32 PM, Alin Gabriel Serdean 
wrote:

> People from AppVeyor are nice and included the Windows 10 DDK (driver 
> development kit).
>
> This patch allows AppVeyor to compile the Win10 target.
>
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  appveyor.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/appveyor.yml b/appveyor.yml index da31764c1..49fcae4b5 
> 100644
> --- a/appveyor.yml
> +++ b/appveyor.yml
> @@ -41,6 +41,6 @@ build_script:
>  - C:\MinGW\msys\1.0\bin\bash -lc "cp 
> /c/pthreads-win32/Pre-built.2/dll/x86/*.dll
> /c/openvswitch/."
>  - C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe"
>  - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./boot.sh"
> -- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure 
> CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi 
> -lwbemuuid
> -lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2
> --with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\"
> --with-vstudiotargetver=\"Win8,Win8.1\""
> +- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure
> CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi 
> -lwbemuuid
> -lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2
> --with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\"
>  - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make"
>  - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make 
> datapath_windows_analyze"
> --
> 2.16.1.windows.1
>
> ___
>

Acked-by: Shashank Ram 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[Alin Serdean] Thanks for the review! Applied on master.

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

Re: [ovs-dev] [PATCH v2] datapath-windows: Support to selectively compile targets

[aserdean]

Applied on master with the following incremental:
60c66
< + MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win10$(VSTUDIO_CONFIG)"
/property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
---
> +MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win10$(VSTUDIO_CONFIG)"
/property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
73c79
< + MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Clean /property:Configuration="Win10$(VSTUDIO_CONFIG)"
/property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
---
> +MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Clean /property:Configuration="Win10$(VSTUDIO_CONFIG)"
/property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
80c86

Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele aserd...@ovn.org
Trimis: Friday, March 2, 2018 1:05 AM
Către: 'Anand Kumar' ; 'Shashank Ram'
; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v2] datapath-windows: Support to selectively
compile targets

There is a small nit, win 10 targets contain regular ' ' instead of a '\t'
in the Makefile. I will change it and apply on master.

Thanks a lot for the patch.

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Anand Kumar
Trimis: Tuesday, February 27, 2018 11:40 PM
Către: Shashank Ram ; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v2] datapath-windows: Support to selectively
compile targets

Acked-by: Anand Kumar 

Thanks,
Anand Kumar

On 2/27/18, 11:57 AM, "ovs-dev-boun...@openvswitch.org on behalf of Shashank
Ram"  wrote:

Adds support to selectively compile kernel driver for
target versions. This is useful when environments to
compile for all targets might not be available on the
user's machine, or if the user wants to only compile
some targets selectively.

Also once appveyor has support to build Win10 targets,
we will not pass the "--with-vstudiotargetver" to the
configure script.

Signed-off-by: Shashank Ram 
---
 Documentation/intro/install/windows.rst |  5 -
 Makefile.am | 15 +++
 appveyor.yml|  4 ++--
 m4/openvswitch.m4   | 28
+++-
 4 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/Documentation/intro/install/windows.rst
b/Documentation/intro/install/windows.rst

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

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

Re: [ovs-dev] [PATCH v2] datapath-windows: Support to selectively compile targets

[aserdean]

There is a small nit, win 10 targets contain regular ' ' instead of a '\t'
in the Makefile. I will change it and apply on master.

Thanks a lot for the patch.

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Anand Kumar
Trimis: Tuesday, February 27, 2018 11:40 PM
Către: Shashank Ram ; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v2] datapath-windows: Support to selectively
compile targets

Acked-by: Anand Kumar 

Thanks,
Anand Kumar

On 2/27/18, 11:57 AM, "ovs-dev-boun...@openvswitch.org on behalf of Shashank
Ram"  wrote:

Adds support to selectively compile kernel driver for
target versions. This is useful when environments to
compile for all targets might not be available on the
user's machine, or if the user wants to only compile
some targets selectively.

Also once appveyor has support to build Win10 targets,
we will not pass the "--with-vstudiotargetver" to the
configure script.

Signed-off-by: Shashank Ram 
---
 Documentation/intro/install/windows.rst |  5 -
 Makefile.am | 15 +++
 appveyor.yml|  4 ++--
 m4/openvswitch.m4   | 28
+++-
 4 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/Documentation/intro/install/windows.rst
b/Documentation/intro/install/windows.rst

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

Re: [ovs-dev] [PATCH v2] datapath-windows: On Debug builds, dump NBL info based on OVS_DBG_DEFAULT macro

[aserdean]

Thanks for the patch Anand.
I applied it on master.

Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Anand Kumar
Trimis: Wednesday, February 28, 2018 7:00 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v2] datapath-windows: On Debug builds, dump NBL
info based on OVS_DBG_DEFAULT macro

Currently nbl information is getting dumped whenever a nbl is copied or
allocated, since OVS_DBG_DEFAULT is set to OVS_DBG_INFO for debug builds,
which affects the ovs performance. Instead dump nbl information only when
OVS_DBG_DEFAULT is set to OVS_LOG_LOUD

Signed-off-by: Anand Kumar 

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

Re: [ovs-dev] [PATCH v2] datapath-windows: On Debug builds, dump NBL info based on OVS_DBG_DEFAULT macro

[aserdean]

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Anand Kumar
Trimis: Wednesday, February 28, 2018 7:00 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v2] datapath-windows: On Debug builds, dump NBL
info based on OVS_DBG_DEFAULT macro

Currently nbl information is getting dumped whenever a nbl is copied or
allocated, since OVS_DBG_DEFAULT is set to OVS_DBG_INFO for debug builds,
which affects the ovs performance. Instead dump nbl information only when
OVS_DBG_DEFAULT is set to OVS_LOG_LOUD

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/BufferMgmt.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)


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

Re: [ovs-dev] [PATCH] datapath-windows: fix hash creation on ct mark

[aserdean]

Thanks Ben and Anand. I applied from master to branch-2.6.

Thanks,
Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Ben Pfaff
Trimis: Wednesday, February 21, 2018 8:41 PM
Către: Alin Gabriel Serdean 
Cc: d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapath-windows: fix hash creation on ct
mark

On Wed, Feb 21, 2018 at 04:57:29PM +0200, Alin Gabriel Serdean wrote:
> Use key->ct.mark instead of key->ct.zone when generating the hash over 
> the mark.
> 
> Signed-off-by: Alin Gabriel Serdean 

Seems obviously correct.

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

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

Re: [ovs-dev] [PATCH] datapath-windows: Support to selectively compile targets

[aserdean]

Trimming the message a bit.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Shashank Ram
Trimis: Thursday, February 8, 2018 7:50 PM
Către: aserd...@ovn.org; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Support to selectively
compile targets

Hi Alin, thanks for the review.
I personally feel we should be consistent and run configure, and have a
single make command to build both user space and kernel. What part did you
find complicated?
[Alin Serdean] I.e. if I configure to target 8. And after I need to target
10 I need to do a reconfigure (similar, for debug and or other platforms).
The configure part is particularly slow on Windows.
For convenience the old part with selecting Debug/Release and
trying to build for all the compilers found in the system is still there, so
building both
userspace and kernel will still be in a single command.
I don't see a huge issue to specify two or more make commands to build a
particular target of the kernel
via the shell.

Thanks,
Shashank


From: aserd...@ovn.org 
Sent: Thursday, February 8, 2018 9:32:59 AM
To: Shashank Ram; d...@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Support to selectively
compile targets

It looks a bit complicated.

What do you think about the following approach:

git diff datapath-windows/automake.mk
diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 3820041f6..164567734 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -85,6 +85,26 @@ EXTRA_DIST += \
datapath-windows/ovsext/precompsrc.c \
datapath-windows/ovsext/resource.h

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

Re: [ovs-dev] [PATCH] datapath-windows: Support to selectively compile targets

[aserdean]

It looks a bit complicated.

What do you think about the following approach:

git diff datapath-windows/automake.mk
diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 3820041f6..164567734 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -85,6 +85,26 @@ EXTRA_DIST += \
datapath-windows/ovsext/precompsrc.c \
datapath-windows/ovsext/resource.h

+ARCH=x64
+
 datapath_windows_analyze: all
-   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8.1Analyze"
-   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8Analyze"
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8.1Analyze" //p:Platform=$(ARCH)
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8Analyze" //p:Platform=$(ARCH)
+
+datapath_windows_win_8_debug: all
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8Debug" //p:Platform=$(ARCH)
+
+datapath_windows_win_8_release: all
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8Release" //p:Platform=$(ARCH)
+
+datapath_windows_win_8_1_debug: all
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8.1Debug" //p:Platform=$(ARCH)
+
+datapath_windows_win_8_1_release: all
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8.1Release" //p:Platform=$(ARCH)
+
+datapath_windows_win_10_debug: all
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win10Debug" //p:Platform=$(ARCH)
+
+datapath_windows_win_10_release: all
+   MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win10Release" //p:Platform=$(ARCH)

./boot.sh; ./configure CC=./build-aux/cccl LD="$(which link)" LIBS="-lws2_32
-liphlpapi -lwbemuuid -lole32 -loleaut32" --prefix="C:/openvswitch/usr"
--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc"
--with-pthread="C:/pthread"; make clean && make -j6; make
datapath_windows_win_8_debug datapath_windows_win_8_release
datapath_windows_win_8_1_debug datapath_windows_win_8_1_release
datapath_windows_win_10_debug datapath_windows_win_10_release

On the plus side you don't need to rerun the configure when you want to
change things. As another plus size you just pass in the parameters for the
userspace and compile the
Debug/release version which the user prefers.

Thanks,
Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Shashank Ram
Trimis: Tuesday, February 6, 2018 11:30 PM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Support to selectively compile
targets

Adds support to selectively compile kernel driver for target versions. This
is useful when environments to compile for all targets might not be
available on the user's machine, or if the user wants to only compile some
targets selectively.

Also once appveyor has support to build Win10 targets, we will not pass the
"--with-vstudiotargetver" to the configure script.

Signed-off-by: Shashank Ram 
---

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

Re: [ovs-dev] [PATCH v1 1/2] datapath-windows: Allow compiling all targets using SDK 10.0

[aserdean]

I applied the patch set on master. Thanks!

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele aserd...@ovn.org
Trimis: Tuesday, February 6, 2018 5:49 PM
Către: 'Shashank Ram' ; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v1 1/2] datapath-windows: Allow compiling all
targets using SDK 10.0

Thanks a lot for the series.

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Shashank Ram
Trimis: Thursday, February 1, 2018 4:59 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v1 1/2] datapath-windows: Allow compiling all
targets using SDK 10.0

Previously, Win8/8.1 targets would use SDK8.1. However, its recommended to
use the newer SDK as newer VS versions typically drop support for older SDKs
later on. This patch adds support to compile all targets (Win8/8.1/10) using
the 10.0 SDK.

Note that his patch does not drop support for older SDKs.

Signed-off-by: Shashank Ram 
---
 datapath-windows/Package/package.VcxProj | 28 --
 datapath-windows/ovsext/ovsext.vcxproj   | 50
+++-
 2 files changed, 49 insertions(+), 29 deletions(-)


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

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

Re: [ovs-dev] [PATCH v1 2/2] datapath-windows: Specify platform arch during compilation

[aserdean]

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Shashank Ram
Trimis: Thursday, February 1, 2018 4:59 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v1 2/2] datapath-windows: Specify platform arch
during compilation

Newer compilers expect the platorm architecture to be passed.

Signed-off-by: Shashank Ram 
---
 Makefile.am | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)


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

Re: [ovs-dev] [PATCH v1 1/2] datapath-windows: Allow compiling all targets using SDK 10.0

[aserdean]

Thanks a lot for the series.

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org
[mailto:ovs-dev-boun...@openvswitch.org] În numele Shashank Ram
Trimis: Thursday, February 1, 2018 4:59 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v1 1/2] datapath-windows: Allow compiling all
targets using SDK 10.0

Previously, Win8/8.1 targets would use SDK8.1. However, its recommended to
use the newer SDK as newer VS versions typically drop support for older SDKs
later on. This patch adds support to compile all targets (Win8/8.1/10) using
the 10.0 SDK.

Note that his patch does not drop support for older SDKs.

Signed-off-by: Shashank Ram 
---
 datapath-windows/Package/package.VcxProj | 28 --
 datapath-windows/ovsext/ovsext.vcxproj   | 50
+++-
 2 files changed, 49 insertions(+), 29 deletions(-)


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

Re: [ovs-dev] [PATCH v2] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[aserdean]

Much better 😊

Acked-by: Alin Gabriel Serdean 

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] 
În numele Anand Kumar
Trimis: Saturday, February 3, 2018 12:43 AM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH v2] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +  
datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 6 ++
 3 files changed, 11 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..28fe2bf 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)  {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid ICMP packet detected, header cannot be 
+ NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..8cbab24 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)  {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid TCP packet detected, header cannot be 
+ NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid TCP packet detected, tcp_flags %hu", 
+ tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid TCP packet detected, SYN+ACK flags not allowed,"
+  "tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..678bedb 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,10 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, &storage);
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid ICMP packet detected, icmp->type %u",
+  icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +338,8 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid packet detected, protocol not supported"
+  " ipProto %u", ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
--
2.9.3.windows.1

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

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

Re: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in conntrack for invalid ct state.

[aserdean]

-Mesaj original-
De la: Anand Kumar [mailto:kumaran...@vmware.com] 
Trimis: Saturday, February 3, 2018 12:28 AM
Către: Alin Serdean ; d...@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Hi Alin,

Thanks for the quick review. I will expand log messages to “Invalid XXX packet 
detected”.
I’m not sure if I follow your comment about “s/syn/SYN/g”. 

Do you want me to change it to Uppercase?
[Alin Serdean] Yes please 😊.

Thanks,
Anand Kumar

On 2/2/18, 2:06 PM, "Alin Serdean"  wrote:

Looks good just a small nit:
s/syn/SYN/g
s/ack/ACK/g

Also I would prefer if you drop the text `Invalid!`. Either just remove it 
or expand it, i.e.:
"Invalid! ICMPhdr cannot be NULL" => "Invalid ICMP packet detected the 
header cannot be NULL"

Thanks,
Alin.

-Mesaj original-
De la: ovs-dev-boun...@openvswitch.org 
[mailto:ovs-dev-boun...@openvswitch.org] În numele Anand Kumar
Trimis: Friday, February 2, 2018 11:19 PM
Către: d...@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Add trace level logs in 
conntrack for invalid ct state.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Conntrack-icmp.c | 1 +  
datapath-windows/ovsext/Conntrack-tcp.c  | 4 
 datapath-windows/ovsext/Conntrack.c  | 4 
 3 files changed, 9 insertions(+)

diff --git a/datapath-windows/ovsext/Conntrack-icmp.c 
b/datapath-windows/ovsext/Conntrack-icmp.c
index 4da0665..d86feed 100644
--- a/datapath-windows/ovsext/Conntrack-icmp.c
+++ b/datapath-windows/ovsext/Conntrack-icmp.c
@@ -61,6 +61,7 @@ BOOLEAN
 OvsConntrackValidateIcmpPacket(const ICMPHdr *icmp)  {
 if (!icmp) {
+OVS_LOG_TRACE("Invalid! ICMPhdr cannot be NULL");
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index f8e85a2..65eaac5 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -444,12 +444,14 @@ BOOLEAN
 OvsConntrackValidateTcpPacket(const TCPHdr *tcp)  {
 if (!tcp) {
+OVS_LOG_TRACE("Invalid! TCPHdr cannot be NULL");
 return FALSE;
 }
 
 UINT16 tcp_flags = ntohs(tcp->flags);
 
 if (OvsCtInvalidTcpFlags(tcp_flags)) {
+OVS_LOG_TRACE("Invalid! tcp_flags %hu", tcp_flags);
 return FALSE;
 }
 
@@ -457,6 +459,8 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp)
  * totally new connections (syn) or already established, not partially
  * open (syn+ack). */
 if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) {
+OVS_LOG_TRACE("Invalid! syn+ack flags not allowed, tcp_flags %hu",
+  tcp_flags);
 return FALSE;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 43c9dd3..7e413c6 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -317,6 +317,9 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 const ICMPHdr *icmp;
 icmp = OvsGetIcmp(curNbl, l4Offset, &storage);
 if (!OvsConntrackValidateIcmpPacket(icmp)) {
+if(icmp) {
+OVS_LOG_TRACE("Invalid! icmp->type %u", icmp->type);
+}
 state = OVS_CS_F_INVALID;
 break;
 }
@@ -334,6 +337,7 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
 break;
 }
 default:
+OVS_LOG_TRACE("Invalid! Not supported protocol, ipProto %u", 
+ ipProto);
 state = OVS_CS_F_INVALID;
 break;
 }
--
2.9.3.windows.1

___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIFBA&c=uilaK90D4TOVoH58JNXRgQ&r=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us&m=yuxpORhg-xij1o9VvWANs9QOyywwfr7YO_EDT-QvqQ4&s=ainXigtUgyesCYog0X4639gDSFv1mrY4OTsctKNaH0M&e=



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

Re: [ovs-dev] [PATCH] ovn.at: Fix IPv6 periodic RA test on Windows

[aserdean]

Thanks for the review!

I have a "\n" which isn't OK. Also, even with that changed I get the
following:
+++ /c/_2018/january/23/ovs/tests/testsuite.dir/at-groups/2378/stdout
2018-01-23 22:58:49 +0200
@@ -1 +1 @@
-ff4001010001050105dc03044080fff
faef003043080000
0fd0f
+ff8001010001050105dc03044080fff
faef003043080000
0fd0f
I will look into it and send another revision!

Thanks,
Alin.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, January 23, 2018 2:20 AM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] ovn.at: Fix IPv6 periodic RA test on
Windows
> 
> On Sun, Jan 14, 2018 at 07:05:16PM +0200, Alin Gabriel Serdean wrote:
> > One issue with this test is that MSYS mangles the shorter form of the
IPv6
> address.
> > To solve this, we switch to the longer notation of it.
> >
> > Another issue is that `printf` command does not add the leading `0` to
the
> packet.
> > We switch to a more platform independent `awk` substitution.
> >
> > Co-authored-by: Mark Michelson 
> > Signed-off-by: Mark Michelson 
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Acked-by: Ben Pfaff 

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

Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client backup test on Win

[aserdean]

(facepalm). Thanks for pointing me to the right direction. I will look into
it.

Thanks,
Alin.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, January 23, 2018 2:24 AM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] ovsdb-client.at: Fix ovsdb-client backup
test
> on Win
> 
> On Sun, Jan 14, 2018 at 08:26:47PM +0200, Alin Gabriel Serdean wrote:
> > The test:
> > 1948. ovsdb-client.at:15: testing ovsdb-client backup and restore
> > fails on Windows with:
> > --- /dev/null   2018-01-14 20:09:57 +0200
> > +++ /c/_2018/january/14/ovs/tests/testsuite.dir/at-groups/1948/stderr
> > @@ -0,0 +1,3 @@
> > +ovsdb-server: ovsdb error: backup: unexpected file format
> > +ovsdb-server: Failed to read from child (The pipe has been ended.
> > +)
> > ./ovsdb-client.at:111: exit code was 1, expected 0
> >
> > The root cause is that when redirecting output defaults to the Windows
> > line
> > endings(CRLF):
> > $ file db
> > db: ASCII text, with very long lines
> > $ file backup
> > backup: ASCII text, with very long lines, with CRLF line terminators
> >
> > Add a `dos2unix` command to convert to the line endings expected by
> > ovsdb-server.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Thank you for the fix.
> 
> Hmm, maybe this is a bug in "ovsdb-client backup".  Maybe it should do
> something like:
> 
> #ifdef _WIN32
> fflush(stdout);
> _setmode(STDOUT_FILENO, _O_BINARY); #endif
> 
> and then the test wouldn't have to change at all?  What do you think?
> 
> I don't know whether anything similar is needed for stdin for
"ovsdb-client
> restore".

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

Re: [ovs-dev] [PATCH] ovn-northd.at: Fix MSYS IPv6 interpretation

[aserdean]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Wednesday, January 10, 2018 11:16 PM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] ovn-northd.at: Fix MSYS IPv6 interpretation
> 
> On Wed, Jan 10, 2018 at 09:34:11PM +0200, Alin Gabriel Serdean wrote:
> > Similar to commit 847dc1c22c0b4af2ba33d15c311be32d34257429, MSYS
> > mangles the
> > IPv6 address:
> > 'ovn-nbctl: ro-sw: invalid network address: aef0;1\64'
> >
> > This patch uses the full form of the IPv6 address instead of its
> > shorter notation.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Acked-by: Ben Pfaff 
> 
> Is there a regular expression (etc.) that could find these problems?  If
so,
> then we could add a build-time check that flags them before they get into
> the tree, or we could add something to checkpatch to find them at review
> time.
[Alin Serdean] Thanks for the quick review, applied on master.
Thanks for the suggestion, I'll try to see what I can come up with.
I would rather switch to msys2 because you can add MSYS2_ARG_CONV_EXCL
to bypass the argument mangling
(https://github.com/msys2/msys2/wiki/Porting#filesystem-namespaces),
making things much cleaner.
Unfortunately this is not present on MSYS.

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

Re: [ovs-dev] [PATCH] appveyor: Bump OpenSSL version

[aserdean]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Wednesday, January 10, 2018 11:19 PM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] appveyor: Bump OpenSSL version
> 
> On Wed, Jan 10, 2018 at 09:06:16PM +0200, Alin Gabriel Serdean wrote:
> > The URL https://slproweb.com/download/Win32OpenSSL-1_0_2L.exe is no
> > longer valid.
> >
> > Bump the version of OpenSSL to 1_0_2m.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> I wonder whether there's a stable URL to get the latest version at a given
> time?
> 
> Acked-by: Ben Pfaff 
[Alin Serdean] Thanks for the pointer. I'll try to see what I can find.
Thanks for the quick review applied on master.

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

Re: [ovs-dev] [PATCH] windows: Fix x64 build

[aserdean]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Wednesday, January 10, 2018 7:30 PM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] windows: Fix x64 build
> 
> On Wed, Jan 10, 2018 at 07:27:52PM +0200, Alin Gabriel Serdean wrote:
> > MSVC complains:
> > error C4013: 'atomic_storeX' undefined; assuming extern returning int
> >
> > atomic_storeX - is no longer defined. This patch adds back its
> implementation.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> You might want to add a Fixes: tag.
> 
> Acked-by: Ben Pfaff 
[Alin Serdean] Thanks for the quick review. I changed the tag and applied on
master.

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

Re: [ovs-dev] [PATCH 0/3] Fixes while compiling with 1709 and VS2017

[aserdean]

Thanks Ben! Applied on master.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Friday, January 5, 2018 5:14 PM
> To: aserd...@ovn.org
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 0/3] Fixes while compiling with 1709 and
> VS2017
> 
> OK.
> 
> Acked-by: Ben Pfaff 
> 
> 
> On Fri, Jan 05, 2018 at 03:59:02PM +0200, aserd...@ovn.org wrote:
> > Sure. Do you mind if I add your acked-by on each one?
> >
> > Thanks,
> > Alin.
> >
> > > -Original Message-
> > > From: Ben Pfaff [mailto:b...@ovn.org]
> > > Sent: Thursday, January 4, 2018 11:37 PM
> > > To: Alin Gabriel Serdean 
> > > Cc: d...@openvswitch.org
> > > Subject: Re: [ovs-dev] [PATCH 0/3] Fixes while compiling with 1709 and
> > > VS2017
> > >
> > > On Mon, Nov 06, 2017 at 02:07:15PM -0800, Ben Pfaff wrote:
> > > > On Mon, Nov 06, 2017 at 05:51:54PM +0200, Alin Gabriel Serdean
wrote:
> > > > > This series includes fixes that were found while compiling using
WDK
> > > > > 1709 and Visual Studio 2017.
> > > > >
> > > > > Alin Gabriel Serdean (3):
> > > > >   datapath-windows: Add directory to .gitignore
> > > > >   datapath-windows: Change include type in Iphelper.h
> > > > >   datapath-windows: Add include directory to ovsext project
> > > >
> > > > All of these seem reasonable to me.
> > >
> > > Do you want to apply these?
> >

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

Re: [ovs-dev] [PATCH 0/3] Fixes while compiling with 1709 and VS2017

[aserdean]

Sure. Do you mind if I add your acked-by on each one?

Thanks,
Alin.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Thursday, January 4, 2018 11:37 PM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 0/3] Fixes while compiling with 1709 and
> VS2017
> 
> On Mon, Nov 06, 2017 at 02:07:15PM -0800, Ben Pfaff wrote:
> > On Mon, Nov 06, 2017 at 05:51:54PM +0200, Alin Gabriel Serdean wrote:
> > > This series includes fixes that were found while compiling using WDK
> > > 1709 and Visual Studio 2017.
> > >
> > > Alin Gabriel Serdean (3):
> > >   datapath-windows: Add directory to .gitignore
> > >   datapath-windows: Change include type in Iphelper.h
> > >   datapath-windows: Add include directory to ovsext project
> >
> > All of these seem reasonable to me.
> 
> Do you want to apply these?

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

Re: [ovs-dev] [PATCH] MSI: Use platform specific netcfg location

[aserdean]

Thanks for the review. Applied on master.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Thursday, January 4, 2018 11:36 PM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] MSI: Use platform specific netcfg location
> 
> On Mon, Dec 11, 2017 at 08:22:27PM +0200, Alin Gabriel Serdean wrote:
> > We use the command `netcfg` to install the Windows datapath.
> >
> > Since we have both 32 and 64 bit installers available point it to the
> > platform specific binary.
> >
> > Found while testing.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Acked-by: Ben Pfaff 

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

Re: [ovs-dev] [PATCH] travis: Use pip2 instead of pip for OSX build.

[aserdean]

I applied this on master.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of aserd...@ovn.org
> Sent: Monday, December 11, 2017 4:23 PM
> To: 'Ilya Maximets' ; ovs-dev@openvswitch.org
> Cc: 'Lance Richardson' ; 'Heetae Ahn'
> 
> Subject: Re: [ovs-dev] [PATCH] travis: Use pip2 instead of pip for OSX
build.
> 
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Ilya Maximets
> > Sent: Friday, December 8, 2017 2:53 PM
> > To: ovs-dev@openvswitch.org
> > Cc: Ilya Maximets ; Lance Richardson
> > ; Heetae Ahn 
> > Subject: [ovs-dev] [PATCH] travis: Use pip2 instead of pip for OSX
build.
> >
> > xcode8.3 is a new default image for OS X on Travis-CI, but it does not
> have
> > 'pip':
> >
> > pip install --user six
> > ./.travis/osx-prepare.sh: line 3: pip: command not found
> >
> > 'pip2' or 'pip3' should be used explicitly instead:
> >
> > https://github.com/travis-ci/travis-ci/issues/8829
> >
> > Signed-off-by: Ilya Maximets 
> 
> I haven't tested it, but I saw other people complaining about the same
issue
> on travis-ci.
> 
> Acked-by: Alin Gabriel Serdean 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.

[aserdean]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, December 12, 2017 12:40 AM
> To: Alin Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.
> 
> On Mon, Dec 11, 2017 at 10:34:06PM +, Alin Serdean wrote:
> >
> >
> > > -Original Message-
> > > From: Ben Pfaff [mailto:b...@ovn.org]
> > > Sent: Tuesday, December 12, 2017 12:30 AM
> > > To: Alin Serdean 
> > > Cc: d...@openvswitch.org
> > > Subject: Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.
> > >
> > > On Mon, Dec 11, 2017 at 08:23:44PM +, Alin Serdean wrote:
> > > > LGTM. Just two nits.
> > >
> > > > > +/* Terminal error state that indicates that nothing useful can be
> done.
> > > > > + * The most likely reason is that the database server
> > > > > + doesn't actually have
> > > > [Alin Serdean] doesn't have, maybe?
> > >
> > > OK, thanks.
> > >
> > > > > @@ -1118,7 +1153,7 @@ ovsdb_idl_condition_clone(struct
> > > > > ovsdb_idl_condition *dst,
> > > > >   * arranges to send the new condition to the database server.
> > > > >   *
> > > > >   * Return the next conditional update sequence number. When
> > > > > this
> > > > > - * value and ovsdb_idl_get_condition_seqno() matchs, the 'idl'
> > > > > + * value and ovsdb_idl_get_condition_seqno() matches, the 'idl'
> > > > >   * contains rows that match the 'condition'.
> > > > >   */
> > > > >  unsigned int
> > > > > --
> > > > s/.  *\//. *\//
> > >
> > > I don't understand that suggestion.  Maybe you were suggesting that
> > > */ should be on the same line as the last word; if so, OK, sure.
> > [Alin Serdean] I think in some places there is `.  */` vs `. */` , but 
> > maybe my
> email client messed up the formatting.
> 
> Oh.  Got it.  I'll try to be more consistent.
> 
> (I was a little confused by regex syntax.  '.' and '*' are special
> characters.)
[Alin Serdean] My bad, I forgot to escape them 😊.

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

Re: [ovs-dev] [PATCH] datapath-windows: Correct endianness for deleting zone.

[aserdean]

I was concerned a bit about binary compatibility between the future
userspace binaries and older kernel versions.

But Windows users usually just use the MSI (because of the signing issues).

Thanks both!

Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Sairam Venugopal
> Sent: Friday, December 8, 2017 1:26 AM
> To: aserd...@ovn.org; 'Justin Pettit' ;
> d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] datapath-windows: Correct endianness for
> deleting zone.
> 
> We can hold off on back-porting this. This change mainly affects the
delete
> by 5-tuple API.
> 
> https://github.com/openvswitch/ovs/commit/817a76577fec3f03310d7d3a5a
> 10df01340ee8ad
> 
> Unless we plan to backport that patch, we should hold off on backporting
this
> as well.
> 
> Thanks,
> Sairam
> 
> 
> 
> On 12/6/17, 9:41 AM, "aserd...@ovn.org"  wrote:
> 
> >> -Original Message-
> >> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> >> boun...@openvswitch.org] On Behalf Of Justin Pettit
> >> Sent: Tuesday, December 5, 2017 9:30 AM
> >> To: d...@openvswitch.org; Alin Gabriel Serdean ;
> >> Sairam Venugopal 
> >> Subject: [ovs-dev] [PATCH] datapath-windows: Correct endianness for
> >> deleting zone.
> >>
> >> The zone Netlink attribute is supposed to be in network-byte order,
> >> but
> >the
> >> Windows code for deleting conntrack entries was treating it as
> >> host-byte order.
> >>
> >> Found by inspection.
> >>
> >> Signed-off-by: Justin Pettit 
> >
> >Thanks for the patch, Justin.  I tested the patch, and everything looks
> >good.
> >I want to hold on the acknowledge until all of us are on the same page.
> >
> >This will make the windows datapath consistent with future patches from
> >the userspace.
> >For consistency I would like to backport the patch all the way to
> >branch-2.6.
> >
> >What do you think Sai?
> >
> >Thanks,
> >Alin.
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH v1] datapath-windows: Add support for deleting conntrack entry by 5-tuple.

[aserdean]

Thanks all!

I applied this on master.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Yi-Hung Wei
> Sent: Wednesday, November 29, 2017 9:28 PM
> To: aserd...@ovn.org; Anand Kumar 
> Cc: ovs dev 
> Subject: Re: [ovs-dev] [PATCH v1] datapath-windows: Add support for
> deleting conntrack entry by 5-tuple.
> 
> Hi Anand and Alin,
> 
> I have some updates about supporting delete conntrack entry by 5-tuple on
> my v2 patch series ( https://mail.openvswitch.org/pipermail/ovs-dev/2017-
> November/341140.html ).
> 
> In the v2 series, I made some changes on the dpif-netlink implementation (
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-
> November/341142.html ), and I am not sure if it will affect the windows
> datapath implementation.
> 
> Given that the dpif-netlink implementation is still under review, if it
will
> affect the windows datapath implementation, do you mind if we wait a bit
till
> the dpif-netlink implementation is upstream?
> 
> Thanks,
> 
> -Yi-Hung
> 
> On Wed, Nov 29, 2017 at 10:19 AM,  wrote:
> 
> > I wanted to give it a few days to see if another review pops up.
> >
> > Mind if I wait until tomorrow?
> >
> > Thanks,
> > Alin.
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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