[ovs-dev] You have been nominated for the £2 million charity support

[Barrister Bobby Smith]

You have been nominated for the charity program to support Prince Alwaleed Bin 
Tala, please contact:
http://bit.do/23awsa
for more information.

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

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

Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

[Darrell Ball]

It might be good to add a test that checks that “all the fields” associated 
with tunnel header can be
matched in the underlay bridge context
I did a quick test with your patch:
ip_proto/nw_proto was fine and matching GRE
I could also successfully match on ip_dst/nw_dst but not on ip_src/nw_src, for 
example.



On 5/9/17, 4:55 AM, "ovs-dev-boun...@openvswitch.org on behalf of Zoltán 
Balogh"  wrote:

Hi,

I created a patch that updates flow and base_flow L2/L3 members when 
pushing packet to tunnel, and fixes statistics on underlay bridge. 
I posted it to the original thread:

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DMay_332265.html=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=lNfYVOoPGZRl8beVCPBv23EX5aIqrWrIRcpodjAIkwE=6UWX4CTnpSwFgj4kj0bhBaDTfh5fldH_61-lbc9AG8U=
 

As I see, you have not reverted the "Avoid recirculation" patch yet. If my 
new patch is accepted then it's not needed to revert the original commit. Could 
you have a look at the new patch, please?

If you are going to revert the original patch, then I'm going to merge the 
new patch with the original one and resend it to the list.

Best regards,
Zoltan


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
[mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Joe Stringer
> Sent: Tuesday, May 09, 2017 5:41 AM
> To: William Tu 
> Cc: ovs-dev 
> Subject: Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation 
on datapath."
> 
> On 8 May 2017 at 17:35, William Tu  wrote:
> > Hi Joe and Greg,
> >
> > Maybe it's better I put this revert tunneling patch (1/2) and its
> > tunnel-tests (2/2) in one patch, so the "make check" can pass?
> 
> They can be separate. It's currently broken, the revert will fix it.
> The test can be an independent submission. I'd rather not fold yet
> more changes into the revert.
> ___
> dev mailing list
> d...@openvswitch.org
> 
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=lNfYVOoPGZRl8beVCPBv23EX5aIqrWrIRcpodjAIkwE=x9d45zty5nZTMZsW3A2BSENnB72aAhk6Ed8hMQ3h_xA=
 
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=lNfYVOoPGZRl8beVCPBv23EX5aIqrWrIRcpodjAIkwE=x9d45zty5nZTMZsW3A2BSENnB72aAhk6Ed8hMQ3h_xA=
 






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

Re: [ovs-dev] [PATCH 2/2] sparse: Avoid warnings compiling stream-ssl.c.

[Darrell Ball]

I don’t see the Sparse complaint

I am using

dball@ubuntu:~/ovs$ openssl version
OpenSSL 1.0.2g  1 Mar 2016
dball@ubuntu:~/ovs$ uname -a
Linux ubuntu 4.4.0-75-generic #96-Ubuntu SMP Thu Apr 20 09:56:33 UTC 2017 
x86_64 x86_64 x86_64 GNU/Linux

The line number quoted in the commit message here
“/usr/include/openssl/e_os2.h:275:12: this was the original definition”
makes no sense for me

That being said, I cannot see a problem with the proposed change
and since we don’t understand the Sparse complaint, we can either
ignore it or spend more time in the slim hope we can understand it
and it turns out to be useful.

I think it is better to ignore it

Acked-by: Darrell Ball 



On 5/5/17, 9:39 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

This change avoids the following "sparse" warnings:

/usr/include/inttypes.h:105:10: warning: preprocessor token PRIu64 redefined
/usr/include/openssl/e_os2.h:275:12: this was the original definition

With this change, the build is again "sparse" clean.

I did not look into the details of this particular issue.  However, the
upshot is that including  before any OpenSSL header avoids the
warning and the opposite order provokes it.

Signed-off-by: Ben Pfaff 
---
 lib/dhparams.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/dhparams.h b/lib/dhparams.h
index 54f9c5fbc01b..9bf03e51ed7e 100644
--- a/lib/dhparams.h
+++ b/lib/dhparams.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008 Nicira, Inc.
+ * Copyright (c) 2008, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
 #ifndef DHPARAMS_H
 #define DHPARAMS_H 1
 
+#include 
 #include 
 
 DH *get_dh1024(void);
-- 
2.10.2

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

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=8bO30Qvgkz6yoS5Af-9gGrNcJcPgq-_wwIOJF8ynpdU=pf3zcGbDebqT24IX4YIPYGjLKoe6cNT04fIUw89Z5WU=
 


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

Re: [ovs-dev] ovn: SFC Patch V3

[John McDowall]

Mickey,

Thanks for the review. I need some help understanding a couple of things:


1)   The proposed change, I could see the previous logic where we inserted 
the flow back in the ingress pipeline just after the IN_CHAIN stage. The 
changes you suggest seem to imply that the action is still insert after the 
_IN_CHAIN stage but in the egress (OUT) pipeline. I am missing something here – 
can you give me some more info?

+for (int ii = 0; ii < MFF_N_LOG_REGS; ii++) {
+ds_put_format(, "reg%d = 0; ", ii);
+}
+ds_put_format(, "next(pipeline=ingress, table=%d); };",
+ovn_stage_get_table(S_SWITCH_IN_CHAIN) + 1);
+ovn_lflow_add(lflows, od, S_SWITCH_IN_CHAIN, ingress_inner_priority,
+lcc_match, ds_cstr());

Replace the line above by:

ovn_lflow_add(lflows, od, S_SWITCH_OUT_SFC_LOOPBACK, 100,
lcc_match, ds_cstr());




2)   I can try and put some checks in for loop avoidance. Can you think of 
scenarios that would cause this, a badly configured port-pair could perhaps 
cause it (if the eth egress of the port-pair was configured as the ingress 
eth.) Any other scenarios that come to mind ?

Regards

John
From: Mickey Spiegel 
Date: Monday, April 24, 2017 at 6:39 PM
To: John McDowall 
Cc: "ovs-dev@openvswitch.org" 
Subject: Re: [ovs-dev] ovn: SFC Patch V3


On Mon, Apr 24, 2017 at 12:56 PM, 
> wrote:
From: John McDowall 
>


Fixed changes from Mickey's last review.

Changes

1) Fixed re-circulation rules

Still a few modifications required. See comments inline. I just typed some 
stuff out, have not run, built, or tested anything.

2) Fixed match statement - match is only applied to beginnning of chain in
   each direction.
3) Fixed array length of chain of VNFs. I have tested thi sup to three VNFs
   in a chain and it looks like it works in both directions.

Areas to review

1) The logic now supports hair-pinnign the flow back to the original source to
   ensure that the MAC learnign problem is addressed. I tested this using
   ovn-trace - any better testing that I should do?

Current todo list

1) I have standalone tests need to add tests to ovs/ovn framework.
2) Load-balancing support for port-pair-groups
3) Publish more detailed examples.
4) Submit suggestions to change and shorted the CLI names.

Simple example using ovn-trace

#!/bin/sh
#
clear
ovn-nbctl ls-add swt1

ovn-nbctl lsp-add swt1 swt1-appc
ovn-nbctl lsp-add swt1 swt1-apps
ovn-nbctl lsp-add swt1 swt1-vnfp1
ovn-nbctl lsp-add swt1 swt1-vnfp2

ovn-nbctl lsp-set-addresses swt1-appc "00:00:00:00:00:01 192.168.33.1"
ovn-nbctl lsp-set-addresses swt1-apps "00:00:00:00:00:02 192.168.33.2"
ovn-nbctl lsp-set-addresses swt1-vnfp1 00:00:00:00:00:03
ovn-nbctl lsp-set-addresses swt1-vnfp2 00:00:00:00:00:04
#
# Configure Service chain
#
ovn-nbctl lsp-pair-add swt1 swt1-vnfp1 swt1-vnfp2 pp1
ovn-nbctl lsp-chain-add swt1 pc1
ovn-nbctl lsp-pair-group-add pc1 ppg1
ovn-nbctl lsp-pair-group-add-port-pair ppg1 pp1
ovn-nbctl lsp-chain-classifier-add swt1 pc1 swt1-appc "entry-lport" 
"bi-directional" pcc1
#
ovn-sbctl dump-flows
#
# Run trace command
printf "\n-Flow 1 -\n\n"
ovn-trace --detailed  swt1 'inport == "swt1-appc" && eth.src == 
00:00:00:00:00:01 && eth.dst == 00:00:00:00:00:02'
printf "\n-Flow 2 -\n\n"
ovn-trace --detailed  swt1 'inport == "swt1-vnfp1" && eth.src == 
00:00:00:00:00:01 && eth.dst == 00:00:00:00:00:02'
printf "\n-Flow 3 -\n\n"
ovn-trace --detailed  swt1 'inport == "swt1-apps" && eth.dst == 
00:00:00:00:00:01 && eth.src == 00:00:00:00:00:02'
printf "\n-Flow 4 -\n\n"
ovn-trace --detailed  swt1 'inport == "swt1-vnfp2" && eth.dst == 
00:00:00:00:00:01 && eth.src == 00:00:00:00:00:02'
#
# Cleanup
#
ovn-nbctl lsp-chain-classifier-del pcc1
ovn-nbctl lsp-pair-group-del ppg1
ovn-nbctl lsp-chain-del pc1
ovn-nbctl lsp-pair-del pp1
ovn-nbctl ls-del swt1

Reported at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2016-March/040381.html
Reported at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2016-May/041359.html

Signed-off-by: John 

[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 
---
 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->key.zone) {
+OvsNatDeleteEntry(entry);
+}
+}
+}
+}
+
+/*
+ 

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

[Yin Lin]

From: Anand Kumar 

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

Co-Authored-by: Anand Kumar 
Co-Authored-by: Darrell Ball 
Signed-off-by: Yin Lin 
---
 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, mark, labels, helper, 
);
 return status;
 }
 
diff --git 

[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 
---
 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.UdpChecksumFailed);
+}
+} else {
+addrField = >daddr;
+

Re: [ovs-dev] [PATCH] docs: Improve formatting for daemon options in a few manpages.

[Andy Zhou]

On Mon, May 8, 2017 at 7:43 AM, Ben Pfaff  wrote:
> daemon.man is meant to have a heading above it, but in a few manpages its
> text was running directly into the previous documentation because this had
> been overlooked.
>
> By adding .PP to daemon.man, we make this problem less severe if the
> heading is similarly omitted in future manpages, since at least it will
> then have its own paragraph instead of running into the previous one.
>
> Signed-off-by: Ben Pfaff 
Acked-by: Andy Zhou 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V8 00/26] Introducing HW offload support for openvswitch

[Joe Stringer]

On 9 May 2017 at 12:05, Flavio Leitner  wrote:
> On Sun, May 07, 2017 at 10:55:32AM +0300, Roi Dayan wrote:
>>
>>
>> On 03/05/2017 18:58, Federico Iezzi wrote:
>> > On Wed, May 3, 2017 at 5:07 PM, Roi Dayan  wrote:
>> > > This patch series introduces rule offload functionality to dpif-netlink
>> > > via netdev ports new flow offloading API. The user can specify whether to
>> > > enable rule offloading or not via OVS configuration. Netdev providers
>> > > are able to implement netdev flow offload API in order to offload rules.
>> > >
>> > > This patch series also implements one offload scheme for netdev-linux,
>> > > using TC flower classifier, which was chosen because its sort of natural
>> > > to state OVS DP rules for this classifier. However, the code can be
>> > > extended to support other classifiers such as U32, eBPF, etc which 
>> > > support
>> > > offload as well.
>> > >
>> > > The use-case we are currently addressing is the newly sriov switchdev 
>> > > mode
>> > > in the Linux kernel which was introduced in version 4.8.
>> > > This series was tested against sriov vfs vports representors of the
>> > > Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.
>> > >
>> > >
>> > > V7->V8
>> > > - Refactor dpif logging functions and use them in dpif-netlink
>> > > - Ignore internal devices from netdev hashmap
>> > > - Refactor netdev hmap naming to prefix netdev_ports_*
>> > > - Use single hashmap with 2 nodes for ufid/tc mapping
>> > > - Verify ifindex is valid in netdev_hmap_port_add
>> > > - Close netdev in netdev_tc_flow_get error flow
>> > > - Improve comments for flow offload api
>> > > - Reorder flow api output args to be last args
>> > > - Remove redundant netdev_flow_support
>> > > - Fix using uninitialized var 's'
>> > >
>> > > Not done:
>> > > refactor netdev_ports_* functions to accept a typed obj
>> > > (e.g. netdev_ports struct) instead of void*.
>> > > We can do it as a follow-up commit later.
>> > >
>> > > V6->V7:
>> > > - Fix L3 IPv4 matching got broken
>> > > - Refactor offloads test and testsuite to have same prefix
>> > > - Better handling of unsupported match attributes
>> > >
>> > > V5->V6:
>> > > - Rebase over master branch, fix compilation issue
>> > > - Add Nicira copyright to tc interface
>> > >
>> > > V4->V5:
>> > > - Fix compat
>> > > - Fix VXLAN IPv6 tunnel matching
>> > > - Fix order of actions in dump flows
>> > > - Update ovs-dpctl man page about the addtion of type to dump-flows
>> > >
>> > > Travis
>> > > https://travis-ci.org/roidayan/ovs/builds/213735371
>> > > AppVeyor
>> > > https://ci.appveyor.com/project/roidayan/ovs/build/1.0.18
>> > >
>> > > V3->V4:
>> > > - Move declarations to the right commit with implementation
>> > > - Fix tc_get_flower flow return false success
>> > > - Fix memory leaks - not releasing tc_transact replies
>> > > - Fix travis failure for OSX compilation
>> > > - Fix loop in dpif_netlink_flow_dump_next
>> > > - Fix declared default value for tc-policy in vswitch.xml
>> > > - Refactor loop in netdev_tc_flow_dump_next
>> > > - Add missing error checks in parse_flow_put
>> > > - Fix handling modify request where old rule is in hw and new
>> > >   rule is not supported and needs to be in sw.
>> > > - Use 2 hashmaps instead of 1 for faster reverse lookup of ufid from 
>> > > tc
>> > > - Init ports when enabling hw-offload after OVS is running
>> > >
>> > > TODO: Fix breaking of datapath compilation
>> > >   Fix testsuite failures
>> > >
>> > > Travis
>> > > https://travis-ci.org/roidayan/ovs/builds/210549325
>> > > AppVeyor
>> > > https://ci.appveyor.com/project/roidayan/ovs/build/1.0.15
>> > >
>> > > V2->V3:
>> > > - Code styling fixes
>> > > - Bug fixes
>> > > - Using already available macros/functions to match current OVS code
>> > > - Refactored code according to V2 review
>> > > - Replaced bool option skip-hw for string option tc-policy
>> > > - Added hw offload tests using policy skip_hw
>> > > - Fixed most compatability compiling issues
>> > > - Travis
>> > > https://travis-ci.org/roidayan/ovs/builds/199610124
>> > > - AppVeyor
>> > > https://ci.appveyor.com/project/roidayan/ovs/build/1.0.14
>> > > - Fixed compiling with DPDK enabled
>> > >
>> > > TODO:
>> > > - need to fix datapath compiling issues found in travis after adding 
>> > > tc
>> > >   compatability headers
>> > > - need to fix failing test cases because of get_ifindex
>> > >
>> > > V1->V2:
>> > > - Added generic netdev flow offloads API.
>> > > - Implemented relevant flow API in netdev-linux (and netdev-vport).
>> > > - Added a other_config hw-offload option to enable offloading 
>> > > (defaults to false).
>> > > - Fixed coding style to 

Re: [ovs-dev] 答复: Re: ?答复: Re: 答复: Re: [PATCH] ovn-controller: Support vxlan tunnel in ovn

[Ben Pfaff]

The solution presented so far breaks OVN current and planned features.
If you have a solution that doesn't break OVN current or planned
features, let's talk about it.  Until then, it is not interesting.

On Tue, May 09, 2017 at 09:29:07AM +0800, wang.qia...@zte.com.cn wrote:
> Hi Ben and Mikey, thank you for your review and analysis.
> 
> As we discribed below, vxlan is used very common, and the use case that we 
> mentioned below is typical architecture of telecom operators' networks.
> So, we think it is very necessary to support vxlan between ovs and HW 
> switch. If ovn does not support the use case we discribed before, we think 
> the the 
> ovn features may be imperfect. For this reason, we do the changes as this 
> patch.
> 
> Because vxlan tunnel can not carry enough information like genev to 
> fulfill ovn current pipeline, so we do some assumptions. As Michkey and 
> Ben said, 
> these assumptions may not hold going forward, such as S_SWITCH_IN_L2_LKUP 
> and  more than one chassisredirect ports and so on.
> 
> We think these incompatibles are not impossibly hard problems. If we have 
> the consensus of the necessity to support vxlan, we think we could 
> together 
> to talk over a blueprint that less influence of current planning features 
> and more effective to solve the problem we mentioned. 
> 
> Waiting for your suggestions,Thanks
> 
> 
> 
> 
> 
> 
> Mickey Spiegel 
> 发件人： ovs-dev-boun...@openvswitch.org
> 2017/05/08 04:58
>  
> 收件人：xu.r...@zte.com.cn, 
> 抄送：  ovs dev , 
> ovs-dev-boun...@openvswitch.org
> 主题：  Re: [ovs-dev]   答复: Re: 答复: Re: [PATCH] 
> ovn-controller: Support vxlan tunnel in ovn
> 
> 
> There are some assumptions that you are making which need to be called 
> out.
> These assumptions may not hold going forward. In fact I refer to two
> different patches below that are currently under review, that break your
> assumptions.
> 
> On Fri, May 5, 2017 at 7:18 PM,  wrote:
> 
> > Hi,Russell
> >
> > We think vxlan is the most commonly used tunnel encapsulation in the
> > overlay network openstack,ovn should better consider it.
> >
> > As my workmate wang qianyu said,we would consider computer node connect
> > with existing hardware switches which associates with SR-IOV as VTEP.
> >
> > After discussion, we feel that as long as the following changes for 
> vxlan
> > tunnel in the table0:
> >
> > 1.For local switch, move MFF_TUN_ID to MFF_LOG_DATAPATH, resubmit to
> > OFTABLE_ETH_UCAST(table29)
> >
> 
> It looks like you are overloading OFTABLE_ETH_UCAST that you define here
> with S_SWITCH_IN_L2_LKUP in ovn/northd/ovn-northd.c. Hardcoding the value
> to 29 in ovn/controller/lflow.h is not the way to do this. This pipeline
> stage does move back as new features are added. In fact it is now table 31
> due to the recent addition of 2 tables for DNS lookup.
> 
> 
> > //---In table29, we can find out dst port based on dst mac
> >
> 
> You are assuming that outport determination is only based on
> S_SWITCH_IN_L2_LKUP with no impact from any other ingress pipeline stages.
> This may not always be true, which I think is the point of Ben's 
> complaint.
> For example the SFC patch that is currently under review (
> http://patchwork.ozlabs.org/patch/754427/) may set outport and then do
> "output" in the ingress pipeline, in a pipeline stage other than
> S_SWITCH_IN_L2_LKUP.
> 
> The alternative is to go through the entire ingress pipeline, but here you
> have a problem since you do not know the inport. The current VTEP-centric
> VXLAN code assumes that there is only one port binding per datapath from
> the VTEP chassis. For the general case that you are trying to address, 
> this
> assumption does not hold, so you cannot properly determine the inport. The
> inport may affect the ultimate decision on outport. This is certainly the
> case for the SFC patch currently under review.
> 
> You are also assuming that inport does not affect anything in the egress
> pipeline. This seems to be true at the moment, but this might not always 
> be
> true as features are added.
> 
> The existing VTEP functionality does not rely on the assumptions that you
> made, but since you changed the logic to determine inport in case of 
> VXLAN,
> you are changing existing functionality.
> 
> 
> > 2.For local chassisredirect port, move MFF_TUN_ID to MFF_LOG_DATAPATH, 
> set
> > port tunnel_key to MFF_LOG_OUTPORT and then resubmit to
> > OFTABLE_LOCAL_OUTPUT.
> > //---In table 33, we can find out dst local sw and sw patch port based 
> on
> > the local chassisredirect port,and then follow the exsiting flows.
> >
> 
> At the moment, the only case where a tunnel is used for a datapath
> representing a logical router is when the outport is a chassisredirect
> port. Your code assumes that will always be the case. If we do what you 
> are
> suggesting, then that becomes a restriction for all logical router 
> features
> going 

Re: [ovs-dev] [PATCH] OVN localport type support

[Ben Pfaff]

Thanks.

This is a good summary of some of what the patch does.  Should it be in
the commit message?

On Tue, May 09, 2017 at 05:09:43PM +0200, Daniel Alvarez Sanchez wrote:
> Hi,
> 
> I've submitted a new patch v3 where I removed the external-id
> "ovn-localport"
> from the Interface which I used to identify a port as localport in
> physical.c.
> 
> Instead, I have passed another parameter "local_lports" to physical_run.
> When
> inserting flows in table 32, I'm inserting higher priority drop flows for
> every
> local localport.
> 
> In order to illustrate this:
> 
> LS1 switch1 with 3 ports: 2 ports and 1 localport
> $ ovn-nbctl show switch1
> switch c2a81271-b77f-4019-b877-6428c8b647d7 (switch1)
> port p2
> addresses: ["00:00:00:01:01:11 20.0.0.11"]
> port lp1
> type: localport
> addresses: ["00:00:00:01:01:05 20.0.0.5"]
> port p1
> addresses: ["00:00:00:01:01:10 20.0.0.10"]
> 
> Two HVs: hv1 and hv2.
> 
> p1 (tunnel_key = 2) is in hv1
> p2 (tunnel_key = 4) is in hv2
> lp1 (tunnel_key = 1)  is in both hv1 and hv2
> 
> (When i say that a port is in a certain hv i'm saying that there's an OVS
> port
>  with external-id:iface-id=")
> 
> Table 32 in hv2 looks like:
> 
>  cookie=0x0, duration=2077.204s, table=32, n_packets=2, n_bytes=196,
> idle_age=114, priority=150,reg14=0x1,reg15=0x2,metadata=0x5 actions=drop
>  cookie=0x0, duration=2077.214s, table=32, n_packets=0, n_bytes=0,
> idle_age=2077, priority=100,reg15=0x2,metadata=0x5
> actions=load:0x5->NXM_NX_TUN_ID[0..23],set_field:0x2->tun_metadata0,move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30],output:34
> 
> Which means that if a packet is directed to p1 (reg15=0x2) send it through
> the tunnel unless it comes from lp1 (reg14=0x1) when it'll be dropped.
> 
> 
> Table 32 in hv1 looks like:
> 
>  cookie=0x0, duration=15.193s, table=32, n_packets=0, n_bytes=0,
> idle_age=15, priority=150,reg14=0x1,reg15=0x4,metadata=0x5 actions=drop
>  cookie=0x0, duration=15.193s, table=32, n_packets=0, n_bytes=0,
> idle_age=15, priority=100,reg15=0x4,metadata=0x5
> actions=load:0x5->NXM_NX_TUN_ID[0..23],set_field:0x4->tun_metadata0,move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30],output:28
> 
> Which means that if a packet is directed to p1 (reg15=0x4) send it through
> the tunnel unless it comes from lp1 (reg14=0x1) when it'll be dropped.
> 
> 
> 
> 
> On Fri, May 5, 2017 at 5:51 PM, Ben Pfaff  wrote:
> 
> > [oops, adding back the list]
> >
> > On Fri, May 05, 2017 at 08:51:01AM -0700, Ben Pfaff wrote:
> > > On Fri, May 05, 2017 at 02:58:45PM +0200, Daniel Alvarez Sanchez wrote:
> > > > Thanks a lot Ben for taking the time to review the patch and submit
> > > > the 3 patch series.
> > > >
> > > > On Wed, May 3, 2017 at 11:54 PM, Ben Pfaff  wrote:
> > > >
> > > > > On Tue, Apr 25, 2017 at 11:05:28AM +, Daniel Alvarez wrote:
> > > > > > This patch introduces a new type of OVN ports called "localport".
> > > > > > These ports will be present in every hypervisor and may have the
> > > > > > same IP/MAC addresses. They are not bound to any chassis and
> > traffic
> > > > > > to these ports will never go through a tunnel.
> > > > > >
> > > > > > Its main use case is the OpenStack metadata API support which
> > relies
> > > > > > on a local agent running on every hypervisor and serving metadata
> > to
> > > > > > VM's locally. This service is described in detail at [0].
> > > > > >
> > > > > > Signed-off-by: Daniel Alvarez 
> > > > >
> > > > > In consider_local_datapath(), the ovn-localport-port logic only
> > fires if
> > > > > ovn-localport-port is completely unset.  Usually, it's better to make
> > > > > sure that everything that ovn-controller sets happens every time,
> > > > > because this fixes up damage if any occurs.  So, for example, in this
> > > > > case, we would tend to always set ovn-localport-port.  (Sometimes,
> > this
> > > > > can be expensive, and so we come up with ways to avoid it, but I
> > don't
> > > > > have a reason to believe that it is expensive in this case.)
> > > > >
> > > > Ack. However, now that I've made previous change (only setting it on
> > > > the Interface, I think we can keep it like this because that
> > > > "ovn-localport-port logic" is simply setting the external-id if
> > > > unset. Or you would just prefer to set it anyways? There's no more
> > > > logic apart from this now that I removed setting it also in the Port
> > > > table.
> > >
> > > I'm not sure I understand.  I'll see when I read the next version, no
> > > problem.
> > >
> > > > > The documentation for external-ids:ovn-localport-port should say
> > what
> > > > > the key's value is.
> > > > >
> > > > > Actually, the purpose of external-ids:ovn-localport-port is not
> > entirely
> > > > > clear to me.  The other documented keys in this category have the
> > > > > following uses:
> > > > >
> > > > > external-ids:ovn-localnet-port
> > > > > 

Re: [ovs-dev] [PATCH ovs V8 12/26] netdev-tc-offloads: Add flower mask to priority map

[Flavio Leitner]

On Tue, May 09, 2017 at 03:23:12PM -0300, Flavio Leitner wrote:
> On Wed, May 03, 2017 at 06:08:03PM +0300, Roi Dayan wrote:
> > From: Paul Blakey 
> > 
> > Flower classifer requires a different priority per mask,
> > so we hash the mask and generate a new priority for
> > each new mask used.
> > 
> > Signed-off-by: Paul Blakey 
> > Reviewed-by: Roi Dayan 
> > Reviewed-by: Simon Horman 
> > ---
> >  lib/netdev-tc-offloads.c | 38 ++
> >  1 file changed, 38 insertions(+)
> > 
> > diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> > index 9faea97..7e33fff 100644
> > --- a/lib/netdev-tc-offloads.c
> > +++ b/lib/netdev-tc-offloads.c
> > @@ -209,6 +209,44 @@ find_ufid(int prio, int handle, struct netdev *netdev, 
> > ovs_u128 *ufid)
> >  return (data != NULL);
> >  }
> >  
> > +struct prio_map_data {
> > +struct hmap_node node;
> > +struct tc_flower_key mask;
> > +ovs_be16 protocol;
> > +uint16_t prio;
> > +};
> > +
> > +static uint16_t
> > +get_prio_for_tc_flower(struct tc_flower *flower)
> > +{
> > +static struct hmap prios = HMAP_INITIALIZER();
> > +static struct ovs_mutex prios_lock = OVS_MUTEX_INITIALIZER;
> > +static int last_prio = 0;
> > +size_t key_len = sizeof(struct tc_flower_key);
> > +size_t hash = hash_bytes(>mask, key_len,
> > + (OVS_FORCE uint32_t) flower->key.eth_type);
> > +struct prio_map_data *data;
> > +struct prio_map_data *new_data;
> > +
> > +ovs_mutex_lock(_lock);
> > +HMAP_FOR_EACH_WITH_HASH(data, node, hash, ) {
> > +if (!memcmp(>mask, >mask, key_len)
> > +&& data->protocol == flower->key.eth_type) {
> > +ovs_mutex_unlock(_lock);
> > +return data->prio;
> > +}
> > +}
> > +
> > +new_data = xzalloc(sizeof *new_data);
> > +memcpy(_data->mask, >mask, key_len);
> > +new_data->prio = ++last_prio;
> 
> Well, not sure if it is realistic to have more than 65k different
> masks which is the tc limit, but here it silently overflows.  I
> suspect that tc-flower will fail to insert the flow, but I haven't
> looked further.  It may be harmless, but wanna to point out anyways.

Turns out that Marcelo had already pointed out this issue on V3.
https://mail.openvswitch.org/pipermail/ovs-dev/2017-February/328957.html

I guess at least it deserves a comment explaining briefly why
it isn't a problem. 

-- 
Flavio

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

Re: [ovs-dev] [PATCH ovs V8 00/26] Introducing HW offload support for openvswitch

[Flavio Leitner]

On Sun, May 07, 2017 at 10:55:32AM +0300, Roi Dayan wrote:
> 
> 
> On 03/05/2017 18:58, Federico Iezzi wrote:
> > On Wed, May 3, 2017 at 5:07 PM, Roi Dayan  wrote:
> > > This patch series introduces rule offload functionality to dpif-netlink
> > > via netdev ports new flow offloading API. The user can specify whether to
> > > enable rule offloading or not via OVS configuration. Netdev providers
> > > are able to implement netdev flow offload API in order to offload rules.
> > > 
> > > This patch series also implements one offload scheme for netdev-linux,
> > > using TC flower classifier, which was chosen because its sort of natural
> > > to state OVS DP rules for this classifier. However, the code can be
> > > extended to support other classifiers such as U32, eBPF, etc which support
> > > offload as well.
> > > 
> > > The use-case we are currently addressing is the newly sriov switchdev mode
> > > in the Linux kernel which was introduced in version 4.8.
> > > This series was tested against sriov vfs vports representors of the
> > > Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.
> > > 
> > > 
> > > V7->V8
> > > - Refactor dpif logging functions and use them in dpif-netlink
> > > - Ignore internal devices from netdev hashmap
> > > - Refactor netdev hmap naming to prefix netdev_ports_*
> > > - Use single hashmap with 2 nodes for ufid/tc mapping
> > > - Verify ifindex is valid in netdev_hmap_port_add
> > > - Close netdev in netdev_tc_flow_get error flow
> > > - Improve comments for flow offload api
> > > - Reorder flow api output args to be last args
> > > - Remove redundant netdev_flow_support
> > > - Fix using uninitialized var 's'
> > > 
> > > Not done:
> > > refactor netdev_ports_* functions to accept a typed obj
> > > (e.g. netdev_ports struct) instead of void*.
> > > We can do it as a follow-up commit later.
> > > 
> > > V6->V7:
> > > - Fix L3 IPv4 matching got broken
> > > - Refactor offloads test and testsuite to have same prefix
> > > - Better handling of unsupported match attributes
> > > 
> > > V5->V6:
> > > - Rebase over master branch, fix compilation issue
> > > - Add Nicira copyright to tc interface
> > > 
> > > V4->V5:
> > > - Fix compat
> > > - Fix VXLAN IPv6 tunnel matching
> > > - Fix order of actions in dump flows
> > > - Update ovs-dpctl man page about the addtion of type to dump-flows
> > > 
> > > Travis
> > > https://travis-ci.org/roidayan/ovs/builds/213735371
> > > AppVeyor
> > > https://ci.appveyor.com/project/roidayan/ovs/build/1.0.18
> > > 
> > > V3->V4:
> > > - Move declarations to the right commit with implementation
> > > - Fix tc_get_flower flow return false success
> > > - Fix memory leaks - not releasing tc_transact replies
> > > - Fix travis failure for OSX compilation
> > > - Fix loop in dpif_netlink_flow_dump_next
> > > - Fix declared default value for tc-policy in vswitch.xml
> > > - Refactor loop in netdev_tc_flow_dump_next
> > > - Add missing error checks in parse_flow_put
> > > - Fix handling modify request where old rule is in hw and new
> > >   rule is not supported and needs to be in sw.
> > > - Use 2 hashmaps instead of 1 for faster reverse lookup of ufid from 
> > > tc
> > > - Init ports when enabling hw-offload after OVS is running
> > > 
> > > TODO: Fix breaking of datapath compilation
> > >   Fix testsuite failures
> > > 
> > > Travis
> > > https://travis-ci.org/roidayan/ovs/builds/210549325
> > > AppVeyor
> > > https://ci.appveyor.com/project/roidayan/ovs/build/1.0.15
> > > 
> > > V2->V3:
> > > - Code styling fixes
> > > - Bug fixes
> > > - Using already available macros/functions to match current OVS code
> > > - Refactored code according to V2 review
> > > - Replaced bool option skip-hw for string option tc-policy
> > > - Added hw offload tests using policy skip_hw
> > > - Fixed most compatability compiling issues
> > > - Travis
> > > https://travis-ci.org/roidayan/ovs/builds/199610124
> > > - AppVeyor
> > > https://ci.appveyor.com/project/roidayan/ovs/build/1.0.14
> > > - Fixed compiling with DPDK enabled
> > > 
> > > TODO:
> > > - need to fix datapath compiling issues found in travis after adding 
> > > tc
> > >   compatability headers
> > > - need to fix failing test cases because of get_ifindex
> > > 
> > > V1->V2:
> > > - Added generic netdev flow offloads API.
> > > - Implemented relevant flow API in netdev-linux (and netdev-vport).
> > > - Added a other_config hw-offload option to enable offloading 
> > > (defaults to false).
> > > - Fixed coding style to conform with OVS.
> > > - Policy removed for now. (Will be discussed how best implemented 
> > > later).
> > > 
> > > 
> > > Thanks,
> > > Paul & Roi
> > > 
> > > 
> > > 

Re: [ovs-dev] [PATCH ovs V8 12/26] netdev-tc-offloads: Add flower mask to priority map

[Flavio Leitner]

On Wed, May 03, 2017 at 06:08:03PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Flower classifer requires a different priority per mask,
> so we hash the mask and generate a new priority for
> each new mask used.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/netdev-tc-offloads.c | 38 ++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 9faea97..7e33fff 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -209,6 +209,44 @@ find_ufid(int prio, int handle, struct netdev *netdev, 
> ovs_u128 *ufid)
>  return (data != NULL);
>  }
>  
> +struct prio_map_data {
> +struct hmap_node node;
> +struct tc_flower_key mask;
> +ovs_be16 protocol;
> +uint16_t prio;
> +};
> +
> +static uint16_t
> +get_prio_for_tc_flower(struct tc_flower *flower)
> +{
> +static struct hmap prios = HMAP_INITIALIZER();
> +static struct ovs_mutex prios_lock = OVS_MUTEX_INITIALIZER;
> +static int last_prio = 0;
> +size_t key_len = sizeof(struct tc_flower_key);
> +size_t hash = hash_bytes(>mask, key_len,
> + (OVS_FORCE uint32_t) flower->key.eth_type);
> +struct prio_map_data *data;
> +struct prio_map_data *new_data;
> +
> +ovs_mutex_lock(_lock);
> +HMAP_FOR_EACH_WITH_HASH(data, node, hash, ) {
> +if (!memcmp(>mask, >mask, key_len)
> +&& data->protocol == flower->key.eth_type) {
> +ovs_mutex_unlock(_lock);
> +return data->prio;
> +}
> +}
> +
> +new_data = xzalloc(sizeof *new_data);
> +memcpy(_data->mask, >mask, key_len);
> +new_data->prio = ++last_prio;

Well, not sure if it is realistic to have more than 65k different
masks which is the tc limit, but here it silently overflows.  I
suspect that tc-flower will fail to insert the flow, but I haven't
looked further.  It may be harmless, but wanna to point out anyways.

Flavio



> +new_data->protocol = flower->key.eth_type;
> +hmap_insert(, _data->node, hash);
> +ovs_mutex_unlock(_lock);
> +
> +return new_data->prio;
> +}
> +
>  int
>  netdev_tc_flow_flush(struct netdev *netdev)
>  {
> -- 
> 2.7.4
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio

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

Re: [ovs-dev] [PATCH ovs V8 13/26] netdev-tc-offloads: Implement netdev flow put using tc interface

[Flavio Leitner]

On Wed, May 03, 2017 at 06:08:04PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Currently only tunnel offload is supported.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/dpif-netlink.c   |   4 +-
>  lib/netdev-tc-offloads.c | 392 
> ++-
>  2 files changed, 385 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 81d513d..7e6c647 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -1937,8 +1937,6 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
> dpif_flow_put *put)
>  return EOPNOTSUPP;
>  }
>  
> -memset(, 0, sizeof match);
> -
>  err = parse_key_and_mask_to_match(put->key, put->key_len, put->mask,
>put->mask_len, );
>  if (err) {
> @@ -2011,7 +2009,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
> dpif_flow_put *put)
>  
>  VLOG_DBG("added flow");
>  } else if (err != EEXIST) {
> -VLOG_ERR_RL(, "failed to offload flow: %s", ovs_strerror(err));
> +VLOG_ERR_RL(, "failed adding flow: %s", ovs_strerror(err));
>  }
>  
>  out:
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 7e33fff..e3daf62 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -461,16 +461,392 @@ netdev_tc_flow_dump_next(struct netdev_flow_dump *dump,
>  return false;
>  }
>  
> +static int
> +parse_put_flow_set_action(struct tc_flower *flower, const struct nlattr *set,
> +  size_t set_len)
> +{
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> +const struct nlattr *set_attr;
> +size_t set_left;
> +
> +NL_ATTR_FOR_EACH_UNSAFE(set_attr, set_left, set, set_len) {
> +if (nl_attr_type(set_attr) == OVS_KEY_ATTR_TUNNEL) {
> +const struct nlattr *tunnel = nl_attr_get(set_attr);
> +const size_t tunnel_len = nl_attr_get_size(set_attr);
> +const struct nlattr *tun_attr;
> +size_t tun_left;
> +
> +flower->set.set = true;
> +NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
> +switch (nl_attr_type(tun_attr)) {
> +case OVS_TUNNEL_KEY_ATTR_ID: {
> +flower->set.id = nl_attr_get_be64(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_IPV4_SRC: {
> +flower->set.ipv4.ipv4_src = nl_attr_get_be32(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_IPV4_DST: {
> +flower->set.ipv4.ipv4_dst = nl_attr_get_be32(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_IPV6_SRC: {
> +flower->set.ipv6.ipv6_src =
> +nl_attr_get_in6_addr(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_IPV6_DST: {
> +flower->set.ipv6.ipv6_dst =
> +nl_attr_get_in6_addr(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_TP_SRC: {
> +flower->set.tp_src = nl_attr_get_be16(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_TP_DST: {
> +flower->set.tp_dst = nl_attr_get_be16(tun_attr);
> +}
> +break;
> +}
> +}
> +} else {
> +VLOG_DBG_RL(, "unsupported set action type: %d",
> +nl_attr_type(set_attr));
> +return EOPNOTSUPP;
> +}
> +}
> +return 0;
> +}
> +
> +static int
> +test_key_and_mask(struct match *match) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> +const struct flow *key = >flow;
> +struct flow *mask = >wc.masks;
> +
> +if (mask->pkt_mark) {
> +VLOG_DBG_RL(, "offloading attribute pkt_mark isn't supported");
> +return EOPNOTSUPP;
> +}
> +
> +if (mask->recirc_id && key->recirc_id != 0) {
> +VLOG_DBG_RL(, "offloading attribute recirc_id isn't supported");
> +return EOPNOTSUPP;
> +}
> +mask->recirc_id = 0;
> +
> +if (mask->dp_hash) {
> +VLOG_DBG_RL(, "offloading attribute dp_hash isn't supported");
> +return EOPNOTSUPP;
> +}
> +
> +if (mask->conj_id) {
> +VLOG_DBG_RL(, "offloading attribute conj_id isn't supported");
> +return EOPNOTSUPP;
> +}
> +
> +if (mask->skb_priority) {
> +VLOG_DBG_RL(, "offloading attribute skb_priority isn't 
> supported");
> +return EOPNOTSUPP;
> +}
> +
> +if (mask->actset_output) {
> +

Re: [ovs-dev] [PATCH ovs V8 02/26] netdev: Adding a new netdev api to be used for offloading flows

[Flavio Leitner]

On Wed, May 03, 2017 at 06:07:53PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/automake.mk  |   2 +
>  lib/netdev-bsd.c |   2 +
>  lib/netdev-dpdk.c|   1 +
>  lib/netdev-dummy.c   |   2 +
>  lib/netdev-linux.c   |  15 +++--
>  lib/netdev-linux.h   |   9 +++
>  lib/netdev-provider.h|  71 ++
>  lib/netdev-tc-offloads.c | 154 
> +++
>  lib/netdev-tc-offloads.h |  42 +
>  lib/netdev-vport.c   |  11 +++-
>  lib/netdev.c |  98 ++
>  lib/netdev.h |  23 +++
>  12 files changed, 425 insertions(+), 5 deletions(-)
>  create mode 100644 lib/netdev-tc-offloads.c
>  create mode 100644 lib/netdev-tc-offloads.h
> 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 3d57610..a7c8009 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -354,6 +354,8 @@ lib_libopenvswitch_la_SOURCES += \
>   lib/dpif-netlink.h \
>   lib/tc.h \
>   lib/tc.c \
> + lib/netdev-tc-offloads.h \
> + lib/netdev-tc-offloads.c \
>   lib/if-notifier.c \
>   lib/if-notifier.h \
>   lib/netdev-linux.c \
> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> index 94c515d..5b54d79 100644
> --- a/lib/netdev-bsd.c
> +++ b/lib/netdev-bsd.c
> @@ -1547,6 +1547,8 @@ netdev_bsd_update_flags(struct netdev *netdev_, enum 
> netdev_flags off,
>  netdev_bsd_rxq_recv, \
>  netdev_bsd_rxq_wait, \
>  netdev_bsd_rxq_drain,\
> + \
> +NO_OFFLOAD_API   \
>  }
>  
>  const struct netdev_class netdev_bsd_class =
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ddc651b..9ad96cd 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3314,6 +3314,7 @@ unlock:
>  RXQ_RECV, \
>  NULL,   /* rx_wait */ \
>  NULL,   /* rxq_drain */   \
> +NO_OFFLOAD_API\
>  }
>  
>  static const struct netdev_class dpdk_class =
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index 0657434..7e1383f 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -1409,6 +1409,8 @@ netdev_dummy_update_flags(struct netdev *netdev_,
>  netdev_dummy_rxq_recv,  \
>  netdev_dummy_rxq_wait,  \
>  netdev_dummy_rxq_drain, \
> +\
> +NO_OFFLOAD_API  \
>  }
>  
>  static const struct netdev_class dummy_class =
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index a6bb515..f7941fd 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -73,6 +73,7 @@
>  #include "openvswitch/vlog.h"
>  #include "util.h"
>  #include "tc.h"
> +#include "netdev-tc-offloads.h"
>  
>  VLOG_DEFINE_THIS_MODULE(netdev_linux);
>  
> @@ -2783,7 +2784,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum 
> netdev_flags off,
>  }
>  
>  #define NETDEV_LINUX_CLASS(NAME, CONSTRUCT, GET_STATS,  \
> -   GET_FEATURES, GET_STATUS)\
> +   GET_FEATURES, GET_STATUS,\
> +   FLOW_OFFLOAD_API)\
>  {   \
>  NAME,   \
>  false,  /* is_pmd */\
> @@ -2852,6 +2854,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum 
> netdev_flags off,
>  netdev_linux_rxq_recv,  \
>  netdev_linux_rxq_wait,  \
>  netdev_linux_rxq_drain, \
> +\
> +FLOW_OFFLOAD_API\
>  }
>  
>  const struct netdev_class netdev_linux_class =
> @@ -2860,7 +2864,8 @@ const struct netdev_class netdev_linux_class =
>  netdev_linux_construct,
>  netdev_linux_get_stats,
>  netdev_linux_get_features,
> -netdev_linux_get_status);
> +netdev_linux_get_status,
> +LINUX_FLOW_OFFLOAD_API);
>  
>  const struct netdev_class netdev_tap_class =
>  NETDEV_LINUX_CLASS(
> @@ -2868,7 +2873,8 @@ const struct netdev_class netdev_tap_class =
>  netdev_linux_construct_tap,
>  netdev_tap_get_stats,
>

[ovs-dev] Payment of $1,295.00 from 268131

[Stripe Support]

Congratulations! You have just received a payment of $1,295.00. You can view 
the full details of this payment in your dashboard:


LOG IN TO THE DASHBOARD

We'll be here to help you with any step along the way. You can find answers to 
most questions and get in touch with us at https://support.stripe.com/.

Yours,

The Stripe team

 Charge Summary ?






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

[ovs-dev] Best 2017 Project Management Software

[Betty Rivera]

Find The Best 2017 Project Management Software



Learn More










Increase  workflow efficiency and project visibility






Create  a single point of access for all relevant information






Identify   problem areas and maximize opportunities












One 
Minute Could Save You Days of Frustration


With 
the multitude of project management software solutions available today, 
selecting the right system for your organization can be challenging. Software 
Advice is a trusted resource for software buyers. We provide detailed 
reviews and research on thousands of software applications.


Get free price quotes and relevant recommendations
and find the right solution for your needs!

Compare Systems




























 

Re: [ovs-dev] [RFC PATCH] netdev-dpdk: Add Tx intermediate queue for vhost ports.

[Eelco Chaudron]

On 23/04/17 17:41, Bhanuprakash Bodireddy wrote:

> This commit adds the intermediate queue for vHost-user ports. It
> improves the throughput in multiple virtual machines deployments and
> also in cases with VM doing packet forwarding in kernel stack.
>
> This patch is aligned with intermediate queue implementation for dpdk
> ports that can be found here: https://patchwork.ozlabs.org/patch/723309/

This patch and the one above combined will increase throughput in general
however to the cost of additional latency (see some numbers below).

However I still would like to see both patches applied with a flush every
tx batch. This still increase performance if the rx batch has overlapping
egress ports, but lacks the latency increase.

It would be nice if you could do your latency tests with this flush included
to see if you get the same results I got with this patch and the earlier 
one.


> Signed-off-by: Bhanuprakash Bodireddy 
> Signed-off-by: Antonio Fischetti 
> Co-authored-by: Antonio Fischetti 
> ---
> - Limited testing is done with this RFC patch, test scenarios includes
>   VM doing ip forwarding(Linux stack) and running testpmd in the guest.
> - Sanity testing is done with multiple VMs to check for any 
locking/crashes.
> - Much of the testing is done with single queue, and very basic 
testing with MQ.

> - No throughput/latency tests are done at this point.

I did do some quick latency and throughput tests (with only this patch 
applied).
Same test setup as for the other patch set, i.e. two 82599ES 10G port 
with 64 byte

packets being send at wire speed:

Physical to Virtual test:

flows
Number  plainpatch +
of flows  git clonepatch  flush
  =  =  =
10  594589980065937833914
32  387221165963106530133
50  328371358618946618711
100 313254059537525857226
500 296449956129015273006
1000293195252330895178038


Physical to Virtual to Physical test:

Number  plainpatch +
of flows  git clonepatch  flush
  =  =  =
10  324064726595263652217
32  213687220603132834941
50  198179519124762897763
100 179467817980842014881
500 168675616720141657513
1000167779516285781612480

The results for the latency tests mimics your test case 2 form the previous
patch set, sending 10G traffic @ wire speed:

= GIT CLONE
Pkt size  min(ns)  avg(ns)  max(ns)
 512  10,011   12,100   281,915
1024   7,8709,313   193,116
1280   7,8629,036   194,439
1518   8,2159,417   204,782

= PATCH
Pkt size  min(ns)  avg(ns)  max(ns)
 512  25,044   28,244   774,921
1024  29,029   33,031   218,653
1280  26,464   30,097   203,083
1518  25,870   29,412   204,165

= PATCH + FLUSH
Pkt size  min(ns)  avg(ns)  max(ns)
 512  10,492   13,655   281,538
1024   8,4079,784   205,095
1280   8,3999,750   194,888
1518   8,3679,722   196,973


> TODO:
> - Retry logic in 'netdev_dpdk_vhost_tx_burst' should be handled 
appropriately to
>   lessen the throughput impact when multiple vHost-user port serviced 
by same PMD.
>   An option could be to allow configurable 'retries' option and the 
default being
>   no retries. During testing it was found that the second retry 
couldn't add a single
>   packet to RX queue most of the times with ip forwarding in kernel 
stack*.


Taking the above into consideration see my review comment inline. Also 
I'll assume this

patch will be applied together with the other patch set.

>
>  lib/dpif-netdev.c |  51 +-
>  lib/netdev-dpdk.c | 117 
++

>  lib/netdev-dummy.c|   1 +
>  lib/netdev-linux.c|   1 +
>  lib/netdev-provider.h |   3 ++
>  lib/netdev-vport.c|   3 +-
>  lib/netdev.c  |   9 
>  lib/netdev.h  |   1 +
>  8 files changed, 156 insertions(+), 30 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index a14a2eb..4710985 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -344,6 +344,8 @@ struct dp_netdev_rxq {
>  struct dp_netdev_pmd_thread *pmd;  /* pmd thread that will poll 
this queue. */

>  };
>
> +#define LAST_USED_QID_NONE -1
> +
>  /* A port in a netdev-based datapath. */
>  struct dp_netdev_port {
>  odp_port_t port_no;
> @@ -494,6 +496,8 @@ struct tx_port {
>  int qid;
>  long long last_used;
>  struct hmap_node node;
> +int last_used_qid;/* Last queue id where packets could be
> + enqueued. */
>  };
>
>  /* PMD: Poll modes drivers.  PMD accesses devices via polling to 
eliminate

> @@ -3033,6 +3037,26 @@ cycles_count_end(struct 

Re: [ovs-dev] [PATCH] OVN localport type support

[Daniel Alvarez Sanchez]

Hi,

I've submitted a new patch v3 where I removed the external-id
"ovn-localport"
from the Interface which I used to identify a port as localport in
physical.c.

Instead, I have passed another parameter "local_lports" to physical_run.
When
inserting flows in table 32, I'm inserting higher priority drop flows for
every
local localport.

In order to illustrate this:

LS1 switch1 with 3 ports: 2 ports and 1 localport
$ ovn-nbctl show switch1
switch c2a81271-b77f-4019-b877-6428c8b647d7 (switch1)
port p2
addresses: ["00:00:00:01:01:11 20.0.0.11"]
port lp1
type: localport
addresses: ["00:00:00:01:01:05 20.0.0.5"]
port p1
addresses: ["00:00:00:01:01:10 20.0.0.10"]

Two HVs: hv1 and hv2.

p1 (tunnel_key = 2) is in hv1
p2 (tunnel_key = 4) is in hv2
lp1 (tunnel_key = 1)  is in both hv1 and hv2

(When i say that a port is in a certain hv i'm saying that there's an OVS
port
 with external-id:iface-id=")

Table 32 in hv2 looks like:

 cookie=0x0, duration=2077.204s, table=32, n_packets=2, n_bytes=196,
idle_age=114, priority=150,reg14=0x1,reg15=0x2,metadata=0x5 actions=drop
 cookie=0x0, duration=2077.214s, table=32, n_packets=0, n_bytes=0,
idle_age=2077, priority=100,reg15=0x2,metadata=0x5
actions=load:0x5->NXM_NX_TUN_ID[0..23],set_field:0x2->tun_metadata0,move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30],output:34

Which means that if a packet is directed to p1 (reg15=0x2) send it through
the tunnel unless it comes from lp1 (reg14=0x1) when it'll be dropped.


Table 32 in hv1 looks like:

 cookie=0x0, duration=15.193s, table=32, n_packets=0, n_bytes=0,
idle_age=15, priority=150,reg14=0x1,reg15=0x4,metadata=0x5 actions=drop
 cookie=0x0, duration=15.193s, table=32, n_packets=0, n_bytes=0,
idle_age=15, priority=100,reg15=0x4,metadata=0x5
actions=load:0x5->NXM_NX_TUN_ID[0..23],set_field:0x4->tun_metadata0,move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30],output:28

Which means that if a packet is directed to p1 (reg15=0x4) send it through
the tunnel unless it comes from lp1 (reg14=0x1) when it'll be dropped.




On Fri, May 5, 2017 at 5:51 PM, Ben Pfaff  wrote:

> [oops, adding back the list]
>
> On Fri, May 05, 2017 at 08:51:01AM -0700, Ben Pfaff wrote:
> > On Fri, May 05, 2017 at 02:58:45PM +0200, Daniel Alvarez Sanchez wrote:
> > > Thanks a lot Ben for taking the time to review the patch and submit
> > > the 3 patch series.
> > >
> > > On Wed, May 3, 2017 at 11:54 PM, Ben Pfaff  wrote:
> > >
> > > > On Tue, Apr 25, 2017 at 11:05:28AM +, Daniel Alvarez wrote:
> > > > > This patch introduces a new type of OVN ports called "localport".
> > > > > These ports will be present in every hypervisor and may have the
> > > > > same IP/MAC addresses. They are not bound to any chassis and
> traffic
> > > > > to these ports will never go through a tunnel.
> > > > >
> > > > > Its main use case is the OpenStack metadata API support which
> relies
> > > > > on a local agent running on every hypervisor and serving metadata
> to
> > > > > VM's locally. This service is described in detail at [0].
> > > > >
> > > > > Signed-off-by: Daniel Alvarez 
> > > >
> > > > In consider_local_datapath(), the ovn-localport-port logic only
> fires if
> > > > ovn-localport-port is completely unset.  Usually, it's better to make
> > > > sure that everything that ovn-controller sets happens every time,
> > > > because this fixes up damage if any occurs.  So, for example, in this
> > > > case, we would tend to always set ovn-localport-port.  (Sometimes,
> this
> > > > can be expensive, and so we come up with ways to avoid it, but I
> don't
> > > > have a reason to believe that it is expensive in this case.)
> > > >
> > > Ack. However, now that I've made previous change (only setting it on
> > > the Interface, I think we can keep it like this because that
> > > "ovn-localport-port logic" is simply setting the external-id if
> > > unset. Or you would just prefer to set it anyways? There's no more
> > > logic apart from this now that I removed setting it also in the Port
> > > table.
> >
> > I'm not sure I understand.  I'll see when I read the next version, no
> > problem.
> >
> > > > The documentation for external-ids:ovn-localport-port should say
> what
> > > > the key's value is.
> > > >
> > > > Actually, the purpose of external-ids:ovn-localport-port is not
> entirely
> > > > clear to me.  The other documented keys in this category have the
> > > > following uses:
> > > >
> > > > external-ids:ovn-localnet-port
> > > > external-ids:ovn-l2gateway-port
> > > > external-ids:ovn-l3gateway-port
> > > >
> > > > ovn-controller creates these ports itself and it adds
> these
> > > > keys to them to indicate that it owns them; otherwise
> that
> > > > would be ambiguous and therefore it would be risky for
> > > > ovn-controller to later delete them.
> > > >
> > > > 

[ovs-dev] [PATCH v3] OVN localport type support

[Daniel Alvarez]

This patch introduces a new type of OVN ports called "localport".
These ports will be present in every hypervisor and may have the
same IP/MAC addresses. They are not bound to any chassis and traffic
to these ports will never go through a tunnel.

Its main use case is the OpenStack metadata API support which relies
on a local agent running on every hypervisor and serving metadata to
VM's locally. This service is described in detail at [0].

[0] https://review.openstack.org/#/c/452811/

Signed-off-by: Daniel Alvarez 
---
 ovn/controller/binding.c|   3 +-
 ovn/controller/ovn-controller.c |   2 +-
 ovn/controller/physical.c   |  34 ++-
 ovn/controller/physical.h   |   4 +-
 ovn/northd/ovn-northd.8.xml |   8 +--
 ovn/northd/ovn-northd.c |   6 +-
 ovn/ovn-architecture.7.xml  |  25 ++--
 ovn/ovn-nb.xml  |   9 +++
 ovn/ovn-sb.xml  |  14 +
 tests/ovn.at| 122 
 10 files changed, 210 insertions(+), 17 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 95e9deb..83a7543 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -380,7 +380,8 @@ consider_local_datapath(struct controller_ctx *ctx,
 if (iface_rec && qos_map && ctx->ovs_idl_txn) {
 get_qos_params(binding_rec, qos_map);
 }
-our_chassis = true;
+   if(strcmp(binding_rec->type, "localport"))
+our_chassis = true;
 } else if (!strcmp(binding_rec->type, "l2gateway")) {
 const char *chassis_id = smap_get(_rec->options,
   "l2gateway-chassis");
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index f22551d..0f4dd35 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -655,7 +655,7 @@ main(int argc, char *argv[])
 
 physical_run(, mff_ovn_geneve,
  br_int, chassis, _zones, ,
- _table, _datapaths);
+ _table, _datapaths, _lports);
 
 ofctrl_put(_table, _ct_zones,
get_nb_cfg(ctx.ovnsb_idl));
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 457fc45..c98b305 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -293,7 +293,8 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
   const struct sbrec_port_binding *binding,
   const struct sbrec_chassis *chassis,
   struct ofpbuf *ofpacts_p,
-  struct hmap *flow_table)
+  struct hmap *flow_table,
+  const struct sset *local_lports)
 {
 uint32_t dp_key = binding->datapath->tunnel_key;
 uint32_t port_key = binding->tunnel_key;
@@ -601,6 +602,32 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
 } else {
 /* Remote port connected by tunnel */
 
+/* Table 32, priority 150.
+ * ===
+ *
+ * Drop traffic originated from a localport to a remote destination.
+ */
+const char *localport;
+SSET_FOR_EACH (localport, local_lports) {
+/* Iterate over all local logical ports and insert a drop
+ * rule with higher priority for every localport in this
+ * datapath. */
+const struct sbrec_port_binding *pb = lport_lookup_by_name(
+lports, localport);
+if (pb && pb->datapath->tunnel_key == dp_key &&
+!strcmp(pb->type, "localport")) {
+match_init_catchall();
+ofpbuf_clear(ofpacts_p);
+/* Match localport logical in_port. */
+match_set_reg(, MFF_LOG_INPORT - MFF_REG0,
+  pb->tunnel_key);
+/* Match MFF_LOG_DATAPATH, MFF_LOG_OUTPORT. */
+match_set_metadata(, htonll(dp_key));
+match_set_reg(, MFF_LOG_OUTPORT - MFF_REG0, port_key);
+ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, 0,
+, ofpacts_p);
+}
+}
 /* Table 32, priority 100.
  * ===
  *
@@ -769,7 +796,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id 
mff_ovn_geneve,
  const struct ovsrec_bridge *br_int,
  const struct sbrec_chassis *chassis,
  const struct simap *ct_zones, struct lport_index *lports,
- struct hmap *flow_table, struct hmap *local_datapaths)
+ struct hmap *flow_table, struct hmap *local_datapaths,
+ const struct sset *local_lports)
 {
 
 /* This bool tracks physical mapping changes. */
@@ -891,7 +919,7 @@ physical_run(struct controller_ctx 

[ovs-dev] Sync on PTAP, EXT-382 and NSH: Minutes of meeting Wed 2017-05-09, 14:00 CET

[Jan Scheurich]

Participants
*   Ben M, Zoltan, Georg, Jan, Simon

Review/Discussion of current patch packages
1.  RTNETLINK tunnel configuration - v3 still under review by Joe S (no 
update)
2.  L3 Tunneling
a.  v4: Ben P has merged 1st patch. Comment on 2nd patch fixed in v5
b.  v5: out on mailing list. Waiting for review by Ben
3.  PTAP - v2 (based on L3 Tunneling v5 and fully updated) to be sent this 
week
a.  To be reviewed by Simon and/or Ben P
4.  Basic NSH MD1 fields (according to google doc)
a.  v2 (rebased to PTAP v2) to be sent this week. To be reviewed by ???
5.  Gen encap/decap for Ethernet & NSH
a.  First working gitlab version internally reviewed
b.  Currently being refactored. Ready for mailing list perhaps 2 weeks from 
now
6.  L3 config based on RTNETLINK - not started. Jiri to talk to Eric if it 
can be started (no update)

Status/Planning
*   Lead time is getting an issue for 2.8 (branch date around July 1st)
  We need to focus on review and merge. Start with L3 tunneling v5 and 
RTNETLINK v3 then PTAP v2 and Basic NSH v2 when they are out.
*   Simon has some more bandwidth now and agreed to support Ben P with 
reviews. THANKS!
  We will send notifications to Simon/Ben when we push things for review.

Other
*   Ben Mackcrane explained slow progress in ONF due to ONF/ON.lab merger. 
Can possibly first document the assigned point codes outside formal ONF spec.
*   Discussed again the problem of Encap/Decap actions in Action Sets: 
execution order undefined, support for multiple encap/decap actions? In OVS we 
will not support encap/decap in Write Actions, but we would really like to have 
support for encap in groups. What to do?
*   Jan to provide access to private gitlab repo to Simon and Ben M.
  https://gitlab.com/JScheurich/ovs
*   Jan to call for next meeting in two weeks

BR, Jan

  -Original Appointment-
  From: Jan Scheurich
  Sent: Sunday, 18 December, 2016 15:34
  To: Jan Scheurich; Zoltán Balogh; Yang, Yi Y (yi.y.y...@intel.com); Jiri 
Benc (jb...@redhat.com); Pravin Shelar; Simon Horman 
(simon.hor...@netronome.com); 'ja...@ovn.org'; 'Ben Pfaff'; 
'ben.mackcr...@corsa.com'; d...@openvswitch.org; Georg Schmuecking
  Cc: Maria Pilar Benito Diez
  Subject: Sync on PTAP, EXT-382 and NSH - Tue 2017-05-09 17:00 CET
  When: Tuesday, 09 May, 2017 14:00-15:30 (UTC+01:00) Amsterdam, Berlin, 
Bern, Rome, Stockholm, Vienna.
  Where: Skype Meeting


  Moved to 2pm CET to accommodate Ben, who is in Boston that week.
  Perhaps also Yi can join at that time slot.


  Hi,

  Let's have a look at the status and work out a plan how to accelerate the 
review and merging in order to achieve the agreed target to upstream these 
changes in time for OVS 2.8.

  Thank you,
  Jan

  Link to the Google design doc:
  
https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8/edit


  
.
  --> Join Skype Meeting
  This is an online meeting for Skype for Business, the professional meetings 
and communications app formerly known as Lync.
  Join by phone

  +492115343925 (Germany)English 
(United States)
  89925 (Germany)   English (United States)

  Find a local number

  Conference ID: 70849799
   Forgot your dial-in PIN? 
|Help


  To join a Lync / Skype for Business meeting from an Ericsson standard 
video room, add 77 before the Conference ID (e.g. 771234567 where 1234567 is 
the conference ID).To join from a video room outside of Ericsson add one of 
the domains after 77 and Conference ID (e.g. 771234567@ .ericsson.net, 
where =emea/apac/amcs).  For assistance contact the IT Service Desk.
  [!OC([1033])!]
  
.



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

Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on datapath."

[Zoltán Balogh]

Hi,

I created a patch that updates flow and base_flow L2/L3 members when pushing 
packet to tunnel, and fixes statistics on underlay bridge. 
I posted it to the original thread:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332265.html

As I see, you have not reverted the "Avoid recirculation" patch yet. If my new 
patch is accepted then it's not needed to revert the original commit. Could you 
have a look at the new patch, please?

If you are going to revert the original patch, then I'm going to merge the new 
patch with the original one and resend it to the list.

Best regards,
Zoltan


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Joe Stringer
> Sent: Tuesday, May 09, 2017 5:41 AM
> To: William Tu 
> Cc: ovs-dev 
> Subject: Re: [ovs-dev] [PATCH 1/2] Revert "tunneling: Avoid recirculation on 
> datapath."
> 
> On 8 May 2017 at 17:35, William Tu  wrote:
> > Hi Joe and Greg,
> >
> > Maybe it's better I put this revert tunneling patch (1/2) and its
> > tunnel-tests (2/2) in one patch, so the "make check" can pass?
> 
> They can be separate. It's currently broken, the revert will fix it.
> The test can be an independent submission. I'd rather not fold yet
> more changes into the revert.
> ___
> 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 v5] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

[Zoltán Balogh]

Hi,

Thank you for your comments. Actually, I was wrong in my previous e-mail when I 
stated that packet is truncated only at the end of the pipeline, when output is 
applied. The packet size is set according max_len set by truncate when 
tunnel_push is applied. So the size of packet is correct.

The root cause of the problem, why stats in underlay bridge is wrong, is that 
statistics will be incremented by the packet number and packet size set by the 
upcall_xlate(). 

static void
upcall_xlate(struct udpif *udpif, struct upcall *upcall,
 struct ofpbuf *odp_actions, struct flow_wildcards *wc)
{
struct dpif_flow_stats stats;
struct xlate_in xin;

stats.n_packets = 1;
stats.n_bytes = dp_packet_size(upcall->packet);
stats.used = time_msec();
stats.tcp_flags = ntohs(upcall->flow->tcp_flags);
...
}

Since we excluded recirculation in the "Avoid recirculation" patch, there is no 
second upcall when packet enters tunnel, but stats created by "first" and only 
upcall are used for statistics of both integrate and underlay bridges. And 
that's not correct.

I completed my old patch to solve this problem by adding two new members to 
struct rule_dpif and modify stats with their values.

Here comes the patch:


Since tunneling and forwarding via patch port uses clone action, the tunneled
packet will not be parsed by miniflow_extract() and flow data will not be
updated within clone. So when a tunnel header is added to the packet, the MAC
and IP data of struct flow will not be updated according to the newly created
tunnel header.

This patch stores MAC and IP data of flow before calling
apply_nested_clone_actions() in build_tunnel_send(), then restores the data
after apply_nested_clone_actions() has returned.

Furthermore, it introduces truncate_packet_size and tunnel_header_size struct
rule_dpif members in order to correct n_bytes statistics of OF flows.
---
 ofproto/ofproto-dpif-xlate.c | 92 
 ofproto/ofproto-dpif.c   | 14 ++-
 ofproto/ofproto-dpif.h   |  5 +++
 tests/ofproto-dpif.at| 11 +-
 4 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b308f21..55015d7 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3141,6 +3141,33 @@ tnl_send_arp_request(struct xlate_ctx *ctx, const struct 
xport *out_dev,
 dp_packet_uninit();
 }
 
+/*
+ * Copy IP data of 'flow->tunnel' into 'flow' and 'base_flow'.
+ */
+static void
+propagate_tunnel_data_to_flow(struct flow *flow, struct flow *base_flow)
+{
+flow->nw_dst = flow->tunnel.ip_dst;
+flow->nw_src = flow->tunnel.ip_src;
+flow->ipv6_dst = flow->tunnel.ipv6_dst;
+flow->ipv6_src = flow->tunnel.ipv6_src;
+
+flow->nw_tos = flow->tunnel.ip_tos;
+flow->nw_ttl = flow->tunnel.ip_ttl;
+flow->tp_dst = flow->tunnel.tp_dst;
+flow->tp_src = flow->tunnel.tp_src;
+
+base_flow->nw_dst = flow->nw_dst;
+base_flow->nw_src = flow->nw_src;
+base_flow->ipv6_dst = flow->ipv6_dst;
+base_flow->ipv6_src = flow->ipv6_src;
+
+base_flow->nw_tos = flow->nw_tos;
+base_flow->nw_ttl = flow->nw_ttl;
+base_flow->tp_dst = flow->tp_dst;
+base_flow->tp_src = flow->tp_src;
+}
+
 static int
 build_tunnel_send(struct xlate_ctx *ctx, const struct xport *xport,
   const struct flow *flow, odp_port_t tunnel_odp_port)
@@ -3156,6 +3183,11 @@ build_tunnel_send(struct xlate_ctx *ctx, const struct 
xport *xport,
 int err;
 char buf_sip6[INET6_ADDRSTRLEN];
 char buf_dip6[INET6_ADDRSTRLEN];
+/* Structures to backup Ethernet and IP data of flow and base_flow. */
+struct flow old_flow = ctx->xin->flow;
+struct flow old_base_flow = ctx->base_flow;
+/* Variable to backup actual tunnel header size. */
+uint64_t old_ths = 0;
 
 err = tnl_route_lookup_flow(flow, _ip6, _ip6, _dev);
 if (err) {
@@ -3216,16 +3248,57 @@ build_tunnel_send(struct xlate_ctx *ctx, const struct 
xport *xport,
 tnl_push_data.tnl_port = odp_to_u32(tunnel_odp_port);
 tnl_push_data.out_port = odp_to_u32(out_dev->odp_port);
 
+/* After tunnel header has been added, MAC and IP data of flow and
+ * base_flow need to be set properly, since there is not recirculation
+ * any more when sending packet to tunnel. */
+if (!eth_addr_is_zero(ctx->xin->flow.dl_dst)) {
+ctx->xin->flow.dl_dst = dmac;
+ctx->base_flow.dl_dst = ctx->xin->flow.dl_dst;
+}
+
+ctx->xin->flow.dl_src = smac;
+ctx->base_flow.dl_src = ctx->xin->flow.dl_src;
+
+propagate_tunnel_data_to_flow(>xin->flow, >base_flow);
+
+if (!tnl_params.is_ipv6) {
+ctx->xin->flow.dl_type = htons(ETH_TYPE_IP);
+} else {
+ctx->xin->flow.dl_type = htons(ETH_TYPE_IPV6);
+}
+
+if (tnl_push_data.tnl_type == OVS_VPORT_TYPE_GRE) {
+ctx->xin->flow.nw_proto = IPPROTO_GRE;
+} else {
+

Re: [ovs-dev] Wachtwoord verlopen vandaag

[Cynthia Sullivan]

From: Cynthia Sullivan
Sent: Tuesday, May 9, 2017 3:19 AM
To: Cynthia Sullivan
Subject: Wachtwoord verlopen vandaag

Geachte e-mail gebruiker.

Uw Outlook EMAIL ACCOUNT PASSWORD verloopt Vandaag, en u wordt verzocht het 
binnen 24 uur opnieuw te 
valideren
Of anders zal uw Outlook-e-mailaccount worden uitgeschakeld om te updaten.

Klik op RE-VALIDATE en volg de 
instructies.

Help Desk Administrator.
ICT-helpdesk
ADMIN TEAM
(c) Copyright (c) 2017 Microsoft
Alle rechten voorbehouden.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 1/1] dpif-netdev: The pmd-*-show commands will show info in core order

[Eelco Chaudron]

The "ovs-appctl dpif-netdev/pmd-rxq-show" and "ovs-appctl
dpif-netdev/pmd-stats-show" commands show their output per core_id,
sorted on the hash location. My OCD was kicking in when using these
commands, hence this change to display them in natural core_id order.

In addition I had to change a test case that would fail if the cores
where not in order in the hash list. This is due to OVS assigning
queues to cores based on the order in the hash list. The test case now
checks if any core has the set of queues in the given order.

Manually tested this on my setup, and ran clang-analyze.

Signed-off-by: Eelco Chaudron 
---
 lib/dpif-netdev.c | 55 ++-
 tests/pmd.at  |  7 ---
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4ee5d05..30aa5e9 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -922,13 +922,59 @@ pmd_info_show_rxq(struct ds *reply, struct 
dp_netdev_pmd_thread *pmd)
 }
 }
 
+static int
+compare_poll_thread_list(const void *a_, const void *b_)
+{
+const struct dp_netdev_pmd_thread *a, *b;
+
+a = *(struct dp_netdev_pmd_thread **)a_;
+b = *(struct dp_netdev_pmd_thread **)b_;
+
+if (a->core_id < b->core_id) {
+return -1;
+}
+if (a->core_id > b->core_id) {
+return 1;
+}
+return 0;
+}
+
+/* Create a sorted list of pmd's from the dp->poll_threads cmap. We can use
+ * this list, as long as we do not go to quiescent state. */
+static void
+sorted_poll_thread_list(struct dp_netdev *dp,
+struct dp_netdev_pmd_thread ***list,
+size_t *n)
+{
+struct dp_netdev_pmd_thread *pmd;
+struct dp_netdev_pmd_thread **pmd_list;
+size_t k = 0, n_pmds;
+
+n_pmds = cmap_count(>poll_threads);
+pmd_list = xcalloc(n_pmds, sizeof *pmd_list);
+
+CMAP_FOR_EACH (pmd, node, >poll_threads) {
+if (k >= n_pmds) {
+break;
+}
+pmd_list[k++] = pmd;
+}
+
+qsort(pmd_list, k, sizeof *pmd_list, compare_poll_thread_list);
+
+*list = pmd_list;
+*n = k;
+}
+
 static void
 dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, const char *argv[],
  void *aux)
 {
 struct ds reply = DS_EMPTY_INITIALIZER;
 struct dp_netdev_pmd_thread *pmd;
+struct dp_netdev_pmd_thread **pmd_list;
 struct dp_netdev *dp = NULL;
+size_t i, n;
 enum pmd_info_type type = *(enum pmd_info_type *) aux;
 
 ovs_mutex_lock(_netdev_mutex);
@@ -947,7 +993,13 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 return;
 }
 
-CMAP_FOR_EACH (pmd, node, >poll_threads) {
+sorted_poll_thread_list(dp, _list, );
+for (i = 0; i < n; i++) {
+pmd = pmd_list[i];
+if (!pmd) {
+break;
+}
+
 if (type == PMD_INFO_SHOW_RXQ) {
 pmd_info_show_rxq(, pmd);
 } else {
@@ -970,6 +1022,7 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 }
 }
 }
+free(pmd_list);
 
 ovs_mutex_unlock(_netdev_mutex);
 
diff --git a/tests/pmd.at b/tests/pmd.at
index 2816d45..05755b3 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -53,6 +53,7 @@ m4_define([CHECK_PMD_THREADS_CREATED], [
 ])
 
 m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
\)[[0-9]]*:/\1\2:/"])
+m4_define([SED_NUMA_CORE_QUEUE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
\)[[0-9]]*:/\1\2:/;s/\(queue-id: \)\(0 2 4 6\|1 3 5 
7\)/\1/"])
 m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
 
 AT_SETUP([PMD - creating a thread/add-port])
@@ -126,13 +127,13 @@ TMP=$(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])
 AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x3])
 CHECK_PMD_THREADS_CREATED([2], [], [+$TMP])
 
-AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed SED_NUMA_CORE_PATTERN], 
[0], [dnl
+AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed 
SED_NUMA_CORE_QUEUE_PATTERN], [0], [dnl
 pmd thread numa_id  core_id :
isolated : false
-   port: p0queue-id: 0 2 4 6
+   port: p0queue-id: 
 pmd thread numa_id  core_id :
isolated : false
-   port: p0queue-id: 1 3 5 7
+   port: p0queue-id: 
 ])
 
 TMP=$(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])
-- 
2.7.4

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

Re: [ovs-dev] [PATCH v2 1/1] dpif-netdev: The pmd-*-show commands will show info in core order

[Eelco Chaudron]

Please Ignore as it has the wrong sign-off header :(

//Eelco

On 09/05/17 09:00, Eelco Chaudron wrote:

From: root 

The "ovs-appctl dpif-netdev/pmd-rxq-show" and "ovs-appctl
dpif-netdev/pmd-stats-show" commands show their output per core_id,
sorted on the hash location. My OCD was kicking in when using these
commands, hence this change to display them in natural core_id order.

In addition I had to change a test case that would fail if the cores
where not in order in the hash list. This is due to OVS assigning
queues to cores based on the order in the hash list. The test case now
checks if any core has the set of queues in the given order.

Manually tested this on my setup, and ran clang-analyze.

Signed-off-by: root 
---
  lib/dpif-netdev.c | 55 ++-
  tests/pmd.at  |  7 ---
  2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4ee5d05..30aa5e9 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -922,13 +922,59 @@ pmd_info_show_rxq(struct ds *reply, struct 
dp_netdev_pmd_thread *pmd)
  }
  }
  
+static int

+compare_poll_thread_list(const void *a_, const void *b_)
+{
+const struct dp_netdev_pmd_thread *a, *b;
+
+a = *(struct dp_netdev_pmd_thread **)a_;
+b = *(struct dp_netdev_pmd_thread **)b_;
+
+if (a->core_id < b->core_id) {
+return -1;
+}
+if (a->core_id > b->core_id) {
+return 1;
+}
+return 0;
+}
+
+/* Create a sorted list of pmd's from the dp->poll_threads cmap. We can use
+ * this list, as long as we do not go to quiescent state. */
+static void
+sorted_poll_thread_list(struct dp_netdev *dp,
+struct dp_netdev_pmd_thread ***list,
+size_t *n)
+{
+struct dp_netdev_pmd_thread *pmd;
+struct dp_netdev_pmd_thread **pmd_list;
+size_t k = 0, n_pmds;
+
+n_pmds = cmap_count(>poll_threads);
+pmd_list = xcalloc(n_pmds, sizeof *pmd_list);
+
+CMAP_FOR_EACH (pmd, node, >poll_threads) {
+if (k >= n_pmds) {
+break;
+}
+pmd_list[k++] = pmd;
+}
+
+qsort(pmd_list, k, sizeof *pmd_list, compare_poll_thread_list);
+
+*list = pmd_list;
+*n = k;
+}
+
  static void
  dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, const char *argv[],
   void *aux)
  {
  struct ds reply = DS_EMPTY_INITIALIZER;
  struct dp_netdev_pmd_thread *pmd;
+struct dp_netdev_pmd_thread **pmd_list;
  struct dp_netdev *dp = NULL;
+size_t i, n;
  enum pmd_info_type type = *(enum pmd_info_type *) aux;
  
  ovs_mutex_lock(_netdev_mutex);

@@ -947,7 +993,13 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
  return;
  }
  
-CMAP_FOR_EACH (pmd, node, >poll_threads) {

+sorted_poll_thread_list(dp, _list, );
+for (i = 0; i < n; i++) {
+pmd = pmd_list[i];
+if (!pmd) {
+break;
+}
+
  if (type == PMD_INFO_SHOW_RXQ) {
  pmd_info_show_rxq(, pmd);
  } else {
@@ -970,6 +1022,7 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
  }
  }
  }
+free(pmd_list);
  
  ovs_mutex_unlock(_netdev_mutex);
  
diff --git a/tests/pmd.at b/tests/pmd.at

index 2816d45..05755b3 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -53,6 +53,7 @@ m4_define([CHECK_PMD_THREADS_CREATED], [
  ])
  
  m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id \)[[0-9]]*:/\1\2:/"])

+m4_define([SED_NUMA_CORE_QUEUE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
\)[[0-9]]*:/\1\2:/;s/\(queue-id: \)\(0 2 4 6\|1 3 5 
7\)/\1/"])
  m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
  
  AT_SETUP([PMD - creating a thread/add-port])

@@ -126,13 +127,13 @@ TMP=$(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])
  AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x3])
  CHECK_PMD_THREADS_CREATED([2], [], [+$TMP])
  
-AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed SED_NUMA_CORE_PATTERN], [0], [dnl

+AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed 
SED_NUMA_CORE_QUEUE_PATTERN], [0], [dnl
  pmd thread numa_id  core_id :
isolated : false
-   port: p0queue-id: 0 2 4 6
+   port: p0queue-id: 
  pmd thread numa_id  core_id :
isolated : false
-   port: p0queue-id: 1 3 5 7
+   port: p0queue-id: 
  ])
  
  TMP=$(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])



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