[ovs-dev] Please help add me to AUTHORS

2022-06-02 Thread jinjun gao 
Hi guys,
I'm a contributor in datapath-windows part with jinj...@vmware.com. Can you
please help add me to the AUTHORS? Thanks!

Name: Jinjun Gao
Mail: gjin...@gmail.com
-- 
Best Regards,
Jinjun Gao
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 8/8] appveyor: Bump outdated links and add artifacts

2020-09-25 Thread Jinjun Gao 
Thanks for working on it. It looks good to me.
+Ack. 

- Jinjun
 

On 2020/9/24, 2:24 PM, "Alin Gabriel Serdean"  wrote:

Bump OpenSSL.

Add release and debug configuration.

Build and add Windows installer to generated artifacts.

Build and zip prebuilt version.

Co-authored-by: Yonggang Luo 
Signed-off-by: Yonggang Luo 
Co-authored-by: Jinjun Gao 
Signed-off-by: Jinjun Gao 
Signed-off-by: Alin Gabriel Serdean 
---
 appveyor.yml | 40 
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/appveyor.yml b/appveyor.yml
index 6e2b2e9e2..25c3f69fb 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -3,26 +3,27 @@ image: Visual Studio 2019
 branches:
   only:
   - master
-clone_folder: C:\openvswitch
+configuration:
+  - Debug
+  - Release
+clone_folder: C:\openvswitch_compile
 init:
 - ps: $env:PATH ="C:\Python37;"+$env:PATH
 - ps: New-Item -Type HardLink -Path "C:\Python37\python3.exe" -Value 
"C:\Python37\python.exe"
 - ps: >-
 mkdir C:\ovs-build-downloads

-$source = 
"https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fslproweb.com%2Fdownload%2FWin32OpenSSL-1_0_2t.exe&data=02%7C01%7Cjinjung%40vmware.com%7C79ef2d3f21874ad6a00108d860527958%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637365254633884741&sdata=tWKNMO9p8DtmxO9Llbe2JJLTzfgbdvDRU%2F%2F71f88P0o%3D&reserved=0";
+mkdir C:\openvswitch\driver

-$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_2t.exe"
+$source = 
"https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fslproweb.com%2Fdownload%2FWin64OpenSSL-1_0_2u.exe&data=02%7C01%7Cjinjung%40vmware.com%7C79ef2d3f21874ad6a00108d860527958%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637365254633884741&sdata=fxm0tyK1J37WGnrfMy%2BoSam%2Fkbj2pVGHG3fseEqNFRc%3D&reserved=0";

-Invoke-WebRequest $source -OutFile $destination
-
-cd C:\pthreads-win32
+$destination = "C:\ovs-build-downloads\Win64OpenSSL-1_0_2u.exe"

-7z x C:\pthreads-win32\pthreads-win32.zip
+Invoke-WebRequest $source -OutFile $destination

 cd C:\ovs-build-downloads

-.\Win32OpenSSL-1_0_2t.exe /silent /verysilent /sp- /suppressmsgboxes
+.\Win64OpenSSL-1_0_2u.exe /silent /verysilent /sp- /suppressmsgboxes

 Start-Sleep -s 30

@@ -32,13 +33,28 @@ init:

 python3 -m pip install pypiwin32 --disable-pip-version-check

+cd C:\openvswitch_compile
+
 build_script:
 - '"C:\Program Files (x86)\Microsoft Visual 
Studio\2019\Community\VC\Auxiliary\Build\vcvars64.bat"'
 - C:\MinGW\msys\1.0\bin\bash -lc "echo \"C:/MinGW /mingw\" > /etc/fstab"
 - C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe"
 # Build pthreads
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/pthreads4w-code && nmake all 
install"
-- 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 -lShlwapi -liphlpapi 
-lwbemuuid -lole32 -loleaut32\" --with-pthread=c:/PTHREADS-BUILT/ 
--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"
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch_compile && ./boot.sh"
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch_compile && ./configure 
CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -lShlwapi -liphlpapi 
-lwbemuuid -lole32 -loleaut32\" --prefix=C:/openvswitch/usr 
--localstatedir=C:/openvswitch/var --sysconfdir=C:/openvswitch/etc 
--with-pthread=c:/PTHREADS-BUILT/ --enable-ssl --with-openssl=C:/OpenSSL-Win64 
--with-vstudiotarget=\"%CONFIGURATION%\""
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch_compile && make -j 4"
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch_compile && make 
datapath_windows_analyze"
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch_compile && make 
install"
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch_compile && make 
windows_installer"
+- cp C:\PTHREADS-BUILT\bin\pthreadVC3.dll C:\openvswitch\usr\bin
+- cp C:\PTHREADS-BUILT\bin\pthreadVC3.dll C:\openvswitch\usr\sbin
+- ps: cp 
C:\openvswitch_compile\datapath-windows\x64\Win10$env:CONFIGURATION\package\* 
C:\openvswitch\driver
+- ps: cp

Re: [ovs-dev] Bond support on Windows OVS

2020-08-24 Thread Jinjun Gao 
Thanks for quick response, Alin.

If we use Windows team cmdlets, it will rely on Hyper-V role enable. Does it 
possible to support on disable Hyper-V role with your patchset 
(https://patchwork.ozlabs.org/project/openvswitch/patch/20181008232602.30924-1-aserd...@ovn.org/)?


PS > New-VMSwitch -Name external -NetAdapterName external \
 -AllowManagementOS $false


Regards,
-Jinjun

From: Alin Serdean 
Sent: Monday, August 24, 2020 8:37 PM
To: Jinjun Gao ; Anand Kumar 
Cc: ovs-dev@openvswitch.org 
Subject: RE: Bond support on Windows OVS


Hi Jinjun,



Basically we need to use `New-NetSwitchTeam` to make OVS aware of multiple 
adapters:

[1] 
https://docs.openvswitch.org/en/latest/intro/install/windows/#add-multiple-nics-to-be-managed-by-ovs<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.openvswitch.org%2Fen%2Flatest%2Fintro%2Finstall%2Fwindows%2F%23add-multiple-nics-to-be-managed-by-ovs&data=02%7C01%7Cjinjung%40vmware.com%7C6efeecbbd32d4dbe034d08d8482a70ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637338694408254428&sdata=lrHnPpddOX2A1pHt4lpk8tAMOs6kzATGfWdwNd2VRoM%3D&reserved=0>



For bonding in particular we need to add some(or all) of those interfaces 
inside OVS bonds:

[2] 
https://blog.scottlowe.org/2012/10/19/link-aggregation-and-lacp-with-open-vswitch/<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fblog.scottlowe.org%2F2012%2F10%2F19%2Flink-aggregation-and-lacp-with-open-vswitch%2F&data=02%7C01%7Cjinjung%40vmware.com%7C6efeecbbd32d4dbe034d08d8482a70ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637338694408254428&sdata=2AP6BY4FCYw%2FsZ8tB4POCohd%2FUw9F0IomfFrKg8dktY%3D&reserved=0>



Please let me know if you want me to send a snippet that combines [1] and [2].



In particular use cases, we can also create a switch over LBFO team NIC as per:

https://docs.microsoft.com/en-us/powershell/module/netlbfo/new-netlbfoteam?view=win10-ps<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fpowershell%2Fmodule%2Fnetlbfo%2Fnew-netlbfoteam%3Fview%3Dwin10-ps&data=02%7C01%7Cjinjung%40vmware.com%7C6efeecbbd32d4dbe034d08d8482a70ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637338694408264418&sdata=fsYUFMfaQ%2FHD1JI3wlOHtmTcyDXGHs%2ByQ6UVUantwo8%3D&reserved=0>

but in that case from the OVS side we see one single external NIC.



Thanks,

Alin.



From: Jinjun Gao<mailto:jinj...@vmware.com>
Sent: Monday, August 24, 2020 9:09 AM
To: Alin Serdean<mailto:aserd...@cloudbasesolutions.com>; Anand 
Kumar<mailto:kumaran...@vmware.com>
Cc: ovs-dev@openvswitch.org<mailto:ovs-dev@openvswitch.org>
Subject: Bond support on Windows OVS



Hi Alin and Anand,



I'm investigating how to support bond with dual or more physical NICs on 
Windows OVS.  I heard from Anand that you ever discussed it with community guys 
and had a good idea about how to implement it. Can you please provide some 
input on it to us?



Thanks,

-Jinjun


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

[ovs-dev] Bond support on Windows OVS

2020-08-23 Thread Jinjun Gao 
Hi Alin and Anand,

I'm investigating how to support bond with dual or more physical NICs on 
Windows OVS.  I heard from Anand that you ever discussed it with community guys 
and had a good idea about how to implement it. Can you please provide some 
input on it to us?

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

Re: [ovs-dev] [PATCH] datapath-windows: Update flow key in SET action

2020-07-28 Thread Jinjun Gao 
Thanks for review, Alin.

I have updated the patch and uploaded v2 patch. Please help review again.
Also, the PR (https://github.com/openvswitch/ovs/pull/326) has pasted in issue 
(https://github.com/openvswitch/ovs-issues/issues/190). Please help link them.

- Jinjun


From: Alin Serdean 
Date: Tuesday, July 28, 2020 at 11:42 PM
To: Jinjun Gao , "d...@openvswitch.org" 
, Anand Kumar 
Cc: Rui Cao , Jinjun Gao 
Subject: RE: [PATCH] datapath-windows: Update flow key in SET action

Thanks a lot for testing and fixing this as discussed offline.

Can you please split the assignment via the form:
“key->ipKey.nwDst = ipHdr->daddr = ipAttr->ipv4_dst”

It confuses the SDV tool from msft for some reason:
https://docs.microsoft.com/en-us/windows-hardware/drivers/develop/static-driver-verifier-known-issues<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fwindows-hardware%2Fdrivers%2Fdevelop%2Fstatic-driver-verifier-known-issues&data=02%7C01%7Cjinjung%40vmware.com%7Ca74ea7845f494560b53f08d8330cd381%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637315477473836583&sdata=xbZ1YNsOVYOrJzG9wFlkoatGe0pX9%2BEA0SsOn%2FKG2g8%3D&reserved=0>

Thanks,
Alin.

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

[ovs-dev] [PATCH v2] datapath-windows: Update flow key in SET action

2020-07-28 Thread Jinjun Gao 
The flow key is not updated when process OVS_ACTION_ATTR_SET action.
It will impact follow-up actions, such as, conntrack module cannot
find created conntrack entry if passing old flow key to it.

Reported-by: Rui Cao 
Signed-off-by: Jinjun Gao 
---
v2: Separated assignment to happy MSFT SDV tool
---
 datapath-windows/ovsext/Actions.c | 31 ---
 datapath-windows/ovsext/Actions.h |  3 +++
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 4a11cea..4f43369 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1259,6 +1259,7 @@ OvsActionMplsPush(OvsForwardingContext *ovsFwdCtx,
  */
 static __inline NDIS_STATUS
 OvsUpdateEthHeader(OvsForwardingContext *ovsFwdCtx,
+   OvsFlowKey *key,
const struct ovs_key_ethernet *ethAttr)
 {
 PNET_BUFFER curNb;
@@ -1285,9 +1286,11 @@ OvsUpdateEthHeader(OvsForwardingContext *ovsFwdCtx,
 }
 ethHdr = (EthHdr *)(bufferStart + NET_BUFFER_CURRENT_MDL_OFFSET(curNb));

-RtlCopyMemory(ethHdr->Destination, ethAttr->eth_dst,
-   sizeof ethHdr->Destination);
-RtlCopyMemory(ethHdr->Source, ethAttr->eth_src, sizeof ethHdr->Source);
+RtlCopyMemory(ethHdr->Destination, ethAttr->eth_dst, ETH_ADDR_LENGTH);
+RtlCopyMemory(ethHdr->Source, ethAttr->eth_src, ETH_ADDR_LENGTH);
+/* Update l2 flow key */
+RtlCopyMemory(key->l2.dlDst, ethAttr->eth_dst, ETH_ADDR_LENGTH);
+RtlCopyMemory(key->l2.dlSrc, ethAttr->eth_src, ETH_ADDR_LENGTH);

 return NDIS_STATUS_SUCCESS;
 }
@@ -1376,6 +1379,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
+  OvsFlowKey *key,
   const struct ovs_key_udp *udpAttr)
 {
 PUINT8 bufferStart;
@@ -1400,15 +1404,19 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
 udpHdr->check = ChecksumUpdate16(udpHdr->check, udpHdr->source,
  udpAttr->udp_src);
 udpHdr->source = udpAttr->udp_src;
+key->ipKey.l4.tpSrc = udpAttr->udp_src;
 }
 if (udpHdr->dest != udpAttr->udp_dst) {
 udpHdr->check = ChecksumUpdate16(udpHdr->check, udpHdr->dest,
  udpAttr->udp_dst);
 udpHdr->dest = udpAttr->udp_dst;
+key->ipKey.l4.tpDst = udpAttr->udp_dst;
 }
 } else {
 udpHdr->source = udpAttr->udp_src;
+key->ipKey.l4.tpSrc = udpAttr->udp_src;
 udpHdr->dest = udpAttr->udp_dst;
+key->ipKey.l4.tpDst = udpAttr->udp_dst;
 }

 return NDIS_STATUS_SUCCESS;
@@ -1423,6 +1431,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
+  OvsFlowKey *key,
   const struct ovs_key_tcp *tcpAttr)
 {
 PUINT8 bufferStart;
@@ -1447,11 +1456,13 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 tcpHdr->check = ChecksumUpdate16(tcpHdr->check, tcpHdr->source,
  tcpAttr->tcp_src);
 tcpHdr->source = tcpAttr->tcp_src;
+key->ipKey.l4.tpSrc = tcpAttr->tcp_src;
 }
 if (tcpHdr->dest != tcpAttr->tcp_dst) {
 tcpHdr->check = ChecksumUpdate16(tcpHdr->check, tcpHdr->dest,
  tcpAttr->tcp_dst);
 tcpHdr->dest = tcpAttr->tcp_dst;
+key->ipKey.l4.tpDst = tcpAttr->tcp_dst;
 }

 return NDIS_STATUS_SUCCESS;
@@ -1579,6 +1590,7 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
+OvsFlowKey *key,
 const struct ovs_key_ipv4 *ipAttr)
 {
 PUINT8 bufferStart;
@@ -1632,6 +1644,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
 ipAttr->ipv4_src);
 }
 ipHdr->saddr = ipAttr->ipv4_src;
+key->ipKey.nwSrc = ipAttr->ipv4_src;
 }
 if (ipHdr->daddr != ipAttr->ipv4_dst) {
 if (tcpHdr) {
@@ -1647,6 +1660,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
 ipAttr->ipv4_dst);
 }
 ipHdr->daddr = ipAttr->ipv4_dst;
+key->ipKey.nwDst = ipAttr->ipv4_dst;
 }
 if (ipHdr->protocol != ipAttr->ipv4_proto) {
 UINT16 oldProto = (ipHdr->protocol << 16) & 0xff00;
@@ -1661,6 +1675,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
 ipHdr->check = ChecksumUpdate16(ipHdr->check, oldProto

[ovs-dev] [PATCH] datapath-windows: Update flow key in SET action

2020-07-27 Thread Jinjun Gao 
The flow key is not updated when process OVS_ACTION_ATTR_SET action.
It will impact follow-up actions, such as, conntrack module cannot
find created conntrack entry if passing older flow key to it.

Reported-by: Rui Cao 
Signed-off-by: Jinjun Gao 
---
 datapath-windows/ovsext/Actions.c | 41 +++
 datapath-windows/ovsext/Actions.h |  3 +++
 2 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 699d2cd..0bc5444 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1284,6 +1284,7 @@ OvsActionMplsPush(OvsForwardingContext *ovsFwdCtx,
  */
 static __inline NDIS_STATUS
 OvsUpdateEthHeader(OvsForwardingContext *ovsFwdCtx,
+   OvsFlowKey *key,
const struct ovs_key_ethernet *ethAttr)
 {
 PNET_BUFFER curNb;
@@ -1310,9 +1311,11 @@ OvsUpdateEthHeader(OvsForwardingContext *ovsFwdCtx,
 }
 ethHdr = (EthHdr *)(bufferStart + NET_BUFFER_CURRENT_MDL_OFFSET(curNb));

-RtlCopyMemory(ethHdr->Destination, ethAttr->eth_dst,
-   sizeof ethHdr->Destination);
-RtlCopyMemory(ethHdr->Source, ethAttr->eth_src, sizeof ethHdr->Source);
+RtlCopyMemory(ethHdr->Destination, ethAttr->eth_dst, ETH_ADDR_LENGTH);
+RtlCopyMemory(ethHdr->Source, ethAttr->eth_src, ETH_ADDR_LENGTH);
+/* Update l2 flow key */
+RtlCopyMemory(key->l2.dlDst, ethAttr->eth_dst, ETH_ADDR_LENGTH);
+RtlCopyMemory(key->l2.dlSrc, ethAttr->eth_src, ETH_ADDR_LENGTH);

 return NDIS_STATUS_SUCCESS;
 }
@@ -1401,6 +1404,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
+  OvsFlowKey *key,
   const struct ovs_key_udp *udpAttr)
 {
 PUINT8 bufferStart;
@@ -1424,16 +1428,16 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
 if (udpHdr->source != udpAttr->udp_src) {
 udpHdr->check = ChecksumUpdate16(udpHdr->check, udpHdr->source,
  udpAttr->udp_src);
-udpHdr->source = udpAttr->udp_src;
+key->ipKey.l4.tpSrc = udpHdr->source = udpAttr->udp_src;
 }
 if (udpHdr->dest != udpAttr->udp_dst) {
 udpHdr->check = ChecksumUpdate16(udpHdr->check, udpHdr->dest,
  udpAttr->udp_dst);
-udpHdr->dest = udpAttr->udp_dst;
+key->ipKey.l4.tpDst = udpHdr->dest = udpAttr->udp_dst;
 }
 } else {
-udpHdr->source = udpAttr->udp_src;
-udpHdr->dest = udpAttr->udp_dst;
+key->ipKey.l4.tpSrc = udpHdr->source = udpAttr->udp_src;
+key->ipKey.l4.tpDst = udpHdr->dest = udpAttr->udp_dst;
 }

 return NDIS_STATUS_SUCCESS;
@@ -1448,6 +1452,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
+  OvsFlowKey *key,
   const struct ovs_key_tcp *tcpAttr)
 {
 PUINT8 bufferStart;
@@ -1471,12 +1476,12 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 if (tcpHdr->source != tcpAttr->tcp_src) {
 tcpHdr->check = ChecksumUpdate16(tcpHdr->check, tcpHdr->source,
  tcpAttr->tcp_src);
-tcpHdr->source = tcpAttr->tcp_src;
+key->ipKey.l4.tpSrc = tcpHdr->source = tcpAttr->tcp_src;
 }
 if (tcpHdr->dest != tcpAttr->tcp_dst) {
 tcpHdr->check = ChecksumUpdate16(tcpHdr->check, tcpHdr->dest,
  tcpAttr->tcp_dst);
-tcpHdr->dest = tcpAttr->tcp_dst;
+key->ipKey.l4.tpDst = tcpHdr->dest = tcpAttr->tcp_dst;
 }

 return NDIS_STATUS_SUCCESS;
@@ -1604,6 +1609,7 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
+OvsFlowKey *key,
 const struct ovs_key_ipv4 *ipAttr)
 {
 PUINT8 bufferStart;
@@ -1656,7 +1662,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
 ipHdr->check = ChecksumUpdate32(ipHdr->check, ipHdr->saddr,
 ipAttr->ipv4_src);
 }
-ipHdr->saddr = ipAttr->ipv4_src;
+key->ipKey.nwSrc = ipHdr->saddr = ipAttr->ipv4_src;
 }
 if (ipHdr->daddr != ipAttr->ipv4_dst) {
 if (tcpHdr) {
@@ -1671,7 +1677,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
 ipHdr->check = ChecksumUpdate32(ipHdr->check, ipHdr->daddr,
 ipAttr->ipv4_dst);
 }
-ipHdr->daddr = ipAttr-

[ovs-dev] [PATCH v3] datapath-windows: Reset ct_mark/ct_label to support ALG

2020-07-22 Thread Jinjun Gao 
The ct_mark/ct_label setting on related connection keep the same
behavior with Linux datapath. If one CT entry has parent/master entry,
its ct_mark and ct_label should inherit from the corresponding part
of parent/master entry at initialization.

Signed-off-by: Jinjun Gao 
---
v3: Resolve Alin's comment and use RtlCopyMemory to replace memcpy
v2: Resolve Anand's comment to add braces for if-block.
---
 datapath-windows/ovsext/Conntrack.c | 86 +++--
 1 file changed, 54 insertions(+), 32 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index d065591..2610d62 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -789,60 +789,82 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
 static __inline VOID
 OvsConntrackSetMark(OvsFlowKey *key,
 POVS_CT_ENTRY entry,
-UINT32 value,
-UINT32 mask,
+MD_MARK *mark,
 BOOLEAN *markChanged)
 {
-UINT32 newMark;
-newMark = value | (entry->mark & ~(mask));
-if (entry->mark != newMark) {
+POVS_CT_ENTRY parent = entry->parent;
+BOOLEAN changed = FALSE;
+UINT32 newMark = 0;
+
+if (parent && parent->mark) {
+newMark = parent->mark;
+changed = TRUE;
+} else if (mark) {
+newMark = mark->value | (entry->mark & ~(mark->mask));
+changed = TRUE;
+}
+
+if (changed && entry->mark != newMark) {
 entry->mark = newMark;
 key->ct.mark = newMark;
 *markChanged = TRUE;
 }
 }

+static __inline BOOLEAN
+OvsConntrackIsLabelsNonZero(const struct ovs_key_ct_labels *labels)
+{
+UINT8 i;
+
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+if (labels->ct_labels_32[i]) {
+return TRUE;
+}
+}
+
+return FALSE;
+}
+
 static __inline void
 OvsConntrackSetLabels(OvsFlowKey *key,
   POVS_CT_ENTRY entry,
-  struct ovs_key_ct_labels *val,
-  struct ovs_key_ct_labels *mask,
+  MD_LABELS *labels,
   BOOLEAN *labelChanged)
 {
-ovs_u128 v, m, pktMdLabel = {0};
-memcpy(&v, val, sizeof v);
-memcpy(&m, mask, sizeof m);
-memcpy(&pktMdLabel, &entry->labels, sizeof(struct ovs_key_ct_labels));
+POVS_CT_ENTRY parent = entry->parent;

-pktMdLabel.u64.lo = v.u64.lo | (pktMdLabel.u64.lo & ~(m.u64.lo));
-pktMdLabel.u64.hi = v.u64.hi | (pktMdLabel.u64.hi & ~(m.u64.hi));
+/* Inherit master's labels at labels initialization, if any. */
+if (!OvsConntrackIsLabelsNonZero(&entry->labels) &&
+parent && OvsConntrackIsLabelsNonZero(&parent->labels)) {
+RtlCopyMemory(&entry->labels, &parent->labels, OVS_CT_LABELS_LEN);
+*labelChanged = TRUE;
+}
+
+/* Update labels according to value of ct_label in ct commit */
+if (labels && OvsConntrackIsLabelsNonZero(&labels->mask)) {
+UINT8 i;
+UINT32 *dst = entry->labels.ct_labels_32;
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+dst[i] = (dst[i] & ~(labels->mask.ct_labels_32[i])) |
+ (labels->value.ct_labels_32[i] & 
labels->mask.ct_labels_32[i]);
+}

-if (!NdisEqualMemory(&entry->labels, &pktMdLabel,
- sizeof(struct ovs_key_ct_labels))) {
 *labelChanged = TRUE;
 }
-NdisMoveMemory(&entry->labels, &pktMdLabel,
-   sizeof(struct ovs_key_ct_labels));
-NdisMoveMemory(&key->ct.labels, &pktMdLabel,
-   sizeof(struct ovs_key_ct_labels));
+
+/* Update flow key's ct labels */
+NdisMoveMemory(&key->ct.labels, &entry->labels, OVS_CT_LABELS_LEN);
 }

 static void
 OvsCtSetMarkLabel(OvsFlowKey *key,
-   POVS_CT_ENTRY entry,
-   MD_MARK *mark,
-   MD_LABELS *labels,
-   BOOLEAN *triggerUpdateEvent)
+  POVS_CT_ENTRY entry,
+  MD_MARK *mark,
+  MD_LABELS *labels,
+  BOOLEAN *triggerUpdateEvent)
 {
-if (mark) {
-OvsConntrackSetMark(key, entry, mark->value, mark->mask,
-triggerUpdateEvent);
-}
-
-if (labels) {
-OvsConntrackSetLabels(key, entry, &labels->value, &labels->mask,
-  triggerUpdateEvent);
-}
+OvsConntrackSetMark(key, entry, mark, triggerUpdateEvent);
+OvsConntrackSetLabels(key, entry, labels, triggerUpdateEvent);
 }

 /*
--
2.7.4

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

Re: [ovs-dev] [ovs-dev v2] datapath-windows: Reset ct_mark/ct_label to support ALG

2020-07-22 Thread Jinjun Gao 
Alin, thanks for review. I have updated the patch to resolve your comment and 
use RtlCopyMemory to replace memcpy. The PR also is updated: 
https://github.com/openvswitch/ovs/pull/324 .

- Jinjun


From: Alin Serdean 
Date: Thursday, July 23, 2020 at 3:47 AM
To: Jinjun Gao , "d...@openvswitch.org" 
, Anand Kumar 
Cc: Jinjun Gao 
Subject: RE: [ovs-dev v2] datapath-windows: Reset ct_mark/ct_label to support 
ALG

Thanks a lot for the patch.

In general please prefer to use RtlCopyMemory instead of memcpy: 
https://community.osr.com/discussion/242667/rtlcopymemory-vs-memcpy<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcommunity.osr.com%2Fdiscussion%2F242667%2Frtlcopymemory-vs-memcpy&data=02%7C01%7Cjinjung%40vmware.com%7Ca21bd1a7cd5241cab02508d82e781646%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637310440598279369&sdata=%2BinfwJ6Fw7%2FoJ1KKh67mGhgNJAiv1OT2G0T3O%2BVMQYg%3D&reserved=0>

Just a few more nits inlined.

Alin.

From: Jinjun Gao<mailto:jinj...@vmware.com>
Sent: Monday, July 20, 2020 6:00 PM
To: d...@openvswitch.org<mailto:d...@openvswitch.org>; Alin 
Serdean<mailto:aserd...@cloudbasesolutions.com>; 
kumaran...@vmware.com<mailto:kumaran...@vmware.com>
Cc: Jinjun Gao<mailto:jinj...@vmware.com>
Subject: [ovs-dev v2] datapath-windows: Reset ct_mark/ct_label to support ALG

The ct_mark/ct_label setting on related connection keep the same
behavior with Linux datapath. If one CT entry has parent/master entry,
its ct_mark and ct_label should inherit from the corresponding part
of parent/master entry at initialization.

Signed-off-by: Jinjun Gao 
---
v2: Resolve Anand's comment to add braces for if-block.
---
 datapath-windows/ovsext/Conntrack.c | 88 +++--
 1 file changed, 56 insertions(+), 32 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index d065591..cda6d8b 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -789,60 +789,84 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
 static __inline VOID
 OvsConntrackSetMark(OvsFlowKey *key,
 POVS_CT_ENTRY entry,
-UINT32 value,
-UINT32 mask,
+MD_MARK *mark,
 BOOLEAN *markChanged)
 {
-UINT32 newMark;
-newMark = value | (entry->mark & ~(mask));
-if (entry->mark != newMark) {
+POVS_CT_ENTRY parent = entry->parent;
+BOOLEAN changed = FALSE;
+UINT32 newMark = 0;
+
+if (parent && parent->mark) {
+newMark = parent->mark;
+changed = TRUE;
+} else if (mark) {
+newMark = mark->value | (entry->mark & ~(mark->mask));
+changed = TRUE;
+}
+
+if (changed && entry->mark != newMark) {
 entry->mark = newMark;
 key->ct.mark = newMark;
 *markChanged = TRUE;
 }
 }

+static __inline BOOLEAN
+OvsConntrackIsLabelsNonZero(const struct ovs_key_ct_labels *labels)
+{
+UINT8 i;
+
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+if (labels->ct_labels_32[i]) {
+return TRUE;
+}
+}
+
+return FALSE;
+}
+
 static __inline void
 OvsConntrackSetLabels(OvsFlowKey *key,
   POVS_CT_ENTRY entry,
-  struct ovs_key_ct_labels *val,
-  struct ovs_key_ct_labels *mask,
+  MD_LABELS *labels,
   BOOLEAN *labelChanged)
 {
-ovs_u128 v, m, pktMdLabel = {0};
-memcpy(&v, val, sizeof v);
-memcpy(&m, mask, sizeof m);
-memcpy(&pktMdLabel, &entry->labels, sizeof(struct ovs_key_ct_labels));
+POVS_CT_ENTRY parent = entry->parent;

-pktMdLabel.u64.lo = v.u64.lo | (pktMdLabel.u64.lo & ~(m.u64.lo));
-pktMdLabel.u64.hi = v.u64.hi | (pktMdLabel.u64.hi & ~(m.u64.hi));
+/* Inherit master's labels at labels initialization, if any. */
+if (!OvsConntrackIsLabelsNonZero(&entry->labels) &&
+parent && OvsConntrackIsLabelsNonZero(&parent->labels)) {
+memcpy(&entry->labels, &parent->labels, OVS_CT_LABELS_LEN);
+*labelChanged = TRUE;
+}
+
+/* Use the same computing method with Linux kernel datapath.
+ * It is more clean and easy understanding.
+ */
[Alin]Nit: You should drop the comment above. It is more useful to
say what you are actually computing.
+if (labels && OvsConntrackIsLabelsNonZero(&labels->mask)) {
+UINT8 i;
+UINT32 * dst = entry->labels.ct_labels_32;
[Alin] Nit: Remove the ‘ ‘. UINT32 *dst
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+dst[i] = (dst[i] & ~(labels->mask.ct_labels_32[i])) |
+ (labels->value.ct_labels_32[i] & 
labels->mask.ct_labels_32[i]);

[ovs-dev] [ovs-dev v2] datapath-windows: Reset ct_mark/ct_label to support ALG

2020-07-20 Thread Jinjun Gao 
The ct_mark/ct_label setting on related connection keep the same
behavior with Linux datapath. If one CT entry has parent/master entry,
its ct_mark and ct_label should inherit from the corresponding part
of parent/master entry at initialization.

Signed-off-by: Jinjun Gao 
---
v2: Resolve Anand's comment to add braces for if-block.
---
 datapath-windows/ovsext/Conntrack.c | 88 +++--
 1 file changed, 56 insertions(+), 32 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index d065591..cda6d8b 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -789,60 +789,84 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
 static __inline VOID
 OvsConntrackSetMark(OvsFlowKey *key,
 POVS_CT_ENTRY entry,
-UINT32 value,
-UINT32 mask,
+MD_MARK *mark,
 BOOLEAN *markChanged)
 {
-UINT32 newMark;
-newMark = value | (entry->mark & ~(mask));
-if (entry->mark != newMark) {
+POVS_CT_ENTRY parent = entry->parent;
+BOOLEAN changed = FALSE;
+UINT32 newMark = 0;
+
+if (parent && parent->mark) {
+newMark = parent->mark;
+changed = TRUE;
+} else if (mark) {
+newMark = mark->value | (entry->mark & ~(mark->mask));
+changed = TRUE;
+}
+
+if (changed && entry->mark != newMark) {
 entry->mark = newMark;
 key->ct.mark = newMark;
 *markChanged = TRUE;
 }
 }

+static __inline BOOLEAN
+OvsConntrackIsLabelsNonZero(const struct ovs_key_ct_labels *labels)
+{
+UINT8 i;
+
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+if (labels->ct_labels_32[i]) {
+return TRUE;
+}
+}
+
+return FALSE;
+}
+
 static __inline void
 OvsConntrackSetLabels(OvsFlowKey *key,
   POVS_CT_ENTRY entry,
-  struct ovs_key_ct_labels *val,
-  struct ovs_key_ct_labels *mask,
+  MD_LABELS *labels,
   BOOLEAN *labelChanged)
 {
-ovs_u128 v, m, pktMdLabel = {0};
-memcpy(&v, val, sizeof v);
-memcpy(&m, mask, sizeof m);
-memcpy(&pktMdLabel, &entry->labels, sizeof(struct ovs_key_ct_labels));
+POVS_CT_ENTRY parent = entry->parent;

-pktMdLabel.u64.lo = v.u64.lo | (pktMdLabel.u64.lo & ~(m.u64.lo));
-pktMdLabel.u64.hi = v.u64.hi | (pktMdLabel.u64.hi & ~(m.u64.hi));
+/* Inherit master's labels at labels initialization, if any. */
+if (!OvsConntrackIsLabelsNonZero(&entry->labels) &&
+parent && OvsConntrackIsLabelsNonZero(&parent->labels)) {
+memcpy(&entry->labels, &parent->labels, OVS_CT_LABELS_LEN);
+*labelChanged = TRUE;
+}
+
+/* Use the same computing method with Linux kernel datapath.
+ * It is more clean and easy understanding.
+ */
+if (labels && OvsConntrackIsLabelsNonZero(&labels->mask)) {
+UINT8 i;
+UINT32 * dst = entry->labels.ct_labels_32;
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+dst[i] = (dst[i] & ~(labels->mask.ct_labels_32[i])) |
+ (labels->value.ct_labels_32[i] & 
labels->mask.ct_labels_32[i]);
+}

-if (!NdisEqualMemory(&entry->labels, &pktMdLabel,
- sizeof(struct ovs_key_ct_labels))) {
 *labelChanged = TRUE;
 }
-NdisMoveMemory(&entry->labels, &pktMdLabel,
-   sizeof(struct ovs_key_ct_labels));
-NdisMoveMemory(&key->ct.labels, &pktMdLabel,
-   sizeof(struct ovs_key_ct_labels));
+
+/* Update flow key's ct labels */
+NdisMoveMemory(&key->ct.labels, &entry->labels, OVS_CT_LABELS_LEN);
 }

 static void
 OvsCtSetMarkLabel(OvsFlowKey *key,
-   POVS_CT_ENTRY entry,
-   MD_MARK *mark,
-   MD_LABELS *labels,
-   BOOLEAN *triggerUpdateEvent)
+  POVS_CT_ENTRY entry,
+  MD_MARK *mark,
+  MD_LABELS *labels,
+  BOOLEAN *triggerUpdateEvent)
 {
-if (mark) {
-OvsConntrackSetMark(key, entry, mark->value, mark->mask,
-triggerUpdateEvent);
-}
-
-if (labels) {
-OvsConntrackSetLabels(key, entry, &labels->value, &labels->mask,
-  triggerUpdateEvent);
-}
+OvsConntrackSetMark(key, entry, mark, triggerUpdateEvent);
+OvsConntrackSetLabels(key, entry, labels, triggerUpdateEvent);
 }

 /*
--
2.7.4

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

Re: [ovs-dev] [ovs-dev 1/1] datapath-windows: Reset ct_mark/ct_label to support ALG

2020-07-20 Thread Jinjun Gao 
Thanks for review, Anand. I have sent v2 patch. Please review it again.

PS: I also create pull request for it 
https://github.com/openvswitch/ovs/pull/324

-Jinjun

From: Anand Kumar 
Sent: Monday, July 20, 2020 10:12 PM
To: Jinjun Gao ; d...@openvswitch.org 
; aserd...@cloudbasesolutions.com 

Subject: Re: [ovs-dev 1/1] datapath-windows: Reset ct_mark/ct_label to support 
ALG

Hi Jinjun,

Thanks for the patch. It looks good to me, just a minor comment on the style.

Thanks,
Anand Kumar

On 16/07/20, 10:13 AM, "Jinjun Gao"  wrote:

The ct_mark/ct_label setting on related connection keep the same
behavior with Linux datapath. If one CT entry has parent/master entry,
its ct_mark and ct_label should inherit from the corresponding part
of parent/master entry at initialization.

Signed-off-by: Jinjun Gao 
---
 datapath-windows/ovsext/Conntrack.c | 87 
+++--
 1 file changed, 55 insertions(+), 32 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index d065591..83baf99 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -789,60 +789,83 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
 static __inline VOID
 OvsConntrackSetMark(OvsFlowKey *key,
 POVS_CT_ENTRY entry,
-UINT32 value,
-UINT32 mask,
+MD_MARK *mark,
 BOOLEAN *markChanged)
 {
-UINT32 newMark;
-newMark = value | (entry->mark & ~(mask));
-if (entry->mark != newMark) {
+POVS_CT_ENTRY parent = entry->parent;
+BOOLEAN changed = FALSE;
+UINT32 newMark = 0;
+
+if (parent && parent->mark) {
+newMark = parent->mark;
+changed = TRUE;
+} else if (mark) {
+newMark = mark->value | (entry->mark & ~(mark->mask));
+changed = TRUE;
+}
+
+if (changed && entry->mark != newMark) {
 entry->mark = newMark;
 key->ct.mark = newMark;
 *markChanged = TRUE;
 }
 }

+static __inline BOOLEAN
+OvsConntrackIsLabelsNonZero(const struct ovs_key_ct_labels *labels)
+{
+UINT8 i;
+
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+if (labels->ct_labels_32[i])
[Anand] : Please add braces for the if block.
+return TRUE;
+}
+
+return FALSE;
+}
+
 static __inline void
 OvsConntrackSetLabels(OvsFlowKey *key,
   POVS_CT_ENTRY entry,
-  struct ovs_key_ct_labels *val,
-  struct ovs_key_ct_labels *mask,
+  MD_LABELS *labels,
   BOOLEAN *labelChanged)
 {
-ovs_u128 v, m, pktMdLabel = {0};
-memcpy(&v, val, sizeof v);
-memcpy(&m, mask, sizeof m);
-memcpy(&pktMdLabel, &entry->labels, sizeof(struct ovs_key_ct_labels));
+POVS_CT_ENTRY parent = entry->parent;

-pktMdLabel.u64.lo = v.u64.lo | (pktMdLabel.u64.lo & ~(m.u64.lo));
-pktMdLabel.u64.hi = v.u64.hi | (pktMdLabel.u64.hi & ~(m.u64.hi));
+/* Inherit master's labels at labels initialization, if any. */
+if (!OvsConntrackIsLabelsNonZero(&entry->labels) &&
+parent && OvsConntrackIsLabelsNonZero(&parent->labels)) {
+memcpy(&entry->labels, &parent->labels, OVS_CT_LABELS_LEN);
+*labelChanged = TRUE;
+}
+
+/* Use the same computing method with Linux kernel datapath.
+ * It is more clean and easy understanding.
+ */
+if (labels && OvsConntrackIsLabelsNonZero(&labels->mask)) {
+UINT8 i;
+UINT32 * dst = entry->labels.ct_labels_32;
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+dst[i] = (dst[i] & ~(labels->mask.ct_labels_32[i])) |
+ (labels->value.ct_labels_32[i] & 
labels->mask.ct_labels_32[i]);
+}

-if (!NdisEqualMemory(&entry->labels, &pktMdLabel,
- sizeof(struct ovs_key_ct_labels))) {
 *labelChanged = TRUE;
 }
-NdisMoveMemory(&entry->labels, &pktMdLabel,
-   sizeof(struct ovs_key_ct_labels));
-NdisMoveMemory(&key->ct.labels, &pktMdLabel,
-   sizeof(struct ovs_key_ct_labels));
+
+/* Update flow key's ct labels */
+NdisMoveMemory(&key->ct.labels, &entry->labels, OVS_CT_LABELS_LEN);
 }

 stat

[ovs-dev] [ovs-dev 1/1] datapath-windows: Reset ct_mark/ct_label to support ALG

2020-07-15 Thread Jinjun Gao 
The ct_mark/ct_label setting on related connection keep the same
behavior with Linux datapath. If one CT entry has parent/master entry,
its ct_mark and ct_label should inherit from the corresponding part
of parent/master entry at initialization.

Signed-off-by: Jinjun Gao 
---
 datapath-windows/ovsext/Conntrack.c | 87 +++--
 1 file changed, 55 insertions(+), 32 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index d065591..83baf99 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -789,60 +789,83 @@ OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
 static __inline VOID
 OvsConntrackSetMark(OvsFlowKey *key,
 POVS_CT_ENTRY entry,
-UINT32 value,
-UINT32 mask,
+MD_MARK *mark,
 BOOLEAN *markChanged)
 {
-UINT32 newMark;
-newMark = value | (entry->mark & ~(mask));
-if (entry->mark != newMark) {
+POVS_CT_ENTRY parent = entry->parent;
+BOOLEAN changed = FALSE;
+UINT32 newMark = 0;
+
+if (parent && parent->mark) {
+newMark = parent->mark;
+changed = TRUE;
+} else if (mark) {
+newMark = mark->value | (entry->mark & ~(mark->mask));
+changed = TRUE;
+}
+
+if (changed && entry->mark != newMark) {
 entry->mark = newMark;
 key->ct.mark = newMark;
 *markChanged = TRUE;
 }
 }
 
+static __inline BOOLEAN
+OvsConntrackIsLabelsNonZero(const struct ovs_key_ct_labels *labels)
+{
+UINT8 i;
+
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+if (labels->ct_labels_32[i])
+return TRUE;
+}
+
+return FALSE;
+}
+
 static __inline void
 OvsConntrackSetLabels(OvsFlowKey *key,
   POVS_CT_ENTRY entry,
-  struct ovs_key_ct_labels *val,
-  struct ovs_key_ct_labels *mask,
+  MD_LABELS *labels,
   BOOLEAN *labelChanged)
 {
-ovs_u128 v, m, pktMdLabel = {0};
-memcpy(&v, val, sizeof v);
-memcpy(&m, mask, sizeof m);
-memcpy(&pktMdLabel, &entry->labels, sizeof(struct ovs_key_ct_labels));
+POVS_CT_ENTRY parent = entry->parent;
 
-pktMdLabel.u64.lo = v.u64.lo | (pktMdLabel.u64.lo & ~(m.u64.lo));
-pktMdLabel.u64.hi = v.u64.hi | (pktMdLabel.u64.hi & ~(m.u64.hi));
+/* Inherit master's labels at labels initialization, if any. */
+if (!OvsConntrackIsLabelsNonZero(&entry->labels) &&
+parent && OvsConntrackIsLabelsNonZero(&parent->labels)) {
+memcpy(&entry->labels, &parent->labels, OVS_CT_LABELS_LEN);
+*labelChanged = TRUE;
+}
+
+/* Use the same computing method with Linux kernel datapath.
+ * It is more clean and easy understanding.
+ */
+if (labels && OvsConntrackIsLabelsNonZero(&labels->mask)) {
+UINT8 i;
+UINT32 * dst = entry->labels.ct_labels_32;
+for (i = 0; i < OVS_CT_LABELS_LEN_32; i++) {
+dst[i] = (dst[i] & ~(labels->mask.ct_labels_32[i])) |
+ (labels->value.ct_labels_32[i] & 
labels->mask.ct_labels_32[i]);
+}
 
-if (!NdisEqualMemory(&entry->labels, &pktMdLabel,
- sizeof(struct ovs_key_ct_labels))) {
 *labelChanged = TRUE;
 }
-NdisMoveMemory(&entry->labels, &pktMdLabel,
-   sizeof(struct ovs_key_ct_labels));
-NdisMoveMemory(&key->ct.labels, &pktMdLabel,
-   sizeof(struct ovs_key_ct_labels));
+
+/* Update flow key's ct labels */
+NdisMoveMemory(&key->ct.labels, &entry->labels, OVS_CT_LABELS_LEN);
 }
 
 static void
 OvsCtSetMarkLabel(OvsFlowKey *key,
-   POVS_CT_ENTRY entry,
-   MD_MARK *mark,
-   MD_LABELS *labels,
-   BOOLEAN *triggerUpdateEvent)
+  POVS_CT_ENTRY entry,
+  MD_MARK *mark,
+  MD_LABELS *labels,
+  BOOLEAN *triggerUpdateEvent)
 {
-if (mark) {
-OvsConntrackSetMark(key, entry, mark->value, mark->mask,
-triggerUpdateEvent);
-}
-
-if (labels) {
-OvsConntrackSetLabels(key, entry, &labels->value, &labels->mask,
-  triggerUpdateEvent);
-}
+OvsConntrackSetMark(key, entry, mark, triggerUpdateEvent);
+OvsConntrackSetLabels(key, entry, labels, triggerUpdateEvent);
 }
 
 /*
-- 
2.7.4

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

[ovs-dev] Reset ct_mark and ct_lables in conntrack entry

2020-07-13 Thread Jinjun Gao 
Hi Alin/Anand,

We have a project that needs to support ALG FTP on Windows OVS. I have added 
CT_HELP and CT_TUPLE_MASTER in previous merged patch. Now, we also need to 
support ct_mark and ct_labels. We have tested that FTP data traffic cannot 
inherit ct_mark and ct_labels from its master FTP control traffic in current 
implementation. I have cooked a patch to implement it. But I have a confusion 
that need to discuss with you guys.

In our project, there is a rule like “ruleid (0x1000) from(any) to(any) 
service(ftp) allow”. This rule will be pushed to OVS by controller. The ruleid 
will be set in ct_mark field only in ftp control flow:
actions=ct(commit,zone=61439,mark=0x3f1/0x,label=0x1018/0x1fff,helper=ftp),
 the ftp data flow has no ct_mark field: 
actions=ct(commit,zone=61439,label=0x80/0x1380).

In general, the CT entry cannot derive the ct_mark at connection commit because 
there is no ct_mark field in flow rule’s ct actions. For FTP data traffic, it 
has master conntrack entry. If its master conntrack entry has ct_mark, should 
FTP data conntrack entry inherit its master’s ct_mark even though it has no 
ct_mark field in ct actions?

I checked userspace datapath ALG FTP support code. Seems it will not inherit 
master’s ct_mark/ct_labels if it has no ct_mark/ct_labels field in ct commit 
actions.
https://github.com/openvswitch/ovs/blob/master/lib/conntrack.c#L1374
 if (conn && setmark) {.   setmark should be NULL if there is no 
ct_mark field in ct commit actions. If setmark is NULL in FTP data traffic, it 
will cannot inherit master’s mark even master has mark.
set_mark(pkt, conn, setmark[0], setmark[1]);
}

if (conn && setlabel) { 
set_label(pkt, conn, &setlabel[0], &setlabel[1]);
}

I don’t know Linux datapath how to implement it, but for the same ALG FTP rule 
in our project, the FTP data traffic can inherit master’s mark in Linux OVS 
even though it has no ct_mark field in ct commit actions.
Here, do you have any suggestion? The FTP data flow should always inherit 
master’s mark/labels even though it has no ct_mark/ct_labels field in ct commit 
actions. Or it should always consider its ct_mark/ct_labels field in ct commit 
actions at firstly.

Thanks,
- Jinjun

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

Re: [ovs-dev] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

2020-06-30 Thread Jinjun Gao 
Thanks for review, Alin.

Please see the comments in inline [Jinjun].

--
Jinjun


From: Alin Serdean 
Date: Tuesday, June 30, 2020 at 5:34 PM
To: Jinjun Gao , "d...@openvswitch.org" 

Cc: Anand Kumar 
Subject: RE: [ovs-dev] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

Thanks for sending the patch.

Running the command:
make datapath_windows_analyze
resulted in the following:
ovsext\Conntrack.c(873): warning C28167: The function 'OvsCtExecute_' changes 
the IRQL and does not
restore the IRQL before it exits. It should be annotated to reflect the change 
or the IRQL should
be restored. IRQL was last set to 2 at line 955.
[datapath-windows\ovsext\ovsext.vcxproj]

There is a lock acquired but it is not released on the failure path.

You can fold in following incremental:

[Jinjun]: Will add it, thanks.

$ git diff datapath-windows/ovsext/Conntrack.c
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 414b3f4b2..717d04f49 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -974,6 +974,7 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
   OVS_CT_POOL_TAG);
 if (!entry->helper_name) {
 OVS_LOG_ERROR("Error while allocating memory");
+OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql);
 return NDIS_STATUS_RESOURCES;
 }

The rest looks good, but I have the following question:

 /* FTP ACTIVE - Server initiates the connection */
 if ((incomingKey.src.addr.ipv4 == entryKey.src.addr.ipv4) &&
-(incomingKey.src.port == entryKey.src.port) &&
Why is the line above removed?
 [Jinjun]: I used pyftpdlib as ftp server to test this patch. Pyftpdlib doesn’t 
use 20 as data port and it chooses a random port as data port in active mode. 
It causes the incomingKey cannot match any entry in related table. In this 
case, the data flow cannot find control flow and add it as tuple master. After 
deleting above line, it works! If we don’t consider to support random data port 
in active mode, we need not to remove above line. It should be better to 
support random data port case in active mode. How do you think about it? If you 
agree, I can add some comments here to clear it.

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

[ovs-dev] [ovs-dev v2] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

2020-06-30 Thread Jinjun Gao 
Add helper and master if existing to a conntrack entry:
1, For CTA_HELP, only support FTP/TFTP;
2, For CTA_TUPLE_MASTER, only support FTP.

Signed-off-by: Jinjun Gao 
---
v2:
  1, Add CTA_TUPLE_MASTER support in CT event subscribe/report system.
  2, Release CT entry lock in fail path.
  3, Add more comments.
---
 datapath-windows/ovsext/Conntrack-related.c |  5 ++-
 datapath-windows/ovsext/Conntrack.c | 50 +
 datapath-windows/ovsext/Conntrack.h |  1 +
 3 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-related.c 
b/datapath-windows/ovsext/Conntrack-related.c
index 950be98..a5bba5c 100644
--- a/datapath-windows/ovsext/Conntrack-related.c
+++ b/datapath-windows/ovsext/Conntrack-related.c
@@ -47,8 +47,11 @@ OvsCtRelatedKeyAreSame(OVS_CT_KEY incomingKey, OVS_CT_KEY 
entryKey)
 }

 /* FTP ACTIVE - Server initiates the connection */
+/* Some ftp server, such as pyftpdlib, may use random (>1024) data port
+ * except 20. In this case, the incomingKey's src port is different with
+ * entryKey's src port.
+ */
 if ((incomingKey.src.addr.ipv4 == entryKey.src.addr.ipv4) &&
-(incomingKey.src.port == entryKey.src.port) &&
 (incomingKey.dst.addr.ipv4 == entryKey.dst.addr.ipv4) &&
 (incomingKey.dst.port == entryKey.dst.port) &&
 (incomingKey.dl_type == entryKey.dl_type) &&
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 55917c4..d065591 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -246,7 +246,6 @@ OvsPostCtEventEntry(POVS_CT_ENTRY entry, UINT8 type)
 {
 OVS_CT_EVENT_ENTRY ctEventEntry = {0};
 NdisMoveMemory(&ctEventEntry.entry, entry, sizeof(OVS_CT_ENTRY));
-ctEventEntry.entry.parent = NULL;
 ctEventEntry.type = type;
 OvsPostCtEvent(&ctEventEntry);
 }
@@ -480,6 +479,9 @@ OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN forceDelete)
 RemoveEntryList(&entry->link);
 OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql);
 NdisFreeSpinLock(&(entry->lock));
+if (entry->helper_name) {
+OvsFreeMemoryWithTag(entry->helper_name, OVS_CT_POOL_TAG);
+}
 OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
 NdisInterlockedDecrement((PLONG)&ctTotalEntries);
 return;
@@ -883,6 +885,7 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
 BOOLEAN triggerUpdateEvent = FALSE;
 BOOLEAN entryCreated = FALSE;
 POVS_CT_ENTRY entry = NULL;
+POVS_CT_ENTRY parent = NULL;
 PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
 OvsConntrackKeyLookupCtx ctx = { 0 };
 LOCK_STATE_EX lockStateTable;
@@ -959,8 +962,6 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,

 if (OvsDetectFtpPacket(key)) {
 /* FTP parser will always be loaded */
-UNREFERENCED_PARAMETER(helper);
-
 status = OvsCtHandleFtp(curNbl, key, layers, currentTime, entry,
 (ntohs(key->ipKey.l4.tpDst) == IPPORT_FTP));
 if (status != NDIS_STATUS_SUCCESS) {
@@ -968,10 +969,25 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
 }
 }

+parent = entry->parent;
+/* The entry should have the same helper name with parent's */
+if (!entry->helper_name &&
+(helper || (parent && parent->helper_name))) {
+
+helper = helper ? helper : parent->helper_name;
+entry->helper_name = OvsAllocateMemoryWithTag(strlen(helper) + 1,
+  OVS_CT_POOL_TAG);
+if (!entry->helper_name) {
+OVS_LOG_ERROR("Error while allocating memory");
+OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql);
+return NDIS_STATUS_RESOURCES;
+}
+memcpy(entry->helper_name, helper, strlen(helper) + 1);
+}
+
 /* Add original tuple information to flow Key */
 if (entry->key.dl_type == ntohs(ETH_TYPE_IPV4)) {
-if (entry->parent != NULL) {
-POVS_CT_ENTRY parent = entry->parent;
+if (parent != NULL) {
 OVS_ACQUIRE_SPIN_LOCK(&(parent->lock), irql);
 OvsCtUpdateTuple(key, &parent->key);
 OVS_RELEASE_SPIN_LOCK(&(parent->lock), irql);
@@ -1042,8 +1058,8 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 if (helper == NULL) {
 return NDIS_STATUS_INVALID_PARAMETER;
 }
-if (strcmp("ftp", helper) != 0) {
-/* Only support FTP */
+if (strcmp("ftp", helper) != 0 && strcmp("tftp", helper) != 0) 
{
+/* Only support FTP/TFTP */
 return NDIS_STATUS_NOT_SUPPORTED;

[ovs-dev] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

2020-06-23 Thread Jinjun Gao 
Add helper and master if existing to a conntrack entry:
1, For CTA_HELP, only support FTP/TFTP;
2, For CTA_TUPLE_MASTER, only support FTP.

Signed-off-by: Jinjun Gao 
---
 datapath-windows/ovsext/Conntrack-related.c |  1 -
 datapath-windows/ovsext/Conntrack.c | 40 ++---
 datapath-windows/ovsext/Conntrack.h |  1 +
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-related.c 
b/datapath-windows/ovsext/Conntrack-related.c
index 950be98..3bdd52f 100644
--- a/datapath-windows/ovsext/Conntrack-related.c
+++ b/datapath-windows/ovsext/Conntrack-related.c
@@ -48,7 +48,6 @@ OvsCtRelatedKeyAreSame(OVS_CT_KEY incomingKey, OVS_CT_KEY 
entryKey)

 /* FTP ACTIVE - Server initiates the connection */
 if ((incomingKey.src.addr.ipv4 == entryKey.src.addr.ipv4) &&
-(incomingKey.src.port == entryKey.src.port) &&
 (incomingKey.dst.addr.ipv4 == entryKey.dst.addr.ipv4) &&
 (incomingKey.dst.port == entryKey.dst.port) &&
 (incomingKey.dl_type == entryKey.dl_type) &&
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index ba56116..864095f 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -480,6 +480,9 @@ OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN forceDelete)
 RemoveEntryList(&entry->link);
 OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql);
 NdisFreeSpinLock(&(entry->lock));
+if (entry->helper_name) {
+OvsFreeMemoryWithTag(entry->helper_name, OVS_CT_POOL_TAG);
+}
 OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
 NdisInterlockedDecrement((PLONG)&ctTotalEntries);
 return;
@@ -956,8 +959,6 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,

 if (OvsDetectFtpPacket(key)) {
 /* FTP parser will always be loaded */
-UNREFERENCED_PARAMETER(helper);
-
 status = OvsCtHandleFtp(curNbl, key, layers, currentTime, entry,
 (ntohs(key->ipKey.l4.tpDst) == IPPORT_FTP));
 if (status != NDIS_STATUS_SUCCESS) {
@@ -965,6 +966,17 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
 }
 }

+if (!entry->helper_name && helper) {
+entry->helper_name = OvsAllocateMemoryWithTag(strlen(helper) + 1,
+  OVS_CT_POOL_TAG);
+if (!entry->helper_name) {
+OVS_LOG_ERROR("Error while allocating memory");
+return NDIS_STATUS_RESOURCES;
+}
+
+memcpy(entry->helper_name, helper, strlen(helper) + 1);
+}
+
 /* Add original tuple information to flow Key */
 if (entry->key.dl_type == ntohs(ETH_TYPE_IPV4)) {
 if (entry->parent != NULL) {
@@ -1039,8 +1051,8 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 if (helper == NULL) {
 return NDIS_STATUS_INVALID_PARAMETER;
 }
-if (strcmp("ftp", helper) != 0) {
-/* Only support FTP */
+if (strcmp("ftp", helper) != 0 && strcmp("tftp", helper) != 0) 
{
+/* Only support FTP/TFTP */
 return NDIS_STATUS_NOT_SUPPORTED;
 }
 break;
@@ -1680,6 +1692,26 @@ OvsCreateNlMsgFromCtEntry(POVS_CT_ENTRY entry,
 }
 }

+if (entry->helper_name) {
+UINT32 offset;
+offset = NlMsgStartNested(&nlBuf, CTA_HELP);
+if (!offset) {
+return NDIS_STATUS_FAILURE;
+}
+if (!NlMsgPutTailString(&nlBuf, CTA_HELP_NAME, entry->helper_name)) {
+return STATUS_INVALID_BUFFER_SIZE;
+}
+NlMsgEndNested(&nlBuf, offset);
+}
+
+if (entry->parent) {
+status = MapCtKeyTupleToNl(&nlBuf, CTA_TUPLE_MASTER,
+   &((POVS_CT_ENTRY)entry->parent)->key);
+if (status != NDIS_STATUS_SUCCESS) {
+   return STATUS_UNSUCCESSFUL;
+}
+}
+
 /* CTA_STATUS is required but not implemented. Default to 0 */
 if (!NlMsgPutTailU32(&nlBuf, CTA_STATUS, 0)) {
 return STATUS_INVALID_BUFFER_SIZE;
diff --git a/datapath-windows/ovsext/Conntrack.h 
b/datapath-windows/ovsext/Conntrack.h
index bc6580d..23b0058 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -108,6 +108,7 @@ typedef struct OVS_CT_ENTRY {
 struct ovs_key_ct_labels labels;
 NAT_ACTION_INFO natInfo;
 PVOID   parent; /* Points to main connection */
+PCHAR   helper_name;
 } OVS_CT_ENTRY, *POVS_CT_ENTRY;

 typedef struct OVS_CT_REL_ENTRY {
--
1.8.5.6
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Windows OVS datapath conntrack support CTA_HELP and CTA_TUPLE_MASTER

2020-05-27 Thread Jinjun Gao 
Hi Alin/Guys,

We have an ALG feature that needs Windows OVS conntrack to support CTA_HELP and 
CTA_TUPLE_MASTER attributes. I noticed these two attributes has not been 
implemented in current Windows OVS datapath. Do you know the context why them 
are not implemented at original design and implement? What details do I need to 
pay attention to if I want to implement them?

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

[ovs-dev] [ovs-dev, v2] datapath-windows: Don't delete internal port

2019-12-08 Thread Jinjun Gao via dev 
According to the microsoft doc:
https://docs.microsoft.com/en-us/windows-hardware/drivers/network/hyper-v-extensible-switch-port-and-network-adapter-states
Below OID request sequence is validation:
 OID_SWITCH_NIC_CONNECT -> OID_SWITCH_NIC_DISCONNECT
  ^   |
  |   V
 OID_SWITCH_NIC_CREATE  <- OID_SWITCH_NIC_DELETE

In above sequence, the windows extensible switch interface assumes the
OID_SWITCH_PORT_CREATE has issued and the port has been created
successfully. If delete the internal port in HvDisconnectNic(),
HvCreateNic() will fail when received OID_SWITCH_NIC_CREATE late because
there is no corresponding port.

Signed-off-by: Jinjun Gao 
---
datapath-windows/ovsext/Vport.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 57e7510..9f1587f 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -628,6 +628,7 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
 event.upcallPid = vport->upcallPid;
 RtlCopyMemory(&event.ovsName, &vport->ovsName, sizeof event.ovsName);
 event.type = OVS_EVENT_LINK_DOWN;
+OvsPostVportEvent(&event);

 /*
  * Delete the port from the hash tables accessible to userspace. After this
@@ -635,13 +636,18 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
  */
 if (OvsIsRealExternalVport(vport)) {
 OvsRemoveAndDeleteVport(NULL, switchContext, vport, FALSE, TRUE);
-OvsPostVportEvent(&event);
 }

 if (isInternalPort) {
 OvsUnBindVportWithIpHelper(vport, switchContext);
-OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE);
-OvsPostVportEvent(&event);
+/*
+ * Don't delete the port from the hash tables here for internal port
+ * because the internal port cannot be recreated in HvCreateNic(). It
+ * only can be created in HvCreatePort() by issuing
+ * OID_SWITCH_PORT_CREATE. We should wait extensible switch interface
+ * to issue OID_SWITCH_PORT_TEARDOWN and OID_SWITCH_PORT_DELETE to
+ * delete the internal port.
+ */
 }
 NdisReleaseRWLock(switchContext->dispatchLock, &lockState);

--
1.8.5.6

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

[ovs-dev] datapath-windows: Don't delete internal port

2019-12-02 Thread Jinjun Gao via dev 
According to the microsoft doc:
https://docs.microsoft.com/en-us/windows-hardware/drivers/network/hyper-v-extensible-switch-port-and-network-adapter-states
Below OID request sequence is validation:
 OID_SWITCH_NIC_CONNECT -> OID_SWITCH_NIC_DISCONNECT
  ^   |
  |   V
 OID_SWITCH_NIC_CREATE  <- OID_SWITCH_NIC_DELETE

In above sequence, the windows extensible switch interface assumes the
OID_SWITCH_PORT_CREATE has issued and the port has been created
successfully. If delete the internal port in HvDisconnectNic(),
HvCreateNic() will fail when received OID_SWITCH_NIC_CREATE late because
there is no corresponding port.

Signed-off-by: Jinjun Gao 
---
datapath-windows/ovsext/Vport.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 57e7510..c3362d7 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -640,8 +640,14 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
 if (isInternalPort) {
 OvsUnBindVportWithIpHelper(vport, switchContext);
-OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE);
-OvsPostVportEvent(&event);
+/*
+ * Don't delete the port from the hash tables here for internal port
+ * because the internal port cannot be recreated in HvCreateNic(). It
+ * only can be created in HvCreatePort() by issuing
+ * OID_SWITCH_PORT_CREATE. We should wait extensible switch interface
+ * to issue OID_SWITCH_PORT_TEARDOWN and OID_SWITCH_PORT_DELETE to
+ * delete the internal port.
+ */
 }
 NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
--
1.8.5.6
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev