Re: [ovs-dev] [PATCH] datapath-windows: Include ICMP type and code fields to find a matching NAT entry

[Yin Lin]

Hi Anand,

Please do not change NAT code to include other fields in ct_endpoint. NAT
doesn't care about those fields. As long as the address and port are
identical, we will reverse NAT it. For example, even if we receive an ICMP
reply, we will match it against a ICMP request entry to do the reverse NAT.
We don't support ipv6 yet and don't want to be bothered by the extra fields
in src.addr and dst.addr either. So those FIELD_COMPARE are the exact
things we want to compare, nothing less.

Best regards,
Yin Lin

On Fri, Jun 23, 2017 at 2:27 PM, Anand Kumar <kumaran...@vmware.com> wrote:

> Update OvsNatKeyAreSame() and OvsHashNatKey() to include ICMP type and code
> fields, so that ICMP_ECHO_REQUEST packets are not matched with
> ICMP_ECHO_REPLY
> packets and vice versa.
>
> Signed-off-by: Anand Kumar <kumaran...@vmware.com>
> ---
>  datapath-windows/ovsext/Conntrack-nat.c | 32
> ++--
>  1 file changed, 10 insertions(+), 22 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Conntrack-nat.c
> b/datapath-windows/ovsext/Conntrack-nat.c
> index ae6b92c..f3c282c 100644
> --- a/datapath-windows/ovsext/Conntrack-nat.c
> +++ b/datapath-windows/ovsext/Conntrack-nat.c
> @@ -13,17 +13,11 @@ PLIST_ENTRY ovsUnNatTable = NULL;
>  static __inline UINT32
>  OvsHashNatKey(const OVS_CT_KEY *key)
>  {
> -UINT32 hash = 0;
> -#define HASH_ADD(field) \
> -hash = OvsJhashBytes(>field, sizeof(key->field), hash)
> -
> -HASH_ADD(src.addr.ipv4_aligned);
> -HASH_ADD(dst.addr.ipv4_aligned);
> -HASH_ADD(src.port);
> -HASH_ADD(dst.port);
> -HASH_ADD(zone);
> -#undef HASH_ADD
> -return hash;
> +UINT32 hash[3];
> +hash[0] = OvsJhashBytes(>src, sizeof(key->src), 0);
> +hash[1] = OvsJhashBytes(>dst, sizeof(key->dst), 0);
> +hash[2] = OvsJhashBytes(>zone, sizeof(key->zone), 0);
> +return OvsJhashWords(hash, 3, OVS_HASH_BASIS);
>  }
>
>  /*
> @@ -35,17 +29,11 @@ OvsHashNatKey(const OVS_CT_KEY *key)
>  static __inline BOOLEAN
>  OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
>  {
> -// XXX: Compare IPv6 key as well
> -#define FIELD_COMPARE(field) \
> -if (key1->field != key2->field) return FALSE
> -
> -FIELD_COMPARE(src.addr.ipv4_aligned);
> -FIELD_COMPARE(dst.addr.ipv4_aligned);
> -FIELD_COMPARE(src.port);
> -FIELD_COMPARE(dst.port);
> -FIELD_COMPARE(zone);
> -return TRUE;
> -#undef FIELD_COMPARE
> +return ((NdisEqualMemory(>src, >src,
> + sizeof(struct ct_endpoint))) &&
> +(NdisEqualMemory(>dst, >dst,
> + sizeof(struct ct_endpoint))) &&
> +(key1->zone == key2->zone));
>  }
>
>  /*
> --
> 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: Fix potential memory leak while creating conntrack entry

[Yin Lin]

Hi Sai,

I meant that  if an entry is returned by this function, it's always going
to be "explicitly created", rather than "an existing entry". If that is
that case, you can remove all lines that is related to entryCreated in this
function and determine whether entryCreated is true from the caller. I'm
bringing it up because you are refactoring this function for simplicity and
coherence anyway. But of course it's your decision whether to make it in a
separate patch.

Best regards,
Yin Lin

On Tue, Jun 20, 2017 at 3:29 PM, Sairam Venugopal <vsai...@vmware.com>
wrote:

> How will you do that in this code? We need to distinguish between an
> existing entry and entry explicitly created. If you want to send out an
> incremental patch, feel free to.
>
> Thanks,
> Sairam
>
> From: Yin Lin
> Date: Tuesday, June 20, 2017 at 3:26 PM
>
> To: Sairam Venugopal
> Subject: Re: [ovs-dev] [PATCH] datapath-windows: Fix potential memory
> leak while creating conntrack entry
>
> Hi Sai,
>
> In that case, can we get rid of the boolean and ask the caller to fire the
> event based on whether entry == NULL? For this particular function I don't
> see any reason to take an extra parameter. If one return value can be
> easily inferred from the other return value, there is no need to return two.
>
> Best regards,
> Yin Lin
>
> On Tue, Jun 20, 2017 at 2:38 PM, Sairam Venugopal <vsai...@vmware.com>
> wrote:
>
>> This variable is used to determine if a CT Entry was created without any
>> errors. We then rely on this variable to fire an event to state that a new
>> CT Entry was created.
>>
>> This event was previously inside the OvsCtAddEntry but had to be pulled
>> out to include Mark and Label fields.
>>
>>
>> From: Yin Lin
>> Date: Tuesday, June 20, 2017 at 2:34 PM
>> To: Sairam Venugopal
>> Subject: Re: [ovs-dev] [PATCH] datapath-windows: Fix potential memory
>> leak while creating conntrack entry
>>
>> Hi Sai,
>>
>> Thanks for the fix. One question though. I noticed that entryCreated is
>> TRUE if and only if entry == NULL at the end of this function. So why do we
>> need this variable?
>>
>> Best regards,
>> Yin Lin
>>
>> On Tue, Jun 20, 2017 at 1:36 PM, Sairam Venugopal <vsai...@vmware.com>
>> wrote:
>>
>>> OvsCtAddEntry returns TRUE or FALSE depending on whether
>>> OvsNatTranslateCtEntry was successful or not. In the case of an
>>> unsuccesful NAT translation, this will fail to insert the newly created
>>> entry to the Conntrack Table. This entry needs to be freed and the states
>>> should be accordingly in the flowKey instead of returning out.
>>>
>>> Consolidated the parentEntry lookup and assignment portion across
>>> different protocols and some minor refactoring to make the code more
>>> readable.
>>>
>>> Tests Done: Enabled driver verifier and tested the following:
>>> - TCP & ICMP traffic through Conntrack Module.
>>> - Flushed Conntrack Entries while traffic was flowing.
>>> - Uninstalled and re-installed the driver when traffic was in progress.
>>>
>>> Signed-off-by: Sairam Venugopal <vsai...@vmware.com>
>>> ---
>>>  datapath-windows/ovsext/Conntrack.c | 59 +-
>>> ---
>>>  1 file changed, 27 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/datapath-windows/ovsext/Conntrack.c
>>> b/datapath-windows/ovsext/Conntrack.c
>>> index 68ed395..bf9c4f4 100644
>>> --- a/datapath-windows/ovsext/Conntrack.c
>>> +++ b/datapath-windows/ovsext/Conntrack.c
>>> @@ -214,9 +214,18 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
>>>   BOOLEAN *entryCreated)
>>>  {
>>>  POVS_CT_ENTRY entry = NULL;
>>> -*entryCreated = FALSE;
>>>  UINT32 state = 0;
>>> +POVS_CT_ENTRY parentEntry;
>>>  PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
>>> +
>>> +*entryCreated = FALSE;
>>> +state |= OVS_CS_F_NEW;
>>> +
>>> +parentEntry = OvsCtRelatedLookup(ctx->key, currentTime);
>>> +if (parentEntry != NULL) {
>>> +state |= OVS_CS_F_RELATED;
>>> +}
>>> +
>>>  switch (ipProto)
>>>  {
>>>  case IPPROTO_TCP:
>>> @@ -228,23 +237,8 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
>>>  goto invalid;
>>>  }
>>>
>>> -state |= OVS_CS_F_NEW;
>>> -POVS_CT_ENTRY

[ovs-dev] [PATCH v9 3/4] datapath-windows: NAT integration with conntrack

[Yin Lin]

This patch integrates NAT module with existing conntrack module. NAT
action is now supported.

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Actions.c  | 119 ++-
 datapath-windows/ovsext/Actions.h  |  20 
 datapath-windows/ovsext/Conntrack.c| 201 +
 datapath-windows/ovsext/Conntrack.h|  25 ++--
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 5 files changed, 282 insertions(+), 85 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index c3f0362..668825b 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1380,7 +1380,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_udp *udpAttr)
 {
@@ -1427,7 +1427,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_tcp *tcpAttr)
 {
@@ -1465,12 +1465,125 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 
 /*
  *
+ * OvsUpdateAddressAndPort --
+ *  Updates the source/destination IP and port fields in
+ *  ovsFwdCtx.curNbl inline based on the specified key.
+ *
+ */
+NDIS_STATUS
+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
+UINT32 newAddr, UINT16 newPort,
+BOOLEAN isSource, BOOLEAN isTx)
+{
+PUINT8 bufferStart;
+UINT32 hdrSize;
+OVS_PACKET_HDR_INFO *layers = >layers;
+IPHdr *ipHdr;
+TCPHdr *tcpHdr = NULL;
+UDPHdr *udpHdr = NULL;
+UINT32 *addrField = NULL;
+UINT16 *portField = NULL;
+UINT16 *checkField = NULL;
+BOOLEAN l4Offload = FALSE;
+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+
+ASSERT(layers->value != 0);
+
+if (layers->isTcp || layers->isUdp) {
+hdrSize = layers->l4Offset +
+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
+} else {
+hdrSize = layers->l3Offset + sizeof (*ipHdr);
+}
+
+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize);
+if (!bufferStart) {
+return NDIS_STATUS_RESOURCES;
+}
+
+ipHdr = (IPHdr *)(bufferStart + layers->l3Offset);
+
+if (layers->isTcp) {
+tcpHdr = (TCPHdr *)(bufferStart + layers->l4Offset);
+} else if (layers->isUdp) {
+udpHdr = (UDPHdr *)(bufferStart + layers->l4Offset);
+}
+
+csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl,
+  TcpIpChecksumNetBufferListInfo);
+/*
+ * Adjust the IP header inline as dictated by the action, and also update
+ * the IP and the TCP checksum for the data modified.
+ *
+ * In the future, this could be optimized to make one call to
+ * ChecksumUpdate32(). Ignoring this for now, since for the most common
+ * case, we only update the TTL.
+ */
+
+if (isSource) {
+addrField = >saddr;
+if (tcpHdr) {
+portField = >source;
+checkField = >check;
+l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.TcpChecksum :
+((BOOLEAN)csumInfo.Receive.TcpChecksumSucceeded ||
+ (BOOLEAN)csumInfo.Receive.TcpChecksumFailed);
+} else if (udpHdr) {
+portField = >source;
+checkField = >check;
+l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.UdpChecksum :
+((BOOLEAN)csumInfo.Receive.UdpChecksumSucceeded ||
+ (BOOLEAN)csumInfo.Receive.UdpChecksumFailed);
+}
+} else {
+addrField = >daddr;
+if (tcpHdr) {
+portField = >dest;
+checkField = >check;
+} else if (udpHdr) {
+portField = >dest;
+checkField = >check;
+}
+}
+if (*addrField != newAddr) {
+UINT32 oldAddr = *addrField;
+if (checkField && *checkField != 0) {
+if (l4Offload) {
+/* Recompute IP pseudo checksum */
+*checkField = ~(*checkField);
+*checkField = ChecksumUpdate32(*checkField, oldAddr,
+   newAddr);
+*checkField = ~(*checkField);
+} else {
+*checkField = ChecksumUpd

[ovs-dev] [PATCH v9 2/4] datapath-windows: Add NAT module in conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-nat.c | 433 
 datapath-windows/ovsext/Conntrack-nat.h |  39 +++
 3 files changed, 474 insertions(+)
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 49011f12..36018ea 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,7 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
+   datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
+   datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
new file mode 100644
index 000..ae6b92c
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -0,0 +1,433 @@
+#include "Conntrack-nat.h"
+#include "Jhash.h"
+
+PLIST_ENTRY ovsNatTable = NULL;
+PLIST_ENTRY ovsUnNatTable = NULL;
+
+/*
+ *---
+ * OvsHashNatKey
+ * Hash NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline UINT32
+OvsHashNatKey(const OVS_CT_KEY *key)
+{
+UINT32 hash = 0;
+#define HASH_ADD(field) \
+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
+
+HASH_ADD(src.addr.ipv4_aligned);
+HASH_ADD(dst.addr.ipv4_aligned);
+HASH_ADD(src.port);
+HASH_ADD(dst.port);
+HASH_ADD(zone);
+#undef HASH_ADD
+return hash;
+}
+
+/*
+ *---
+ * OvsNatKeyAreSame
+ * Compare NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline BOOLEAN
+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
+{
+// XXX: Compare IPv6 key as well
+#define FIELD_COMPARE(field) \
+if (key1->field != key2->field) return FALSE
+
+FIELD_COMPARE(src.addr.ipv4_aligned);
+FIELD_COMPARE(dst.addr.ipv4_aligned);
+FIELD_COMPARE(src.port);
+FIELD_COMPARE(dst.port);
+FIELD_COMPARE(zone);
+return TRUE;
+#undef FIELD_COMPARE
+}
+
+/*
+ *---
+ * OvsNatGetBucket
+ * Returns the row of NAT table that has the same hash as the given NAT
+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
+ * instead.
+ *---
+ */
+static __inline PLIST_ENTRY
+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
+{
+uint32_t hash = OvsHashNatKey(key);
+if (isReverse) {
+return [hash & NAT_HASH_TABLE_MASK];
+} else {
+return [hash & NAT_HASH_TABLE_MASK];
+}
+}
+
+/*
+ *---
+ * OvsNatInit
+ * Initialize NAT related resources.
+ *---
+ */
+NTSTATUS OvsNatInit()
+{
+ASSERT(ovsNatTable == NULL);
+
+/* Init the Hash Buffer */
+ovsNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsNatTable == NULL) {
+goto failNoMem;
+}
+
+ovsUnNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsUnNatTable == NULL) {
+goto freeNatTable;
+}
+
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+InitializeListHead([i]);
+InitializeListHead([i]);
+}
+return STATUS_SUCCESS;
+
+freeNatTable:
+OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
+failNoMem:
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/*
+ *
+ * OvsNatFlush
+ * Flushes out all NAT entries that match the given zone.
+ *
+ */
+VOID OvsNatFlush(UINT16 zone)
+{
+PLIST_ENTRY link, next;
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+LIST_FORALL_SAFE([i], link, next) {
+POVS_NAT_ENTRY entry =
+CONTAINING_RECORD(link, OVS_NAT_ENTRY, link);
+/* zone is a non-zero value */
+if (!zone || zone == entry-&

[ovs-dev] [PATCH v9 4/4] windows-datapath: Temporary workaround checksum issue with NAT

[Yin Lin]

From: Alin Gabriel Serdean 

There is a known bug with NAT where checksum computation is wrong on
the RX path if offload is enabled. This patch works around the problem
by always computing a software checksum and should be reverted once
we figure out the root cause of checksum error.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 668825b..3ea066b 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1573,6 +1573,44 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
 }
 *portField = newPort;
 }
+PNET_BUFFER_LIST curNbl = ovsFwdCtx->curNbl;
+PNET_BUFFER_LIST newNbl = NULL;
+if (layers->isTcp) {
+UINT32 mss = OVSGetTcpMSS(curNbl);
+if (mss) {
+OVS_LOG_TRACE("l4Offset %d", layers->l4Offset);
+newNbl = OvsTcpSegmentNBL(ovsFwdCtx->switchContext, curNbl, layers,
+  mss, 0, FALSE);
+if (newNbl == NULL) {
+OVS_LOG_ERROR("Unable to segment NBL");
+return NDIS_STATUS_FAILURE;
+}
+/* Clear out LSO flags after this point */
+NET_BUFFER_LIST_INFO(newNbl, TcpLargeSendNetBufferListInfo) = 0;
+}
+}
+/* If we didn't split the packet above, make a copy now */
+if (newNbl == NULL) {
+csumInfo.Value = NET_BUFFER_LIST_INFO(curNbl,
+  TcpIpChecksumNetBufferListInfo);
+OvsApplySWChecksumOnNB(layers, curNbl, );
+}
+
+if (newNbl) {
+curNbl = newNbl;
+OvsCompleteNBLForwardingCtx(ovsFwdCtx,
+L"Complete after cloning NBL for 
encapsulation");
+OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
+ newNbl, ovsFwdCtx->srcVportNo, 0,
+ NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
+ ovsFwdCtx->completionList,
+ >layers, FALSE);
+ovsFwdCtx->curNbl = newNbl;
+}
+
+NET_BUFFER_LIST_INFO(curNbl,
+ TcpIpChecksumNetBufferListInfo) = 0;
+
 return NDIS_STATUS_SUCCESS;
 }
 
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH v8 4/4] windows-datapath: Temporary workaround checksum issue with NAT

[Yin Lin]

From: Alin Gabriel Serdean 

There is a known bug with NAT where checksum computation is wrong on
the RX path if offload is enabled. This patch works around the problem
by always computing a software checksum and should be reverted once
we figure out the root cause of checksum error.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 668825b..3ea066b 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1573,6 +1573,44 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
 }
 *portField = newPort;
 }
+PNET_BUFFER_LIST curNbl = ovsFwdCtx->curNbl;
+PNET_BUFFER_LIST newNbl = NULL;
+if (layers->isTcp) {
+UINT32 mss = OVSGetTcpMSS(curNbl);
+if (mss) {
+OVS_LOG_TRACE("l4Offset %d", layers->l4Offset);
+newNbl = OvsTcpSegmentNBL(ovsFwdCtx->switchContext, curNbl, layers,
+  mss, 0, FALSE);
+if (newNbl == NULL) {
+OVS_LOG_ERROR("Unable to segment NBL");
+return NDIS_STATUS_FAILURE;
+}
+/* Clear out LSO flags after this point */
+NET_BUFFER_LIST_INFO(newNbl, TcpLargeSendNetBufferListInfo) = 0;
+}
+}
+/* If we didn't split the packet above, make a copy now */
+if (newNbl == NULL) {
+csumInfo.Value = NET_BUFFER_LIST_INFO(curNbl,
+  TcpIpChecksumNetBufferListInfo);
+OvsApplySWChecksumOnNB(layers, curNbl, );
+}
+
+if (newNbl) {
+curNbl = newNbl;
+OvsCompleteNBLForwardingCtx(ovsFwdCtx,
+L"Complete after cloning NBL for 
encapsulation");
+OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
+ newNbl, ovsFwdCtx->srcVportNo, 0,
+ NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
+ ovsFwdCtx->completionList,
+ >layers, FALSE);
+ovsFwdCtx->curNbl = newNbl;
+}
+
+NET_BUFFER_LIST_INFO(curNbl,
+ TcpIpChecksumNetBufferListInfo) = 0;
+
 return NDIS_STATUS_SUCCESS;
 }
 
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH v8 2/4] datapath-windows: Add NAT module in conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-nat.c | 433 
 datapath-windows/ovsext/Conntrack-nat.h |  39 +++
 3 files changed, 474 insertions(+)
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 49011f12..36018ea 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,7 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
+   datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
+   datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
new file mode 100644
index 000..ae6b92c
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -0,0 +1,433 @@
+#include "Conntrack-nat.h"
+#include "Jhash.h"
+
+PLIST_ENTRY ovsNatTable = NULL;
+PLIST_ENTRY ovsUnNatTable = NULL;
+
+/*
+ *---
+ * OvsHashNatKey
+ * Hash NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline UINT32
+OvsHashNatKey(const OVS_CT_KEY *key)
+{
+UINT32 hash = 0;
+#define HASH_ADD(field) \
+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
+
+HASH_ADD(src.addr.ipv4_aligned);
+HASH_ADD(dst.addr.ipv4_aligned);
+HASH_ADD(src.port);
+HASH_ADD(dst.port);
+HASH_ADD(zone);
+#undef HASH_ADD
+return hash;
+}
+
+/*
+ *---
+ * OvsNatKeyAreSame
+ * Compare NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline BOOLEAN
+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
+{
+// XXX: Compare IPv6 key as well
+#define FIELD_COMPARE(field) \
+if (key1->field != key2->field) return FALSE
+
+FIELD_COMPARE(src.addr.ipv4_aligned);
+FIELD_COMPARE(dst.addr.ipv4_aligned);
+FIELD_COMPARE(src.port);
+FIELD_COMPARE(dst.port);
+FIELD_COMPARE(zone);
+return TRUE;
+#undef FIELD_COMPARE
+}
+
+/*
+ *---
+ * OvsNatGetBucket
+ * Returns the row of NAT table that has the same hash as the given NAT
+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
+ * instead.
+ *---
+ */
+static __inline PLIST_ENTRY
+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
+{
+uint32_t hash = OvsHashNatKey(key);
+if (isReverse) {
+return [hash & NAT_HASH_TABLE_MASK];
+} else {
+return [hash & NAT_HASH_TABLE_MASK];
+}
+}
+
+/*
+ *---
+ * OvsNatInit
+ * Initialize NAT related resources.
+ *---
+ */
+NTSTATUS OvsNatInit()
+{
+ASSERT(ovsNatTable == NULL);
+
+/* Init the Hash Buffer */
+ovsNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsNatTable == NULL) {
+goto failNoMem;
+}
+
+ovsUnNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsUnNatTable == NULL) {
+goto freeNatTable;
+}
+
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+InitializeListHead([i]);
+InitializeListHead([i]);
+}
+return STATUS_SUCCESS;
+
+freeNatTable:
+OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
+failNoMem:
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/*
+ *
+ * OvsNatFlush
+ * Flushes out all NAT entries that match the given zone.
+ *
+ */
+VOID OvsNatFlush(UINT16 zone)
+{
+PLIST_ENTRY link, next;
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+LIST_FORALL_SAFE([i], link, next) {
+POVS_NAT_ENTRY entry =
+CONTAINING_RECORD(link, OVS_NAT_ENTRY, link);
+/* zone is a non-zero value */
+if (!zone || zone == entry-&

[ovs-dev] [PATCH v8 3/4] datapath-windows: NAT integration with conntrack

[Yin Lin]

This patch integrates NAT module with existing conntrack module. NAT
action is now supported.

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Actions.c  | 119 ++-
 datapath-windows/ovsext/Actions.h  |  20 
 datapath-windows/ovsext/Conntrack.c| 201 +
 datapath-windows/ovsext/Conntrack.h|  25 ++--
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 5 files changed, 282 insertions(+), 85 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index c3f0362..668825b 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1380,7 +1380,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_udp *udpAttr)
 {
@@ -1427,7 +1427,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_tcp *tcpAttr)
 {
@@ -1465,12 +1465,125 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 
 /*
  *
+ * OvsUpdateAddressAndPort --
+ *  Updates the source/destination IP and port fields in
+ *  ovsFwdCtx.curNbl inline based on the specified key.
+ *
+ */
+NDIS_STATUS
+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
+UINT32 newAddr, UINT16 newPort,
+BOOLEAN isSource, BOOLEAN isTx)
+{
+PUINT8 bufferStart;
+UINT32 hdrSize;
+OVS_PACKET_HDR_INFO *layers = >layers;
+IPHdr *ipHdr;
+TCPHdr *tcpHdr = NULL;
+UDPHdr *udpHdr = NULL;
+UINT32 *addrField = NULL;
+UINT16 *portField = NULL;
+UINT16 *checkField = NULL;
+BOOLEAN l4Offload = FALSE;
+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+
+ASSERT(layers->value != 0);
+
+if (layers->isTcp || layers->isUdp) {
+hdrSize = layers->l4Offset +
+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
+} else {
+hdrSize = layers->l3Offset + sizeof (*ipHdr);
+}
+
+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize);
+if (!bufferStart) {
+return NDIS_STATUS_RESOURCES;
+}
+
+ipHdr = (IPHdr *)(bufferStart + layers->l3Offset);
+
+if (layers->isTcp) {
+tcpHdr = (TCPHdr *)(bufferStart + layers->l4Offset);
+} else if (layers->isUdp) {
+udpHdr = (UDPHdr *)(bufferStart + layers->l4Offset);
+}
+
+csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl,
+  TcpIpChecksumNetBufferListInfo);
+/*
+ * Adjust the IP header inline as dictated by the action, and also update
+ * the IP and the TCP checksum for the data modified.
+ *
+ * In the future, this could be optimized to make one call to
+ * ChecksumUpdate32(). Ignoring this for now, since for the most common
+ * case, we only update the TTL.
+ */
+
+if (isSource) {
+addrField = >saddr;
+if (tcpHdr) {
+portField = >source;
+checkField = >check;
+l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.TcpChecksum :
+((BOOLEAN)csumInfo.Receive.TcpChecksumSucceeded ||
+ (BOOLEAN)csumInfo.Receive.TcpChecksumFailed);
+} else if (udpHdr) {
+portField = >source;
+checkField = >check;
+l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.UdpChecksum :
+((BOOLEAN)csumInfo.Receive.UdpChecksumSucceeded ||
+ (BOOLEAN)csumInfo.Receive.UdpChecksumFailed);
+}
+} else {
+addrField = >daddr;
+if (tcpHdr) {
+portField = >dest;
+checkField = >check;
+} else if (udpHdr) {
+portField = >dest;
+checkField = >check;
+}
+}
+if (*addrField != newAddr) {
+UINT32 oldAddr = *addrField;
+if (checkField && *checkField != 0) {
+if (l4Offload) {
+/* Recompute IP pseudo checksum */
+*checkField = ~(*checkField);
+*checkField = ChecksumUpdate32(*checkField, oldAddr,
+   newAddr);
+*checkField = ~(*checkField);
+} else {
+*checkField = ChecksumUpd

[ovs-dev] [PATCH v8 1/4] datapath-windows: Add support for NAT in conntrack

[Yin Lin]

From: Anand Kumar <kumaran...@vmware.com>

Add support for parsing netlink attributes related to NAT
in conntrack.

Co-Authored-by: Anand Kumar <kumaran...@vmware.com>
Co-Authored-by: Darrell Ball <dlu...@gmail.com>
Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 83 -
 datapath-windows/ovsext/Conntrack.h | 17 
 datapath-windows/ovsext/Flow.c  |  4 +-
 3 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 609ae5a..6b3435c 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -651,7 +651,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
   UINT16 zone,
   MD_MARK *mark,
   MD_LABELS *labels,
-  PCHAR helper)
+  PCHAR helper,
+  PNAT_ACTION_INFO natInfo)
 {
 NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_CT_ENTRY entry = NULL;
@@ -660,6 +661,9 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
 UINT64 currentTime;
 NdisGetCurrentSystemTime((LARGE_INTEGER *) );
 
+/* XXX: Not referenced for now */
+UNREFERENCED_PARAMETER(natInfo);
+
 /* Retrieve the Conntrack Key related fields from packet */
 OvsCtSetupLookupCtx(key, zone, , curNbl, layers->l4Offset);
 
@@ -753,11 +757,14 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
 PCHAR helper = NULL;
+NAT_ACTION_INFO natActionInfo;
 PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
 OVS_PACKET_HDR_INFO *layers = >layers;
 PNET_BUFFER_LIST newNbl = NULL;
+NAT_ACTION_INFO natActionInfo;
 NDIS_STATUS status;
 
+memset(, 0, sizeof natActionInfo);
 status = OvsDetectCtPacket(fwdCtx, key, );
 if (status != NDIS_STATUS_SUCCESS) {
 return status;
@@ -780,6 +787,78 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 if (ctAttr) {
 labels = NlAttrGet(ctAttr);
 }
+natActionInfo.natAction = NAT_ACTION_NONE;
+ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_NAT);
+if (ctAttr) {
+/* Pares Nested NAT attributes. */
+PNL_ATTR natAttr;
+unsigned int left;
+BOOLEAN hasMinIp = FALSE;
+BOOLEAN hasMinPort = FALSE;
+BOOLEAN hasMaxIp = FALSE;
+BOOLEAN hasMaxPort = FALSE;
+NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) {
+enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr);
+switch(sub_type_nest) {
+case OVS_NAT_ATTR_SRC:
+case OVS_NAT_ATTR_DST:
+natActionInfo.natAction |=
+((sub_type_nest == OVS_NAT_ATTR_SRC)
+? NAT_ACTION_SRC : NAT_ACTION_DST);
+break;
+case OVS_NAT_ATTR_IP_MIN:
+   if (natAttr->nlaLen < NLA_HDRLEN) {
+OVS_LOG_ERROR("Incorrect header length for "
+  "OVS_NAT_ATTR_IP_MIN message.");
+break;
+}
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMinIp = TRUE;
+break;
+case OVS_NAT_ATTR_IP_MAX:
+if (natAttr->nlaLen < NLA_HDRLEN) {
+OVS_LOG_ERROR("Incorrect header length for "
+  "OVS_NAT_ATTR_IP_MAX message.");
+break;
+}
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMaxIp = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MIN:
+natActionInfo.minPort = NlAttrGetU16(natAttr);
+hasMinPort = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MAX:
+natActionInfo.maxPort = NlAttrGetU16(natAttr);
+hasMaxPort = TRUE;
+break;
+case OVS_NAT_ATTR_PERSISTENT:
+case OVS_NAT_ATTR_PROTO_HASH:
+case OVS_NAT_ATTR_PROTO_RANDOM:
+break;
+}
+}
+if (natActionInfo.natAction == NAT_ACTION_NONE) {
+natActionInfo.natAction = NAT_ACTION_REVERSE;
+}
+if (hasMinIp && !hasMaxIp) {
+memcpy(,
+   ,
+   sizeof(natActionInfo.maxAddr));
+}
+if (hasMinPort && !hasMaxPort) {
+natActionInfo.maxPort = natActionInfo.minPort;
+}
+if (hasMinPort || hasMaxPort) {
+if (natActionInfo.natAction & NAT_ACTION_SRC) {
+natActionInfo.natAction |= NAT_ACTION_SRC_PORT;
+} else if (natActionInfo.natAction & NAT_ACTION_DST) {
+natActionInfo.natA

Re: [ovs-dev] [PATCH v7 3/4] datapath-windows: NAT integration with conntrack

[Yin Lin]

Hi Alin,

Thanks for the feedback. They are very sharp catches! I have address all of 
them except the last one. The last one is a memory leak that has been there for 
a while and not touched by my patch. The fix is not easy either. Take a look at 
the following code:

OVS_CT_ENTRY *
OvsConntrackCreateIcmpEntry(UINT64 now)
{
struct conn_icmp *conn;

conn = OvsAllocateMemoryWithTag(sizeof(struct conn_icmp),
OVS_CT_POOL_TAG);
if (!conn) {
return NULL;
}

conn->state = ICMPS_FIRST;

OvsConntrackUpdateExpiration(>up, now,
 icmp_timeouts[conn->state]);

return >up;
}

We allocate a conn_icmp structure in the heap, but only returns a reference to 
conn->up, so the caller cannot even release the memory.

We'll have to create another patch to address this. Sai, can you take care of 
the memory leak?

Best regards,
Yin Lin

-Original Message-
From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com] 
Sent: Tuesday, May 23, 2017 7:18 AM
To: Yin Lin <li...@vmware.com>; d...@openvswitch.org
Subject: RE: [ovs-dev] [PATCH v7 3/4] datapath-windows: NAT integration with 
conntrack

Hi Yin,

Sorry it took a while to review the patch.

I just have a few inlined comments. I am stripping the code a bit to be more 
readable.

I will run some tests tonight over the current series to see that everything is 
ok from a functionality perspective.

Thanks,
Alin.

> 
> This patch integrates NAT module with existing conntrack module. NAT
> action is now supported.
> 
> Signed-off-by: Yin Lin <li...@vmware.com>
> diff --git a/datapath-windows/automake.mk b/datapath-
> windows/automake.mk index 7177630..f1632cc 100644
> --- a/datapath-windows/automake.mk
> +++ b/datapath-windows/automake.mk
> @@ -16,9 +16,9 @@ EXTRA_DIST += \
>   datapath-windows/ovsext/Conntrack-icmp.c \
>   datapath-windows/ovsext/Conntrack-other.c \
>   datapath-windows/ovsext/Conntrack-related.c \
> -datapath-windows/ovsext/Conntrack-nat.c \
> + datapath-windows/ovsext/Conntrack-nat.c \
>   datapath-windows/ovsext/Conntrack-tcp.c \
> -datapath-windows/ovsext/Conntrack-nat.h \
> + datapath-windows/ovsext/Conntrack-nat.h \
[Alin Serdean] Just use tab instead of 4 spaces in patch 2/4
>   datapath-windows/ovsext/Conntrack.c \
>   datapath-windows/ovsext/Conntrack.h \
>   datapath-windows/ovsext/Datapath.c \
[Alin Serdean] <== cut >
> +
> +/* NatInfo is always initialized to be disabled, so that if NAT action
> + * fails, we will not end up deleting an non-existent NAT entry.
> + */
> +if (natInfo != NULL && OvsIsForwardNat(natInfo->natAction)) {
> +entry->natInfo = *natInfo;
> +if (!OvsNatCtEntry(entry)) {
> +return FALSE;
> +}
> +ctx->hash = OvsHashCtKey(>key);
> +} else {
> +entry->natInfo.natAction = natInfo->natAction;
[Alin Serdean] Shouldn't this be guarded for natInfo==NULL?
> +}
> +
[Alin Serdean] <== cut >
> -OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
> -return entry;
> +break;
>  }
>  default:
>  goto invalid;
>  }
> 
> +if (commit && !entry) {
> +return NULL;
> +}
> +if (entry && !OvsCtAddEntry(entry, ctx, natInfo, currentTime)) {
[Alin Serdean] Shouldn't we delete the 'entry' here if OvsCtAddEntry failed?
> +return NULL;
> +}
> +OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
> +return entry;
>  invalid:
>  state |= OVS_CS_F_INVALID;
>  OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL); @@ -269,11
> +289,11 @@ invalid:
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 3/4] datapath-windows: NAT integration with conntrack

[Yin Lin]

See inline comments for reply.

-Original Message-
From: Sairam Venugopal 
Sent: Tuesday, May 16, 2017 5:09 PM
To: Yin Lin <li...@vmware.com>; d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v7 3/4] datapath-windows: NAT integration with 
conntrack

Please find my comments inline.

Thanks,
Sairam




On 5/9/17, 3:59 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yin Lin" 
<ovs-dev-boun...@openvswitch.org on behalf of li...@vmware.com> wrote:

>This patch integrates NAT module with existing conntrack module. NAT
>action is now supported.
>
>Signed-off-by: Yin Lin <li...@vmware.com>
>---
> datapath-windows/automake.mk   |   4 +-
> datapath-windows/ovsext/Actions.c  | 119 -
> datapath-windows/ovsext/Actions.h  |  20 
> datapath-windows/ovsext/Conntrack.c| 187 +
> datapath-windows/ovsext/Conntrack.h|  25 +++--
> datapath-windows/ovsext/ovsext.vcxproj |   2 +
> 6 files changed, 274 insertions(+), 83 deletions(-)
>
>diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
>index 7177630..f1632cc 100644
>--- a/datapath-windows/automake.mk
>+++ b/datapath-windows/automake.mk
>@@ -16,9 +16,9 @@ EXTRA_DIST += \
>   datapath-windows/ovsext/Conntrack-icmp.c \
>   datapath-windows/ovsext/Conntrack-other.c \
>   datapath-windows/ovsext/Conntrack-related.c \
>-datapath-windows/ovsext/Conntrack-nat.c \
>+  datapath-windows/ovsext/Conntrack-nat.c \
>   datapath-windows/ovsext/Conntrack-tcp.c \
>-datapath-windows/ovsext/Conntrack-nat.h \
>+  datapath-windows/ovsext/Conntrack-nat.h \
>   datapath-windows/ovsext/Conntrack.c \
>   datapath-windows/ovsext/Conntrack.h \
>   datapath-windows/ovsext/Datapath.c \
>diff --git a/datapath-windows/ovsext/Actions.c 
>b/datapath-windows/ovsext/Actions.c
>index e2eae9a..a808d2c 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -1380,7 +1380,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext 
>*ovsFwdCtx,
>  *  based on the specified key.
>  *
>  */
>-static __inline NDIS_STATUS
>+NDIS_STATUS
> OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
>   const struct ovs_key_udp *udpAttr)
> {
>@@ -1427,7 +1427,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
>  *  based on the specified key.
>  *
>  */
>-static __inline NDIS_STATUS
>+NDIS_STATUS
> OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
>   const struct ovs_key_tcp *tcpAttr)
> {
>@@ -1465,12 +1465,125 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
> 
> /*
>  *
>+ * OvsUpdateAddressAndPort --
>+ *  Updates the source/destination IP and port fields in
>+ *  ovsFwdCtx.curNbl inline based on the specified key.
>+ *
>+ */
>+NDIS_STATUS
>+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
>+UINT32 newAddr, UINT16 newPort,
>+BOOLEAN isSource, BOOLEAN isTx)
>+{
>+PUINT8 bufferStart;
>+UINT32 hdrSize;
>+OVS_PACKET_HDR_INFO *layers = >layers;
>+IPHdr *ipHdr;
>+TCPHdr *tcpHdr = NULL;
>+UDPHdr *udpHdr = NULL;
>+UINT32 *addrField = NULL;
>+UINT16 *portField = NULL;
>+UINT16 *checkField = NULL;
>+BOOLEAN l4Offload = FALSE;
>+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
>+
>+ASSERT(layers->value != 0);
>+
>+if (layers->isTcp || layers->isUdp) {
>+hdrSize = layers->l4Offset +
>+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
>+} else {
>+hdrSize = layers->l3Offset + sizeof (*ipHdr);
>+}
>+
>+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize);
>+if (!bufferStart) {
>+return NDIS_STATUS_RESOURCES;
>+}
>+
>+ipHdr = (IPHdr *)(bufferStart + layers->l3Offset);
>+
>+if (layers->isTcp) {
>+tcpHdr = (TCPHdr *)(bufferStart + layers->l4Offset);
>+} else if (layers->isUdp) {
>+udpHdr = (UDPHdr *)(bufferStart + layers->l4Offset);
>+}
>+
>+csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl,
>+  TcpIpChecksumNetBufferListInfo);
>+/*
>+ * Adjust the IP header inline as dictated by the action, and also update
>+ * the IP and the TCP checksum for the data modified

Re: [ovs-dev] [PATCH v7 2/4] datapath-windows: Add NAT module in conntrack

[Yin Lin]

Thanks Sai for the review! I fixed most of them and explained the remaining 
ones in the inline comments.

-Original Message-
From: Sairam Venugopal 
Sent: Tuesday, May 16, 2017 4:50 PM
To: Yin Lin <li...@vmware.com>; d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v7 2/4] datapath-windows: Add NAT module in 
conntrack

Hi Yin,

Thanks for the patch. Please find my comments inline.

Thanks,
Sairam




On 5/9/17, 3:59 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yin Lin" 
<ovs-dev-boun...@openvswitch.org on behalf of li...@vmware.com> wrote:

>Signed-off-by: Yin Lin <li...@vmware.com>
>---
> datapath-windows/automake.mk|   2 +
> datapath-windows/ovsext/Conntrack-nat.c | 424 
> datapath-windows/ovsext/Conntrack-nat.h |  39 +++
> 3 files changed, 465 insertions(+)
> create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
> create mode 100644 datapath-windows/ovsext/Conntrack-nat.h
>
>diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
>index 4f7b55a..7177630 100644
>--- a/datapath-windows/automake.mk
>+++ b/datapath-windows/automake.mk
>@@ -16,7 +16,9 @@ EXTRA_DIST += \
>   datapath-windows/ovsext/Conntrack-icmp.c \
>   datapath-windows/ovsext/Conntrack-other.c \
>   datapath-windows/ovsext/Conntrack-related.c \
>+datapath-windows/ovsext/Conntrack-nat.c \
>   datapath-windows/ovsext/Conntrack-tcp.c \
>+datapath-windows/ovsext/Conntrack-nat.h \
>   datapath-windows/ovsext/Conntrack.c \
>   datapath-windows/ovsext/Conntrack.h \
>   datapath-windows/ovsext/Datapath.c \
>diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
>b/datapath-windows/ovsext/Conntrack-nat.c
>new file mode 100644
>index 000..edf5f8f
>--- /dev/null
>+++ b/datapath-windows/ovsext/Conntrack-nat.c
>@@ -0,0 +1,424 @@
>+#include "Conntrack-nat.h"
>+#include "Jhash.h"
>+
>+PLIST_ENTRY ovsNatTable = NULL;
>+PLIST_ENTRY ovsUnNatTable = NULL;
>+
>+/*
>+ *---
>+ * OvsHashNatKey
>+ * Hash NAT related fields in a Conntrack key.
>+ *---
>+ */
>+static __inline UINT32
>+OvsHashNatKey(const OVS_CT_KEY *key)
>+{
>+UINT32 hash = 0;
>+#define HASH_ADD(field) \
>+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
>+
>+HASH_ADD(src.addr.ipv4_aligned);
>+HASH_ADD(dst.addr.ipv4_aligned);
>+HASH_ADD(src.port);
>+HASH_ADD(dst.port);
>+HASH_ADD(zone);
>+#undef HASH_ADD
>+return hash;
>+}
>+
>+/*
>+ *---
>+ * OvsNatKeyAreSame
>+ * Compare NAT related fields in a Conntrack key.
>+ *---
>+ */
>+static __inline BOOLEAN
>+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
>+{
>+// XXX: Compare IPv6 key as well
>+#define FIELD_COMPARE(field) \

[Sai]: Nit: move return statement to next line. 
[Yin]: This is a MACRO and I don't want to spread it over lines. (Note that I 
don't even add a semicolon at the end to make it look like a function)

>+if (key1->field != key2->field) return FALSE
>+
>+FIELD_COMPARE(src.addr.ipv4_aligned);
>+FIELD_COMPARE(dst.addr.ipv4_aligned);
>+FIELD_COMPARE(src.port);
>+FIELD_COMPARE(dst.port);
>+FIELD_COMPARE(zone);
>+return TRUE;
>+#undef FIELD_COMPARE
>+}
>+
>+/*
>+ *---

[Sai]: Nit - OvsNatGetBucket
[Yin]: Fixed

>+ * OvsNaGetBucket
>+ * Returns the row of NAT table that has the same hash as the given NAT
>+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
>+ * instead.
>+ *---
>+ */
>+static __inline PLIST_ENTRY
>+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
>+{
>+uint32_t hash = OvsHashNatKey(key);
>+if (isReverse) {
>+return [hash & NAT_HASH_TABLE_MASK];
>+} else {
>+return [hash & NAT_HASH_TABLE_MASK];
>+}
>+}
>+
>+/*
>+ *---
>+ * OvsNatInit
>+ * Initialize NAT related resources.
>+ *---
>+ */
>+NTSTATUS OvsNatInit()
>+{
>+ASSERT(ovsNatTable == NULL);
>+
>+/* Init the Hash Buffer */
>+ovsNatTable = OvsAllocateMemoryWithTag(
>+sizeof(LIST_ENTRY) * NAT

[ovs-dev] [PATCH v7 4/4] windows-datapath: Temporary workaround checksum issue with NAT

[Yin Lin]

From: Alin Gabriel Serdean 

There is a known bug with NAT where checksum computation is wrong on
the RX path if offload is enabled. This patch works around the problem
by always computing a software checksum and should be reverted once
we figure out the root cause of checksum error.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index a808d2c..141fcb1 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1573,6 +1573,44 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
 }
 *portField = newPort;
 }
+PNET_BUFFER_LIST curNbl = ovsFwdCtx->curNbl;
+PNET_BUFFER_LIST newNbl = NULL;
+if (layers->isTcp) {
+UINT32 mss = OVSGetTcpMSS(curNbl);
+if (mss) {
+OVS_LOG_TRACE("l4Offset %d", layers->l4Offset);
+newNbl = OvsTcpSegmentNBL(ovsFwdCtx->switchContext, curNbl, layers,
+  mss, 0, FALSE);
+if (newNbl == NULL) {
+OVS_LOG_ERROR("Unable to segment NBL");
+return NDIS_STATUS_FAILURE;
+}
+/* Clear out LSO flags after this point */
+NET_BUFFER_LIST_INFO(newNbl, TcpLargeSendNetBufferListInfo) = 0;
+}
+}
+/* If we didn't split the packet above, make a copy now */
+if (newNbl == NULL) {
+csumInfo.Value = NET_BUFFER_LIST_INFO(curNbl,
+  TcpIpChecksumNetBufferListInfo);
+OvsApplySWChecksumOnNB(layers, curNbl, );
+}
+
+if (newNbl) {
+curNbl = newNbl;
+OvsCompleteNBLForwardingCtx(ovsFwdCtx,
+L"Complete after cloning NBL for 
encapsulation");
+OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
+ newNbl, ovsFwdCtx->srcVportNo, 0,
+ NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
+ ovsFwdCtx->completionList,
+ >layers, FALSE);
+ovsFwdCtx->curNbl = newNbl;
+}
+
+NET_BUFFER_LIST_INFO(curNbl,
+ TcpIpChecksumNetBufferListInfo) = 0;
+
 return NDIS_STATUS_SUCCESS;
 }
 
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH v7 2/4] datapath-windows: Add NAT module in conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-nat.c | 424 
 datapath-windows/ovsext/Conntrack-nat.h |  39 +++
 3 files changed, 465 insertions(+)
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 4f7b55a..7177630 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,7 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
+datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
+datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
new file mode 100644
index 000..edf5f8f
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -0,0 +1,424 @@
+#include "Conntrack-nat.h"
+#include "Jhash.h"
+
+PLIST_ENTRY ovsNatTable = NULL;
+PLIST_ENTRY ovsUnNatTable = NULL;
+
+/*
+ *---
+ * OvsHashNatKey
+ * Hash NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline UINT32
+OvsHashNatKey(const OVS_CT_KEY *key)
+{
+UINT32 hash = 0;
+#define HASH_ADD(field) \
+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
+
+HASH_ADD(src.addr.ipv4_aligned);
+HASH_ADD(dst.addr.ipv4_aligned);
+HASH_ADD(src.port);
+HASH_ADD(dst.port);
+HASH_ADD(zone);
+#undef HASH_ADD
+return hash;
+}
+
+/*
+ *---
+ * OvsNatKeyAreSame
+ * Compare NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline BOOLEAN
+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
+{
+// XXX: Compare IPv6 key as well
+#define FIELD_COMPARE(field) \
+if (key1->field != key2->field) return FALSE
+
+FIELD_COMPARE(src.addr.ipv4_aligned);
+FIELD_COMPARE(dst.addr.ipv4_aligned);
+FIELD_COMPARE(src.port);
+FIELD_COMPARE(dst.port);
+FIELD_COMPARE(zone);
+return TRUE;
+#undef FIELD_COMPARE
+}
+
+/*
+ *---
+ * OvsNaGetBucket
+ * Returns the row of NAT table that has the same hash as the given NAT
+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
+ * instead.
+ *---
+ */
+static __inline PLIST_ENTRY
+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
+{
+uint32_t hash = OvsHashNatKey(key);
+if (isReverse) {
+return [hash & NAT_HASH_TABLE_MASK];
+} else {
+return [hash & NAT_HASH_TABLE_MASK];
+}
+}
+
+/*
+ *---
+ * OvsNatInit
+ * Initialize NAT related resources.
+ *---
+ */
+NTSTATUS OvsNatInit()
+{
+ASSERT(ovsNatTable == NULL);
+
+/* Init the Hash Buffer */
+ovsNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsNatTable == NULL) {
+goto failNoMem;
+}
+
+ovsUnNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsUnNatTable == NULL) {
+goto freeNatTable;
+}
+
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+InitializeListHead([i]);
+InitializeListHead([i]);
+}
+return STATUS_SUCCESS;
+
+freeNatTable:
+OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
+failNoMem:
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/*
+ *
+ * OvsNatFlush
+ * Flushes out all NAT entries that match the given zone.
+ *
+ */
+VOID OvsNatFlush(UINT16 zone)
+{
+PLIST_ENTRY link, next;
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+LIST_FORALL_SAFE([i], link, next) {
+POVS_NAT_ENTRY entry =
+CONTAINING_RECORD(link, OVS_NAT_ENTRY, link);
+/* zone is a non-zero value */
+if (!zone || zone == entry-&

[ovs-dev] [PATCH v7 1/4] datapath-windows: Add support for NAT in conntrack

[Yin Lin]

From: Anand Kumar <kumaran...@vmware.com>

Add support for parsing netlink attributes related to NAT
in conntrack.

Co-Authored-by: Anand Kumar <kumaran...@vmware.com>
Co-Authored-by: Darrell Ball <dlu...@gmail.com>
Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 73 -
 datapath-windows/ovsext/Conntrack.h | 17 +
 datapath-windows/ovsext/Flow.c  |  4 +-
 3 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index dce0c1b..9824368 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -645,7 +645,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
   UINT16 zone,
   MD_MARK *mark,
   MD_LABELS *labels,
-  PCHAR helper)
+  PCHAR helper,
+  PNAT_ACTION_INFO natInfo)
 {
 NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_CT_ENTRY entry = NULL;
@@ -654,6 +655,9 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
 UINT64 currentTime;
 NdisGetCurrentSystemTime((LARGE_INTEGER *) );
 
+/* XXX: Not referenced for now */
+UNREFERENCED_PARAMETER(natInfo);
+
 /* Retrieve the Conntrack Key related fields from packet */
 OvsCtSetupLookupCtx(key, zone, , curNbl, layers->l4Offset);
 
@@ -730,11 +734,14 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
 PCHAR helper = NULL;
+NAT_ACTION_INFO natActionInfo;
 PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
 OVS_PACKET_HDR_INFO *layers = >layers;
 PNET_BUFFER_LIST newNbl = NULL;
+NAT_ACTION_INFO natActionInfo;
 NDIS_STATUS status;
 
+memset(, 0, sizeof natActionInfo);
 status = OvsDetectCtPacket(fwdCtx, key, );
 if (status != NDIS_STATUS_SUCCESS) {
 return status;
@@ -757,6 +764,68 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 if (ctAttr) {
 labels = NlAttrGet(ctAttr);
 }
+natActionInfo.natAction = NAT_ACTION_NONE;
+ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_NAT);
+if (ctAttr) {
+/* Pares Nested NAT attributes. */
+PNL_ATTR natAttr;
+unsigned int left;
+BOOLEAN hasMinIp = FALSE;
+BOOLEAN hasMinPort = FALSE;
+BOOLEAN hasMaxIp = FALSE;
+BOOLEAN hasMaxPort = FALSE;
+NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) {
+enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr);
+switch(sub_type_nest) {
+case OVS_NAT_ATTR_SRC:
+case OVS_NAT_ATTR_DST:
+natActionInfo.natAction |=
+((sub_type_nest == OVS_NAT_ATTR_SRC)
+? NAT_ACTION_SRC : NAT_ACTION_DST);
+break;
+case OVS_NAT_ATTR_IP_MIN:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMinIp = TRUE;
+break;
+case OVS_NAT_ATTR_IP_MAX:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMaxIp = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MIN:
+natActionInfo.minPort = NlAttrGetU16(natAttr);
+hasMinPort = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MAX:
+natActionInfo.maxPort = NlAttrGetU16(natAttr);
+hasMaxPort = TRUE;
+break;
+case OVS_NAT_ATTR_PERSISTENT:
+case OVS_NAT_ATTR_PROTO_HASH:
+case OVS_NAT_ATTR_PROTO_RANDOM:
+break;
+}
+}
+if (natActionInfo.natAction == NAT_ACTION_NONE) {
+natActionInfo.natAction = NAT_ACTION_REVERSE;
+}
+if (hasMinIp && !hasMaxIp) {
+memcpy(,
+   ,
+   sizeof(natActionInfo.maxAddr));
+}
+if (hasMinPort && !hasMaxPort) {
+natActionInfo.maxPort = natActionInfo.minPort;
+}
+if (hasMinPort || hasMaxPort) {
+if (natActionInfo.natAction & NAT_ACTION_SRC) {
+natActionInfo.natAction |= NAT_ACTION_SRC_PORT;
+} else if (natActionInfo.natAction & NAT_ACTION_DST) {
+natActionInfo.natAction |= NAT_ACTION_DST_PORT;
+}
+}
+}
 ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_HELPER);
 if (ctAttr) {
 helper = NlAttrGetString(ctAttr);
@@ -776,7 +845,7 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 }
 /* If newNbl is not allocated, use the current Nbl*/
 status = OvsCtExecute_(newNbl != NULL ? newNbl : curNbl, key, layers,
-   commit, force, zone, mark, labels, helper);
+   commit, force, zone

[ovs-dev] [PATCH v7 3/4] datapath-windows: NAT integration with conntrack

[Yin Lin]

This patch integrates NAT module with existing conntrack module. NAT
action is now supported.

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/automake.mk   |   4 +-
 datapath-windows/ovsext/Actions.c  | 119 -
 datapath-windows/ovsext/Actions.h  |  20 
 datapath-windows/ovsext/Conntrack.c| 187 +
 datapath-windows/ovsext/Conntrack.h|  25 +++--
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 6 files changed, 274 insertions(+), 83 deletions(-)

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 7177630..f1632cc 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,9 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
-datapath-windows/ovsext/Conntrack-nat.c \
+   datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
-datapath-windows/ovsext/Conntrack-nat.h \
+   datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index e2eae9a..a808d2c 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1380,7 +1380,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_udp *udpAttr)
 {
@@ -1427,7 +1427,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_tcp *tcpAttr)
 {
@@ -1465,12 +1465,125 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 
 /*
  *
+ * OvsUpdateAddressAndPort --
+ *  Updates the source/destination IP and port fields in
+ *  ovsFwdCtx.curNbl inline based on the specified key.
+ *
+ */
+NDIS_STATUS
+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
+UINT32 newAddr, UINT16 newPort,
+BOOLEAN isSource, BOOLEAN isTx)
+{
+PUINT8 bufferStart;
+UINT32 hdrSize;
+OVS_PACKET_HDR_INFO *layers = >layers;
+IPHdr *ipHdr;
+TCPHdr *tcpHdr = NULL;
+UDPHdr *udpHdr = NULL;
+UINT32 *addrField = NULL;
+UINT16 *portField = NULL;
+UINT16 *checkField = NULL;
+BOOLEAN l4Offload = FALSE;
+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+
+ASSERT(layers->value != 0);
+
+if (layers->isTcp || layers->isUdp) {
+hdrSize = layers->l4Offset +
+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
+} else {
+hdrSize = layers->l3Offset + sizeof (*ipHdr);
+}
+
+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize);
+if (!bufferStart) {
+return NDIS_STATUS_RESOURCES;
+}
+
+ipHdr = (IPHdr *)(bufferStart + layers->l3Offset);
+
+if (layers->isTcp) {
+tcpHdr = (TCPHdr *)(bufferStart + layers->l4Offset);
+} else if (layers->isUdp) {
+udpHdr = (UDPHdr *)(bufferStart + layers->l4Offset);
+}
+
+csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl,
+  TcpIpChecksumNetBufferListInfo);
+/*
+ * Adjust the IP header inline as dictated by the action, and also update
+ * the IP and the TCP checksum for the data modified.
+ *
+ * In the future, this could be optimized to make one call to
+ * ChecksumUpdate32(). Ignoring this for now, since for the most common
+ * case, we only update the TTL.
+ */
+
+if (isSource) {
+addrField = >saddr;
+if (tcpHdr) {
+portField = >source;
+checkField = >check;
+l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.TcpChecksum :
+((BOOLEAN)csumInfo.Receive.TcpChecksumSucceeded ||
+ (BOOLEAN)csumInfo.Receive.TcpChecksumFailed);
+} else if (udpHdr) {
+portField = >source;
+checkField = >check;
+l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.UdpChecksum :
+((BOOLEAN)csumInfo.Receive.UdpChecksumSucceeded ||
+ (BOOLEAN)csumInfo.Receive.UdpChecksumF

Re: [ovs-dev] [PATCH v6 3/4] datapath-windows: NAT integration with conntrack

[Yin Lin]

Hi Sai,

Thanks for the feedback! When I removed "static __inline", I meant to make the 
function public. It's not a matter of coding standard or style, but a matter of 
feature. Please note the changes I made to Actions.h as well. The functions I 
made public are utility functions. It's not justifiable why 
OvsUpdateAddressAndPort should be public but OvsUpdateUdpPorts should be 
private, for example.

Please check the inline comments for the reply to your other concerns.

Best regards,
Yin Lin



-Original Message-
From: Sairam Venugopal 
Sent: Monday, May 8, 2017 5:07 PM
To: Yin Lin <li...@vmware.com>; d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v6 3/4] datapath-windows: NAT integration with 
conntrack

Hi Yin,

Thanks for the patches. I had some immediate review comments. 

- OvsUpdateAddressAndPort does not have a return type
- Refrain from changing other code that your patch does not require. Please 
send out separate incremental patch.
- This is the correct way to define static inline functions - "static __inline 
NDIS_STATUS”. Don’t change this.
Update your code to follow this instead.

Please find the other comments inline.

Thanks,
Sairam



On 5/8/17, 3:38 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yin Lin" 
<ovs-dev-boun...@openvswitch.org on behalf of li...@vmware.com> wrote:

>Signed-off-by: Yin Lin <li...@vmware.com>
>---
> datapath-windows/automake.mk   |   4 +-
> datapath-windows/ovsext/Actions.c  | 118 -
> datapath-windows/ovsext/Actions.h  |  20 
> datapath-windows/ovsext/Conntrack.c| 187 +
> datapath-windows/ovsext/Conntrack.h|  25 +++--
> datapath-windows/ovsext/ovsext.vcxproj |   2 +
> 6 files changed, 273 insertions(+), 83 deletions(-)
>
>diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
>index 7177630..f1632cc 100644
>--- a/datapath-windows/automake.mk
>+++ b/datapath-windows/automake.mk
>@@ -16,9 +16,9 @@ EXTRA_DIST += \
>   datapath-windows/ovsext/Conntrack-icmp.c \
>   datapath-windows/ovsext/Conntrack-other.c \
>   datapath-windows/ovsext/Conntrack-related.c \
>-datapath-windows/ovsext/Conntrack-nat.c \
>+  datapath-windows/ovsext/Conntrack-nat.c \
>   datapath-windows/ovsext/Conntrack-tcp.c \
>-datapath-windows/ovsext/Conntrack-nat.h \
>+  datapath-windows/ovsext/Conntrack-nat.h \
>   datapath-windows/ovsext/Conntrack.c \
>   datapath-windows/ovsext/Conntrack.h \
>   datapath-windows/ovsext/Datapath.c \
>diff --git a/datapath-windows/ovsext/Actions.c 
>b/datapath-windows/ovsext/Actions.c
>index e2eae9a..d1938f3 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -1380,7 +1380,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext 
>*ovsFwdCtx,
>  *  based on the specified key.
>  *
>  */

[Sai]: Please don’t change this. This is the correct - "static __inline 
NDIS_STATUS"
>-static __inline NDIS_STATUS
>+NDIS_STATUS
> OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
>   const struct ovs_key_udp *udpAttr)
> {
>@@ -1427,7 +1427,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
>  *  based on the specified key.
>  *
>  */
>-static __inline NDIS_STATUS
>+NDIS_STATUS

[Sai]: Please don’t change this. This is the correct - "static __inline 
NDIS_STATUS"


> OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
>   const struct ovs_key_tcp *tcpAttr)
> {
>@@ -1465,12 +1465,124 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
> 
> /*
>  *
>+ * OvsUpdateAddressAndPort --
>+ *  Updates the source/destination IP and port fields in
>+ *  ovsFwdCtx.curNbl inline based on the specified key.
>+ *
>+ */

[Sai]: Missing return type
[Yin]: Good catch. Fixed.
>+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
>+UINT32 newAddr, UINT16 newPort,
>+BOOLEAN isSource, BOOLEAN isTx)
>+{
>+PUINT8 bufferStart;
>+UINT32 hdrSize;
>+OVS_PACKET_HDR_INFO *layers = >layers;
>+IPHdr *ipHdr;
>+TCPHdr *tcpHdr = NULL;
>+UDPHdr *udpHdr = NULL;
>+UINT32 *addrField = NULL;
>+UINT16 *portField = NULL;
>+UINT16 *checkField = NULL;
>+BOOLEAN l4Offload = FALSE;
>+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
>+
>+ASSERT(layers->value != 0);
&

[ovs-dev] [PATCH v6 4/4] Signed-off-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com>

[Yin Lin]

From: Alin Gabriel Serdean 

---
 datapath-windows/ovsext/Actions.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index d1938f3..bb1e6ea 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1572,6 +1572,44 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
 }
 *portField = newPort;
 }
+PNET_BUFFER_LIST curNbl = ovsFwdCtx->curNbl;
+PNET_BUFFER_LIST newNbl = NULL;
+if (layers->isTcp) {
+UINT32 mss = OVSGetTcpMSS(curNbl);
+if (mss) {
+OVS_LOG_TRACE("l4Offset %d", layers->l4Offset);
+newNbl = OvsTcpSegmentNBL(ovsFwdCtx->switchContext, curNbl, layers,
+  mss, 0);
+if (newNbl == NULL) {
+OVS_LOG_ERROR("Unable to segment NBL");
+return NDIS_STATUS_FAILURE;
+}
+/* Clear out LSO flags after this point */
+NET_BUFFER_LIST_INFO(newNbl, TcpLargeSendNetBufferListInfo) = 0;
+}
+}
+/* If we didn't split the packet above, make a copy now */
+if (newNbl == NULL) {
+csumInfo.Value = NET_BUFFER_LIST_INFO(curNbl,
+  TcpIpChecksumNetBufferListInfo);
+OvsApplySWChecksumOnNB(layers, curNbl, );
+}
+
+if (newNbl) {
+curNbl = newNbl;
+OvsCompleteNBLForwardingCtx(ovsFwdCtx,
+L"Complete after cloning NBL for 
encapsulation");
+OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
+ newNbl, ovsFwdCtx->srcVportNo, 0,
+ NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
+ ovsFwdCtx->completionList,
+ >layers, FALSE);
+ovsFwdCtx->curNbl = newNbl;
+}
+
+NET_BUFFER_LIST_INFO(curNbl,
+ TcpIpChecksumNetBufferListInfo) = 0;
+
 return NDIS_STATUS_SUCCESS;
 }
 
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH v6 3/4] datapath-windows: NAT integration with conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/automake.mk   |   4 +-
 datapath-windows/ovsext/Actions.c  | 118 -
 datapath-windows/ovsext/Actions.h  |  20 
 datapath-windows/ovsext/Conntrack.c| 187 +
 datapath-windows/ovsext/Conntrack.h|  25 +++--
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 6 files changed, 273 insertions(+), 83 deletions(-)

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 7177630..f1632cc 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,9 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
-datapath-windows/ovsext/Conntrack-nat.c \
+   datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
-datapath-windows/ovsext/Conntrack-nat.h \
+   datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index e2eae9a..d1938f3 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1380,7 +1380,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_udp *udpAttr)
 {
@@ -1427,7 +1427,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_tcp *tcpAttr)
 {
@@ -1465,12 +1465,124 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 
 /*
  *
+ * OvsUpdateAddressAndPort --
+ *  Updates the source/destination IP and port fields in
+ *  ovsFwdCtx.curNbl inline based on the specified key.
+ *
+ */
+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
+UINT32 newAddr, UINT16 newPort,
+BOOLEAN isSource, BOOLEAN isTx)
+{
+PUINT8 bufferStart;
+UINT32 hdrSize;
+OVS_PACKET_HDR_INFO *layers = >layers;
+IPHdr *ipHdr;
+TCPHdr *tcpHdr = NULL;
+UDPHdr *udpHdr = NULL;
+UINT32 *addrField = NULL;
+UINT16 *portField = NULL;
+UINT16 *checkField = NULL;
+BOOLEAN l4Offload = FALSE;
+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+
+ASSERT(layers->value != 0);
+
+if (layers->isTcp || layers->isUdp) {
+hdrSize = layers->l4Offset +
+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
+} else {
+hdrSize = layers->l3Offset + sizeof (*ipHdr);
+}
+
+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize);
+if (!bufferStart) {
+return NDIS_STATUS_RESOURCES;
+}
+
+ipHdr = (IPHdr *)(bufferStart + layers->l3Offset);
+
+if (layers->isTcp) {
+tcpHdr = (TCPHdr *)(bufferStart + layers->l4Offset);
+} else if (layers->isUdp) {
+udpHdr = (UDPHdr *)(bufferStart + layers->l4Offset);
+}
+
+csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl,
+  TcpIpChecksumNetBufferListInfo);
+/*
+ * Adjust the IP header inline as dictated by the action, and also update
+ * the IP and the TCP checksum for the data modified.
+ *
+ * In the future, this could be optimized to make one call to
+ * ChecksumUpdate32(). Ignoring this for now, since for the most common
+ * case, we only update the TTL.
+ */
+
+if (isSource) {
+addrField = >saddr;
+if (tcpHdr) {
+portField = >source;
+checkField = >check;
+l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.TcpChecksum :
+((BOOLEAN)csumInfo.Receive.TcpChecksumSucceeded ||
+ (BOOLEAN)csumInfo.Receive.TcpChecksumFailed);
+} else if (udpHdr) {
+portField = >source;
+checkField = >check;
+l4Offload = isTx ? (BOOLEAN)csumInfo.Transmit.UdpChecksum :
+((BOOLEAN)csumInfo.Receive.UdpChecksumSucceeded ||
+ (BOOLEAN)csumInfo.Receive.UdpChecksumFailed);
+}
+} else {
+addrField = >daddr;
+if (tcpHdr) {
+portField = >

[ovs-dev] [PATCH v6 2/4] datapath-windows: Add NAT module in conntrack

[Yin Lin]

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

Issue: #
Change-Id: I6f37360c36525548b343f0016304015fec8aba7d
---
 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-nat.c | 424 
 datapath-windows/ovsext/Conntrack-nat.h |  39 +++
 3 files changed, 465 insertions(+)
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 4f7b55a..7177630 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,7 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
+datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
+datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
new file mode 100644
index 000..edf5f8f
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -0,0 +1,424 @@
+#include "Conntrack-nat.h"
+#include "Jhash.h"
+
+PLIST_ENTRY ovsNatTable = NULL;
+PLIST_ENTRY ovsUnNatTable = NULL;
+
+/*
+ *---
+ * OvsHashNatKey
+ * Hash NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline UINT32
+OvsHashNatKey(const OVS_CT_KEY *key)
+{
+UINT32 hash = 0;
+#define HASH_ADD(field) \
+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
+
+HASH_ADD(src.addr.ipv4_aligned);
+HASH_ADD(dst.addr.ipv4_aligned);
+HASH_ADD(src.port);
+HASH_ADD(dst.port);
+HASH_ADD(zone);
+#undef HASH_ADD
+return hash;
+}
+
+/*
+ *---
+ * OvsNatKeyAreSame
+ * Compare NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline BOOLEAN
+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
+{
+// XXX: Compare IPv6 key as well
+#define FIELD_COMPARE(field) \
+if (key1->field != key2->field) return FALSE
+
+FIELD_COMPARE(src.addr.ipv4_aligned);
+FIELD_COMPARE(dst.addr.ipv4_aligned);
+FIELD_COMPARE(src.port);
+FIELD_COMPARE(dst.port);
+FIELD_COMPARE(zone);
+return TRUE;
+#undef FIELD_COMPARE
+}
+
+/*
+ *---
+ * OvsNaGetBucket
+ * Returns the row of NAT table that has the same hash as the given NAT
+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
+ * instead.
+ *---
+ */
+static __inline PLIST_ENTRY
+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
+{
+uint32_t hash = OvsHashNatKey(key);
+if (isReverse) {
+return [hash & NAT_HASH_TABLE_MASK];
+} else {
+return [hash & NAT_HASH_TABLE_MASK];
+}
+}
+
+/*
+ *---
+ * OvsNatInit
+ * Initialize NAT related resources.
+ *---
+ */
+NTSTATUS OvsNatInit()
+{
+ASSERT(ovsNatTable == NULL);
+
+/* Init the Hash Buffer */
+ovsNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsNatTable == NULL) {
+goto failNoMem;
+}
+
+ovsUnNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsUnNatTable == NULL) {
+goto freeNatTable;
+}
+
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+InitializeListHead([i]);
+InitializeListHead([i]);
+}
+return STATUS_SUCCESS;
+
+freeNatTable:
+OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
+failNoMem:
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/*
+ *
+ * OvsNatFlush
+ * Flushes out all NAT entries that match the given zone.
+ *
+ */
+VOID OvsNatFlush(UINT16 zone)
+{
+PLIST_ENTRY link, next;
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+LIST_FORALL_SAFE([i], link, next) {
+POVS_NAT_ENTRY entry =
+CONTAINING_RECORD(link, OVS_NAT_ENTRY, link);
+/* zone is a non-zero value */
+if (!zone || zone == entry-&

[ovs-dev] [PATCH v6 1/4] datapath-windows: Add support for NAT in conntrack

[Yin Lin]

From: Anand Kumar <kumaran...@vmware.com>

Add support for parsing netlink attributes related to NAT
in conntrack.

Co-Authored-by: Anand Kumar <kumaran...@vmware.com>
Co-Authored-by: Darrell Ball <dlu...@gmail.com>
Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 73 -
 datapath-windows/ovsext/Conntrack.h | 17 +
 datapath-windows/ovsext/Flow.c  |  4 +-
 3 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index dce0c1b..9824368 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -645,7 +645,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
   UINT16 zone,
   MD_MARK *mark,
   MD_LABELS *labels,
-  PCHAR helper)
+  PCHAR helper,
+  PNAT_ACTION_INFO natInfo)
 {
 NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_CT_ENTRY entry = NULL;
@@ -654,6 +655,9 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
 UINT64 currentTime;
 NdisGetCurrentSystemTime((LARGE_INTEGER *) );
 
+/* XXX: Not referenced for now */
+UNREFERENCED_PARAMETER(natInfo);
+
 /* Retrieve the Conntrack Key related fields from packet */
 OvsCtSetupLookupCtx(key, zone, , curNbl, layers->l4Offset);
 
@@ -730,11 +734,14 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
 PCHAR helper = NULL;
+NAT_ACTION_INFO natActionInfo;
 PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
 OVS_PACKET_HDR_INFO *layers = >layers;
 PNET_BUFFER_LIST newNbl = NULL;
+NAT_ACTION_INFO natActionInfo;
 NDIS_STATUS status;
 
+memset(, 0, sizeof natActionInfo);
 status = OvsDetectCtPacket(fwdCtx, key, );
 if (status != NDIS_STATUS_SUCCESS) {
 return status;
@@ -757,6 +764,68 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 if (ctAttr) {
 labels = NlAttrGet(ctAttr);
 }
+natActionInfo.natAction = NAT_ACTION_NONE;
+ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_NAT);
+if (ctAttr) {
+/* Pares Nested NAT attributes. */
+PNL_ATTR natAttr;
+unsigned int left;
+BOOLEAN hasMinIp = FALSE;
+BOOLEAN hasMinPort = FALSE;
+BOOLEAN hasMaxIp = FALSE;
+BOOLEAN hasMaxPort = FALSE;
+NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) {
+enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr);
+switch(sub_type_nest) {
+case OVS_NAT_ATTR_SRC:
+case OVS_NAT_ATTR_DST:
+natActionInfo.natAction |=
+((sub_type_nest == OVS_NAT_ATTR_SRC)
+? NAT_ACTION_SRC : NAT_ACTION_DST);
+break;
+case OVS_NAT_ATTR_IP_MIN:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMinIp = TRUE;
+break;
+case OVS_NAT_ATTR_IP_MAX:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMaxIp = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MIN:
+natActionInfo.minPort = NlAttrGetU16(natAttr);
+hasMinPort = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MAX:
+natActionInfo.maxPort = NlAttrGetU16(natAttr);
+hasMaxPort = TRUE;
+break;
+case OVS_NAT_ATTR_PERSISTENT:
+case OVS_NAT_ATTR_PROTO_HASH:
+case OVS_NAT_ATTR_PROTO_RANDOM:
+break;
+}
+}
+if (natActionInfo.natAction == NAT_ACTION_NONE) {
+natActionInfo.natAction = NAT_ACTION_REVERSE;
+}
+if (hasMinIp && !hasMaxIp) {
+memcpy(,
+   ,
+   sizeof(natActionInfo.maxAddr));
+}
+if (hasMinPort && !hasMaxPort) {
+natActionInfo.maxPort = natActionInfo.minPort;
+}
+if (hasMinPort || hasMaxPort) {
+if (natActionInfo.natAction & NAT_ACTION_SRC) {
+natActionInfo.natAction |= NAT_ACTION_SRC_PORT;
+} else if (natActionInfo.natAction & NAT_ACTION_DST) {
+natActionInfo.natAction |= NAT_ACTION_DST_PORT;
+}
+}
+}
 ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_HELPER);
 if (ctAttr) {
 helper = NlAttrGetString(ctAttr);
@@ -776,7 +845,7 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 }
 /* If newNbl is not allocated, use the current Nbl*/
 status = OvsCtExecute_(newNbl != NULL ? newNbl : curNbl, key, layers,
-   commit, force, zone, mark, labels, helper);
+   commit, force, zone

[ovs-dev] [PATCH v5 1/3] datapath-windows: Add support for NAT in conntrack

[Yin Lin]

From: Anand Kumar <kumaran...@vmware.com>

Add support for parsing netlink attributes related to NAT
in conntrack.

Co-Authored-by: Anand Kumar <kumaran...@vmware.com>
Co-Authored-by: Darrell Ball <dlu...@gmail.com>
Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 73 -
 datapath-windows/ovsext/Conntrack.h | 17 +
 datapath-windows/ovsext/Flow.c  |  4 +-
 3 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 8658910..212e0f0 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -637,7 +637,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
   UINT16 zone,
   MD_MARK *mark,
   MD_LABELS *labels,
-  PCHAR helper)
+  PCHAR helper,
+  PNAT_ACTION_INFO natInfo)
 {
 NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_CT_ENTRY entry = NULL;
@@ -646,6 +647,9 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
 UINT64 currentTime;
 NdisGetCurrentSystemTime((LARGE_INTEGER *) );
 
+/* XXX: Not referenced for now */
+UNREFERENCED_PARAMETER(natInfo);
+
 /* Retrieve the Conntrack Key related fields from packet */
 OvsCtSetupLookupCtx(key, zone, , curNbl, layers->l4Offset);
 
@@ -721,11 +725,14 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
 PCHAR helper = NULL;
+NAT_ACTION_INFO natActionInfo;
 PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
 OVS_PACKET_HDR_INFO *layers = >layers;
 
+NAT_ACTION_INFO natActionInfo;
 NDIS_STATUS status;
 
+memset(, 0, sizeof natActionInfo);
 status = OvsDetectCtPacket(key);
 if (status != NDIS_STATUS_SUCCESS) {
 return status;
@@ -748,6 +755,68 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 if (ctAttr) {
 labels = NlAttrGet(ctAttr);
 }
+natActionInfo.natAction = NAT_ACTION_NONE;
+ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_NAT);
+if (ctAttr) {
+/* Pares Nested NAT attributes. */
+PNL_ATTR natAttr;
+unsigned int left;
+BOOLEAN hasMinIp = FALSE;
+BOOLEAN hasMinPort = FALSE;
+BOOLEAN hasMaxIp = FALSE;
+BOOLEAN hasMaxPort = FALSE;
+NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) {
+enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr);
+switch(sub_type_nest) {
+case OVS_NAT_ATTR_SRC:
+case OVS_NAT_ATTR_DST:
+natActionInfo.natAction |=
+((sub_type_nest == OVS_NAT_ATTR_SRC)
+? NAT_ACTION_SRC : NAT_ACTION_DST);
+break;
+case OVS_NAT_ATTR_IP_MIN:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMinIp = TRUE;
+break;
+case OVS_NAT_ATTR_IP_MAX:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMaxIp = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MIN:
+natActionInfo.minPort = NlAttrGetU16(natAttr);
+hasMinPort = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MAX:
+natActionInfo.maxPort = NlAttrGetU16(natAttr);
+hasMaxPort = TRUE;
+break;
+case OVS_NAT_ATTR_PERSISTENT:
+case OVS_NAT_ATTR_PROTO_HASH:
+case OVS_NAT_ATTR_PROTO_RANDOM:
+break;
+}
+}
+if (natActionInfo.natAction == NAT_ACTION_NONE) {
+natActionInfo.natAction = NAT_ACTION_REVERSE;
+}
+if (hasMinIp && !hasMaxIp) {
+memcpy(,
+   ,
+   sizeof(natActionInfo.maxAddr));
+}
+if (hasMinPort && !hasMaxPort) {
+natActionInfo.maxPort = natActionInfo.minPort;
+}
+if (hasMinPort || hasMaxPort) {
+if (natActionInfo.natAction & NAT_ACTION_SRC) {
+natActionInfo.natAction |= NAT_ACTION_SRC_PORT;
+} else if (natActionInfo.natAction & NAT_ACTION_DST) {
+natActionInfo.natAction |= NAT_ACTION_DST_PORT;
+}
+}
+}
 ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_HELPER);
 if (ctAttr) {
 helper = NlAttrGetString(ctAttr);
@@ -767,7 +836,7 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 }
 
 status = OvsCtExecute_(curNbl, key, layers, commit, force,
-   zone, mark, labels, helper);
+   zone, mark, labels, helper, );
 return status;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.h 
b/datapath-windows/ovsext/Conntrack.h
index 87

[ovs-dev] [PATCH v2] datapath-windows: Pass fwdCtx to conntrack

[Yin Lin]

There dependencies in Contrack module such as NAT and fragmentation on
OvsForwardingContext. This patch will make OvsForwardingContext public
in order to implement these functionalities.

Signed-off-by: Yin Lin <li...@vmware.com>
Acked-by: Alin Serdean <aserd...@cloudbasesolutions.com>
---
 datapath-windows/ovsext/Actions.c   | 61 ++---
 datapath-windows/ovsext/Actions.h   | 58 +++
 datapath-windows/ovsext/Conntrack.c |  5 +--
 datapath-windows/ovsext/Conntrack.h |  4 +--
 4 files changed, 65 insertions(+), 63 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 46f84bc..3bd00a7 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -71,63 +71,6 @@ typedef struct _OVS_ACTION_STATS {
 OVS_ACTION_STATS ovsActionStats;
 
 /*
- * There a lot of data that needs to be maintained while executing the pipeline
- * as dictated by the actions of a flow, across different functions at 
different
- * levels. Such data is put together in a 'context' structure. Care should be
- * exercised while adding new members to the structure - only add ones that get
- * used across multiple stages in the pipeline/get used in multiple functions.
- */
-typedef struct OvsForwardingContext {
-POVS_SWITCH_CONTEXT switchContext;
-/* The NBL currently used in the pipeline. */
-PNET_BUFFER_LIST curNbl;
-/* NDIS forwarding detail for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
-/* Array of destination ports for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
-/* send flags while sending 'curNbl' into NDIS. */
-ULONG sendFlags;
-/* Total number of output ports, used + unused, in 'curNbl'. */
-UINT32 destPortsSizeIn;
-/* Total number of used output ports in 'curNbl'. */
-UINT32 destPortsSizeOut;
-/*
- * If 'curNbl' is not owned by OVS, they need to be tracked, if they need 
to
- * be freed/completed.
- */
-OvsCompletionList *completionList;
-/*
- * vport number of 'curNbl' when it is passed from the PIF bridge to the 
INT
- * bridge. ie. during tunneling on the Rx side.
- */
-UINT32 srcVportNo;
-
-/*
- * Tunnel key:
- * - specified in actions during tunneling Tx
- * - extracted from an NBL during tunneling Rx
- */
-OvsIPv4TunnelKey tunKey;
-
-/*
- * Tunneling - Tx:
- * To store the output port, when it is a tunneled port. We don't foresee
- * multiple tunneled ports as outport for any given NBL.
- */
-POVS_VPORT_ENTRY tunnelTxNic;
-
-/*
- * Tunneling - Rx:
- * Points to the Internal port on the PIF Bridge, if the packet needs to be
- * de-tunneled.
- */
-POVS_VPORT_ENTRY tunnelRxNic;
-
-/* header information */
-OVS_PACKET_HDR_INFO layers;
-} OvsForwardingContext;
-
-/*
  * --
  * OvsInitForwardingCtx --
  * Function to init/re-init the 'ovsFwdCtx' context as the actions pipeline
@@ -2032,8 +1975,8 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 }
 }
 
-status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers,
-   key, (const PNL_ATTR)a);
+status = OvsExecuteConntrackAction(, key,
+   (const PNL_ATTR)a);
 if (status != NDIS_STATUS_SUCCESS) {
 OVS_LOG_ERROR("CT Action failed");
 dropReason = L"OVS-conntrack action failed";
diff --git a/datapath-windows/ovsext/Actions.h 
b/datapath-windows/ovsext/Actions.h
index c56c260..1ce6c20 100644
--- a/datapath-windows/ovsext/Actions.h
+++ b/datapath-windows/ovsext/Actions.h
@@ -20,6 +20,64 @@
 #include "Switch.h"
 #include "PacketIO.h"
 
+
+/*
+ * There a lot of data that needs to be maintained while executing the pipeline
+ * as dictated by the actions of a flow, across different functions at 
different
+ * levels. Such data is put together in a 'context' structure. Care should be
+ * exercised while adding new members to the structure - only add ones that get
+ * used across multiple stages in the pipeline/get used in multiple functions.
+ */
+typedef struct OvsForwardingContext {
+POVS_SWITCH_CONTEXT switchContext;
+/* The NBL currently used in the pipeline. */
+PNET_BUFFER_LIST curNbl;
+/* NDIS forwarding detail for 'curNbl'. */
+PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
+/* Array of destination ports for 'curNbl'. */
+PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
+/* send flags while sending 'curNbl' into NDIS. */
+ULONG sendFlags;
+/* Total number of output ports, used + unused, in 'curNbl'. */
+UINT32 destPortsSizeIn;

[ovs-dev] [PATCH 3/3, v4] datapath-windows: NAT integration with conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Actions.c  | 119 -
 datapath-windows/ovsext/Actions.h  |  20 
 datapath-windows/ovsext/Conntrack.c| 188 +
 datapath-windows/ovsext/Conntrack.h|  25 +++--
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 5 files changed, 272 insertions(+), 82 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 3bd00a7..5872ca5 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1331,7 +1331,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_udp *udpAttr)
 {
@@ -1378,7 +1378,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_tcp *tcpAttr)
 {
@@ -1416,12 +1416,125 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 
 /*
  *
+ * OvsUpdateAddressAndPort --
+ *  Updates the source/destination IP and port fields in
+ *  ovsFwdCtx.curNbl inline based on the specified key.
+ *
+ */
+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
+UINT32 newAddr, UINT16 newPort,
+BOOLEAN isSource, BOOLEAN isTx)
+{
+PUINT8 bufferStart;
+UINT32 hdrSize;
+OVS_PACKET_HDR_INFO *layers = >layers;
+IPHdr *ipHdr;
+TCPHdr *tcpHdr = NULL;
+UDPHdr *udpHdr = NULL;
+UINT32 *addrField = NULL;
+UINT16 *portField = NULL;
+UINT16 *checkField = NULL;
+BOOLEAN l4Offload = FALSE;
+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+
+ASSERT(layers->value != 0);
+
+if (layers->isTcp || layers->isUdp) {
+hdrSize = layers->l4Offset +
+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
+} else {
+hdrSize = layers->l3Offset + sizeof (*ipHdr);
+}
+
+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize);
+if (!bufferStart) {
+return NDIS_STATUS_RESOURCES;
+}
+
+ipHdr = (IPHdr *)(bufferStart + layers->l3Offset);
+
+if (layers->isTcp) {
+tcpHdr = (TCPHdr *)(bufferStart + layers->l4Offset);
+} else if (layers->isUdp) {
+udpHdr = (UDPHdr *)(bufferStart + layers->l4Offset);
+}
+
+csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl,
+  TcpIpChecksumNetBufferListInfo);
+/*
+ * Adjust the IP header inline as dictated by the action, and also update
+ * the IP and the TCP checksum for the data modified.
+ *
+ * In the future, this could be optimized to make one call to
+ * ChecksumUpdate32(). Ignoring this for now, since for the most common
+ * case, we only update the TTL.
+ */
+
+if (isSource) {
+addrField = >saddr;
+if (tcpHdr) {
+portField = >source;
+checkField = >check;
+if (isTx) {
+l4Offload = (csumInfo.Transmit.TcpChecksum != 0);
+}
+}
+else if (udpHdr) {
+portField = >source;
+checkField = >check;
+if (isTx) {
+l4Offload = (csumInfo.Transmit.UdpChecksum != 0);
+}
+}
+} else {
+addrField = >daddr;
+if (tcpHdr) {
+portField = >dest;
+checkField = >check;
+} else if (udpHdr) {
+portField = >dest;
+checkField = >check;
+}
+}
+if (*addrField != newAddr) {
+UINT32 oldAddr = *addrField;
+if (checkField && *checkField != 0) {
+if (l4Offload) {
+/* Recompute IP pseudo checksum */
+*checkField = ~(*checkField);
+*checkField = ChecksumUpdate32(*checkField, oldAddr,
+   newAddr);
+*checkField = ~(*checkField);
+} else {
+*checkField = ChecksumUpdate32(*checkField, oldAddr,
+   newAddr);
+}
+}
+if (ipHdr->check != 0) {
+ipHdr->check = ChecksumUpdate32(ipHdr->check, oldAddr,
+newAddr);
+}
+*addrField = newAddr;
+}
+
+if (portField &

[ovs-dev] datapath-windows: Pass fwdCtx to conntrack

[Yin Lin]

There dependencies in Contrack module such as NAT and fragmentation on
OvsForwardingContext. This patch will make OvsForwardingContext public
in order to implement these functionalities.
---
 datapath-windows/ovsext/Actions.c   | 61 ++---
 datapath-windows/ovsext/Actions.h   | 58 +++
 datapath-windows/ovsext/Conntrack.c |  5 +--
 datapath-windows/ovsext/Conntrack.h |  4 +--
 4 files changed, 65 insertions(+), 63 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 46f84bc..3bd00a7 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -71,63 +71,6 @@ typedef struct _OVS_ACTION_STATS {
 OVS_ACTION_STATS ovsActionStats;
 
 /*
- * There a lot of data that needs to be maintained while executing the pipeline
- * as dictated by the actions of a flow, across different functions at 
different
- * levels. Such data is put together in a 'context' structure. Care should be
- * exercised while adding new members to the structure - only add ones that get
- * used across multiple stages in the pipeline/get used in multiple functions.
- */
-typedef struct OvsForwardingContext {
-POVS_SWITCH_CONTEXT switchContext;
-/* The NBL currently used in the pipeline. */
-PNET_BUFFER_LIST curNbl;
-/* NDIS forwarding detail for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
-/* Array of destination ports for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
-/* send flags while sending 'curNbl' into NDIS. */
-ULONG sendFlags;
-/* Total number of output ports, used + unused, in 'curNbl'. */
-UINT32 destPortsSizeIn;
-/* Total number of used output ports in 'curNbl'. */
-UINT32 destPortsSizeOut;
-/*
- * If 'curNbl' is not owned by OVS, they need to be tracked, if they need 
to
- * be freed/completed.
- */
-OvsCompletionList *completionList;
-/*
- * vport number of 'curNbl' when it is passed from the PIF bridge to the 
INT
- * bridge. ie. during tunneling on the Rx side.
- */
-UINT32 srcVportNo;
-
-/*
- * Tunnel key:
- * - specified in actions during tunneling Tx
- * - extracted from an NBL during tunneling Rx
- */
-OvsIPv4TunnelKey tunKey;
-
-/*
- * Tunneling - Tx:
- * To store the output port, when it is a tunneled port. We don't foresee
- * multiple tunneled ports as outport for any given NBL.
- */
-POVS_VPORT_ENTRY tunnelTxNic;
-
-/*
- * Tunneling - Rx:
- * Points to the Internal port on the PIF Bridge, if the packet needs to be
- * de-tunneled.
- */
-POVS_VPORT_ENTRY tunnelRxNic;
-
-/* header information */
-OVS_PACKET_HDR_INFO layers;
-} OvsForwardingContext;
-
-/*
  * --
  * OvsInitForwardingCtx --
  * Function to init/re-init the 'ovsFwdCtx' context as the actions pipeline
@@ -2032,8 +1975,8 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 }
 }
 
-status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers,
-   key, (const PNL_ATTR)a);
+status = OvsExecuteConntrackAction(, key,
+   (const PNL_ATTR)a);
 if (status != NDIS_STATUS_SUCCESS) {
 OVS_LOG_ERROR("CT Action failed");
 dropReason = L"OVS-conntrack action failed";
diff --git a/datapath-windows/ovsext/Actions.h 
b/datapath-windows/ovsext/Actions.h
index c56c260..1ce6c20 100644
--- a/datapath-windows/ovsext/Actions.h
+++ b/datapath-windows/ovsext/Actions.h
@@ -20,6 +20,64 @@
 #include "Switch.h"
 #include "PacketIO.h"
 
+
+/*
+ * There a lot of data that needs to be maintained while executing the pipeline
+ * as dictated by the actions of a flow, across different functions at 
different
+ * levels. Such data is put together in a 'context' structure. Care should be
+ * exercised while adding new members to the structure - only add ones that get
+ * used across multiple stages in the pipeline/get used in multiple functions.
+ */
+typedef struct OvsForwardingContext {
+POVS_SWITCH_CONTEXT switchContext;
+/* The NBL currently used in the pipeline. */
+PNET_BUFFER_LIST curNbl;
+/* NDIS forwarding detail for 'curNbl'. */
+PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
+/* Array of destination ports for 'curNbl'. */
+PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
+/* send flags while sending 'curNbl' into NDIS. */
+ULONG sendFlags;
+/* Total number of output ports, used + unused, in 'curNbl'. */
+UINT32 destPortsSizeIn;
+/* Total number of used output ports in 'curNbl'. */
+UINT32 destPortsSizeOut;
+/*
+ * If 'curNbl' is not owned by OVS, they need to be 

[ovs-dev] [PATCH 4/4 v3] datapath-windows: NAT integration with conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Actions.c  | 119 -
 datapath-windows/ovsext/Actions.h  |  20 
 datapath-windows/ovsext/Conntrack.c| 188 +
 datapath-windows/ovsext/Conntrack.h|  25 +++--
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 5 files changed, 272 insertions(+), 82 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 3bd00a7..5872ca5 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1331,7 +1331,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_udp *udpAttr)
 {
@@ -1378,7 +1378,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_tcp *tcpAttr)
 {
@@ -1416,12 +1416,125 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 
 /*
  *
+ * OvsUpdateAddressAndPort --
+ *  Updates the source/destination IP and port fields in
+ *  ovsFwdCtx.curNbl inline based on the specified key.
+ *
+ */
+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
+UINT32 newAddr, UINT16 newPort,
+BOOLEAN isSource, BOOLEAN isTx)
+{
+PUINT8 bufferStart;
+UINT32 hdrSize;
+OVS_PACKET_HDR_INFO *layers = >layers;
+IPHdr *ipHdr;
+TCPHdr *tcpHdr = NULL;
+UDPHdr *udpHdr = NULL;
+UINT32 *addrField = NULL;
+UINT16 *portField = NULL;
+UINT16 *checkField = NULL;
+BOOLEAN l4Offload = FALSE;
+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+
+ASSERT(layers->value != 0);
+
+if (layers->isTcp || layers->isUdp) {
+hdrSize = layers->l4Offset +
+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
+} else {
+hdrSize = layers->l3Offset + sizeof (*ipHdr);
+}
+
+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize);
+if (!bufferStart) {
+return NDIS_STATUS_RESOURCES;
+}
+
+ipHdr = (IPHdr *)(bufferStart + layers->l3Offset);
+
+if (layers->isTcp) {
+tcpHdr = (TCPHdr *)(bufferStart + layers->l4Offset);
+} else if (layers->isUdp) {
+udpHdr = (UDPHdr *)(bufferStart + layers->l4Offset);
+}
+
+csumInfo.Value = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl,
+  TcpIpChecksumNetBufferListInfo);
+/*
+ * Adjust the IP header inline as dictated by the action, and also update
+ * the IP and the TCP checksum for the data modified.
+ *
+ * In the future, this could be optimized to make one call to
+ * ChecksumUpdate32(). Ignoring this for now, since for the most common
+ * case, we only update the TTL.
+ */
+
+if (isSource) {
+addrField = >saddr;
+if (tcpHdr) {
+portField = >source;
+checkField = >check;
+if (isTx) {
+l4Offload = (csumInfo.Transmit.TcpChecksum != 0);
+}
+}
+else if (udpHdr) {
+portField = >source;
+checkField = >check;
+if (isTx) {
+l4Offload = (csumInfo.Transmit.UdpChecksum != 0);
+}
+}
+} else {
+addrField = >daddr;
+if (tcpHdr) {
+portField = >dest;
+checkField = >check;
+} else if (udpHdr) {
+portField = >dest;
+checkField = >check;
+}
+}
+if (*addrField != newAddr) {
+UINT32 oldAddr = *addrField;
+if (checkField && *checkField != 0) {
+if (l4Offload) {
+/* Recompute IP pseudo checksum */
+*checkField = ~(*checkField);
+*checkField = ChecksumUpdate32(*checkField, oldAddr,
+   newAddr);
+*checkField = ~(*checkField);
+} else {
+*checkField = ChecksumUpdate32(*checkField, oldAddr,
+   newAddr);
+}
+}
+if (ipHdr->check != 0) {
+ipHdr->check = ChecksumUpdate32(ipHdr->check, oldAddr,
+newAddr);
+}
+*addrField = newAddr;
+}
+
+if (portField &

[ovs-dev] [PATCH 3/4 v3] datapath-windows: Add NAT module in conntrack

[Yin Lin]

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

Issue: #
Change-Id: I6f37360c36525548b343f0016304015fec8aba7d
---
 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-nat.c | 438 
 datapath-windows/ovsext/Conntrack-nat.h |  39 +++
 3 files changed, 479 insertions(+)
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 53983ae..296e785 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,7 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
+datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
+datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
new file mode 100644
index 000..17b312c
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -0,0 +1,438 @@
+#include "Conntrack-nat.h"
+#include "Jhash.h"
+
+PLIST_ENTRY ovsNatTable = NULL;
+PLIST_ENTRY ovsUnNatTable = NULL;
+static PNDIS_RW_LOCK_EX ovsNatLock;
+
+/*
+ *---
+ * OvsHashNatKey
+ * Hash NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline UINT32
+OvsHashNatKey(const OVS_CT_KEY *key)
+{
+UINT32 hash = 0;
+#define HASH_ADD(field) \
+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
+
+HASH_ADD(src.addr.ipv4_aligned);
+HASH_ADD(dst.addr.ipv4_aligned);
+HASH_ADD(src.port);
+HASH_ADD(dst.port);
+HASH_ADD(zone);
+#undef HASH_ADD
+return hash;
+}
+
+/*
+ *---
+ * OvsNatKeyAreSame
+ * Compare NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline BOOLEAN
+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
+{
+// XXX: Compare IPv6 key as well
+#define FIELD_COMPARE(field) \
+if (key1->field != key2->field) return FALSE
+
+FIELD_COMPARE(src.addr.ipv4_aligned);
+FIELD_COMPARE(dst.addr.ipv4_aligned);
+FIELD_COMPARE(src.port);
+FIELD_COMPARE(dst.port);
+FIELD_COMPARE(zone);
+return TRUE;
+#undef FIELD_COMPARE
+}
+
+/*
+ *---
+ * OvsNaGetBucket
+ * Returns the row of NAT table that has the same hash as the given NAT
+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
+ * instead.
+ *---
+ */
+static __inline PLIST_ENTRY
+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
+{
+uint32_t hash = OvsHashNatKey(key);
+if (isReverse) {
+return [hash & NAT_HASH_TABLE_MASK];
+} else {
+return [hash & NAT_HASH_TABLE_MASK];
+}
+}
+
+/*
+ *---
+ * OvsNatInit
+ * Initialize NAT related resources.
+ *---
+ */
+NTSTATUS OvsNatInit(POVS_SWITCH_CONTEXT context)
+{
+ASSERT(ovsNatTable == NULL);
+
+/* Init the sync-lock */
+ovsNatLock = NdisAllocateRWLock(context->NdisFilterHandle);
+if (ovsNatLock == NULL) {
+goto failNoMem;
+}
+
+/* Init the Hash Buffer */
+ovsNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsNatTable == NULL) {
+goto freeNatLock;
+}
+
+ovsUnNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsUnNatTable == NULL) {
+goto freeNatTable;
+}
+
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+InitializeListHead([i]);
+InitializeListHead([i]);
+}
+return STATUS_SUCCESS;
+
+freeNatTable:
+OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
+freeNatLock:
+NdisFreeRWLock(ovsNatLock);
+failNoMem:
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/*
+ *
+ * OvsNatFlush
+ * Flushes out all NAT entries that match the given zone.
+ *
+ */
+VOID OvsNatFlush(UINT16 zone)
+{
+LOCK_STATE_EX lockState;
+PLIST_ENTRY lin

[ovs-dev] [PATCH 2/4 v3] datapath-windows: Add support for NAT in conntrack

[Yin Lin]

Add support for parsing netlink attributes related to NAT
in conntrack.

Co-Authored-by: Anand Kumar <kumaran...@vmware.com>
Co-Authored-by: Darrell Ball <dlu...@gmail.com>
Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 73 -
 datapath-windows/ovsext/Conntrack.h | 17 +
 datapath-windows/ovsext/Flow.c  |  4 +-
 3 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 23de526..404f2dc 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -636,7 +636,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
   UINT16 zone,
   MD_MARK *mark,
   MD_LABELS *labels,
-  PCHAR helper)
+  PCHAR helper,
+  PNAT_ACTION_INFO natInfo)
 {
 NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_CT_ENTRY entry = NULL;
@@ -645,6 +646,9 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
 UINT64 currentTime;
 NdisGetCurrentSystemTime((LARGE_INTEGER *) );
 
+/* XXX: Not referenced for now */
+UNREFERENCED_PARAMETER(natInfo);
+
 /* Retrieve the Conntrack Key related fields from packet */
 OvsCtSetupLookupCtx(key, zone, , curNbl, layers->l4Offset);
 
@@ -712,11 +716,14 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
 PCHAR helper = NULL;
+NAT_ACTION_INFO natActionInfo;
 PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
 OVS_PACKET_HDR_INFO *layers = >layers;
 
+NAT_ACTION_INFO natActionInfo;
 NDIS_STATUS status;
 
+memset(, 0, sizeof natActionInfo);
 status = OvsDetectCtPacket(key);
 if (status != NDIS_STATUS_SUCCESS) {
 return status;
@@ -738,6 +745,68 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 if (ctAttr) {
 labels = NlAttrGet(ctAttr);
 }
+natActionInfo.natAction = NAT_ACTION_NONE;
+ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_NAT);
+if (ctAttr) {
+/* Pares Nested NAT attributes. */
+PNL_ATTR natAttr;
+unsigned int left;
+BOOLEAN hasMinIp = FALSE;
+BOOLEAN hasMinPort = FALSE;
+BOOLEAN hasMaxIp = FALSE;
+BOOLEAN hasMaxPort = FALSE;
+NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) {
+enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr);
+switch(sub_type_nest) {
+case OVS_NAT_ATTR_SRC:
+case OVS_NAT_ATTR_DST:
+natActionInfo.natAction |=
+((sub_type_nest == OVS_NAT_ATTR_SRC)
+? NAT_ACTION_SRC : NAT_ACTION_DST);
+break;
+case OVS_NAT_ATTR_IP_MIN:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMinIp = TRUE;
+break;
+case OVS_NAT_ATTR_IP_MAX:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMaxIp = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MIN:
+natActionInfo.minPort = NlAttrGetU16(natAttr);
+hasMinPort = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MAX:
+natActionInfo.maxPort = NlAttrGetU16(natAttr);
+hasMaxPort = TRUE;
+break;
+case OVS_NAT_ATTR_PERSISTENT:
+case OVS_NAT_ATTR_PROTO_HASH:
+case OVS_NAT_ATTR_PROTO_RANDOM:
+break;
+}
+}
+if (natActionInfo.natAction == NAT_ACTION_NONE) {
+natActionInfo.natAction = NAT_ACTION_REVERSE;
+}
+if (hasMinIp && !hasMaxIp) {
+memcpy(,
+   ,
+   sizeof(natActionInfo.maxAddr));
+}
+if (hasMinPort && !hasMaxPort) {
+natActionInfo.maxPort = natActionInfo.minPort;
+}
+if (hasMinPort || hasMaxPort) {
+if (natActionInfo.natAction & NAT_ACTION_SRC) {
+natActionInfo.natAction |= NAT_ACTION_SRC_PORT;
+} else if (natActionInfo.natAction & NAT_ACTION_DST) {
+natActionInfo.natAction |= NAT_ACTION_DST_PORT;
+}
+}
+}
 ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_HELPER);
 if (ctAttr) {
 helper = NlAttrGetString(ctAttr);
@@ -751,7 +820,7 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 }
 
 status = OvsCtExecute_(curNbl, key, layers,
-   commit, zone, mark, labels, helper);
+   commit, zone, mark, labels, helper, );
 return status;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.h 
b/datapath-windows/ovsext/Conntrack.h
index 87d7eeb..1ad289f 100644
--- a/datapath-windows/ovs

[ovs-dev] [PATCH 1/4 v3] datapath-windows: Pass fwdCtx to conntrack

[Yin Lin]

---
 datapath-windows/ovsext/Actions.c   | 61 ++---
 datapath-windows/ovsext/Actions.h   | 58 +++
 datapath-windows/ovsext/Conntrack.c |  5 +--
 datapath-windows/ovsext/Conntrack.h |  4 +--
 4 files changed, 65 insertions(+), 63 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 46f84bc..3bd00a7 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -71,63 +71,6 @@ typedef struct _OVS_ACTION_STATS {
 OVS_ACTION_STATS ovsActionStats;
 
 /*
- * There a lot of data that needs to be maintained while executing the pipeline
- * as dictated by the actions of a flow, across different functions at 
different
- * levels. Such data is put together in a 'context' structure. Care should be
- * exercised while adding new members to the structure - only add ones that get
- * used across multiple stages in the pipeline/get used in multiple functions.
- */
-typedef struct OvsForwardingContext {
-POVS_SWITCH_CONTEXT switchContext;
-/* The NBL currently used in the pipeline. */
-PNET_BUFFER_LIST curNbl;
-/* NDIS forwarding detail for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
-/* Array of destination ports for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
-/* send flags while sending 'curNbl' into NDIS. */
-ULONG sendFlags;
-/* Total number of output ports, used + unused, in 'curNbl'. */
-UINT32 destPortsSizeIn;
-/* Total number of used output ports in 'curNbl'. */
-UINT32 destPortsSizeOut;
-/*
- * If 'curNbl' is not owned by OVS, they need to be tracked, if they need 
to
- * be freed/completed.
- */
-OvsCompletionList *completionList;
-/*
- * vport number of 'curNbl' when it is passed from the PIF bridge to the 
INT
- * bridge. ie. during tunneling on the Rx side.
- */
-UINT32 srcVportNo;
-
-/*
- * Tunnel key:
- * - specified in actions during tunneling Tx
- * - extracted from an NBL during tunneling Rx
- */
-OvsIPv4TunnelKey tunKey;
-
-/*
- * Tunneling - Tx:
- * To store the output port, when it is a tunneled port. We don't foresee
- * multiple tunneled ports as outport for any given NBL.
- */
-POVS_VPORT_ENTRY tunnelTxNic;
-
-/*
- * Tunneling - Rx:
- * Points to the Internal port on the PIF Bridge, if the packet needs to be
- * de-tunneled.
- */
-POVS_VPORT_ENTRY tunnelRxNic;
-
-/* header information */
-OVS_PACKET_HDR_INFO layers;
-} OvsForwardingContext;
-
-/*
  * --
  * OvsInitForwardingCtx --
  * Function to init/re-init the 'ovsFwdCtx' context as the actions pipeline
@@ -2032,8 +1975,8 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 }
 }
 
-status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers,
-   key, (const PNL_ATTR)a);
+status = OvsExecuteConntrackAction(, key,
+   (const PNL_ATTR)a);
 if (status != NDIS_STATUS_SUCCESS) {
 OVS_LOG_ERROR("CT Action failed");
 dropReason = L"OVS-conntrack action failed";
diff --git a/datapath-windows/ovsext/Actions.h 
b/datapath-windows/ovsext/Actions.h
index c56c260..1ce6c20 100644
--- a/datapath-windows/ovsext/Actions.h
+++ b/datapath-windows/ovsext/Actions.h
@@ -20,6 +20,64 @@
 #include "Switch.h"
 #include "PacketIO.h"
 
+
+/*
+ * There a lot of data that needs to be maintained while executing the pipeline
+ * as dictated by the actions of a flow, across different functions at 
different
+ * levels. Such data is put together in a 'context' structure. Care should be
+ * exercised while adding new members to the structure - only add ones that get
+ * used across multiple stages in the pipeline/get used in multiple functions.
+ */
+typedef struct OvsForwardingContext {
+POVS_SWITCH_CONTEXT switchContext;
+/* The NBL currently used in the pipeline. */
+PNET_BUFFER_LIST curNbl;
+/* NDIS forwarding detail for 'curNbl'. */
+PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
+/* Array of destination ports for 'curNbl'. */
+PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
+/* send flags while sending 'curNbl' into NDIS. */
+ULONG sendFlags;
+/* Total number of output ports, used + unused, in 'curNbl'. */
+UINT32 destPortsSizeIn;
+/* Total number of used output ports in 'curNbl'. */
+UINT32 destPortsSizeOut;
+/*
+ * If 'curNbl' is not owned by OVS, they need to be tracked, if they need 
to
+ * be freed/completed.
+ */
+OvsCompletionList *completionList;
+/*
+ * vport number of 'curNbl' when it is passed from the PIF bridge to the 
INT

Re: [ovs-dev] [PATCH 3/3 v2] datapath-windows: NAT integration with conntrack

[Yin Lin]

Hi Alin,

Was the checksum error observed in my v2 patch? In v1 patch, I did not
consider the TCP offloading case, which result in TCP checksum error when
checksum computing is offloaded. It's already fixed in v2. If you are still
seeing errors, let me know your setup so I can take a look. Thanks!

Best regards,
Yin Lin

On Thu, Apr 6, 2017 at 7:39 PM, Alin Serdean <
aserd...@cloudbasesolutions.com> wrote:

> Hi Yin,
>
> I'm back sorry it took a while .
>
> When applying `OvsUpdateAddressAndPort` for some reason there still is a
> problem with the checksums.
>
> There are two aspects which apparently are hit for some reason:
> 1. normal packet.
> 2. tcp segment.
>
> If I disable tcp/udp checksums(on the NIC) normal packets have the right
> checksums. However, the tcp segments checksums are still wrong.
>
> If I apply a full blown checksum using ` OvsApplySWChecksumOnNB` and `
> OvsTcpSegmentNBL`(just for checksum computations) things are fine with
> tcp/udp checksum enabled/disabled and lso enabled/disabled.
>
> From what I can think of the pseudochecksums are wrong when being sent
> down to a NIC.
>
> I'll try to dig further into this issue since what you apply here is the
> same thing we apply on the set ip/tcp port actions and that works fine.
>
> Thanks,
> Alin.
>
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Yin Lin
> > Sent: Thursday, March 23, 2017 12:12 AM
> > To: d...@openvswitch.org
> > Subject: [ovs-dev] [PATCH 3/3 v2] datapath-windows: NAT integration with
> > conntrack
> >
> > Signed-off-by: Yin Lin <li...@vmware.com>
> ___
> 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 2/3 v2] datapath-windows: Add NAT module in conntrack

[Yin Lin]

Hi Sai,

You are right. Locking is a little bit tricky here than in Conntrack.c
because we cannot grab the lock during the entire Conntrack transaction. I
will have to address thread safety in another patch.

Best regards,
Yin Lin

On Thu, Mar 23, 2017 at 11:58 PM, Sairam Venugopal <vsai...@vmware.com>
wrote:

> Hi Yin,
>
> Thanks for sending over the patch. While it looks good for the most part,
> there are certain indentations that need fixing. I will send those out
> separately.
>
> I see that you have allocated - ovsNatLock and have added in stubs for
> flush and cleanup similar to Conntrack.c.
>
> However, I don’t see you taking out the lock for accessing the NAT tables.
> How are you verifying thread-safety here?
>
> Thanks,
> Sairam
>
>
>
>
> On 3/22/17, 3:11 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yin
> Lin" <ovs-dev-boun...@openvswitch.org on behalf of li...@vmware.com>
> wrote:
>
> >Signed-off-by: Yin Lin <li...@vmware.com>
> >
> >Issue: #
> >Change-Id: I6f37360c36525548b343f0016304015fec8aba7d
> >---
> > datapath-windows/automake.mk|   2 +
> > datapath-windows/ovsext/Conntrack-nat.c | 437
> 
> > datapath-windows/ovsext/Conntrack-nat.h |  39 +++
> > 3 files changed, 478 insertions(+)
> > create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
> > create mode 100644 datapath-windows/ovsext/Conntrack-nat.h
> >
> >diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
> >index 53983ae..296e785 100644
> >--- a/datapath-windows/automake.mk
> >+++ b/datapath-windows/automake.mk
> >@@ -16,7 +16,9 @@ EXTRA_DIST += \
> >   datapath-windows/ovsext/Conntrack-icmp.c \
> >   datapath-windows/ovsext/Conntrack-other.c \
> >   datapath-windows/ovsext/Conntrack-related.c \
> >+datapath-windows/ovsext/Conntrack-nat.c \
> >   datapath-windows/ovsext/Conntrack-tcp.c \
> >+datapath-windows/ovsext/Conntrack-nat.h \
> >   datapath-windows/ovsext/Conntrack.c \
> >   datapath-windows/ovsext/Conntrack.h \
> >   datapath-windows/ovsext/Datapath.c \
> >diff --git a/datapath-windows/ovsext/Conntrack-nat.c
> b/datapath-windows/ovsext/Conntrack-nat.c
> >new file mode 100644
> >index 000..4930694
> >--- /dev/null
> >+++ b/datapath-windows/ovsext/Conntrack-nat.c
> >@@ -0,0 +1,437 @@
> >+#include "Conntrack-nat.h"
> >+#include "Jhash.h"
> >+
> >+PLIST_ENTRY ovsNatTable = NULL;
> >+PLIST_ENTRY ovsUnNatTable = NULL;
> >+static PNDIS_RW_LOCK_EX ovsNatLock;
> >+
> >+/*
> >+ *---
> 
> >+ * OvsHashNatKey
> >+ * Hash NAT related fields in a Conntrack key.
> >+ *---
> 
> >+ */
> >+static __inline UINT32
> >+OvsHashNatKey(const OVS_CT_KEY *key)
> >+{
> >+UINT32 hash = 0;
> >+#define HASH_ADD(field) \
> >+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
> >+
> >+HASH_ADD(src.addr.ipv4_aligned);
> >+HASH_ADD(dst.addr.ipv4_aligned);
> >+HASH_ADD(src.port);
> >+HASH_ADD(dst.port);
> >+HASH_ADD(zone);
> >+#undef HASH_ADD
> >+return hash;
> >+}
> >+
> >+/*
> >+ *---
> 
> >+ * OvsNatKeyAreSame
> >+ * Compare NAT related fields in a Conntrack key.
> >+ *---
> 
> >+ */
> >+static __inline BOOLEAN
> >+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
> >+{
> >+// XXX: Compare IPv6 key as well
> >+#define FIELD_COMPARE(field) \
> >+if (key1->field != key2->field) return FALSE
> >+
> >+FIELD_COMPARE(src.addr.ipv4_aligned);
> >+FIELD_COMPARE(dst.addr.ipv4_aligned);
> >+FIELD_COMPARE(src.port);
> >+FIELD_COMPARE(dst.port);
> >+FIELD_COMPARE(zone);
> >+return TRUE;
> >+#undef FIELD_COMPARE
> >+}
> >+
> >+/*
> >+ *---
> 
> >+ * OvsNaGetBucket
> >+ * Returns the row of NAT table that has the same hash as the given
> NAT
> >+ * hash key. If isReverse is TRUE, returns the row of reverse NAT
> table
> >+ * instead.
> >+ *---
> 

[ovs-dev] [PATCH 3/3 v2] datapath-windows: NAT integration with conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Actions.c   | 179 ---
 datapath-windows/ovsext/Actions.h   |  77 +
 datapath-windows/ovsext/Conntrack-nat.c |   3 +-
 datapath-windows/ovsext/Conntrack.c | 184 
 datapath-windows/ovsext/Conntrack.h |  25 +++--
 datapath-windows/ovsext/ovsext.vcxproj  |   2 +
 6 files changed, 331 insertions(+), 139 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 46f84bc..45b3841 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -71,63 +71,6 @@ typedef struct _OVS_ACTION_STATS {
 OVS_ACTION_STATS ovsActionStats;
 
 /*
- * There a lot of data that needs to be maintained while executing the pipeline
- * as dictated by the actions of a flow, across different functions at 
different
- * levels. Such data is put together in a 'context' structure. Care should be
- * exercised while adding new members to the structure - only add ones that get
- * used across multiple stages in the pipeline/get used in multiple functions.
- */
-typedef struct OvsForwardingContext {
-POVS_SWITCH_CONTEXT switchContext;
-/* The NBL currently used in the pipeline. */
-PNET_BUFFER_LIST curNbl;
-/* NDIS forwarding detail for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
-/* Array of destination ports for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
-/* send flags while sending 'curNbl' into NDIS. */
-ULONG sendFlags;
-/* Total number of output ports, used + unused, in 'curNbl'. */
-UINT32 destPortsSizeIn;
-/* Total number of used output ports in 'curNbl'. */
-UINT32 destPortsSizeOut;
-/*
- * If 'curNbl' is not owned by OVS, they need to be tracked, if they need 
to
- * be freed/completed.
- */
-OvsCompletionList *completionList;
-/*
- * vport number of 'curNbl' when it is passed from the PIF bridge to the 
INT
- * bridge. ie. during tunneling on the Rx side.
- */
-UINT32 srcVportNo;
-
-/*
- * Tunnel key:
- * - specified in actions during tunneling Tx
- * - extracted from an NBL during tunneling Rx
- */
-OvsIPv4TunnelKey tunKey;
-
-/*
- * Tunneling - Tx:
- * To store the output port, when it is a tunneled port. We don't foresee
- * multiple tunneled ports as outport for any given NBL.
- */
-POVS_VPORT_ENTRY tunnelTxNic;
-
-/*
- * Tunneling - Rx:
- * Points to the Internal port on the PIF Bridge, if the packet needs to be
- * de-tunneled.
- */
-POVS_VPORT_ENTRY tunnelRxNic;
-
-/* header information */
-OVS_PACKET_HDR_INFO layers;
-} OvsForwardingContext;
-
-/*
  * --
  * OvsInitForwardingCtx --
  * Function to init/re-init the 'ovsFwdCtx' context as the actions pipeline
@@ -1388,7 +1331,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_udp *udpAttr)
 {
@@ -1435,7 +1378,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_tcp *tcpAttr)
 {
@@ -1474,11 +1417,125 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 /*
  *
  * OvsUpdateIPv4Header --
+ *  Updates the source/destination IP field of IPv4 header in
+ *  ovsFwdCtx.curNbl inline based on the specified key.
+ *
+ */
+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
+UINT32 newAddr, UINT16 newPort,
+BOOLEAN isSource, BOOLEAN isTx)
+{
+PUINT8 bufferStart;
+UINT32 hdrSize;
+OVS_PACKET_HDR_INFO *layers = >layers;
+IPHdr *ipHdr;
+TCPHdr *tcpHdr = NULL;
+UDPHdr *udpHdr = NULL;
+UINT32 *addrField = NULL;
+UINT16 *portField = NULL;
+UINT16 *checkField = NULL;
+BOOLEAN l4Offload = FALSE;
+NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+
+ASSERT(layers->value != 0);
+
+if (layers->isTcp || layers->isUdp) {
+hdrSize = layers->l4Offset +
+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
+} else {
+hdrSize = layers->l3Offset + sizeof (*ipHdr);
+}
+
+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrS

[ovs-dev] [PATCH 2/3 v2] datapath-windows: Add NAT module in conntrack

[Yin Lin]

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

Issue: #
Change-Id: I6f37360c36525548b343f0016304015fec8aba7d
---
 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-nat.c | 437 
 datapath-windows/ovsext/Conntrack-nat.h |  39 +++
 3 files changed, 478 insertions(+)
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 53983ae..296e785 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,7 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
+datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
+datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
new file mode 100644
index 000..4930694
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -0,0 +1,437 @@
+#include "Conntrack-nat.h"
+#include "Jhash.h"
+
+PLIST_ENTRY ovsNatTable = NULL;
+PLIST_ENTRY ovsUnNatTable = NULL;
+static PNDIS_RW_LOCK_EX ovsNatLock;
+
+/*
+ *---
+ * OvsHashNatKey
+ * Hash NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline UINT32
+OvsHashNatKey(const OVS_CT_KEY *key)
+{
+UINT32 hash = 0;
+#define HASH_ADD(field) \
+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
+
+HASH_ADD(src.addr.ipv4_aligned);
+HASH_ADD(dst.addr.ipv4_aligned);
+HASH_ADD(src.port);
+HASH_ADD(dst.port);
+HASH_ADD(zone);
+#undef HASH_ADD
+return hash;
+}
+
+/*
+ *---
+ * OvsNatKeyAreSame
+ * Compare NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline BOOLEAN
+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
+{
+// XXX: Compare IPv6 key as well
+#define FIELD_COMPARE(field) \
+if (key1->field != key2->field) return FALSE
+
+FIELD_COMPARE(src.addr.ipv4_aligned);
+FIELD_COMPARE(dst.addr.ipv4_aligned);
+FIELD_COMPARE(src.port);
+FIELD_COMPARE(dst.port);
+FIELD_COMPARE(zone);
+return TRUE;
+#undef FIELD_COMPARE
+}
+
+/*
+ *---
+ * OvsNaGetBucket
+ * Returns the row of NAT table that has the same hash as the given NAT
+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
+ * instead.
+ *---
+ */
+static __inline PLIST_ENTRY
+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
+{
+uint32_t hash = OvsHashNatKey(key);
+if (isReverse) {
+return [hash & NAT_HASH_TABLE_MASK];
+} else {
+return [hash & NAT_HASH_TABLE_MASK];
+}
+}
+
+/*
+ *---
+ * OvsNatInit
+ * Initialize NAT related resources.
+ *---
+ */
+NTSTATUS OvsNatInit(POVS_SWITCH_CONTEXT context)
+{
+ASSERT(ovsNatTable == NULL);
+
+/* Init the sync-lock */
+ovsNatLock = NdisAllocateRWLock(context->NdisFilterHandle);
+if (ovsNatLock == NULL) {
+goto failNoMem;
+}
+
+/* Init the Hash Buffer */
+ovsNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsNatTable == NULL) {
+goto freeNatLock;
+}
+
+ovsUnNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsUnNatTable == NULL) {
+goto freeNatTable;
+}
+
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+InitializeListHead([i]);
+InitializeListHead([i]);
+}
+return STATUS_SUCCESS;
+
+freeNatTable:
+OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
+freeNatLock:
+NdisFreeRWLock(ovsNatLock);
+failNoMem:
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/*
+ *
+ * OvsNatFlush
+ * Flushes out all NAT entries that match the given zone.
+ *
+ */
+VOID OvsNatFlush(UINT16 zone)
+{
+LOCK_STATE_EX lockState;
+PLIST_ENTRY lin

[ovs-dev] [PATCH 1/3 v2] datapath-windows: Add support for NAT in conntrack

[Yin Lin]

Add support for parsing netlink attributes related to NAT
in conntrack.

Co-Authored-by: Anand Kumar <kumaran...@vmware.com>
Co-Authored-by: Darrell Ball <dlu...@gmail.com>
Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 73 +++--
 datapath-windows/ovsext/Conntrack.h | 17 +
 datapath-windows/ovsext/Flow.c  |  4 +-
 3 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 9f41861..924f260 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -636,7 +636,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
   UINT16 zone,
   MD_MARK *mark,
   MD_LABELS *labels,
-  PCHAR helper)
+  PCHAR helper,
+  PNAT_ACTION_INFO natInfo)
 {
 NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_CT_ENTRY entry = NULL;
@@ -645,6 +646,9 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
 UINT64 currentTime;
 NdisGetCurrentSystemTime((LARGE_INTEGER *) );
 
+/* XXX: Not referenced for now */
+UNREFERENCED_PARAMETER(natInfo);
+
 /* Retrieve the Conntrack Key related fields from packet */
 OvsCtSetupLookupCtx(key, zone, , curNbl, layers->l4Offset);
 
@@ -713,9 +717,10 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
 PCHAR helper = NULL;
-
+NAT_ACTION_INFO natActionInfo;
 NDIS_STATUS status;
 
+memset(, 0, sizeof natActionInfo);
 status = OvsDetectCtPacket(key);
 if (status != NDIS_STATUS_SUCCESS) {
 return status;
@@ -737,6 +742,68 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 if (ctAttr) {
 labels = NlAttrGet(ctAttr);
 }
+natActionInfo.natAction = NAT_ACTION_NONE;
+ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_NAT);
+if (ctAttr) {
+/* Pares Nested NAT attributes. */
+PNL_ATTR natAttr;
+unsigned int left;
+BOOLEAN hasMinIp = FALSE;
+BOOLEAN hasMinPort = FALSE;
+BOOLEAN hasMaxIp = FALSE;
+BOOLEAN hasMaxPort = FALSE;
+NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) {
+enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr);
+switch(sub_type_nest) {
+case OVS_NAT_ATTR_SRC:
+case OVS_NAT_ATTR_DST:
+natActionInfo.natAction |=
+((sub_type_nest == OVS_NAT_ATTR_SRC)
+? NAT_ACTION_SRC : NAT_ACTION_DST);
+break;
+case OVS_NAT_ATTR_IP_MIN:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMinIp = TRUE;
+break;
+case OVS_NAT_ATTR_IP_MAX:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMaxIp = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MIN:
+natActionInfo.minPort = NlAttrGetU16(natAttr);
+hasMinPort = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MAX:
+natActionInfo.maxPort = NlAttrGetU16(natAttr);
+hasMaxPort = TRUE;
+break;
+case OVS_NAT_ATTR_PERSISTENT:
+case OVS_NAT_ATTR_PROTO_HASH:
+case OVS_NAT_ATTR_PROTO_RANDOM:
+break;
+}
+}
+if (natActionInfo.natAction == NAT_ACTION_NONE) {
+natActionInfo.natAction = NAT_ACTION_REVERSE;
+}
+if (hasMinIp && !hasMaxIp) {
+memcpy(,
+   ,
+   sizeof(natActionInfo.maxAddr));
+}
+if (hasMinPort && !hasMaxPort) {
+natActionInfo.maxPort = natActionInfo.minPort;
+}
+if (hasMinPort || hasMaxPort) {
+if (natActionInfo.natAction & NAT_ACTION_SRC) {
+natActionInfo.natAction |= NAT_ACTION_SRC_PORT;
+} else if (natActionInfo.natAction & NAT_ACTION_DST) {
+natActionInfo.natAction |= NAT_ACTION_DST_PORT;
+}
+}
+}
 ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_HELPER);
 if (ctAttr) {
 helper = NlAttrGetString(ctAttr);
@@ -750,7 +817,7 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 }
 
 status = OvsCtExecute_(curNbl, key, layers,
-   commit, zone, mark, labels, helper);
+   commit, zone, mark, labels, helper, );
 return status;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.h 
b/datapath-windows/ovsext/Conntrack.h
index af99885..875c434 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -67,6 +67,15 @@ typedef struct MD_LABELS {
 struct ovs_key_ct_la

Re: [ovs-dev] [PATCH] datapath-windows: geneve option header is in BE

[Yin Lin]

Good catch. Thanks Alin for the fixing this! Wondering why I didn't caught
it earlier.

Acked-by: Yin Lin <li...@vmware.com>

On Fri, Feb 17, 2017 at 4:44 AM, Alin Serdean <
aserd...@cloudbasesolutions.com> wrote:

> At the moment on Windows we suport only LE platforms.
>
> The option header is currently defined in BE.
>
> Found while testing.
>
> Signed-off-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com>
> ---
>  datapath-windows/ovsext/Geneve.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Geneve.h b/datapath-windows/ovsext/
> Geneve.h
> index be8a834..019c0dd 100644
> --- a/datapath-windows/ovsext/Geneve.h
> +++ b/datapath-windows/ovsext/Geneve.h
> @@ -71,10 +71,10 @@ typedef struct GeneveOptionHdr {
>  UINT32   optionClass:16;
>  /* Format of data contained in the option. */
>  UINT32   type:8;
> -/* Reserved. */
> -UINT32   reserved:3;
>  /* Length of option in int32 excluding the option header. */
>  UINT32   length:5;
> +/* Reserved. */
> +UINT32   reserved:3;
>  } GeneveOptionHdr;
>
>  #define GENEVE_CRIT_OPT_TYPE (1 << 7)
> --
> 2.10.2.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

[ovs-dev] [PATCH 1/3] datapath-windows: Add support for NAT in conntrack

[Yin Lin]

Add support for parsing netlink attributes related to NAT
in conntrack.

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 73 +++--
 datapath-windows/ovsext/Conntrack.h | 17 +
 datapath-windows/ovsext/Flow.c  |  4 +-
 3 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index d1be480..8a87dce 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -628,7 +628,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
   UINT16 zone,
   MD_MARK *mark,
   MD_LABELS *labels,
-  PCHAR helper)
+  PCHAR helper,
+  PNAT_ACTION_INFO natInfo)
 {
 NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_CT_ENTRY entry = NULL;
@@ -637,6 +638,9 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
 UINT64 currentTime;
 NdisGetCurrentSystemTime((LARGE_INTEGER *) );
 
+/* XXX: Not referenced for now */
+UNREFERENCED_PARAMETER(natInfo);
+
 /* Retrieve the Conntrack Key related fields from packet */
 OvsCtSetupLookupCtx(key, zone, , curNbl, layers->l4Offset);
 
@@ -699,9 +703,10 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
 PCHAR helper = NULL;
-
+NAT_ACTION_INFO natActionInfo;
 NDIS_STATUS status;
 
+memset(, 0, sizeof natActionInfo);
 status = OvsDetectCtPacket(key);
 if (status != NDIS_STATUS_SUCCESS) {
 return status;
@@ -723,6 +728,68 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 if (ctAttr) {
 labels = NlAttrGet(ctAttr);
 }
+natActionInfo.natAction = NAT_ACTION_NONE;
+ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_NAT);
+if (ctAttr) {
+/* Pares Nested NAT attributes. */
+PNL_ATTR natAttr;
+unsigned int left;
+BOOLEAN hasMinIp = FALSE;
+BOOLEAN hasMinPort = FALSE;
+BOOLEAN hasMaxIp = FALSE;
+BOOLEAN hasMaxPort = FALSE;
+NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) {
+enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr);
+switch(sub_type_nest) {
+case OVS_NAT_ATTR_SRC:
+case OVS_NAT_ATTR_DST:
+natActionInfo.natAction |=
+((sub_type_nest == OVS_NAT_ATTR_SRC)
+? NAT_ACTION_SRC : NAT_ACTION_DST);
+break;
+case OVS_NAT_ATTR_IP_MIN:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMinIp = TRUE;
+break;
+case OVS_NAT_ATTR_IP_MAX:
+memcpy(,
+   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
+hasMaxIp = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MIN:
+natActionInfo.minPort = NlAttrGetU16(natAttr);
+hasMinPort = TRUE;
+break;
+case OVS_NAT_ATTR_PROTO_MAX:
+natActionInfo.maxPort = NlAttrGetU16(natAttr);
+hasMaxPort = TRUE;
+break;
+case OVS_NAT_ATTR_PERSISTENT:
+case OVS_NAT_ATTR_PROTO_HASH:
+case OVS_NAT_ATTR_PROTO_RANDOM:
+break;
+}
+}
+if (natActionInfo.natAction == NAT_ACTION_NONE) {
+natActionInfo.natAction = NAT_ACTION_REVERSE;
+}
+if (hasMinIp && !hasMaxIp) {
+memcpy(,
+   ,
+   sizeof(natActionInfo.maxAddr));
+}
+if (hasMinPort && !hasMaxPort) {
+natActionInfo.maxPort = natActionInfo.minPort;
+}
+if (hasMinPort || hasMaxPort) {
+if (natActionInfo.natAction & NAT_ACTION_SRC) {
+natActionInfo.natAction |= NAT_ACTION_SRC_PORT;
+} else if (natActionInfo.natAction & NAT_ACTION_DST) {
+natActionInfo.natAction |= NAT_ACTION_DST_PORT;
+}
+}
+}
 ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_HELPER);
 if (ctAttr) {
 helper = NlAttrGetString(ctAttr);
@@ -736,7 +803,7 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 }
 
 status = OvsCtExecute_(curNbl, key, layers,
-   commit, zone, mark, labels, helper);
+   commit, zone, mark, labels, helper, );
 return status;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.h 
b/datapath-windows/ovsext/Conntrack.h
index af99885..875c434 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -67,6 +67,15 @@ typedef struct MD_LABELS {
 struct ovs_key_ct_labels mask;
 } MD_LABELS;
 
+typedef enum NAT_ACTION {
+NAT_ACTION_NONE = 0,
+NAT_ACTION_REVERSE = 1 << 0,
+NAT_ACTI

Re: [ovs-dev] [PATCH 1/3] datapath-windows: Add support for NAT in conntrack

[Yin Lin]

Not sure why author field is dropped during "git send-email". But the
original author of this patch is "Anand Kumar<kumaran...@vmware.com>".
Kudos to Anand!

Thanks,
Yin

On Mon, Feb 27, 2017 at 2:13 PM, Yin Lin <li...@vmware.com> wrote:

> Add support for parsing netlink attributes related to NAT
> in conntrack.
>
> Signed-off-by: Yin Lin <li...@vmware.com>
> ---
>  datapath-windows/ovsext/Conntrack.c | 73 ++
> +--
>  datapath-windows/ovsext/Conntrack.h | 17 +
>  datapath-windows/ovsext/Flow.c  |  4 +-
>  3 files changed, 89 insertions(+), 5 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Conntrack.c
> b/datapath-windows/ovsext/Conntrack.c
> index d1be480..8a87dce 100644
> --- a/datapath-windows/ovsext/Conntrack.c
> +++ b/datapath-windows/ovsext/Conntrack.c
> @@ -628,7 +628,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
>UINT16 zone,
>MD_MARK *mark,
>MD_LABELS *labels,
> -  PCHAR helper)
> +  PCHAR helper,
> +  PNAT_ACTION_INFO natInfo)
>  {
>  NDIS_STATUS status = NDIS_STATUS_SUCCESS;
>  POVS_CT_ENTRY entry = NULL;
> @@ -637,6 +638,9 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
>  UINT64 currentTime;
>  NdisGetCurrentSystemTime((LARGE_INTEGER *) );
>
> +/* XXX: Not referenced for now */
> +UNREFERENCED_PARAMETER(natInfo);
> +
>  /* Retrieve the Conntrack Key related fields from packet */
>  OvsCtSetupLookupCtx(key, zone, , curNbl, layers->l4Offset);
>
> @@ -699,9 +703,10 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
>  MD_MARK *mark = NULL;
>  MD_LABELS *labels = NULL;
>  PCHAR helper = NULL;
> -
> +NAT_ACTION_INFO natActionInfo;
>  NDIS_STATUS status;
>
> +memset(, 0, sizeof natActionInfo);
>  status = OvsDetectCtPacket(key);
>  if (status != NDIS_STATUS_SUCCESS) {
>  return status;
> @@ -723,6 +728,68 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
>  if (ctAttr) {
>  labels = NlAttrGet(ctAttr);
>  }
> +natActionInfo.natAction = NAT_ACTION_NONE;
> +ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_NAT);
> +if (ctAttr) {
> +/* Pares Nested NAT attributes. */
> +PNL_ATTR natAttr;
> +unsigned int left;
> +BOOLEAN hasMinIp = FALSE;
> +BOOLEAN hasMinPort = FALSE;
> +BOOLEAN hasMaxIp = FALSE;
> +BOOLEAN hasMaxPort = FALSE;
> +NL_NESTED_FOR_EACH_UNSAFE (natAttr, left, ctAttr) {
> +enum ovs_nat_attr sub_type_nest = NlAttrType(natAttr);
> +switch(sub_type_nest) {
> +case OVS_NAT_ATTR_SRC:
> +case OVS_NAT_ATTR_DST:
> +natActionInfo.natAction |=
> +((sub_type_nest == OVS_NAT_ATTR_SRC)
> +? NAT_ACTION_SRC : NAT_ACTION_DST);
> +break;
> +case OVS_NAT_ATTR_IP_MIN:
> +memcpy(,
> +   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
> +hasMinIp = TRUE;
> +break;
> +case OVS_NAT_ATTR_IP_MAX:
> +memcpy(,
> +   NlAttrData(natAttr), natAttr->nlaLen - NLA_HDRLEN);
> +hasMaxIp = TRUE;
> +break;
> +case OVS_NAT_ATTR_PROTO_MIN:
> +natActionInfo.minPort = NlAttrGetU16(natAttr);
> +hasMinPort = TRUE;
> +break;
> +case OVS_NAT_ATTR_PROTO_MAX:
> +natActionInfo.maxPort = NlAttrGetU16(natAttr);
> +hasMaxPort = TRUE;
> +break;
> +case OVS_NAT_ATTR_PERSISTENT:
> +case OVS_NAT_ATTR_PROTO_HASH:
> +case OVS_NAT_ATTR_PROTO_RANDOM:
> +break;
> +}
> +}
> +if (natActionInfo.natAction == NAT_ACTION_NONE) {
> +natActionInfo.natAction = NAT_ACTION_REVERSE;
> +}
> +if (hasMinIp && !hasMaxIp) {
> +memcpy(,
> +   ,
> +   sizeof(natActionInfo.maxAddr));
> +}
> +if (hasMinPort && !hasMaxPort) {
> +natActionInfo.maxPort = natActionInfo.minPort;
> +}
> +if (hasMinPort || hasMaxPort) {
> +if (natActionInfo.natAction & NAT_ACTION_SRC) {
> +natActionInfo.natAction |= NAT_ACTION_SRC_PORT;
> +} else if (natActionInfo.natAction & NAT_ACTION_DST) {
> +natActionInfo.natAction |= NAT_ACTION_DST_PORT;
> +}
>

[ovs-dev] [PATCH 3/3] datapath-windows: NAT integration with conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/ovsext/Actions.c  | 158 ++---
 datapath-windows/ovsext/Actions.h  |  77 ++
 datapath-windows/ovsext/Conntrack.c| 178 -
 datapath-windows/ovsext/Conntrack.h|  25 +++--
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 5 files changed, 303 insertions(+), 137 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 46f84bc..9f4a22d 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -71,63 +71,6 @@ typedef struct _OVS_ACTION_STATS {
 OVS_ACTION_STATS ovsActionStats;
 
 /*
- * There a lot of data that needs to be maintained while executing the pipeline
- * as dictated by the actions of a flow, across different functions at 
different
- * levels. Such data is put together in a 'context' structure. Care should be
- * exercised while adding new members to the structure - only add ones that get
- * used across multiple stages in the pipeline/get used in multiple functions.
- */
-typedef struct OvsForwardingContext {
-POVS_SWITCH_CONTEXT switchContext;
-/* The NBL currently used in the pipeline. */
-PNET_BUFFER_LIST curNbl;
-/* NDIS forwarding detail for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
-/* Array of destination ports for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
-/* send flags while sending 'curNbl' into NDIS. */
-ULONG sendFlags;
-/* Total number of output ports, used + unused, in 'curNbl'. */
-UINT32 destPortsSizeIn;
-/* Total number of used output ports in 'curNbl'. */
-UINT32 destPortsSizeOut;
-/*
- * If 'curNbl' is not owned by OVS, they need to be tracked, if they need 
to
- * be freed/completed.
- */
-OvsCompletionList *completionList;
-/*
- * vport number of 'curNbl' when it is passed from the PIF bridge to the 
INT
- * bridge. ie. during tunneling on the Rx side.
- */
-UINT32 srcVportNo;
-
-/*
- * Tunnel key:
- * - specified in actions during tunneling Tx
- * - extracted from an NBL during tunneling Rx
- */
-OvsIPv4TunnelKey tunKey;
-
-/*
- * Tunneling - Tx:
- * To store the output port, when it is a tunneled port. We don't foresee
- * multiple tunneled ports as outport for any given NBL.
- */
-POVS_VPORT_ENTRY tunnelTxNic;
-
-/*
- * Tunneling - Rx:
- * Points to the Internal port on the PIF Bridge, if the packet needs to be
- * de-tunneled.
- */
-POVS_VPORT_ENTRY tunnelRxNic;
-
-/* header information */
-OVS_PACKET_HDR_INFO layers;
-} OvsForwardingContext;
-
-/*
  * --
  * OvsInitForwardingCtx --
  * Function to init/re-init the 'ovsFwdCtx' context as the actions pipeline
@@ -1388,7 +1331,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_udp *udpAttr)
 {
@@ -1435,7 +1378,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  *  based on the specified key.
  *
  */
-static __inline NDIS_STATUS
+NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
   const struct ovs_key_tcp *tcpAttr)
 {
@@ -1474,11 +1417,104 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
 /*
  *
  * OvsUpdateIPv4Header --
+ *  Updates the source/destination IP field of IPv4 header in
+ *  ovsFwdCtx.curNbl inline based on the specified key.
+ *
+ */
+OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
+UINT32 newAddr, UINT16 newPort,
+BOOLEAN isSource)
+{
+PUINT8 bufferStart;
+UINT32 hdrSize;
+OVS_PACKET_HDR_INFO *layers = >layers;
+IPHdr *ipHdr;
+TCPHdr *tcpHdr = NULL;
+UDPHdr *udpHdr = NULL;
+UINT32 *addrField = NULL;
+UINT16 *portField = NULL;
+UINT16 *checkField = NULL;
+
+ASSERT(layers->value != 0);
+
+if (layers->isTcp || layers->isUdp) {
+hdrSize = layers->l4Offset +
+  layers->isTcp ? sizeof (*tcpHdr) : sizeof (*udpHdr);
+} else {
+hdrSize = layers->l3Offset + sizeof (*ipHdr);
+}
+
+bufferStart = OvsGetHeaderBySize(ovsFwdCtx, hdrSize);
+if (!bufferStart) {
+return NDIS_STATUS_RESOURCES;
+}
+
+ipHdr = (IPHdr *)(bufferStart + layers->l3Offset);
+
+if (la

[ovs-dev] [PATCH 2/3] datapath-windows: Add NAT module in conntrack

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-nat.c | 436 
 datapath-windows/ovsext/Conntrack-nat.h |  39 +++
 3 files changed, 477 insertions(+)
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
 create mode 100644 datapath-windows/ovsext/Conntrack-nat.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 53983ae..296e785 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -16,7 +16,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
datapath-windows/ovsext/Conntrack-related.c \
+datapath-windows/ovsext/Conntrack-nat.c \
datapath-windows/ovsext/Conntrack-tcp.c \
+datapath-windows/ovsext/Conntrack-nat.h \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
b/datapath-windows/ovsext/Conntrack-nat.c
new file mode 100644
index 000..d34481c
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-nat.c
@@ -0,0 +1,436 @@
+#include "Conntrack-nat.h"
+#include "Jhash.h"
+
+PLIST_ENTRY ovsNatTable = NULL;
+PLIST_ENTRY ovsUnNatTable = NULL;
+static PNDIS_RW_LOCK_EX ovsNatLock;
+
+/*
+ *---
+ * OvsHashNatKey
+ * Hash NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline UINT32
+OvsHashNatKey(const OVS_CT_KEY *key)
+{
+UINT32 hash = 0;
+#define HASH_ADD(field) \
+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
+
+HASH_ADD(src.addr.ipv4_aligned);
+HASH_ADD(dst.addr.ipv4_aligned);
+HASH_ADD(src.port);
+HASH_ADD(dst.port);
+HASH_ADD(zone);
+#undef HASH_ADD
+return hash;
+}
+
+/*
+ *---
+ * OvsNatKeyAreSame
+ * Compare NAT related fields in a Conntrack key.
+ *---
+ */
+static __inline BOOLEAN
+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
+{
+// XXX: Compare IPv6 key as well
+#define FIELD_COMPARE(field) \
+if (key1->field != key2->field) return FALSE
+
+FIELD_COMPARE(src.addr.ipv4_aligned);
+FIELD_COMPARE(dst.addr.ipv4_aligned);
+FIELD_COMPARE(src.port);
+FIELD_COMPARE(dst.port);
+FIELD_COMPARE(zone);
+return TRUE;
+#undef FIELD_COMPARE
+}
+
+/*
+ *---
+ * OvsNaGetBucket
+ * Returns the row of NAT table that has the same hash as the given NAT
+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
+ * instead.
+ *---
+ */
+static __inline PLIST_ENTRY
+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
+{
+uint32_t hash = OvsHashNatKey(key);
+if (isReverse) {
+return [hash & NAT_HASH_TABLE_MASK];
+} else {
+return [hash & NAT_HASH_TABLE_MASK];
+}
+}
+
+/*
+ *---
+ * OvsNatInit
+ * Initialize NAT related resources.
+ *---
+ */
+NTSTATUS OvsNatInit(POVS_SWITCH_CONTEXT context)
+{
+ASSERT(ovsNatTable == NULL);
+
+/* Init the sync-lock */
+ovsNatLock = NdisAllocateRWLock(context->NdisFilterHandle);
+if (ovsNatLock == NULL) {
+goto failNoMem;
+}
+
+/* Init the Hash Buffer */
+ovsNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsNatTable == NULL) {
+goto freeNatLock;
+}
+
+ovsUnNatTable = OvsAllocateMemoryWithTag(
+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
+OVS_CT_POOL_TAG);
+if (ovsUnNatTable == NULL) {
+goto freeNatTable;
+}
+
+for (int i = 0; i < NAT_HASH_TABLE_SIZE; i++) {
+InitializeListHead([i]);
+InitializeListHead([i]);
+}
+return STATUS_SUCCESS;
+
+freeNatTable:
+OvsFreeMemoryWithTag(ovsNatTable, OVS_CT_POOL_TAG);
+freeNatLock:
+NdisFreeRWLock(ovsNatLock);
+failNoMem:
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/*
+ *
+ * OvsNatFlush
+ * Flushes out all NAT entries that match the given zone.
+ *
+ */
+VOID OvsNatFlush(UINT16 zone)
+{
+LOCK_STATE_EX lockState;
+PLIST_ENTRY link, next;
+NdisAcquireRWLockWrite(ovsNatLock, ,

[ovs-dev] [PATCH] Windows: Implement Hyper-V VIF discovery agent.

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 windows/OvsDiscoveryAgent/App.config   |  18 ++
 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj |  83 
 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln|  34 +++
 windows/OvsDiscoveryAgent/OvsDiscoveryService.cs   |  67 ++
 windows/OvsDiscoveryAgent/OvsSwitchMonitor.cs  |  73 +++
 windows/OvsDiscoveryAgent/OvsVsctl.cs  |  84 
 windows/OvsDiscoveryAgent/Program.cs   |  26 +++
 .../OvsDiscoveryAgent/Properties/AssemblyInfo.cs   |  36 
 .../Properties/Settings.Designer.cs|  38 
 .../OvsDiscoveryAgent/Properties/Settings.settings |   9 +
 windows/OvsDiscoveryAgent/VirtualAdapter.cs|  36 
 windows/OvsDiscoveryAgent/VirtualAdapterManager.cs | 228 +
 windows/OvsDiscoveryAgent/VirtualAdapterMonitor.cs | 107 ++
 windows/OvsDiscoveryAgent/WmiMonitor.cs| 193 +
 windows/OvsDiscoveryAgent/WqlCondition.cs  | 168 +++
 windows/OvsDiscoveryAgent/WqlHelper.cs | 132 
 windows/OvsDiscoveryAgent/WqlObject.cs | 133 
 windows/automake.mk|  21 ++
 18 files changed, 1486 insertions(+)
 create mode 100644 windows/OvsDiscoveryAgent/App.config
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryService.cs
 create mode 100644 windows/OvsDiscoveryAgent/OvsSwitchMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/OvsVsctl.cs
 create mode 100644 windows/OvsDiscoveryAgent/Program.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/AssemblyInfo.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/Settings.Designer.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/Settings.settings
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapter.cs
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapterManager.cs
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapterMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/WmiMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlCondition.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlHelper.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlObject.cs

diff --git a/windows/OvsDiscoveryAgent/App.config 
b/windows/OvsDiscoveryAgent/App.config
new file mode 100644
index 000..f2c7a15
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/App.config
@@ -0,0 +1,18 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+ovs-int
+
+
+
+
\ No newline at end of file
diff --git a/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj 
b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
new file mode 100644
index 000..ee709d3
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
@@ -0,0 +1,83 @@
+
+http://schemas.microsoft.com/developer/msbuild/2003;>
+  
+  
+Debug
+AnyCPU
+{2563C1BE-B240-4F63-84C5-01D98D015A3F}
+Exe
+Properties
+OvsDiscoveryAgent
+OvsDiscoveryAgent
+v4.5
+512
+true
+  
+  
+AnyCPU
+true
+full
+false
+bin\Debug\
+DEBUG;TRACE
+prompt
+4
+  
+  
+AnyCPU
+pdbonly
+true
+bin\Release\
+TRACE
+prompt
+4
+  
+  
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+  True
+  True
+  Settings.settings
+
+
+
+  Component
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+  SettingsSingleFileGenerator
+  Settings.Designer.cs
+
+  
+  
+  
+
\ No newline at end of file
diff --git a/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln 
b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
new file mode 100644
index 000..2fe2c81
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
@@ -0,0 +1,34 @@
+
+Microsoft Visual Studio Solution File, Format Version 12.00
+# Visual Studio 14
+VisualStudioVersion = 14.0.25123.0
+MinimumVisualStudioVersion = 10.0.40219.1
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "OvsDiscoveryAgent", 
"OvsDiscoveryAgent.csproj", "{2563C1BE-B240-4F63-84C5-01D98D015A3F}"
+EndProject
+Global
+   GlobalSection(SolutionConfigurationPlatforms) = preSolution
+   Debug|Any CPU = Debug|Any CPU
+   Debug|x64 = Debug|x64
+   Debug|x86 = Debug|x86
+   Release|Any CPU = Release|Any CPU
+   Release|x64 = Release|x64
+   Release|x86 = Release|x86
+   EndGlobalSection
+   GlobalSection(ProjectConfigurationPlatforms) = postSolution
+   {2563C1BE-B240-4F63-84C5-01D98D015A3F}.Debug|Any CPU.ActiveCfg 
= Debug|Any CPU
+   {2563C1BE-B

[ovs-dev] [PATCH v2] Windows: Implement Hyper-V VIF discovery agent.

[Yin Lin]

Signed-off-by: Yin Lin <li...@vmware.com>
---
 windows/OvsDiscoveryAgent/App.config   |  18 ++
 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj |  83 
 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln|  34 +++
 windows/OvsDiscoveryAgent/OvsDiscoveryService.cs   |  67 ++
 windows/OvsDiscoveryAgent/OvsSwitchMonitor.cs  |  73 +++
 windows/OvsDiscoveryAgent/OvsVsctl.cs  |  82 
 windows/OvsDiscoveryAgent/Program.cs   |  26 +++
 .../OvsDiscoveryAgent/Properties/AssemblyInfo.cs   |  36 
 .../Properties/Settings.Designer.cs|  38 
 .../OvsDiscoveryAgent/Properties/Settings.settings |   9 +
 windows/OvsDiscoveryAgent/VirtualAdapter.cs|  36 
 windows/OvsDiscoveryAgent/VirtualAdapterManager.cs | 228 +
 windows/OvsDiscoveryAgent/VirtualAdapterMonitor.cs | 109 ++
 windows/OvsDiscoveryAgent/WmiMonitor.cs| 193 +
 windows/OvsDiscoveryAgent/WqlCondition.cs  | 168 +++
 windows/OvsDiscoveryAgent/WqlHelper.cs | 130 
 windows/OvsDiscoveryAgent/WqlObject.cs | 133 
 windows/automake.mk|  21 ++
 18 files changed, 1484 insertions(+)
 create mode 100644 windows/OvsDiscoveryAgent/App.config
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryService.cs
 create mode 100644 windows/OvsDiscoveryAgent/OvsSwitchMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/OvsVsctl.cs
 create mode 100644 windows/OvsDiscoveryAgent/Program.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/AssemblyInfo.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/Settings.Designer.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/Settings.settings
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapter.cs
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapterManager.cs
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapterMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/WmiMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlCondition.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlHelper.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlObject.cs

diff --git a/windows/OvsDiscoveryAgent/App.config 
b/windows/OvsDiscoveryAgent/App.config
new file mode 100644
index 000..caf9613
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/App.config
@@ -0,0 +1,18 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+ovs-int
+
+
+
+
\ No newline at end of file
diff --git a/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj 
b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
new file mode 100644
index 000..24c69bb
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
@@ -0,0 +1,83 @@
+
+http://schemas.microsoft.com/developer/msbuild/2003;>
+  
+  
+Debug
+AnyCPU
+{2563C1BE-B240-4F63-84C5-01D98D015A3F}
+Exe
+Properties
+OvsDiscoveryAgent
+OvsDiscoveryAgent
+v4.5.2
+512
+true
+  
+  
+AnyCPU
+true
+full
+false
+bin\Debug\
+DEBUG;TRACE
+prompt
+4
+  
+  
+AnyCPU
+pdbonly
+true
+bin\Release\
+TRACE
+prompt
+4
+  
+  
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+  True
+  True
+  Settings.settings
+
+
+
+  Component
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+  SettingsSingleFileGenerator
+  Settings.Designer.cs
+
+  
+  
+  
+
\ No newline at end of file
diff --git a/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln 
b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
new file mode 100644
index 000..b3d6371
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
@@ -0,0 +1,34 @@
+
+Microsoft Visual Studio Solution File, Format Version 12.00
+# Visual Studio 14
+VisualStudioVersion = 14.0.25123.0
+MinimumVisualStudioVersion = 10.0.40219.1
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "OvsDiscoveryAgent", 
"OvsDiscoveryAgent.csproj", "{2563C1BE-B240-4F63-84C5-01D98D015A3F}"
+EndProject
+Global
+   GlobalSection(SolutionConfigurationPlatforms) = preSolution
+   Debug|Any CPU = Debug|Any CPU
+   Debug|x64 = Debug|x64
+   Debug|x86 = Debug|x86
+   Release|Any CPU = Release|Any CPU
+   Release|x64 = Release|x64
+   Release|x86 = Release|x86
+   EndGlobalSection
+   GlobalSection(ProjectConfigurationPlatforms) = postSolution
+   {2563C1BE-B240-4F63-84C5-01D98D015A3F}.Debug|Any CPU.ActiveCfg 
= Debug|Any CPU
+  

[ovs-dev] [PATCH v2] Windows: Implement Hyper-V VIF discovery agent.

[Yin Lin]

Add Makefile changes to build the daemon from console.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document

[Yin Lin]

Hi Alin,

Sorry for the late response. I somehow that the message slip by.

How do you use vswitchd/netlink to watch VIF creation/deletion and Hyper-V
switch changes? Note that through WMI, we monitor the following events:
1. VIF creation/deletion.
2. Hyper-V switch creation/deletion.
3. OVS extension enable/disable on a Hyper-V switch.

Also, note that we only register a callback with WMI so that we are
passively notified when a change we are interested in happens. We do not
run WMI calls in a loop.

If you have a better solution, do you mind elaborate a little more on your
idea?

Best regards,
Yin Lin

On Mon, Dec 5, 2016 at 12:05 PM, Alin Serdean <
aserd...@cloudbasesolutions.com> wrote:

> Sorry for the late reply. We had a few days of bank holiday last week.
>
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Tuesday, November 29, 2016 2:28 AM
> > To: Nithin Raju <nit...@vmware.com>
> > Cc: Yin Lin <yinli...@gmail.com>; d...@openvswitch.org; Alin Serdean
> > <aserd...@cloudbasesolutions.com>; Justin Pettit <jpet...@ovn.org>
> > Subject: Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document
> >
> > OK, I understand now.
> >
> > Having ovs-vswitchd itself add ports would be unprecedented.  Normally,
> we
> > depend on some part of the system integration to do that: libvirt does
> it in
> > modern KVM environments (as you say), a hook script does it on XenServer,
> > and so on.
> [Alin Serdean] I think we need to look at a bigger picture on what we are
> trying to achieve.
> AFAIK, in the case of libvirt/Xen, someone asks them to create an adapter
> and after it is created the result is added to OVSDB.
> On Windows, someone asks vmms (https://technet.microsoft.
> com/en-us/library/dd582295(v=ws.10).aspx) to create a port on a switch,
> rename the port which was created, and after add it to OVSDB afterwards.
> In the case of Windows+OpenStack (with or without OVN) things are already
> handled since the code for port renaming is already there. If someone wants
> to integrate it in his solution, he could use/reuse/implement the code in
> the powershell script which is in our repository (1).
> This daemon is targeted for unexperienced users which do not want/do not
> know how to do the extra step of renaming the port name. This will give us
> better user experience.
> The reasons I would like to add the functionality in vswitchd are
> simplicity and speed (we see the port creation in the windows datapath,
> after it was created by vmms, and during an upcall we could add the port).
> >
> > My preference would be to keep these details of the system integration
> > separate from ovs-vswitchd, since it matches the implementation
> > elsewhere.  I'd expect this to be a pretty simple daemon, which probably
> > wouldn't use much CPU or memory.
> >
> [Alin Serdean] My main problem with the current implementation is that WMI
> calls are slow. Using vswitchd or another monitor that reuses the netlink
> implementation would be better IMO.
>
> (1) https://github.com/openvswitch/ovs/blob/master/
> datapath-windows/misc/OVS.psm1
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp and tcp header

[Yin Lin]

Can we decide if tcp and icmp is null in OvsConntrackValidateTcpPacket? It
makes the function more complete and safer by itself.

On Mon, Nov 28, 2016 at 6:11 AM, Alin Serdean <
aserd...@cloudbasesolutions.com> wrote:

> This patch checks if the TCP or ICMP header exists before trying to use
> them.
>
> The issue was found using the driver under low resources.
>
> Signed-off-by: Alin Gabriel Serdean 
> ---
>  datapath-windows/ovsext/Conntrack.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Conntrack.c
> b/datapath-windows/ovsext/Conntrack.c
> index e663c3b..56a7cbc 100644
> --- a/datapath-windows/ovsext/Conntrack.c
> +++ b/datapath-windows/ovsext/Conntrack.c
> @@ -194,7 +194,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
>  TCPHdr tcpStorage;
>  const TCPHdr *tcp;
>  tcp = OvsGetTcp(curNbl, l4Offset, );
> -if (!OvsConntrackValidateTcpPacket(tcp)) {
> +if (!tcp || !OvsConntrackValidateTcpPacket(tcp)) {
>  goto invalid;
>  }
>
> @@ -215,7 +215,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
>  ICMPHdr storage;
>  const ICMPHdr *icmp;
>  icmp = OvsGetIcmp(curNbl, l4Offset, );
> -if (!OvsConntrackValidateIcmpPacket(icmp)) {
> +if (!icmp || !OvsConntrackValidateIcmpPacket(icmp)) {
>  goto invalid;
>  }
>
> --
> 2.10.2.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

[ovs-dev] Windows: Implement Hyper-V VIF discovery agent

[Yin Lin]

Create a VIF discovery daemon to tag Hyper-V adapters connected to OVS
switch
and add/delete OVS port correspondingly.

Signed-off-by: Yin Lin <li...@vmware.com>
---
 windows/OvsDiscoveryAgent/App.config   |  18 ++
 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj |  83 
 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln|  34 +++
 windows/OvsDiscoveryAgent/OvsDiscoveryService.cs   |  67 ++
 windows/OvsDiscoveryAgent/OvsSwitchMonitor.cs  |  73 +++
 windows/OvsDiscoveryAgent/OvsVsctl.cs  |  82 
 windows/OvsDiscoveryAgent/Program.cs   |  26 +++
 .../OvsDiscoveryAgent/Properties/AssemblyInfo.cs   |  36 
 .../Properties/Settings.Designer.cs|  38 
 .../OvsDiscoveryAgent/Properties/Settings.settings |   9 +
 windows/OvsDiscoveryAgent/VirtualAdapter.cs|  36 
 windows/OvsDiscoveryAgent/VirtualAdapterManager.cs | 228
+
 windows/OvsDiscoveryAgent/VirtualAdapterMonitor.cs | 109 ++
 windows/OvsDiscoveryAgent/WmiMonitor.cs| 193 +
 windows/OvsDiscoveryAgent/WqlCondition.cs  | 168 +++
 windows/OvsDiscoveryAgent/WqlHelper.cs | 130 
 windows/OvsDiscoveryAgent/WqlObject.cs | 133 
 17 files changed, 1463 insertions(+)
 create mode 100644 windows/OvsDiscoveryAgent/App.config
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryService.cs
 create mode 100644 windows/OvsDiscoveryAgent/OvsSwitchMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/OvsVsctl.cs
 create mode 100644 windows/OvsDiscoveryAgent/Program.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/AssemblyInfo.cs
 create mode 100644
windows/OvsDiscoveryAgent/Properties/Settings.Designer.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/Settings.settings
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapter.cs
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapterManager.cs
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapterMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/WmiMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlCondition.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlHelper.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlObject.cs

diff --git a/windows/OvsDiscoveryAgent/App.config
b/windows/OvsDiscoveryAgent/App.config
new file mode 100644
index 000..caf9613
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/App.config
@@ -0,0 +1,18 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+ovs-int
+
+
+
+
\ No newline at end of file
diff --git a/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
new file mode 100644
index 000..24c69bb
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
@@ -0,0 +1,83 @@
+
+http://schemas.microsoft.com/developer/msbuild/2003;>
+  
+  
+Debug
+AnyCPU
+{2563C1BE-B240-4F63-84C5-01D98D015A3F}
+Exe
+Properties
+OvsDiscoveryAgent
+OvsDiscoveryAgent
+v4.5.2
+512
+true
+  
+  
+AnyCPU
+true
+full
+false
+bin\Debug\
+DEBUG;TRACE
+prompt
+4
+  
+  
+AnyCPU
+pdbonly
+true
+bin\Release\
+TRACE
+prompt
+4
+  
+  
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+  True
+  True
+  Settings.settings
+
+
+
+  Component
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+  SettingsSingleFileGenerator
+  Settings.Designer.cs
+
+  
+  
+  
+
\ No newline at end of file
diff --git a/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
new file mode 100644
index 000..b3d6371
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
@@ -0,0 +1,34 @@
+
+Microsoft Visual Studio Solution File, Format Version 12.00
+# Visual Studio 14
+VisualStudioVersion = 14.0.25123.0
+MinimumVisualStudioVersion = 10.0.40219.1
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "OvsDiscoveryAgent",
"OvsDiscoveryAgent.csproj", "{2563C1BE-B240-4F63-84C5-01D98D015A3F}"
+EndProject
+Global
+ GlobalSection(SolutionConfigurationPlatforms) = preSolution
+ Debug|Any CPU = Debug|Any CPU
+ Debug|x64 = Debug|x64
+ Debug|x86 = Debug|x86
+ Release|Any CPU = Release|Any CPU
+ Release|x64 = Release|x64
+ Release|x86 = Release|x86
+ EndGlobalSection
+ GlobalSection(ProjectConfigurationPlatforms) = postSolution
+ {2563C1BE-B240-4F63-84C5-01D98D015A3F}.Debug|Any CPU.ActiveCfg =
Debug|Any CPU
+ {2563C1BE-B240-4F63-84C5-01D98D015A3F}.Debug|Any CPU.Build.0 = Debug|Any
CPU
+

[ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document

[Yin Lin]

Hi,

Below is a document that describes the design and implementation of VIF
discovery agent for Hyper-V that I have been working on. The coding has
already been completed and will be sent out for review in a follow up
patch. The document describes the effort and the choices made. Thanks
Sairam Venugopal for the initial review and edit of the document



Please have a look, and get in touch for any questions or comments.



Thanks!

-- Yin



===


   OVS-Hyper-V-Discovery-Agent Design Document

   ==

On Hyper-V, virtual switches and virtual adapters are created and managed
by Windows infrastructure. This is currently not integrated with OVS. When
a new virtual adapter is connected to a virtual switch with OVS extension
enabled, users need to manually tag it with an OVS port name and manually
use OVS userspace utilities such as ovs-vsctl to add a corresponding OVS
port.



This document describes design of a VIF discovery agent for OVS on Hyper-V
that functions as a counterpart to libvirt on KVM. The agent monitors
virtual switch/adapter modifications, tags the adapter and adds/deletes OVS
port as necessary to automate the VIF creation/deletion process.



The rest of this document describes:

1. Background of OVS port management on Hyper-V

2. Discovery agent functional specification

3. Implementation of discovery agent

4. Build/Deployment environment



For more questions, please contact dev at openvswitch.org



1) Background of OVS port management on Hyper-V



OVS kernel module on Hyper-V[3] is implemented as a extension [2] to the
virtual extensible switch[1] provided by Microsoft. The extension can be
enabled through Hyper-V Virtual Switch Manager GUI or Powershell command
once the OVS driver is installed.



The OVS kernel module recognizes any adapter that connects to such a switch
by requiring user to tag the Hyper-V adapter with an OVS port name and then
add the OVS port to ovsdb with ovs-vsctl command. A Powershell module
called OVS.psm1 has been implemented to facilitate the tagging. Once the
module is imported, admin can use the following command to tag a virtual
adapter with OVS port name:



$vnic | Set-VMNetworkAdapterOVSPort -OVSPortName $vifName



where $vnic is a VMNetworkAdapter instance and $vifName is the OVS port
name. Then the admin needs to add the OVS port by using:



ovs-vsctl add-port $bridge $vifName --set interface $vifName
external_ids:vm-id=$vmId \

external_ids:iface-status=active



where $bridge is the integration bridge name and $vmId is the Hyper-V VM
UUID. The VM ID is currently optional for reporting purposes and will not
affect the actual functionality of the OVS port.



In order to automate this process, the discovery agent needs to actively
monitor two events:

1. Creation/Deletion of a virtual switch that has OVS extension enabled.
This includes

enable/disable-ment of OVS extension on an existing virtual switch.

2. Connection/Disconnection of a VIF to OVS enabled virtual switch.



In order to capture these events, we utilize Windows Management
Instrumentation (WMI)[4] to register a callback [5] when a desired change
in virtualization namespace happens. WMI is also used to perform a full
sync of all existing virtual switches and virtual adapters during the
discovery agent's initial boot.



2) Discovery agent functional specification

---

The discovery agent performs the following three basic duties:

1. Tag(). Tag a virtual adapter with OVS port, by setting its "ElementName"
field in the WMI table "Msvm_EthernetPortAllocationSettingData" to an OVS
port name, such that it can be picked up by OVS kernel module and matched
against a corresponding entry in ovsdb. By default, the OVS port name of a
virtual adapter assigned by the discovery agent is the same as its UUID
assigned by Windows.

2. AddPort(). Add an OVS port to OVS bridge. If a virtual adapter is
connected to an OVS switch but it's missing in ovsdb, the daemon adds it to
a preconfigured integration bridge, as specified by the admin..

3. DelPort(). Delete an OVS port from the OVS integration bridge.



When the discovery agent starts, it scans all virtual adapters that are
connected to the virtual switch with OVS extension enabled. , It runs
 AddPort() for  adapters that are missing  in the preconfigured OVS
integration bridge.



After that, the discovery agent actively monitors the system for any
relevant virtual switch/adapter changes and performs the following actions:

1. When OVS extension is enabled on a virtual switch, perform AddPort() for
all adapters connected to the virtual switch.

2. When a virtual switch with OVS extension is deleted, or when OVS
extension is disabled on a virtual switch, perform DelPort() for all
adapters previously connected to the virtual switch.

3. When a virtual adapter is created