[ovs-dev] [PATCH] ovn-controller: Reduce the number of flows by use conjunction action

[wei]

This patch convert ovn-sb lflow match expr "(1 or 2) and (3 or 4)" to
match 1 aciton connjunction(1, 1/2)
match 2 aciton connjunction(1, 1/2)
match 3 aciton connjunction(1, 2/2)
match 4 aciton connjunction(1, 2/2)
match conj_id=1, action=XXX

NOT support nested conjunction, only use conjunction action in situation "or in 
level 0"
Like (1 or 2) and (3 or ((4 or 5) and (6 or 7))), (4 or 5) and (6 or 7) will 
not be converted conjunction action,
We could call this situation as "or in level 1", in this situation (4 or 5) and 
(6 or 7) will be crossproduct,
so (1 or 2) and (3 or ((4 or 5) and (6 or 7))) -> (1 or 2) and (3 or (4 and 6) 
or (4 and 7) or (5 and 6) or (5 and 7))

In openstack, security group rule will match remote security group and tcp 
port, like
match=(ip4.src == $as_ip4_6a8f4283_ba60_4d1c_9dec_28d027eadef2 && tcp.dst >= 
1 && tcp.dst <= 2))

Use this patch, the number of flows will be significantly reduced

Signed-off-by: wei 
---
 ovn/lib/expr.c | 95 --
 1 file changed, 66 insertions(+), 29 deletions(-)

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 79ff45762..a7e72cef7 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -2426,12 +2426,12 @@ expr_sort(struct expr *expr)
 return expr;
 }
 
-static struct expr *expr_normalize_or(struct expr *expr);
+static struct expr *expr_normalize_or(struct expr *expr, int level);
 
 /* Returns 'expr', which is an AND, reduced to OR(AND(clause)) where
  * a clause is a cmp or a disjunction of cmps on a single field. */
 static struct expr *
-expr_normalize_and(struct expr *expr)
+expr_normalize_and(struct expr *expr, int level)
 {
 ovs_assert(expr->type == EXPR_T_AND);
 
@@ -2472,40 +2472,55 @@ expr_normalize_and(struct expr *expr)
 }
 
 ovs_assert(sub->type == EXPR_T_OR);
-const struct expr_symbol *symbol = expr_is_cmp(sub);
-if (!symbol || symbol->must_crossproduct) {
-struct expr *or = expr_create_andor(EXPR_T_OR);
-struct expr *k;
-
-LIST_FOR_EACH (k, node, >andor) {
-struct expr *and = expr_create_andor(EXPR_T_AND);
-struct expr *m;
-
-LIST_FOR_EACH (m, node, >andor) {
-struct expr *term = m == sub ? k : m;
-if (term->type == EXPR_T_AND) {
-struct expr *p;
-
-LIST_FOR_EACH (p, node, >andor) {
-struct expr *new = expr_clone(p);
+if (level == 0) {
+LIST_FOR_EACH_SAFE (a, b, node, >andor) {
+if (a->type == EXPR_T_CMP) {
+continue;
+} else if (a->type == EXPR_T_AND) {
+ovs_list_remove(>node);
+struct expr *new = expr_normalize_and(a, level + 1);
+ovs_assert(new->type == EXPR_T_CMP || new->type == 
EXPR_T_AND || new->type == EXPR_T_OR);
+expr_insert_andor(sub, b, new);
+} else {
+OVS_NOT_REACHED();
+}
+}
+} else {
+const struct expr_symbol *symbol = expr_is_cmp(sub);
+if (!symbol || symbol->must_crossproduct) {
+struct expr *or = expr_create_andor(EXPR_T_OR);
+struct expr *k;
+
+LIST_FOR_EACH (k, node, >andor) {
+struct expr *and = expr_create_andor(EXPR_T_AND);
+struct expr *m;
+
+LIST_FOR_EACH (m, node, >andor) {
+struct expr *term = m == sub ? k : m;
+if (term->type == EXPR_T_AND) {
+struct expr *p;
+
+LIST_FOR_EACH (p, node, >andor) {
+struct expr *new = expr_clone(p);
+ovs_list_push_back(>andor, >node);
+}
+} else {
+struct expr *new = expr_clone(term);
 ovs_list_push_back(>andor, >node);
 }
-} else {
-struct expr *new = expr_clone(term);
-ovs_list_push_back(>andor, >node);
 }
+ovs_list_push_back(>andor, >node);
 }
-ovs_list_push_back(>andor, >node);
+expr_destroy(expr);
+return expr_normalize_or(or, level + 1);
 }
-expr_destroy(expr);
-return expr_normalize_or(or);
 }
 }
 return expr;
 }
 
 static struct expr *
-expr_normalize_or(struct expr *expr)
+expr_normalize_or(struct expr *expr, int level)
 {
 struct expr *sub, *next;
 
@@ -2513,7 +2528,7 @@ expr_normalize_or(struct expr *expr)
 if (sub->type == EXPR_T_AND) {
 ovs_list_remove(>node);
 
- 

[ovs-dev] [PATCH V2] ovn-controller: Reduce the number of flows by use conjunction action

[wei]

This patch convert ovn-sb lflow match expr "(1 or 2) and (3 or 4)" to
match 1 aciton connjunction(1, 1/2)
match 2 aciton connjunction(1, 1/2)
match 3 aciton connjunction(1, 2/2)
match 4 aciton connjunction(1, 2/2)
match conj_id=1, action=XXX

NOT support nested conjunction, only use conjunction action in situation "or in 
level 0"
Like (1 or 2) and (3 or ((4 or 5) and (6 or 7))), (4 or 5) and (6 or 7) will 
not be converted conjunction action,
We could call this situation as "or in level 1", in this situation (4 or 5) and 
(6 or 7) will be crossproduct,
so (1 or 2) and (3 or ((4 or 5) and (6 or 7))) -> (1 or 2) and (3 or (4 and 6) 
or (4 and 7) or (5 and 6) or (5 and 7))

In openstack, security group rule will match remote security group and tcp 
port, like
match=(ip4.src == $as_ip4_6a8f4283_ba60_4d1c_9dec_28d027eadef2 && tcp.dst >= 
1 && tcp.dst <= 2))

Use this patch, the number of flows will be significantly reduced

Signed-off-by: wei 
---
 ovn/lib/expr.c | 95 --
 1 file changed, 66 insertions(+), 29 deletions(-)

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 79ff45762..a7e72cef7 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -2426,12 +2426,12 @@ expr_sort(struct expr *expr)
 return expr;
 }
 
-static struct expr *expr_normalize_or(struct expr *expr);
+static struct expr *expr_normalize_or(struct expr *expr, int level);
 
 /* Returns 'expr', which is an AND, reduced to OR(AND(clause)) where
  * a clause is a cmp or a disjunction of cmps on a single field. */
 static struct expr *
-expr_normalize_and(struct expr *expr)
+expr_normalize_and(struct expr *expr, int level)
 {
 ovs_assert(expr->type == EXPR_T_AND);
 
@@ -2472,40 +2472,55 @@ expr_normalize_and(struct expr *expr)
 }
 
 ovs_assert(sub->type == EXPR_T_OR);
-const struct expr_symbol *symbol = expr_is_cmp(sub);
-if (!symbol || symbol->must_crossproduct) {
-struct expr *or = expr_create_andor(EXPR_T_OR);
-struct expr *k;
-
-LIST_FOR_EACH (k, node, >andor) {
-struct expr *and = expr_create_andor(EXPR_T_AND);
-struct expr *m;
-
-LIST_FOR_EACH (m, node, >andor) {
-struct expr *term = m == sub ? k : m;
-if (term->type == EXPR_T_AND) {
-struct expr *p;
-
-LIST_FOR_EACH (p, node, >andor) {
-struct expr *new = expr_clone(p);
+if (level == 0) {
+LIST_FOR_EACH_SAFE (a, b, node, >andor) {
+if (a->type == EXPR_T_CMP) {
+continue;
+} else if (a->type == EXPR_T_AND) {
+ovs_list_remove(>node);
+struct expr *new = expr_normalize_and(a, level + 1);
+ovs_assert(new->type == EXPR_T_CMP || new->type == 
EXPR_T_AND || new->type == EXPR_T_OR);
+expr_insert_andor(sub, b, new);
+} else {
+OVS_NOT_REACHED();
+}
+}
+} else {
+const struct expr_symbol *symbol = expr_is_cmp(sub);
+if (!symbol || symbol->must_crossproduct) {
+struct expr *or = expr_create_andor(EXPR_T_OR);
+struct expr *k;
+
+LIST_FOR_EACH (k, node, >andor) {
+struct expr *and = expr_create_andor(EXPR_T_AND);
+struct expr *m;
+
+LIST_FOR_EACH (m, node, >andor) {
+struct expr *term = m == sub ? k : m;
+if (term->type == EXPR_T_AND) {
+struct expr *p;
+
+LIST_FOR_EACH (p, node, >andor) {
+struct expr *new = expr_clone(p);
+ovs_list_push_back(>andor, >node);
+}
+} else {
+struct expr *new = expr_clone(term);
 ovs_list_push_back(>andor, >node);
 }
-} else {
-struct expr *new = expr_clone(term);
-ovs_list_push_back(>andor, >node);
 }
+ovs_list_push_back(>andor, >node);
 }
-ovs_list_push_back(>andor, >node);
+expr_destroy(expr);
+return expr_normalize_or(or, level + 1);
 }
-expr_destroy(expr);
-return expr_normalize_or(or);
 }
 }
 return expr;
 }
 
 static struct expr *
-expr_normalize_or(struct expr *expr)
+expr_normalize_or(struct expr *expr, int level)
 {
 struct expr *sub, *next;
 
@@ -2513,7 +2528,7 @@ expr_normalize_or(struct expr *expr)
 if (sub->type == EXPR_T_AND) {
 ovs_list_remove(>node);
 
- 

[ovs-dev] [PATCH] datapath-windows: Update OvsIPv4TunnelKey flags in geneve decap.

[Anand Kumar]

Set the geneve flags OVS_TNL_F_GENEVE_OPT and OVS_TNL_F_CRT_OPT
in OvsDecapGeneve, so that windows behavior is similiar to linux
https://github.com/openvswitch/ovs/blob/master/datapath/linux/compat/geneve.c#L242

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Geneve.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Geneve.c b/datapath-windows/ovsext/Geneve.c
index 43374e2..77244b1 100644
--- a/datapath-windows/ovsext/Geneve.c
+++ b/datapath-windows/ovsext/Geneve.c
@@ -324,10 +324,10 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT 
switchContext,
 status = STATUS_NDIS_INVALID_PACKET;
 goto dropNbl;
 }
-tunKey->flags = OVS_TNL_F_KEY;
-if (geneveHdr->oam) {
-tunKey->flags |= OVS_TNL_F_OAM;
-}
+/* Update tunnelKey flags. */
+tunKey->flags = OVS_TNL_F_KEY | OVS_TNL_F_GENEVE_OPT |
+(geneveHdr->oam ? OVS_TNL_F_OAM : 0) |
+(geneveHdr->critical ? OVS_TNL_F_CRT_OPT : 0);
 tunKey->tunnelId = GENEVE_VNI_TO_TUNNELID(geneveHdr->vni);
 tunKey->tunOptLen = (uint8)geneveHdr->optLen * 4;
 if (tunKey->tunOptLen > TUN_OPT_MAX_LEN ||
-- 
2.9.3.windows.1

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

Re: [ovs-dev] [PATCH] ovs-atomic: Add C++ compatible implementation.

[Ben Pfaff]

On Tue, Oct 17, 2017 at 02:56:58PM -0700, Yi-Hung Wei wrote:
> On Tue, Oct 17, 2017 at 1:35 PM, Ben Pfaff  wrote:
> > On Mon, Oct 16, 2017 at 05:08:34PM -0700, Yi-Hung Wei wrote:
> >> Thanks for the patch. I can verify that it works great with C++14 support.
> >>
> >> One minor problem is that it may run into some compiler errors if the
> >> C++ compiler does not support C++14 or the support is not enabled.
> >> There was a patch that hit similar issue:
> >> https://patchwork.ozlabs.org/patch/813112/
> >
> > This is a good point.  What if we fold in the following incremental?
> Thanks, I think it resolves the aforementioned issue. Just a few minor
> picks below.
> 
> Acked-by: Yi-Hung Wei 
> 
> > I'm also appending the full patch.
> >
> > diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
> > index 5e4a9c1775b3..c5299fd08512 100644
> > --- a/lib/ovs-atomic.h
> > +++ b/lib/ovs-atomic.h
> > @@ -325,11 +325,11 @@
> >  #include "ovs-atomic-pthreads.h"
> >  #elif __has_extension(c_atomic)
> >  #include "ovs-atomic-clang.h"
> > -#elif defined(__cplusplus) && HAVE_ATOMIC
> > +#elif HAVE_ATOMIC && __cplusplus >= 201103L
> >  #include "ovs-atomic-c++.h"
> > -#elif HAVE_STDATOMIC_H
> > +#elif HAVE_STDATOMIC_H && !defined(__cplusplus)
> >  #include "ovs-atomic-c11.h"
> > -#elif __GNUC__ >= 5
> > +#elif __GNUC__ >= 5 && !defined(__cplusplus)
> >  #error "GCC 5+ should have "
> >  #elif __GNUC__ >= 4 && __GNUC_MINOR__ >= 7
> >  #include "ovs-atomic-gcc4.7+.h"
> Not sure if it's worth to append this changes.
> -#elif __GNUC__ >= 4 && __GNUC_MINOR__ >= 7
> +#elif (defined(__cplusplus) && __GNUC__ >= 5) || \
> +(__GNUC__ >= 4 && __GNUC_MINOR__ >= 7)

Thanks.  I decided to use:
#elif __GNUC__ >= 5 || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 7)

I fixed up the other typos you pointed out (thanks!) and applied this to
master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Donation

[Maria-Elisabeth Schaeffler]

I am Maria-Elisabeth Schaeffler, a German citizen, wife of late Georg W. 
Schaeffler, 75 years old. You can see here: 
en.wikipedia.org/wiki/Maria-Elisabeth_Schaeffler I intend to give to you a 
portion of my Wealth as a free-will financial donation to you. Respond now to 
partake.

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

Re: [ovs-dev] [PATCH] ovs-atomic: Add C++ compatible implementation.

[Yi-Hung Wei]

On Tue, Oct 17, 2017 at 1:35 PM, Ben Pfaff  wrote:
> On Mon, Oct 16, 2017 at 05:08:34PM -0700, Yi-Hung Wei wrote:
>> Thanks for the patch. I can verify that it works great with C++14 support.
>>
>> One minor problem is that it may run into some compiler errors if the
>> C++ compiler does not support C++14 or the support is not enabled.
>> There was a patch that hit similar issue:
>> https://patchwork.ozlabs.org/patch/813112/
>
> This is a good point.  What if we fold in the following incremental?
Thanks, I think it resolves the aforementioned issue. Just a few minor
picks below.

Acked-by: Yi-Hung Wei 

> I'm also appending the full patch.
>
> diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
> index 5e4a9c1775b3..c5299fd08512 100644
> --- a/lib/ovs-atomic.h
> +++ b/lib/ovs-atomic.h
> @@ -325,11 +325,11 @@
>  #include "ovs-atomic-pthreads.h"
>  #elif __has_extension(c_atomic)
>  #include "ovs-atomic-clang.h"
> -#elif defined(__cplusplus) && HAVE_ATOMIC
> +#elif HAVE_ATOMIC && __cplusplus >= 201103L
>  #include "ovs-atomic-c++.h"
> -#elif HAVE_STDATOMIC_H
> +#elif HAVE_STDATOMIC_H && !defined(__cplusplus)
>  #include "ovs-atomic-c11.h"
> -#elif __GNUC__ >= 5
> +#elif __GNUC__ >= 5 && !defined(__cplusplus)
>  #error "GCC 5+ should have "
>  #elif __GNUC__ >= 4 && __GNUC_MINOR__ >= 7
>  #include "ovs-atomic-gcc4.7+.h"
Not sure if it's worth to append this changes.
-#elif __GNUC__ >= 4 && __GNUC_MINOR__ >= 7
+#elif (defined(__cplusplus) && __GNUC__ >= 5) || \
+(__GNUC__ >= 4 && __GNUC_MINOR__ >= 7)
 #include "ovs-atomic-gcc4.7+.h"
 #elif __GNUC__ && defined(__x86_64__)
 #include "ovs-atomic-x86_64.h"

>
> --8<--cut here-->8--
>
> From: Ben Pfaff 
> Date: Tue, 17 Oct 2017 13:34:29 -0700
> Subject: [PATCH] ovs-atomic: Add C++ compatible implementation.
>
> G++ 5 does not implement the _Atomic keyword, which is part of C11 but not
> C++14, so the existing  based atomic implementation doesn't
> work.  This commit adds a new implementation based on the C++14 
Maybe C++11  ?  http://en.cppreference.com/w/cpp/atomic/atomic

> header.
>
> In this area, C++ is pickier about types than C, so a few of the
> definitions in ovs-atomic.h have to be updated to use more precise types
> for integer constants.
>
> This updates the code that generates cxxtest.cc to #include 
> (so that HAVE_ATOMIC is defined) and to automatically regenerate when the
> program is reconfigured (because otherwise the #include ) won't
> get added without a "make clean" step).
>
> "ovs-atomic.h" is not a public header, but apparently some code was
> using it anyway.
>
> Fixes: 9c463631e8145 ("ovs-atomic: Report error for contradictory 
> configuration.")
> Reported-by: Yi-Hung Wei 
> Signed-off-by: Ben Pfaff 
> ---
>  include/openvswitch/automake.mk | 10 ---
>  lib/automake.mk |  1 +
>  lib/ovs-atomic-c++.h| 64 
> +
>  lib/ovs-atomic.h| 18 ++--
>  m4/openvswitch.m4   |  3 ++
>  5 files changed, 84 insertions(+), 12 deletions(-)
>  create mode 100644 lib/ovs-atomic-c++.h
>
> diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
> index eabeb1e971f3..6cb19b84e220 100644
> --- a/include/openvswitch/automake.mk
> +++ b/include/openvswitch/automake.mk
> @@ -41,10 +41,12 @@ if HAVE_CXX
>  # are acceptable as C++.
>  noinst_LTLIBRARIES += include/openvswitch/libcxxtest.la
>  nodist_include_openvswitch_libcxxtest_la_SOURCES = 
> include/openvswitch/cxxtest.cc
> -include/openvswitch/cxxtest.cc: include/openvswitch/automake.mk
> -   $(AM_V_GEN)for header in $(openvswitchinclude_HEADERS); do  \
> - echo $$header;\
> -   done | sed 's,^include/\(.*\)$$,#include <\1>,' > $@
> +include/openvswitch/cxxtest.cc: \
> +   include/openvswitch/automake.mk $(top_builddir)/config.status
> +   $(AM_V_GEN){ echo "#include "; \
> +   for header in $(openvswitchinclude_HEADERS); do \
> + echo $$header; \
> +   done | sed 's,^include/\(.*\)$$,#include <\1>,'; } > $@
>  endif
>
>  # OVS does not use C++ itself, but it provides public header files
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 2415f4cd6c25..ca1cf5dd2524 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -159,6 +159,7 @@ lib_libopenvswitch_la_SOURCES = \
> lib/ofp-version-opt.h \
> lib/ofp-version-opt.c \
> lib/ofpbuf.c \
> +   lib/ovs-atomic-c++.h \
> lib/ovs-atomic-c11.h \
> lib/ovs-atomic-clang.h \
> lib/ovs-atomic-flag-gcc4.7+.h \
> diff --git a/lib/ovs-atomic-c++.h b/lib/ovs-atomic-c++.h
> new file mode 100644
> index ..5bb88536d799
> --- /dev/null
> +++ 

Re: [ovs-dev] [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing mempools.

[Kavanagh, Mark B]

>From: Fischetti, Antonio
>Sent: Tuesday, October 17, 2017 6:04 PM
>To: Kavanagh, Mark B ; d...@openvswitch.org
>Cc: Kevin Traynor ; Aaron Conole ;
>Darrell Ball 
>Subject: RE: [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing
>mempools.
>
>Thanks Mark, comments inline.
>
>-Antonio
>
>> -Original Message-
>> From: Kavanagh, Mark B
>> Sent: Tuesday, October 17, 2017 2:34 PM
>> To: Fischetti, Antonio ; d...@openvswitch.org
>> Cc: Kevin Traynor ; Aaron Conole ;
>> Darrell Ball 
>> Subject: RE: [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing
>> mempools.
>>
>> >From: Fischetti, Antonio
>> >Sent: Monday, October 16, 2017 2:15 PM
>> >To: d...@openvswitch.org
>> >Cc: Kavanagh, Mark B ; Kevin Traynor
>> >; Aaron Conole ; Darrell Ball
>> >; Fischetti, Antonio 
>> >Subject: [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing
>mempools.
>> >
>> >Fix issues on reconfiguration of pre-existing mempools.
>> >This patch avoids to call dpdk_mp_put() - and erroneously
>> >release the mempool - when it already exists.
>> >Create mempool names by considering also the NUMA socket number.
>> >So a name reflects what socket the mempool is allocated on.
>> >This change is needed for the NUMA-awareness feature.
>>
>> Hi Antonio,
>>
>> Is there any particular reason why you've combined patches 1 and 2 of the
>> previous series in a single patch here?
>>
>> I would have thought that these two separate issues would warrant two
>> individual patches (particularly with respect to the reported-by, tested-by
>> tags).
>
>[Antonio]
>I guess I misunderstood your previous review where you asked to squash patches
>1 and 3 into one patch.

Hi Antonio,

I figured as much ;)

>I understood instead to squash the first 2 patches because they were both bug-
>fixes.
>In the next version v7 I'll restore the 2 separate patches.

Thanks - I think that's a much cleaner approach.

>
>>
>> Maybe it's not a big deal, but noted here nonetheless.
>>
>> Apart from that, there are some comments inline.
>>
>> Thanks again,
>> Mark
>>
>> >
>> >CC: Mark B Kavanagh 
>> >CC: Kevin Traynor 
>> >CC: Aaron Conole 
>> >CC: Darrell Ball 
>> >Reported-by: Ciara Loftus 
>> >Tested-by: Ciara Loftus 
>> >Reported-by: Róbert Mulik 
>> >Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
>> >port.")
>> >Signed-off-by: Antonio Fischetti 
>> >---
>> > Test on releasing pre-existing mempools
>> > ===
>> >I've tested this patch by
>> >  - changing at run-time the number of Rx queues:
>> >  ovs-vsctl set Interface dpdk0 type=dpdk options:n_rxq=4
>> >
>> >  - reducing the MTU of the dpdk ports of 1 byte to force
>> >the configuration of an existing mempool:
>> >  ovs-vsctl set Interface dpdk0 mtu_request=1499
>> >
>> >This issue was observed in a PVP test topology with dpdkvhostuserclient
>> >ports. It can happen also with dpdk type ports, eg by reducing the MTU
>> >of 1 byte.
>> >
>> >To replicate the bug scenario in the PVP case it's sufficient to
>> >set 1 dpdkvhostuserclient port, and just boot the VM.
>> >
>> >Below some more details on my own test setup.
>> >
>> > PVP test setup
>> > --
>> >CLIENT_SOCK_DIR=/tmp
>> >SOCK0=dpdkvhostuser0
>> >SOCK1=dpdkvhostuser1
>> >
>> >1 PMD
>> >Add 2 dpdk ports, n_rxq=1
>> >Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost-server-
>path
>> > ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-
>> >path="$CLIENT_SOCK_DIR/$SOCK0"
>> > ovs-vsctl set Interface dpdkvhostuser1 options:vhost-server-
>> >path="$CLIENT_SOCK_DIR/$SOCK1"
>> >
>> >Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1
>> > add-flow br0 in_port=1,action=output:3
>> > add-flow br0 in_port=3,action=output:1
>> > add-flow br0 in_port=4,action=output:2
>> > add-flow br0 in_port=2,action=output:4
>> >
>> > Launch QEMU
>> > ---
>> >As OvS vhu ports are acting as clients, we must specify 'server' in the
>next
>> >command.
>> >VM_IMAGE=
>> >
>> > sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name
>us-
>> >vhost-vm1 -cpu host -enable-kvm -m 4096M -object memory-backend-
>> >file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa
>node,memdev=mem
>> >-mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev
>> >socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev type=vhost-
>> >user,id=mynet1,chardev=char0,vhostforce -device virtio-net-
>> >pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev
>> 

Re: [ovs-dev] [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.

[Kavanagh, Mark B]

>From: Fischetti, Antonio
>Sent: Tuesday, October 17, 2017 6:18 PM
>To: Kavanagh, Mark B ; d...@openvswitch.org
>Cc: Darrell Ball ; Loftus, Ciara ;
>Kevin Traynor ; Aaron Conole 
>Subject: RE: [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
>
>
>
>> -Original Message-
>> From: Kavanagh, Mark B
>> Sent: Tuesday, October 17, 2017 2:34 PM
>> To: Fischetti, Antonio ; d...@openvswitch.org
>> Cc: Darrell Ball ; Loftus, Ciara ;
>> Kevin Traynor ; Aaron Conole 
>> Subject: RE: [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
>>
>> >From: Fischetti, Antonio
>> >Sent: Monday, October 16, 2017 2:15 PM
>> >To: d...@openvswitch.org
>> >Cc: Kavanagh, Mark B ; Darrell Ball
>> >; Loftus, Ciara ; Kevin Traynor
>> >; Aaron Conole ; Fischetti,
>Antonio
>> >
>> >Subject: [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
>> >
>> >Skip initialization of mempool packet areas if this was already
>> >done in a previous call to dpdk_mp_create.
>>
>> Hi Antonio,
>>
>> As stated in my previous review, I believe that this could probably be
>folded
>> into patch 1 of the series (it was patch 3 of v5).
>> However, I don't object strongly to this patch, so I'll leave it to your
>> discretion.
>
>[Antonio]
>I'm keeping this change in a separate patch because it is not related
>to the fixes for the mempool management.
>It's a small improvement, not so much to save CPU cycles, it's actually
>a clean up of the code.

Sure thing.

In that case, Acked-by: Mark Kavanagh 

Thanks,
Mark

>
>>
>> Other than that, LGTM.
>>
>> Thanks,
>> Mark
>>
>> >
>> >CC: Mark B Kavanagh 
>> >CC: Darrell Ball 
>> >CC: Ciara Loftus 
>> >CC: Kevin Traynor 
>> >CC: Aaron Conole 
>> >Signed-off-by: Antonio Fischetti 
>> >---
>> > lib/netdev-dpdk.c | 10 +-
>> > 1 file changed, 5 insertions(+), 5 deletions(-)
>> >
>> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> >index 7f2d7ed..07c438a 100644
>> >--- a/lib/netdev-dpdk.c
>> >+++ b/lib/netdev-dpdk.c
>> >@@ -550,6 +550,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>> >*mp_exists)
>> > if (dmp->mp) {
>> > VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
>> >  dmp->mp_size);
>> >+/* rte_pktmbuf_pool_create has done some initialization of the
>> >+ * rte_mbuf part of each dp_packet. Some OvS specific fields
>> >+ * of the packet still need to be initialized by
>> >+ * ovs_rte_pktmbuf_init. */
>> >+rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
>> > } else if (rte_errno == EEXIST) {
>> > /* A mempool with the same name already exists.  We just
>> >  * retrieve its pointer to be returned to the caller. */
>> >@@ -566,11 +571,6 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>> >*mp_exists)
>> > }
>> > free(mp_name);
>> > if (dmp->mp) {
>> >-/* rte_pktmbuf_pool_create has done some initialization of the
>> >- * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
>> >- * initializes some OVS specific fields of dp_packet.
>> >- */
>> >-rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
>> > return dmp;
>> > }
>> > } while (!(*mp_exists) &&
>> >--
>> >2.4.11

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

Re: [ovs-dev] [PATCH] ovs-atomic: Add C++ compatible implementation.

[Ben Pfaff]

On Mon, Oct 16, 2017 at 05:08:34PM -0700, Yi-Hung Wei wrote:
> Thanks for the patch. I can verify that it works great with C++14 support.
> 
> One minor problem is that it may run into some compiler errors if the
> C++ compiler does not support C++14 or the support is not enabled.
> There was a patch that hit similar issue:
> https://patchwork.ozlabs.org/patch/813112/

This is a good point.  What if we fold in the following incremental?
I'm also appending the full patch.

diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
index 5e4a9c1775b3..c5299fd08512 100644
--- a/lib/ovs-atomic.h
+++ b/lib/ovs-atomic.h
@@ -325,11 +325,11 @@
 #include "ovs-atomic-pthreads.h"
 #elif __has_extension(c_atomic)
 #include "ovs-atomic-clang.h"
-#elif defined(__cplusplus) && HAVE_ATOMIC
+#elif HAVE_ATOMIC && __cplusplus >= 201103L
 #include "ovs-atomic-c++.h"
-#elif HAVE_STDATOMIC_H
+#elif HAVE_STDATOMIC_H && !defined(__cplusplus)
 #include "ovs-atomic-c11.h"
-#elif __GNUC__ >= 5
+#elif __GNUC__ >= 5 && !defined(__cplusplus)
 #error "GCC 5+ should have "
 #elif __GNUC__ >= 4 && __GNUC_MINOR__ >= 7
 #include "ovs-atomic-gcc4.7+.h"

--8<--cut here-->8--

From: Ben Pfaff 
Date: Tue, 17 Oct 2017 13:34:29 -0700
Subject: [PATCH] ovs-atomic: Add C++ compatible implementation.

G++ 5 does not implement the _Atomic keyword, which is part of C11 but not
C++14, so the existing  based atomic implementation doesn't
work.  This commit adds a new implementation based on the C++14 
header.

In this area, C++ is pickier about types than C, so a few of the
definitions in ovs-atomic.h have to be updated to use more precise types
for integer constants.

This updates the code that generates cxxtest.cc to #include 
(so that HAVE_ATOMIC is defined) and to automatically regenerate when the
program is reconfigured (because otherwise the #include ) won't
get added without a "make clean" step).

"ovs-atomic.h" is not a public header, but apparently some code was
using it anyway.

Fixes: 9c463631e8145 ("ovs-atomic: Report error for contradictory 
configuration.")
Reported-by: Yi-Hung Wei 
Signed-off-by: Ben Pfaff 
---
 include/openvswitch/automake.mk | 10 ---
 lib/automake.mk |  1 +
 lib/ovs-atomic-c++.h| 64 +
 lib/ovs-atomic.h| 18 ++--
 m4/openvswitch.m4   |  3 ++
 5 files changed, 84 insertions(+), 12 deletions(-)
 create mode 100644 lib/ovs-atomic-c++.h

diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index eabeb1e971f3..6cb19b84e220 100644
--- a/include/openvswitch/automake.mk
+++ b/include/openvswitch/automake.mk
@@ -41,10 +41,12 @@ if HAVE_CXX
 # are acceptable as C++.
 noinst_LTLIBRARIES += include/openvswitch/libcxxtest.la
 nodist_include_openvswitch_libcxxtest_la_SOURCES = 
include/openvswitch/cxxtest.cc
-include/openvswitch/cxxtest.cc: include/openvswitch/automake.mk
-   $(AM_V_GEN)for header in $(openvswitchinclude_HEADERS); do  \
- echo $$header;\
-   done | sed 's,^include/\(.*\)$$,#include <\1>,' > $@
+include/openvswitch/cxxtest.cc: \
+   include/openvswitch/automake.mk $(top_builddir)/config.status
+   $(AM_V_GEN){ echo "#include "; \
+   for header in $(openvswitchinclude_HEADERS); do \
+ echo $$header; \
+   done | sed 's,^include/\(.*\)$$,#include <\1>,'; } > $@
 endif
 
 # OVS does not use C++ itself, but it provides public header files
diff --git a/lib/automake.mk b/lib/automake.mk
index 2415f4cd6c25..ca1cf5dd2524 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -159,6 +159,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/ofp-version-opt.h \
lib/ofp-version-opt.c \
lib/ofpbuf.c \
+   lib/ovs-atomic-c++.h \
lib/ovs-atomic-c11.h \
lib/ovs-atomic-clang.h \
lib/ovs-atomic-flag-gcc4.7+.h \
diff --git a/lib/ovs-atomic-c++.h b/lib/ovs-atomic-c++.h
new file mode 100644
index ..5bb88536d799
--- /dev/null
+++ b/lib/ovs-atomic-c++.h
@@ -0,0 +1,64 @@
+/* This header implements atomic operation primitives on compilers that
+ * have built-in support for C11   */
+#ifndef IN_OVS_ATOMIC_H
+#error "This header should only be included indirectly via ovs-atomic.h."
+#endif
+
+#include 
+
+#define ATOMIC(TYPE) std::atomic
+
+using std::atomic_init;
+
+using std::memory_order_relaxed;
+using std::memory_order_consume;
+using std::memory_order_acquire;
+using std::memory_order_release;
+using std::memory_order_acq_rel;
+using std::memory_order_seq_cst;
+
+using std::atomic_thread_fence;
+using std::atomic_signal_fence;
+using std::atomic_is_lock_free;
+
+using std::atomic_store;
+using std::atomic_store_explicit;
+
+using std::atomic_compare_exchange_strong;
+using 

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Set suitable value to ovs geneve and vxlan interfaces' mtu.

[Eric Garver]

On Tue, Oct 17, 2017 at 05:53:37PM +0800, JunhanYan wrote:
> From af5c8a50141c7e1a7723a7f992a9e71ad531274d Mon Sep 17 00:00:00 2001
> From: JunhanYan 
> Date: Tue, 17 Oct 2017 17:26:20 +0800
> Subject: [PATCH] In ovs mtu of ovs geneve interface genev_sys_6081 is always
>  0x, mtu of vxlan vxlan_sys_4789 is good for ipv4 vxlan, but for ipv6
>  vxlan, it is still 65470 as ipv4 vxlan, it's not resonable.
> 
> For vxlan, linux kernel do some work for the mtu, so the mtu of
> ipv4 vxlan looks good.
> This patch fixed this issue in ovs, for the udp tunnel,
> calculate the tunnel interface in ovs, and send the final
> value to the kernel from rtnetlink.
> 
> Signed-off-by: JunhanYan 

Hi,

Thanks for the patch. It's worth noting that newer kernels will clamp
the MTU value.

> ---
>  lib/dpif-netlink-rtnl.c | 19 ++-
>  lib/packets.h   |  6 ++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 0c32e7d..d1b227a 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -58,6 +58,12 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
>  #define IFLA_GENEVE_UDP_ZERO_CSUM6_RX 10
>  #endif
> 
> +#define IP_MAX_MTU  0xFFF0U
> +/* IP header + UDP + VXLAN/GENEVE + Ethernet header */
> +#define UDP_TNL_HEADROOM (20 + 8 + 8 + 14)
> +/* IPv6 header + UDP + VXLAN/GENEVE + Ethernet header */
> +#define UDP6_TNL_HEADROOM (40 + 8 + 8 + 14)
> +
>  static const struct nl_policy rtlink_policy[] = {
>  [IFLA_LINKINFO] = { .type = NL_A_NESTED },
>  };
> @@ -281,13 +287,24 @@ dpif_netlink_rtnl_create(const struct
> netdev_tunnel_config *tnl_cfg,

This patch fails to apply because git-am barfs here. This line belongs
on the line above it - likely because your email client wrapped it.
You'll have better results using git-send-email.

>  struct ifinfomsg *ifinfo;
>  struct ofpbuf request;
>  int err;
> +uint16_t tnl_mtu;
> 
>  ofpbuf_init(, 0);
>  nl_msg_put_nlmsghdr(, 0, RTM_NEWLINK, flags);
>  ifinfo = ofpbuf_put_zeros(, sizeof(struct ifinfomsg));
>  ifinfo->ifi_change = ifinfo->ifi_flags = IFF_UP;
>  nl_msg_put_string(, IFLA_IFNAME, name);
> -nl_msg_put_u32(, IFLA_MTU, UINT16_MAX);
> +/* set mtu for udp tunnel*/
> +if (type == OVS_VPORT_TYPE_VXLAN || type == OVS_VPORT_TYPE_GENEVE){

./utilities/checkpatch.py complains about the curly brace here. There
should be a space.

> +   if (in6_addr_is_mapped_ipv4(_cfg->ipv6_dst)){

ditto.

> +   tnl_mtu = IP_MAX_MTU - UDP_TNL_HEADROOM;
> +   }else{

ditto.

> +   tnl_mtu = IP_MAX_MTU - UDP6_TNL_HEADROOM;
> +   }
> +}else{

ditto.

> +   tnl_mtu = UINT16_MAX;
> +}
> +nl_msg_put_u32(, IFLA_MTU, tnl_mtu);
>  linkinfo_off = nl_msg_start_nested(, IFLA_LINKINFO);
>  nl_msg_put_string(, IFLA_INFO_KIND, kind);
>  infodata_off = nl_msg_start_nested(, IFLA_INFO_DATA);
> diff --git a/lib/packets.h b/lib/packets.h
> index 705d0b2..9d56b42 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -1115,6 +1115,12 @@ in6_addr_get_mapped_ipv4(const struct in6_addr *addr)
>  }
>  }
> 
> +static inline bool
> +in6_addr_is_mapped_ipv4(const struct in6_addr *ipv6_addr)
> +{
> +return IN6_IS_ADDR_V4MAPPED(ipv6_addr);
> +}

I don't see why this inline is necessary. Can you use
IN6_IS_ADDR_V4MAPPED() directly?

> +
>  static inline void
>  in6_addr_solicited_node(struct in6_addr *addr, const struct in6_addr *ip6)
>  {
> -- 
> 2.9.3
> 
> ___
> 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] SPENDE

[Llandybie Admin]

Grüße dich,
Mein Name ist Mavis wanczyk, der Gewinner des Power-Jackpots von $ 758,7 
Millionen im August 24, 2017, Mein Jackpot war ein Geschenk von Gott an mich, 
daher hat meine gesamte Familie / Stiftung dies vereinbart. Meine Stiftung 
spendet $ 500,000.00USD an Sie. bitte contac maviswanczy...@yahoo.com für alle 
Details und bitte akzeptieren Sie diesen Gutschein als ein Geschenk von mir und 
meiner Familie.
Freundliche Grüße,Mavis Wanczyk
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] ovsdb-server: Anyways to specify the source interface to be used for remote connections??

[Ben Pfaff]

On Tue, Oct 17, 2017 at 12:52:08PM +0530, Arunkumar Rg wrote:
> In ovsdb-server, we can use "--remote:[ssl|tcp]:[ip-1]:[port]" to specify
> the remote connection to "ip-1".
> 
> Now do we an option or anyways to specify which interface on the box(where
> ovsdb-server is running) to be used as the interface to connect to the
> remote ip-1??

No.

Do other network servers provide such an option?  How do they do it?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/3] ovs-save: Use a file to restore flows instead of heredoc

[Flavio Leitner]

On Mon, 25 Sep 2017 16:44:05 +0200
Timothy Redaelli  wrote:

> This patch makes ovs-save to use a file to restore flows instead of using
> shell script here-document.
> This is needed since eval + here-documents are much slower than reading a file
> with the rules directly.
> 
> Signed-off-by: Timothy Redaelli 
> ---
>  utilities/ovs-save | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 

Acked-by: Flavio Leitner 
Thanks!
fbl

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

Re: [ovs-dev] [PATCH v2 1/3] ovs-save: Use --bundle to restore flows (on OpenFlow 1.4+)

[Flavio Leitner]

On Mon, 25 Sep 2017 16:44:04 +0200
Timothy Redaelli  wrote:

> If possible, use OpenFlow 1.4 atomic bundle transaction to restore flows.
> The patch uses also the highest enabled OpenFlow version to do the queries.
> 
> With the actual implementation, if you have the default OpenFlow version
> disabled then ovs-save fails. This patch also fixes that problem.
> 
> Signed-off-by: Timothy Redaelli 
> ---
>  utilities/ovs-save | 22 +++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 

Acked-by: Flavio Leitner 
Thanks!
fbl

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

Re: [ovs-dev] [PATCH v6 5/5] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.

[Fischetti, Antonio]

Thanks Mark, comments inline.

-Antonio

> -Original Message-
> From: Kavanagh, Mark B
> Sent: Tuesday, October 17, 2017 2:36 PM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Cc: Darrell Ball ; Loftus, Ciara ;
> Kevin Traynor ; Aaron Conole 
> Subject: RE: [PATCH v6 5/5] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
> 
> >From: Fischetti, Antonio
> >Sent: Monday, October 16, 2017 2:15 PM
> >To: d...@openvswitch.org
> >Cc: Kavanagh, Mark B ; Darrell Ball
> >; Loftus, Ciara ; Kevin Traynor
> >; Aaron Conole ; Fischetti, Antonio
> >
> >Subject: [PATCH v6 5/5] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
> >
> >For readability purposes dpdk_mp_put is renamed as dpdk_mp_free.
> >Some other comments are also added to mempool functions.
> 
> Hey Antonio,
> 
> Some minor comments inline.
> 
> Thanks,
> Mark
> 
> >
> >CC: Mark B Kavanagh 
> >CC: Darrell Ball 
> >CC: Ciara Loftus 
> >CC: Kevin Traynor 
> >CC: Aaron Conole 
> >Signed-off-by: Antonio Fischetti 
> >---
> > lib/netdev-dpdk.c | 15 ++-
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >index 5cf1392..ca07918 100644
> >--- a/lib/netdev-dpdk.c
> >+++ b/lib/netdev-dpdk.c
> >@@ -587,6 +587,9 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
> >*mp_exists)
> > return NULL;
> > }
> >
> >+/* Returns a valid pointer when either of the two cases occur:
> >+ * a new mempool was just created or the requested mempool is already
> >+ * existing. */
> 
> It may be clearer to re-write the above as follows:
> 
> "
> Returns a valid pointer when either of the following is true:
>  - a new mempool was just created
>  - a matching mempool already exists
> "
> 

[Antonio]
Thanks, will do that.


> > static struct dpdk_mp *
> > dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool *mp_exists)
> > {
> >@@ -599,8 +602,9 @@ dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool
> >*mp_exists)
> > return dmp;
> > }
> >
> >+/* Release an existing mempool. */
> > static void
> >-dpdk_mp_put(struct dpdk_mp *dmp)
> >+dpdk_mp_free(struct dpdk_mp *dmp)
> > {
> > char *mp_name;
> >
> >@@ -617,8 +621,8 @@ dpdk_mp_put(struct dpdk_mp *dmp)
> > ovs_mutex_unlock(_mp_mutex);
> > }
> >
> >-/* Tries to allocate new mempool on requested_socket_id with
> >- * mbuf size corresponding to requested_mtu.
> >+/* Tries to allocate a new mempool on requested_socket_id with a size
> >+ * determined by requested_mtu and requested Rx/Tx queues.
> 
> This comment needs to be updated, as previously described in review comments 
> of
> patch 1 of the series.

[Antonio] ok, will do.


> 
> 
> >  * On success new configuration will be applied.
> >  * On error, device will be left unchanged. */
> > static int
> >@@ -644,7 +648,8 @@ netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
> > dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
> > return EEXIST;
> > } else {
> >-dpdk_mp_put(dev->dpdk_mp);
> >+/* A new mempool was created, release the previous one. */
> >+dpdk_mp_free(dev->dpdk_mp);
> > dev->dpdk_mp = mp;
> > dev->mtu = dev->requested_mtu;
> > dev->socket_id = dev->requested_socket_id;
> >@@ -1089,7 +1094,7 @@ common_destruct(struct netdev_dpdk *dev)
> > OVS_EXCLUDED(dev->mutex)
> > {
> > rte_free(dev->tx_q);
> >-dpdk_mp_put(dev->dpdk_mp);
> >+dpdk_mp_free(dev->dpdk_mp);
> >
> > ovs_list_remove(>list_node);
> > free(ovsrcu_get_protected(struct ingress_policer *,
> >--
> >2.4.11

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

Re: [ovs-dev] [PATCH v6 3/5] netdev-dpdk: manage empty mempool names.

[Fischetti, Antonio]

Thanks Mark for your suggestions, I'll rework accordingly.

-Antonio

> -Original Message-
> From: Kavanagh, Mark B
> Sent: Tuesday, October 17, 2017 2:35 PM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Cc: Darrell Ball ; Loftus, Ciara ;
> Kevin Traynor ; Aaron Conole 
> Subject: RE: [PATCH v6 3/5] netdev-dpdk: manage empty mempool names.
> 
> >From: Fischetti, Antonio
> >Sent: Monday, October 16, 2017 2:15 PM
> >To: d...@openvswitch.org
> >Cc: Kavanagh, Mark B ; Darrell Ball
> >; Loftus, Ciara ; Kevin Traynor
> >; Aaron Conole ; Fischetti, Antonio
> >
> >Subject: [PATCH v6 3/5] netdev-dpdk: manage empty mempool names.
> 
> It's not just empty names - the name could also be too long. Probably best to
> rephrase the commit name accordingly.
> 
> >
> >In case a mempool name could not be generated log a message
> >and return a null mempool pointer to the caller.
> >
> >CC: Mark B Kavanagh 
> >CC: Darrell Ball 
> >CC: Ciara Loftus 
> >CC: Kevin Traynor 
> >CC: Aaron Conole 
> >Signed-off-by: Antonio Fischetti 
> >---
> > lib/netdev-dpdk.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >index 07c438a..dd5759b 100644
> >--- a/lib/netdev-dpdk.c
> >+++ b/lib/netdev-dpdk.c
> >@@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> > int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
> >h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> > if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> >+VLOG_ERR("Failed to generate a mempool name for \"%s\". "
> >+"Hash:0x%x, mtu:%d, mbufs:%u, ret:%d",
> >+dmp->if_name, h, dmp->mtu, dmp->mp_size, ret);
> 
> A string from ovs_strerror(ret) would probably be more useful than the return
> value itself here.
> I'm not sure how useful the individual values themselves are in an ERR log
> either (more suited to a DBG log).
> -Mark
> 
> > return NULL;
> > }
> > return mp_name;
> >@@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
> >*mp_exists)
> >
> > do {
> > char *mp_name = dpdk_mp_name(dmp);
> >+if (!mp_name) {
> >+rte_free(dmp);
> >+return NULL;
> >+}
> >
> > VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
> >  "with %d Rx and %d Tx queues, socket id:%d.",
> >--
> >2.4.11

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

Re: [ovs-dev] [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.

[Fischetti, Antonio]

> -Original Message-
> From: Kavanagh, Mark B
> Sent: Tuesday, October 17, 2017 2:34 PM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Cc: Darrell Ball ; Loftus, Ciara ;
> Kevin Traynor ; Aaron Conole 
> Subject: RE: [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
> 
> >From: Fischetti, Antonio
> >Sent: Monday, October 16, 2017 2:15 PM
> >To: d...@openvswitch.org
> >Cc: Kavanagh, Mark B ; Darrell Ball
> >; Loftus, Ciara ; Kevin Traynor
> >; Aaron Conole ; Fischetti, Antonio
> >
> >Subject: [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
> >
> >Skip initialization of mempool packet areas if this was already
> >done in a previous call to dpdk_mp_create.
> 
> Hi Antonio,
> 
> As stated in my previous review, I believe that this could probably be folded
> into patch 1 of the series (it was patch 3 of v5).
> However, I don't object strongly to this patch, so I'll leave it to your
> discretion.

[Antonio]
I'm keeping this change in a separate patch because it is not related 
to the fixes for the mempool management. 
It's a small improvement, not so much to save CPU cycles, it's actually 
a clean up of the code. 
 
> 
> Other than that, LGTM.
> 
> Thanks,
> Mark
> 
> >
> >CC: Mark B Kavanagh 
> >CC: Darrell Ball 
> >CC: Ciara Loftus 
> >CC: Kevin Traynor 
> >CC: Aaron Conole 
> >Signed-off-by: Antonio Fischetti 
> >---
> > lib/netdev-dpdk.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >index 7f2d7ed..07c438a 100644
> >--- a/lib/netdev-dpdk.c
> >+++ b/lib/netdev-dpdk.c
> >@@ -550,6 +550,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
> >*mp_exists)
> > if (dmp->mp) {
> > VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
> >  dmp->mp_size);
> >+/* rte_pktmbuf_pool_create has done some initialization of the
> >+ * rte_mbuf part of each dp_packet. Some OvS specific fields
> >+ * of the packet still need to be initialized by
> >+ * ovs_rte_pktmbuf_init. */
> >+rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
> > } else if (rte_errno == EEXIST) {
> > /* A mempool with the same name already exists.  We just
> >  * retrieve its pointer to be returned to the caller. */
> >@@ -566,11 +571,6 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
> >*mp_exists)
> > }
> > free(mp_name);
> > if (dmp->mp) {
> >-/* rte_pktmbuf_pool_create has done some initialization of the
> >- * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
> >- * initializes some OVS specific fields of dp_packet.
> >- */
> >-rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
> > return dmp;
> > }
> > } while (!(*mp_exists) &&
> >--
> >2.4.11

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

Re: [ovs-dev] [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing mempools.

[Fischetti, Antonio]

Thanks Mark, comments inline.

-Antonio

> -Original Message-
> From: Kavanagh, Mark B
> Sent: Tuesday, October 17, 2017 2:34 PM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Cc: Kevin Traynor ; Aaron Conole ;
> Darrell Ball 
> Subject: RE: [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing
> mempools.
> 
> >From: Fischetti, Antonio
> >Sent: Monday, October 16, 2017 2:15 PM
> >To: d...@openvswitch.org
> >Cc: Kavanagh, Mark B ; Kevin Traynor
> >; Aaron Conole ; Darrell Ball
> >; Fischetti, Antonio 
> >Subject: [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing mempools.
> >
> >Fix issues on reconfiguration of pre-existing mempools.
> >This patch avoids to call dpdk_mp_put() - and erroneously
> >release the mempool - when it already exists.
> >Create mempool names by considering also the NUMA socket number.
> >So a name reflects what socket the mempool is allocated on.
> >This change is needed for the NUMA-awareness feature.
> 
> Hi Antonio,
> 
> Is there any particular reason why you've combined patches 1 and 2 of the
> previous series in a single patch here?
> 
> I would have thought that these two separate issues would warrant two
> individual patches (particularly with respect to the reported-by, tested-by
> tags).

[Antonio]
I guess I misunderstood your previous review where you asked to squash patches 
1 and 3 into one patch.
I understood instead to squash the first 2 patches because they were both 
bug-fixes.
In the next version v7 I'll restore the 2 separate patches.

> 
> Maybe it's not a big deal, but noted here nonetheless.
> 
> Apart from that, there are some comments inline.
> 
> Thanks again,
> Mark
> 
> >
> >CC: Mark B Kavanagh 
> >CC: Kevin Traynor 
> >CC: Aaron Conole 
> >CC: Darrell Ball 
> >Reported-by: Ciara Loftus 
> >Tested-by: Ciara Loftus 
> >Reported-by: Róbert Mulik 
> >Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
> >port.")
> >Signed-off-by: Antonio Fischetti 
> >---
> > Test on releasing pre-existing mempools
> > ===
> >I've tested this patch by
> >  - changing at run-time the number of Rx queues:
> >  ovs-vsctl set Interface dpdk0 type=dpdk options:n_rxq=4
> >
> >  - reducing the MTU of the dpdk ports of 1 byte to force
> >the configuration of an existing mempool:
> >  ovs-vsctl set Interface dpdk0 mtu_request=1499
> >
> >This issue was observed in a PVP test topology with dpdkvhostuserclient
> >ports. It can happen also with dpdk type ports, eg by reducing the MTU
> >of 1 byte.
> >
> >To replicate the bug scenario in the PVP case it's sufficient to
> >set 1 dpdkvhostuserclient port, and just boot the VM.
> >
> >Below some more details on my own test setup.
> >
> > PVP test setup
> > --
> >CLIENT_SOCK_DIR=/tmp
> >SOCK0=dpdkvhostuser0
> >SOCK1=dpdkvhostuser1
> >
> >1 PMD
> >Add 2 dpdk ports, n_rxq=1
> >Add 2 vhu ports both of type dpdkvhostuserclient and specify 
> >vhost-server-path
> > ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-
> >path="$CLIENT_SOCK_DIR/$SOCK0"
> > ovs-vsctl set Interface dpdkvhostuser1 options:vhost-server-
> >path="$CLIENT_SOCK_DIR/$SOCK1"
> >
> >Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1
> > add-flow br0 in_port=1,action=output:3
> > add-flow br0 in_port=3,action=output:1
> > add-flow br0 in_port=4,action=output:2
> > add-flow br0 in_port=2,action=output:4
> >
> > Launch QEMU
> > ---
> >As OvS vhu ports are acting as clients, we must specify 'server' in the next
> >command.
> >VM_IMAGE=
> >
> > sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name us-
> >vhost-vm1 -cpu host -enable-kvm -m 4096M -object memory-backend-
> >file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem
> >-mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev
> >socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev type=vhost-
> >user,id=mynet1,chardev=char0,vhostforce -device virtio-net-
> >pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev
> >socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev type=vhost-
> >user,id=mynet2,chardev=char1,vhostforce -device virtio-net-
> >pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic
> >
> > Expected behavior
> > -
> >With this fix OvS shouldn't crash.
> >
> > Test NUMA-Awareness feature
> > ===
> >Mempool names now contains the requested socket id and become like:
> >"ovs_4adb057e_1_2030_20512".
> >
> >Tested with DPDK 17.05.2 (from dpdk-stable branch).
> >NUMA-awareness feature enabled 

Re: [ovs-dev] [PATCH 0/4] prioritizing latency sensitive traffic

[O Mahony, Billy]

Hi All,

As a suggestion for dealing with indicating success or otherwise of ingress 
scheduling configuration and also advertising an Interfaces ingress scheduling 
capability I'm suggesting both these can be written back to the Interface 
tables other_config column.

The schema change (change with respect to the current patch-set) would be like 
this. 

   
   
 
  The format of the ingress_sched field is specified in ovs-fields(7) in
  the ``Matching'' and ``FIELD REFERENCE'' sections.
 
   
+  
+
+A comma separated list of ovs-fields(7) that the interface supports for
+ingress scheduling. If ingress scheduling is not supported this column
+is cleared.
+
+  
+  
+
+If the specified ingress scheduling could not be applied, Open vSwitch
+sets this column to an error description in human readable form.
+Otherwise, Open vSwitch clears this column.
+
+  
 


It would be nice to have input on the feasibility of writing back to the 
Interface table - there is already a few columns that are written to in 
Interface table - e.g stats column and  ofport column. But this would make the 
other_config column both read and write which hopefully doesn't confuse the 
mechanism that notifies Interface table changes from ovsdb into vswitchd. 

Regards,
Billy. 


> -Original Message-
> From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
> Sent: Friday, September 22, 2017 12:37 PM
> To: O Mahony, Billy ; Kevin Traynor
> ; d...@openvswitch.org
> Cc: Mechthild Buescher ; Venkatesan
> Pradeep 
> Subject: RE: [ovs-dev] [PATCH 0/4] prioritizing latency sensitive traffic
> 
> Hi Billy,
> 
> > -Original Message-
> > From: O Mahony, Billy [mailto:billy.o.mah...@intel.com]
> > Sent: Friday, 22 September, 2017 10:52
> 
> > > The next question is how to classify the ingress traffic on the NIC
> > > and insert it into rx queues with different priority. Any scheme
> > > implemented should preferably work with as many NICs as possible.
> > > Use of the new rte_flow API in DPDK seems the right direction to go here.
> >
> > [[BO'M]] This may be getting ahead of where we are but is it important to
> know if a NIC does not support a prioritization scheme?
> > Someone, Darrell I believe mentioned a capability discovery mechanism
> > at one point. I was thinking it was not necessary as functionally
> > nothing changes if prioritization is or is not supported. But maybe in 
> > terms of
> an orchestrator it does make sense - as the it may want to want to make other
> arrangements to protect control traffic in the absence of a working
> prioritization mechanism.
> 
> [Jan] In our use case the configuration of filters for prioritization would 
> happen
> "manually" at OVS deployment time with full knowledge of the NIC type and
> capabilities. A run-time capability discovery mechanism is not really needed 
> for
> that. But it would anyway be good to get a feedback if the configured filter 
> is
> supported by the present NIC or if the prioritization will not work.
> 
> > >
> > > We are very interested in starting the dialogue how to configure the
> > > {queue, priority, filter} mapping in OVS and which filters are most
> > > meaningful to start with and supported by most NICs. Candidates
> > > could include VLAN tags and p- bits, Ethertype and IP DSCP.
> 
> Any feedback as to the viability of filtering on those fields with i40e and 
> ixgbe?
> 
> > >
> > > One thing that we consider important and that we would not want to
> > > lose with prioritization is the possibility to share load over a
> > > number of PMDs with RSS. So preferably the prioritization and RSS
> > > spread over a number of rx queues were orthogonal.
> >
> > [[BO'M]] We have a proposed solution for this now. Which is simply to
> > change the RETA table to avoid RSS'd packets 'polluting' the priority
> > queue. It hasn't been implemented but it should work. That's in the context 
> > of
> DPDK/FlowDirector/XL710 but rte_flow api should allow this too.
> 
> [Jan] Does this mean there is work needed to enhance the NIC firmware, the
> i40e DPDK PMD, or the rte_flow API (or any combination of those)? What about
> the ixgbe PMD in this context? Will the Niantic  support similar 
> classification?
> 
> Do you have a pointer to Fortville documentation that would help us to
> understand how i40e implements the rte_flow API.
> 
> Thanks, Jan

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

Re: [ovs-dev] [PATCH] ovn pacemaker: Provide the option to configure inactivity probe value

[Miguel Angel Ajo Pelayo]

Acked-By: Miguel Angel Ajo 

It makes sense to be able to configure the inactive probe time, also
disabling the echo requests on server, as Ben said I agree would also make
sense in any future patch.

On Mon, Oct 16, 2017 at 9:48 PM, Ben Pfaff  wrote:

> On Mon, Oct 16, 2017 at 10:58:43AM -0700, Ben Pfaff wrote:
> > On Mon, Oct 16, 2017 at 02:50:48PM +0530, Numan Siddique wrote:
> > > On Sat, Oct 14, 2017 at 2:56 AM, Ben Pfaff  wrote:
> > >
> > > > On Fri, Oct 13, 2017 at 12:06:56PM -0400, Russell Bryant wrote:
> > > > > On Fri, Oct 13, 2017 at 8:30 AM, Numan Siddique <
> nusid...@redhat.com>
> > > > wrote:
> > > > > > On Fri, Oct 13, 2017 at 6:05 AM, Andy Zhou 
> wrote:
> > > > > >
> > > > > >> Hi, Numan,
> > > > > >>
> > > > > >> I am curious why default 5 seconds inactivity time does not
> work? Do
> > > > > >> you have more details?
> > > > > >>
> > > > > >> Does the glitch usually happen around the HA switch over?  If
> this
> > > > > >> happens during normal operation,
> > > > > >> Then this is not HA specific issue, but an indication of some
> > > > > >> connectivity issues.
> > > > > >>
> > > > > >
> > > > > > Hi Andy. This happens in the openstack deployment and when the
> > > > > > neutron-server is busy handling lots of API requests.
> > > > > > Normally the deployment would be having 3 controller nodes and
> > > > > > neutron-server would be running in each node.  On each
> controller node,
> > > > > > neutron-server starts around 10 - 12 neutron workers (which are
> > > > separate
> > > > > > processes).  Number of API workers is a configuration option and
> > > > normally
> > > > > > number of cores = no of neutron works if not configured.
> > > > > >
> > > > > > I have tested  in both physical nodes deployment and virtual
> > > > deployment (3
> > > > > > controllers running as vms in a node). Around 40 connections are
> > > > opened to
> > > > > > the OVN north ovsdb-server by all the neutron workers in the
> physical
> > > > > > deployment and around 15 connections are opened in the virtual
> > > > deployment.
> > > > > > When neutron-server is loaded with many API requests, I have
> noticed
> > > > that,
> > > > > > ovsdb-server drops the connections when it doesn't get the echo
> reply
> > > > every
> > > > > > 5 seconds. This leads to lot of reconnections to the
> ovsdb-server and
> > > > the
> > > > > > response from the neutron-server is very slow and bad.  With this
> > > > patch it
> > > > > > seems to work fine.
> > > > > >
> > > > > > The issue is not because of any network issues but because of
> lots of
> > > > > > connections from the neutron-server workers to the ovsdb-server
> and
> > > > failure
> > > > > > by the idl clients to reply to the echo request every 5 seconds
> when
> > > > the
> > > > > > neutron-server is loaded.
> > > > >
> > > > > We have to disable the inactivity probe everywhere each time we
> have
> > > > > done performance testing so far.
> > > >
> > > > Really this seems that it's a bug (or inadequacy) in ovsdb-server.
> It's
> > > > pretty sad that ovsdb-server can't reply within 5 seconds
> > >
> > >
> > > It's actually the ovsdb python idl client which is not able to reply
> within
> > > 5 seconds for the
> > > echo request from ovsdb-server.
> >
> > Oh, I'm surprised that ovsdb-server is doing the echo-requests, I
> > thought that we generally did them from the client end.
>
> One perfectly acceptable approach might be to simply disable
> echo-requests on the server side entirely and do them from the client.
> ___
> 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] Pricelist 16 - 10 - 2017

[Bonesca Mailing]

Deze email nieuwsbrief werd in grafisch HTML formaat verzonden.
Als u deze tekstversie ziet, verkiest uw email programma "gewone tekst" emails.
U kan de originele nieuwsbrief online bekijken:
http://ymlptrack5.com/zOrIBf



GB:  In the attachment you can find our latest pricelist!
DE: Im Anlage finden sie unsere Aktuelle Preisliste!
NL: In de bijlage vind u onze meest recente prijslijst!
FR: Dans l'attachement vous pouvez trouver notre liste des prix
nouveau!
ES: En archivo adjunto encontrará nuestro listado de presios!

Our offer online and for more offers click here (
http://bonesca.mamutweb.com/subdet1.htm )

Kind regards, Mit freundlichen Grüssen, Cordialement,

ber...@bonesca.nl - purchase/sales, Dutch, German, English
ne...@bonesca.nl - sales, Dutch, Arab, English, German
maria...@bonesca.nl - sales, Dutch, French, English, German
t...@bonesca.nl - sales, Vietnamese, Thai, Laothioan, German, English
r...@bonesca.nl - sales, Tamil, Dutch, English
poli...@bonesca.nl - sales: Spanish, French, Portuguese, English

Bonesca Import en Export BV

Schulpengat 9
8321 WC URK
Netherlands
Tel.: +31 (0) 527 70 10 63
Fax: +31 (0) 527 69 04 46
Mail: i...@bonesca.nl
Web: www.bonesca.nl

_
Sign out / Change E-mailadress: 
http://ymlptrack5.com/ughbywyjgsgubbebbgjehjgghewbs
Powered door YourMailingListProvider

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

[ovs-dev] [PATCH 1/1] debian: Do not modify pre-existing defaults file

[Frode Nordahl]

Currently, on installation or upgrade the openvswitch-switch deb package
will in some circumstances modify a pre-existing
/etc/default/openvswitch-switch configuration file.

This does not play well with modeling and configuration management tools
and may lead to unnecessary restarts of the openvswitch-switch service
after the initial restart done as part of the package upgrade. As
restarting the openvswitch-switch affects the datapath this is
something we should try to avoid.

I also believe the current behaviour to be in conflict with best practices
set out in the config files section of the
[Debian Policy](https://www.debian.org/doc/debian-policy/#s-config-files).

This commit addresses this by removing the part of the postinst script
that attempts to append missing documentation parts of the template
and leaves the installed defaults file alone when it exists.

Signed-off-by: Frode Nordahl 
Reported-at: https://github.com/openvswitch/ovs-issues/issues/137
---
diff --git a/debian/changelog b/debian/changelog
index d2b244fca..f1a0e0c77 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,6 +1,7 @@
 openvswitch (2.8.90-1) unstable; urgency=medium

-   - Nothing yet!  Try NEWS...
+   [ Open vSwitch team ]
+   * Do not modify pre-existing /etc/default/openvswitch-switch

  -- Open vSwitch team   Fri, 04 Aug 2017 15:00:00
-0700

diff --git a/debian/openvswitch-switch.postinst
b/debian/openvswitch-switch.postinst
index 3f9b0553f..f8abd40e2 100755
--- a/debian/openvswitch-switch.postinst
+++ b/debian/openvswitch-switch.postinst
@@ -24,14 +24,6 @@ case "$1" in
 TEMPLATE=/usr/share/openvswitch/switch/default.template
 if ! test -e $DEFAULT; then
 cp $TEMPLATE $DEFAULT
-else
-for var in $(awk -F'[ :]' '/^# [_A-Z0-9]+:/{print $2}'
$TEMPLATE)
-do
-if ! grep $var $DEFAULT >/dev/null 2>&1; then
-echo >> $DEFAULT
-sed -n "/$var:/,/$var=/p" $TEMPLATE >> $DEFAULT
-fi
-done
 fi
 # Certain versions of upstream Ubuntu's openvswitch packages (which
 # are forks) may install upstart files which are incompatible
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6 5/5] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.

[Kavanagh, Mark B]

>From: Fischetti, Antonio
>Sent: Monday, October 16, 2017 2:15 PM
>To: d...@openvswitch.org
>Cc: Kavanagh, Mark B ; Darrell Ball
>; Loftus, Ciara ; Kevin Traynor
>; Aaron Conole ; Fischetti, Antonio
>
>Subject: [PATCH v6 5/5] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
>
>For readability purposes dpdk_mp_put is renamed as dpdk_mp_free.
>Some other comments are also added to mempool functions.

Hey Antonio,

Some minor comments inline.

Thanks,
Mark

>
>CC: Mark B Kavanagh 
>CC: Darrell Ball 
>CC: Ciara Loftus 
>CC: Kevin Traynor 
>CC: Aaron Conole 
>Signed-off-by: Antonio Fischetti 
>---
> lib/netdev-dpdk.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 5cf1392..ca07918 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -587,6 +587,9 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>*mp_exists)
> return NULL;
> }
>
>+/* Returns a valid pointer when either of the two cases occur:
>+ * a new mempool was just created or the requested mempool is already
>+ * existing. */

It may be clearer to re-write the above as follows:

"
Returns a valid pointer when either of the following is true:
 - a new mempool was just created
 - a matching mempool already exists
"

> static struct dpdk_mp *
> dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool *mp_exists)
> {
>@@ -599,8 +602,9 @@ dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool
>*mp_exists)
> return dmp;
> }
>
>+/* Release an existing mempool. */
> static void
>-dpdk_mp_put(struct dpdk_mp *dmp)
>+dpdk_mp_free(struct dpdk_mp *dmp)
> {
> char *mp_name;
>
>@@ -617,8 +621,8 @@ dpdk_mp_put(struct dpdk_mp *dmp)
> ovs_mutex_unlock(_mp_mutex);
> }
>
>-/* Tries to allocate new mempool on requested_socket_id with
>- * mbuf size corresponding to requested_mtu.
>+/* Tries to allocate a new mempool on requested_socket_id with a size
>+ * determined by requested_mtu and requested Rx/Tx queues.

This comment needs to be updated, as previously described in review comments of 
patch 1 of the series.


>  * On success new configuration will be applied.
>  * On error, device will be left unchanged. */
> static int
>@@ -644,7 +648,8 @@ netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
> dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
> return EEXIST;
> } else {
>-dpdk_mp_put(dev->dpdk_mp);
>+/* A new mempool was created, release the previous one. */
>+dpdk_mp_free(dev->dpdk_mp);
> dev->dpdk_mp = mp;
> dev->mtu = dev->requested_mtu;
> dev->socket_id = dev->requested_socket_id;
>@@ -1089,7 +1094,7 @@ common_destruct(struct netdev_dpdk *dev)
> OVS_EXCLUDED(dev->mutex)
> {
> rte_free(dev->tx_q);
>-dpdk_mp_put(dev->dpdk_mp);
>+dpdk_mp_free(dev->dpdk_mp);
>
> ovs_list_remove(>list_node);
> free(ovsrcu_get_protected(struct ingress_policer *,
>--
>2.4.11

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

Re: [ovs-dev] [PATCH v6 3/5] netdev-dpdk: manage empty mempool names.

[Kavanagh, Mark B]

>From: Fischetti, Antonio
>Sent: Monday, October 16, 2017 2:15 PM
>To: d...@openvswitch.org
>Cc: Kavanagh, Mark B ; Darrell Ball
>; Loftus, Ciara ; Kevin Traynor
>; Aaron Conole ; Fischetti, Antonio
>
>Subject: [PATCH v6 3/5] netdev-dpdk: manage empty mempool names.

It's not just empty names - the name could also be too long. Probably best to 
rephrase the commit name accordingly.

>
>In case a mempool name could not be generated log a message
>and return a null mempool pointer to the caller.
>
>CC: Mark B Kavanagh 
>CC: Darrell Ball 
>CC: Ciara Loftus 
>CC: Kevin Traynor 
>CC: Aaron Conole 
>Signed-off-by: Antonio Fischetti 
>---
> lib/netdev-dpdk.c | 7 +++
> 1 file changed, 7 insertions(+)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 07c438a..dd5759b 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
>h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
>+VLOG_ERR("Failed to generate a mempool name for \"%s\". "
>+"Hash:0x%x, mtu:%d, mbufs:%u, ret:%d",
>+dmp->if_name, h, dmp->mtu, dmp->mp_size, ret);

A string from ovs_strerror(ret) would probably be more useful than the return 
value itself here.
I'm not sure how useful the individual values themselves are in an ERR log 
either (more suited to a DBG log).
-Mark

> return NULL;
> }
> return mp_name;
>@@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>*mp_exists)
>
> do {
> char *mp_name = dpdk_mp_name(dmp);
>+if (!mp_name) {
>+rte_free(dmp);
>+return NULL;
>+}
>
> VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
>  "with %d Rx and %d Tx queues, socket id:%d.",
>--
>2.4.11

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

Re: [ovs-dev] [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing mempools.

[Kavanagh, Mark B]

>From: Fischetti, Antonio
>Sent: Monday, October 16, 2017 2:15 PM
>To: d...@openvswitch.org
>Cc: Kavanagh, Mark B ; Kevin Traynor
>; Aaron Conole ; Darrell Ball
>; Fischetti, Antonio 
>Subject: [PATCH v6 1/5] netdev-dpdk: fix management of pre-existing mempools.
>
>Fix issues on reconfiguration of pre-existing mempools.
>This patch avoids to call dpdk_mp_put() - and erroneously
>release the mempool - when it already exists.
>Create mempool names by considering also the NUMA socket number.
>So a name reflects what socket the mempool is allocated on.
>This change is needed for the NUMA-awareness feature.

Hi Antonio,

Is there any particular reason why you've combined patches 1 and 2 of the 
previous series in a single patch here?

I would have thought that these two separate issues would warrant two 
individual patches (particularly with respect to the reported-by, tested-by 
tags).

Maybe it's not a big deal, but noted here nonetheless.

Apart from that, there are some comments inline.

Thanks again,
Mark

>
>CC: Mark B Kavanagh 
>CC: Kevin Traynor 
>CC: Aaron Conole 
>CC: Darrell Ball 
>Reported-by: Ciara Loftus 
>Tested-by: Ciara Loftus 
>Reported-by: Róbert Mulik 
>Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
>port.")
>Signed-off-by: Antonio Fischetti 
>---
> Test on releasing pre-existing mempools
> ===
>I've tested this patch by
>  - changing at run-time the number of Rx queues:
>  ovs-vsctl set Interface dpdk0 type=dpdk options:n_rxq=4
>
>  - reducing the MTU of the dpdk ports of 1 byte to force
>the configuration of an existing mempool:
>  ovs-vsctl set Interface dpdk0 mtu_request=1499
>
>This issue was observed in a PVP test topology with dpdkvhostuserclient
>ports. It can happen also with dpdk type ports, eg by reducing the MTU
>of 1 byte.
>
>To replicate the bug scenario in the PVP case it's sufficient to
>set 1 dpdkvhostuserclient port, and just boot the VM.
>
>Below some more details on my own test setup.
>
> PVP test setup
> --
>CLIENT_SOCK_DIR=/tmp
>SOCK0=dpdkvhostuser0
>SOCK1=dpdkvhostuser1
>
>1 PMD
>Add 2 dpdk ports, n_rxq=1
>Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost-server-path
> ovs-vsctl set Interface dpdkvhostuser0 options:vhost-server-
>path="$CLIENT_SOCK_DIR/$SOCK0"
> ovs-vsctl set Interface dpdkvhostuser1 options:vhost-server-
>path="$CLIENT_SOCK_DIR/$SOCK1"
>
>Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1
> add-flow br0 in_port=1,action=output:3
> add-flow br0 in_port=3,action=output:1
> add-flow br0 in_port=4,action=output:2
> add-flow br0 in_port=2,action=output:4
>
> Launch QEMU
> ---
>As OvS vhu ports are acting as clients, we must specify 'server' in the next
>command.
>VM_IMAGE=
>
> sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name us-
>vhost-vm1 -cpu host -enable-kvm -m 4096M -object memory-backend-
>file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa node,memdev=mem
>-mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev
>socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev type=vhost-
>user,id=mynet1,chardev=char0,vhostforce -device virtio-net-
>pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev
>socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev type=vhost-
>user,id=mynet2,chardev=char1,vhostforce -device virtio-net-
>pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic
>
> Expected behavior
> -
>With this fix OvS shouldn't crash.
>
> Test NUMA-Awareness feature
> ===
>Mempool names now contains the requested socket id and become like:
>"ovs_4adb057e_1_2030_20512".
>
>Tested with DPDK 17.05.2 (from dpdk-stable branch).
>NUMA-awareness feature enabled (DPDK/config/common_base).
>
>Created 1 single dpdkvhostuser port type.
>OvS pmd-cpu-mask=FF3 # enable cores on both numa nodes
>QEMU core mask = 0xFC000 # cores for qemu on numa node 1 only
>
> Before launching the VM:
> 
>ovs-appctl dpif-netdev/pmd-rxq-show
>shows core #1 is serving the vhu port.
>
>pmd thread numa_id 0 core_id 1:
>isolated : false
>port: dpdkvhostuser0queue-id: 0
>
> After launching the VM:
> ---
>the vhu port is now managed by core #27
>pmd thread numa_id 1 core_id 27:
>isolated : false
>port: dpdkvhostuser0queue-id: 0
>
>and the log shows a new mempool is allocated on NUMA node 1, while
>the previous one is deleted:
>
>2017-10-06T14:04:55Z|00105|netdev_dpdk|DBG|Allocated
>"ovs_4adb057e_1_2030_20512" mempool with 20512 mbufs

Re: [ovs-dev] [PATCH] vhost: Expose virtio interrupt requirement on rte_vhos API

[Ilya Maximets]

Sorry, CC: list.

On 14.10.2017 01:57, Jan Scheurich wrote:
>> Hi Sugesh,
>>
>> Actually the new API call in DPDK is not needed. A reply by Zhiyong Yang 
>> (http://dpdk.org/ml/archives/dev/2017-September/076504.html) pointed out 
>> that an existing API call provides access to the vring data structure that 
>> contains the interrupt flag. So I will abandon the DPDK patch.
>>
>> Using the existing API I have created a patch on top of Ilya's output 
>> batching v4 series that automatically enables time-based batching on ports 
>> that should benefit from it most: vhostuser(client) using virtio interrupts 
>> as well as internal ports on the Linux (or BSD) host.
> 


I'm not sure about enabling time based batching by default for guests with
enabled interrupts. I see the performance drop for iperf in VM-VM scenario
on my ARMv8 system:
1.33 with 50ms vs. 1.42 Gbps with 0ms and 1.53 Gbps with 500ms.

I'll share more detailed test results, but it's clear that best time
interval is highly system dependent. This means that we should not make
assumptions about it.


Best regards, Ilya Maximets.

>>
>> I still need to do careful testing that the interrupt detection works 
>> reliable. The performance should be the baseline performance of Ilya's patch.
>>
>> BR, Jan
>>
>>> -Original Message-
>>> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at 
>>> openvswitch.org] On Behalf Of Chandran, Sugesh
>>> Sent: Friday, 13 October, 2017 17:34
>>> To: Jan Scheurich 
>>> Cc: dev at openvswitch.org
>>> Subject: Re: [ovs-dev] [PATCH] vhost: Expose virtio interrupt requirement 
>>> on rte_vhos API
>>>
>>> Hi Jan,
>>> The DPDK changes are looks OK to me and will be useful. I am interested in 
>>> testing this patch to see the impact on performance. Are
>>> you planning to share the changes in OVS for these APIs?
>>>
>>>
>>>
>>>
>>> Performance tests with the OVS DPDK datapath have shown that the tx 
>>> throughput over a vhostuser port into a VM with an interrupt-
>>> based virtio driver is limited by the overhead incurred by virtio 
>>> interrupts. The OVS PMD spends up to 30% of its cycles in system calls
>>> kicking the eventfd. Also the core running the vCPU is heavily loaded with 
>>> generating the virtio interrupts in KVM on the host and
>>> handling these interrupts in the virtio-net driver in the guest. This 
>>> limits the throughput to about 500-700 Kpps with a single vCPU.
>>>
>>> OVS is trying to address this issue by batching packets to a vhostuser port 
>>> for some time to limit the virtio interrupt frequency. With a
>>> 50 us batching period we have measured an iperf3  throughput increase by 
>>> 15% and a PMD utilization decrease from 45% to 30%.
>>>
>>> On the other hand, guests using virtio PMDs do not profit from time-based 
>>> tx batching. Instead they experience a 2-3% performance
>>> penalty and an average latency increase of 30-40 us. OVS therefore intends 
>>> to apply time-based tx batching only for vhostuser tx
>>> queues that need to trigger virtio interrupts.
>>>
>>> Today this information is hidden inside the rte_vhost library and not 
>>> accessible to users of the API. This patch adds a function to the
>>> API to query it.
>>>
>>> Signed-off-by: Jan Scheurich >> ericsson.com>
>>>
>>> ---
>>>
>>>  lib/librte_vhost/rte_vhost.h | 12 
>>>  lib/librte_vhost/vhost.c | 19 +++
>>>  2 files changed, 31 insertions(+)
>>>
>>> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
>>> index 8c974eb..d62338b 100644
>>> --- a/lib/librte_vhost/rte_vhost.h
>>> +++ b/lib/librte_vhost/rte_vhost.h
>>> @@ -444,6 +444,18 @@ int rte_vhost_get_vhost_vring(int vid, uint16_t 
>>> vring_idx,
>>>   */
>>>  uint32_t rte_vhost_rx_queue_count(int vid, uint16_t qid);
>>>
>>> +/**
>>> + * Does the virtio driver request interrupts for a vhost tx queue?
>>> + *
>>> + * @param vid
>>> + *  vhost device ID
>>> + * @param qid
>>> + *  virtio queue index in mq case
>>> + * @return
>>> + *  1 if true, 0 if false
>>> + */
>>> +int rte_vhost_tx_interrupt_requested(int vid, uint16_t qid);
>>> +
>>>  #ifdef __cplusplus
>>>  }
>>>  #endif
>>> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
>>> index 0b6aa1c..bd1ebf9 100644
>>> --- a/lib/librte_vhost/vhost.c
>>> +++ b/lib/librte_vhost/vhost.c
>>> @@ -503,3 +503,22 @@ struct virtio_net *
>>>
>>> return *((volatile uint16_t *)>avail->idx) - vq->last_avail_idx;
>>>  }
>>> +
>>> +int rte_vhost_tx_interrupt_requested(int vid, uint16_t qid)
>>> +{
>>> +struct virtio_net *dev;
>>> +struct vhost_virtqueue *vq;
>>> +
>>> +dev = get_device(vid);
>>> +if (dev == NULL)
>>> +return 0;
>>> +
>>> +vq = dev->virtqueue[qid];
>>> +if (vq == NULL)
>>> +return 0;
>>> +
>>> +if (unlikely(vq->enabled == 0 || vq->avail == NULL))
>>> +return 0;
>>> +
>>> +return !(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT);
>>> +}
>>>

Re: [ovs-dev] TOS for BFD packets

[Raymond Burkholder]

> That's a good point, I don't have experience with that (what's the best
DSCP
> class), but we should make sure BFD packet, as control/monitoring traffic,
is
> prioritised over other types of traffic.

As indicated by Pradeep, CS6 is a good value, and I've seen that in various
locations.

> 
> On Tue, Oct 17, 2017 at 11:12 AM, Venkatesan Pradeep <
> venkatesan.prad...@ericsson.com> wrote:
> 
> > Hi Ben,
> >
> > The implementations I've worked on have used tos=0xC0 (DSCP CS6).
> > Given that DSCP CS6 is recommended for network control traffic, and is
> > used by OSPF, BGP etc., I think it is likely that many implementations
> > do that for BFD too. Since BFD flaps have significant impact, marking
> > BFD packets with the right DSCP value will allow the network to apply
> > the right QoS for those packets and prevent BFD flaps due to network
> congestion.
> >


-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

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

Re: [ovs-dev] TOS for BFD packets

[Raymond Burkholder]

> The implementations I've worked on have used tos=0xC0 (DSCP CS6). Given
> that DSCP CS6 is recommended for network control traffic, and is used by
> OSPF, BGP etc., I think it is likely that many implementations do that for
BFD
> too. Since BFD flaps have significant impact, marking BFD packets with the
> right DSCP value will allow the network to apply the right QoS for those
> packets and prevent BFD flaps due to network congestion.

For my two cents. I would agree with the higher precedence marking for BFD,
having worked and am working in that world, and preventing routing protocol
timeouts in congested links is an important network operations practice.

> 
> Regards,
> 
> Pradeep
> 
> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Monday, October 16, 2017 11:13 PM
> To: Venkatesan Pradeep 
> Cc: ovs-dev@openvswitch.org
> Subject: Re: [ovs-dev] TOS for BFD packets
> 
> On Mon, Oct 16, 2017 at 04:32:45PM +, Venkatesan Pradeep wrote:
> > BFD packets are being sent with tos = IPTOS_LOWDELAY |
> > IPTOS_THROUGHPUT which would put them in the best-effort class. It
> > would be better if we send with tos = IPTOS_PREC_INTERNETCONTROL so
> > that they can be appropriately prioritized in the network.
> 
> Is that common practice for other BFD implementations?  If so, then I
agree
> that OVS should follow it also.
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> --
> This message has been scanned for viruses and dangerous content by
> MailScanner, and is believed to be clean.



-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

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

[ovs-dev] [PATCH RFC v2] netdev-dpdk: Allow specification of index for PCI devices

[Ciara Loftus]

Some NICs have only one PCI address associated with multiple ports. This
patch extends the dpdk-devargs option's format to cater for such
devices. Whereas before only one of N ports associated with one PCI
address could be added, now N can.

This patch allows for the following use of the dpdk-devargs option:

ovs-vsctl set Interface myport options:dpdk-devargs=:06:00.0,X

Where X is an unsigned integer representing one of multiple ports
associated with the PCI address provided.

This patch has not been tested.

Signed-off-by: Ciara Loftus 
---
v2:
* Simplify function to find port ID of indexed device
* Hotplug compatibility
* Detach compatibility
* Documentation
* Use strtol instead of atoi

 Documentation/howto/dpdk.rst |  9 +
 Documentation/intro/install/dpdk.rst |  9 +
 NEWS |  2 ++
 lib/netdev-dpdk.c| 67 ++--
 vswitchd/vswitch.xml | 11 --
 5 files changed, 85 insertions(+), 13 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index d123819..9447b71 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -48,6 +48,15 @@ number of dpdk devices found in the log file::
 $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \
 options:dpdk-devargs=:01:00.1
 
+If your PCI device has multiple ports under the same PCI ID, you can use the
+following notation to indicate the specific device you wish to add::
+
+$ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
+options:dpdk-devargs=:01:00.0,0
+
+The above would attempt to use the first device (0) associated with that PCI
+ID. Use ,1 ,2 etc. to access the next.
+
 After the DPDK ports get added to switch, a polling thread continuously polls
 DPDK devices and consumes 100% of the core, as can be checked from ``top`` and
 ``ps`` commands::
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index bb69ae5..d0e87f5 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -271,6 +271,15 @@ ports. For example, to create a userspace bridge named 
``br0`` and add two
 DPDK devices will not be available for use until a valid dpdk-devargs is
 specified.
 
+If your PCI device has multiple ports under the same PCI ID, you can use the
+following notation to indicate the specific device you wish to add::
+
+$ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
+options:dpdk-devargs=:01:00.0,0
+
+The above would attempt to use the first device (0) associated with that PCI
+ID. Use ,1 ,2 etc. to access the next.
+
 Refer to ovs-vsctl(8) and :doc:`/howto/dpdk` for more details.
 
 Performance Tuning
diff --git a/NEWS b/NEWS
index 75f5fa5..ca885a6 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ Post-v2.8.0
chassis "hostname" in addition to a chassis "name".
- Linux kernel 4.13
  * Add support for compiling OVS with the latest Linux 4.13 kernel
+   - DPDK:
+ * Support for adding devices with multiple DPDK ports under one PCI ID.
 
 v2.8.0 - 31 Aug 2017
- ovs-ofctl:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c60f46f..35e15da 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1214,16 +1214,40 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id)
 }
 
 static dpdk_port_t
+dpdk_get_port_by_name_with_index(char *name, int idx, int base_id)
+{
+struct rte_eth_dev_info dev_info;
+char pci_addr[PCI_PRI_STR_SIZE];
+dpdk_port_t offset_port_id = base_id + idx;
+
+if (rte_eth_dev_is_valid_port(offset_port_id)) {
+rte_eth_dev_info_get(offset_port_id, _info);
+rte_pci_device_name(_info.pci_dev->addr, pci_addr,
+sizeof(pci_addr));
+if (!strcmp(pci_addr, name)) {
+return offset_port_id;
+}
+}
+
+VLOG_INFO("Invalid PCI offset %i for device %s", idx, name);
+
+return DPDK_ETH_PORT_ID_INVALID;
+}
+
+static dpdk_port_t
 netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
 const char *devargs, char **errp)
 {
-/* Get the name up to the first comma. */
-char *name = xmemdup0(devargs, strcspn(devargs, ","));
+char *devargs_copy = xmemdup0((devargs), strlen(devargs));
+char *name, *idx_str;
+unsigned idx;
 dpdk_port_t new_port_id = DPDK_ETH_PORT_ID_INVALID;
 
+name = strtok_r(devargs_copy, ",", _copy);
+idx_str = strtok_r(devargs_copy, ",", _copy);
+
 if (!rte_eth_dev_count()
-|| rte_eth_dev_get_port_by_name(name, _port_id)
-|| !rte_eth_dev_is_valid_port(new_port_id)) {
+|| rte_eth_dev_get_port_by_name(name, _port_id)) {
 /* Device not found in DPDK, attempt to attach it */
 if (!rte_eth_dev_attach(devargs, _port_id)) {
 /* Attach successful */
@@ -1232,12 +1256,21 

Re: [ovs-dev] TOS for BFD packets

[Miguel Angel Ajo Pelayo]

That's a good point, I don't have experience with that (what's the best
DSCP class), but we should make sure BFD packet, as control/monitoring
traffic, is prioritised over other types of traffic.

On Tue, Oct 17, 2017 at 11:12 AM, Venkatesan Pradeep <
venkatesan.prad...@ericsson.com> wrote:

> Hi Ben,
>
> The implementations I've worked on have used tos=0xC0 (DSCP CS6). Given
> that DSCP CS6 is recommended for network control traffic, and is used by
> OSPF, BGP etc., I think it is likely that many implementations do that for
> BFD too. Since BFD flaps have significant impact, marking BFD packets with
> the right DSCP value will allow the network to apply the right QoS for
> those packets and prevent BFD flaps due to network congestion.
>
> Regards,
>
> Pradeep
>
> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Monday, October 16, 2017 11:13 PM
> To: Venkatesan Pradeep 
> Cc: ovs-dev@openvswitch.org
> Subject: Re: [ovs-dev] TOS for BFD packets
>
> On Mon, Oct 16, 2017 at 04:32:45PM +, Venkatesan Pradeep wrote:
> > BFD packets are being sent with tos = IPTOS_LOWDELAY |
> > IPTOS_THROUGHPUT which would put them in the best-effort class. It
> > would be better if we send with tos = IPTOS_PREC_INTERNETCONTROL so
> > that they can be appropriately prioritized in the network.
>
> Is that common practice for other BFD implementations?  If so, then I
> agree that OVS should follow it also.
> ___
> 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] dpif-netlink-rtnl: Set suitable value to ovs geneve and vxlan interfaces' mtu.

[JunhanYan]

From af5c8a50141c7e1a7723a7f992a9e71ad531274d Mon Sep 17 00:00:00 2001

From: JunhanYan 
Date: Tue, 17 Oct 2017 17:26:20 +0800
Subject: [PATCH] In ovs mtu of ovs geneve interface genev_sys_6081 is always
 0x, mtu of vxlan vxlan_sys_4789 is good for ipv4 vxlan, but for ipv6
 vxlan, it is still 65470 as ipv4 vxlan, it's not resonable.

For vxlan, linux kernel do some work for the mtu, so the mtu of
ipv4 vxlan looks good.
This patch fixed this issue in ovs, for the udp tunnel,
calculate the tunnel interface in ovs, and send the final
value to the kernel from rtnetlink.

Signed-off-by: JunhanYan 
---
 lib/dpif-netlink-rtnl.c | 19 ++-
 lib/packets.h   |  6 ++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 0c32e7d..d1b227a 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -58,6 +58,12 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
 #define IFLA_GENEVE_UDP_ZERO_CSUM6_RX 10
 #endif

+#define IP_MAX_MTU  0xFFF0U
+/* IP header + UDP + VXLAN/GENEVE + Ethernet header */
+#define UDP_TNL_HEADROOM (20 + 8 + 8 + 14)
+/* IPv6 header + UDP + VXLAN/GENEVE + Ethernet header */
+#define UDP6_TNL_HEADROOM (40 + 8 + 8 + 14)
+
 static const struct nl_policy rtlink_policy[] = {
 [IFLA_LINKINFO] = { .type = NL_A_NESTED },
 };
@@ -281,13 +287,24 @@ dpif_netlink_rtnl_create(const struct 
netdev_tunnel_config *tnl_cfg,

 struct ifinfomsg *ifinfo;
 struct ofpbuf request;
 int err;
+uint16_t tnl_mtu;

 ofpbuf_init(, 0);
 nl_msg_put_nlmsghdr(, 0, RTM_NEWLINK, flags);
 ifinfo = ofpbuf_put_zeros(, sizeof(struct ifinfomsg));
 ifinfo->ifi_change = ifinfo->ifi_flags = IFF_UP;
 nl_msg_put_string(, IFLA_IFNAME, name);
-nl_msg_put_u32(, IFLA_MTU, UINT16_MAX);
+/* set mtu for udp tunnel*/
+if (type == OVS_VPORT_TYPE_VXLAN || type == OVS_VPORT_TYPE_GENEVE){
+   if (in6_addr_is_mapped_ipv4(_cfg->ipv6_dst)){
+   tnl_mtu = IP_MAX_MTU - UDP_TNL_HEADROOM;
+   }else{
+   tnl_mtu = IP_MAX_MTU - UDP6_TNL_HEADROOM;
+   }
+}else{
+   tnl_mtu = UINT16_MAX;
+}
+nl_msg_put_u32(, IFLA_MTU, tnl_mtu);
 linkinfo_off = nl_msg_start_nested(, IFLA_LINKINFO);
 nl_msg_put_string(, IFLA_INFO_KIND, kind);
 infodata_off = nl_msg_start_nested(, IFLA_INFO_DATA);
diff --git a/lib/packets.h b/lib/packets.h
index 705d0b2..9d56b42 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1115,6 +1115,12 @@ in6_addr_get_mapped_ipv4(const struct in6_addr *addr)
 }
 }

+static inline bool
+in6_addr_is_mapped_ipv4(const struct in6_addr *ipv6_addr)
+{
+return IN6_IS_ADDR_V4MAPPED(ipv6_addr);
+}
+
 static inline void
 in6_addr_solicited_node(struct in6_addr *addr, const struct in6_addr *ip6)
 {
--
2.9.3

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

[ovs-dev] [PATCH] Reduce the number of flows by use conjunction action

[wei]

This patch convert ovn-sb lflow expr "(1 or 2) and (3 or 4)" to
match 1 aciton connjunction(1, 1/2)
match 2 aciton connjunction(1, 1/2)
match 3 aciton connjunction(1, 2/2)
match 4 aciton connjunction(1, 2/2)
match conj_id=1, action=

NOT support nested conjunction, only use conjunction action in situation "or in 
level 1"
Like (1 or 2) and (3 or ((4 or 5) and (6 or 7))), (4 or 5) and (6 or 7) will 
not be converted conjunction action,
We could call this situation as "or in level 2", in this situation (4 or 5) and 
(6 or 7) will be crossproduct,
so (1 or 2) and (3 or ((4 or 5) and (6 or 7))) -> (1 or 2) and (3 or (4 and 6) 
or (4 and 7) or (5 and 6) or (5 and 7))

In openstack, security group rule will match remote security group and tcp 
port, like
match=(ip4.src == $as_ip4_6a8f4283_ba60_4d1c_9dec_28d027eadef2 && tcp.dst >= 
1 && tcp.dst <= 2))

Use this patch, the number of flows will be significantly reduced

Signed-off-by: wei 
---
 ovn/lib/expr.c | 95 --
 1 file changed, 66 insertions(+), 29 deletions(-)

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 79ff45762..11abd7eca 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -2426,12 +2426,12 @@ expr_sort(struct expr *expr)
 return expr;
 }
 
-static struct expr *expr_normalize_or(struct expr *expr);
+static struct expr *expr_normalize_or(struct expr *expr, int level);
 
 /* Returns 'expr', which is an AND, reduced to OR(AND(clause)) where
  * a clause is a cmp or a disjunction of cmps on a single field. */
 static struct expr *
-expr_normalize_and(struct expr *expr)
+expr_normalize_and(struct expr *expr, int level)
 {
 ovs_assert(expr->type == EXPR_T_AND);
 
@@ -2472,40 +2472,55 @@ expr_normalize_and(struct expr *expr)
 }
 
 ovs_assert(sub->type == EXPR_T_OR);
-const struct expr_symbol *symbol = expr_is_cmp(sub);
-if (!symbol || symbol->must_crossproduct) {
-struct expr *or = expr_create_andor(EXPR_T_OR);
-struct expr *k;
-
-LIST_FOR_EACH (k, node, >andor) {
-struct expr *and = expr_create_andor(EXPR_T_AND);
-struct expr *m;
-
-LIST_FOR_EACH (m, node, >andor) {
-struct expr *term = m == sub ? k : m;
-if (term->type == EXPR_T_AND) {
-struct expr *p;
-
-LIST_FOR_EACH (p, node, >andor) {
-struct expr *new = expr_clone(p);
+if (level == 0) {
+LIST_FOR_EACH_SAFE (a, b, node, >andor) {
+if (a->type == EXPR_T_CMP) {
+continue;
+} else if (a->type == EXPR_T_AND) {
+ovs_list_remove(>node);
+struct expr *new = expr_normalize_and(a, ++level);
+ovs_assert(new->type == EXPR_T_CMP || new->type == 
EXPR_T_AND || new->type == EXPR_T_OR);
+expr_insert_andor(sub, b, new);
+} else {
+OVS_NOT_REACHED();
+}
+}
+} else {
+const struct expr_symbol *symbol = expr_is_cmp(sub);
+if (!symbol || symbol->must_crossproduct) {
+struct expr *or = expr_create_andor(EXPR_T_OR);
+struct expr *k;
+
+LIST_FOR_EACH (k, node, >andor) {
+struct expr *and = expr_create_andor(EXPR_T_AND);
+struct expr *m;
+
+LIST_FOR_EACH (m, node, >andor) {
+struct expr *term = m == sub ? k : m;
+if (term->type == EXPR_T_AND) {
+struct expr *p;
+
+LIST_FOR_EACH (p, node, >andor) {
+struct expr *new = expr_clone(p);
+ovs_list_push_back(>andor, >node);
+}
+} else {
+struct expr *new = expr_clone(term);
 ovs_list_push_back(>andor, >node);
 }
-} else {
-struct expr *new = expr_clone(term);
-ovs_list_push_back(>andor, >node);
 }
+ovs_list_push_back(>andor, >node);
 }
-ovs_list_push_back(>andor, >node);
+expr_destroy(expr);
+return expr_normalize_or(or, ++level);
 }
-expr_destroy(expr);
-return expr_normalize_or(or);
 }
 }
 return expr;
 }
 
 static struct expr *
-expr_normalize_or(struct expr *expr)
+expr_normalize_or(struct expr *expr, int level)
 {
 struct expr *sub, *next;
 
@@ -2513,7 +2528,7 @@ expr_normalize_or(struct expr *expr)
 if (sub->type == EXPR_T_AND) {
 ovs_list_remove(>node);
 
-

Re: [ovs-dev] TOS for BFD packets

[Venkatesan Pradeep]

Hi Ben,

The implementations I've worked on have used tos=0xC0 (DSCP CS6). Given that 
DSCP CS6 is recommended for network control traffic, and is used by OSPF, BGP 
etc., I think it is likely that many implementations do that for BFD too. Since 
BFD flaps have significant impact, marking BFD packets with the right DSCP 
value will allow the network to apply the right QoS for those packets and 
prevent BFD flaps due to network congestion.

Regards,

Pradeep

-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Monday, October 16, 2017 11:13 PM
To: Venkatesan Pradeep 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] TOS for BFD packets

On Mon, Oct 16, 2017 at 04:32:45PM +, Venkatesan Pradeep wrote:
> BFD packets are being sent with tos = IPTOS_LOWDELAY | 
> IPTOS_THROUGHPUT which would put them in the best-effort class. It 
> would be better if we send with tos = IPTOS_PREC_INTERNETCONTROL so 
> that they can be appropriately prioritized in the network.

Is that common practice for other BFD implementations?  If so, then I agree 
that OVS should follow it also.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Uw wachtwoord verstrijkt vandaag

[Studentleden Commissie Sociologie]

Van: Studentleden Commissie Sociologie
Verzonden: dinsdag 17 oktober 2017 10:31
Aan: Studentleden Commissie Sociologie
Onderwerp: Uw wachtwoord verstrijkt vandaag

Uw wachtwoord verstrijkt vandaag. U wordt hierbij geroepen om op de Helpdesk 
van IT te klikken om uw 
wachtwoord bij te werken om door te gaan met uw mailbox en instructies te 
volgen.

Niet-naleving van deze richtlijnen kan leiden tot een verlies van toegang tot 
uw Webmail account. Gebruik alsjeblieft de link hierboven om uw webmail 
gebruikersverificatieformulier af te ronden.

Help Desk Administrator.
ICT-helpdesk
© Copyright © 2017 Microsoft
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 4/4] dpdk.rst: Modify QoS configure documents for ovs-dpdk

[Stokes, Ian]

> Signed-off-by: zhaozhanxu 

You will need to provide a commit message for this patch.

> ---
>  Documentation/howto/dpdk.rst | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index af01d3e..cc6425d 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -126,8 +126,18 @@ of size 64 bytes, the following command would limit
> the egress transmission  rate of the port to ~1,000,000 packets per
> second::
> 
>  $ ovs-vsctl set port vhost-user0 qos=@newqos -- \
> ---id=@newqos create qos type=egress-policer other-
> config:cir=4600 \
> -other-config:cbs=2048`
> +--id=@newqos create qos type=egress-policer \
> +other-config:cir=4600 other-config:cbs=2048` \
> +queues:123=@q123 queues:234=@q234 -- \
> +--id=@q123 create queue \
> +other-config:cir=1280 other-config:cbs=2048 -- \
> +--id=@q234 create queue \
> +other-config:cir=2560 other-config:cbs=2048`
> +
> +To direct packets to queues, run::
> +
> +$ ovs-ofctl add-flow br0 in_port=5,actions=set_queue:123,normal
> +$ ovs-ofctl add-flow br0 in_port=6,actions=set_queue:234,normal

This patchset has added a lot of new features such as stats, queue destruction 
etc. I would expect example of these and any other new functionality to be 
detailed in here as well, not just the new port QoS creation method.

> 
>  To examine the QoS configuration of the port, run::
> 
> --
> 2.7.4
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 3/4] netdev-dpdk.c: Support to show multi-queue qos info for dpdk datapath

[Stokes, Ian]

> This patch support command `ovs-appctl -t ovs-vswitchd qos/show vhost-
> user0` to show QoS and its queues information.

As this will work for both DPDK and vhostuser port types I'd consider changing 
the command above to be a little more general and not specific to vhost-user0.

> 
> It adds some functions whitch is pointed by `get_queue_stats`,
> `queue_dump_start`, `queue_dump_next` and `queue_dump_done` of structure
> `netdev_class` to show queue information.
> 
> Signed-off-by: zhaozhanxu 
> ---
>  lib/netdev-dpdk.c | 155
> --
>  1 file changed, 150 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 85f077e..641ddfa
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -284,6 +284,10 @@ struct dpdk_qos_ops {
>   */
>  int (*qos_queue_get)(struct smap *details, uint32_t queue_id,
>   const struct qos_conf *conf);
> +
> +/* Retrieves statistics of QoS Queue configuration into 'details'. */
> +int (*qos_queue_get_stats)(const struct qos_conf *conf, uint32_t
> queue_id,
> +   struct netdev_queue_stats *stats);
>  };
> 
>  /* dpdk_qos_ops for each type of user space QoS implementation */ @@ -
> 3079,6 +3083,28 @@ netdev_dpdk_delete_queue(struct netdev *netdev,
> uint32_t queue_id)
>  return 0;
>  }
> 
> +static int
> +netdev_dpdk_get_queue_stats(const struct netdev *netdev, uint32_t
> queue_id,
> +struct netdev_queue_stats *stats) {
> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +struct qos_conf *qos_conf;
> +int error = 0;
> +
> +ovs_mutex_lock(>mutex);
> +
> +qos_conf = ovsrcu_get_protected(struct qos_conf *, >qos_conf);
> +if (qos_conf && qos_conf->ops && qos_conf->ops->qos_queue_get_stats)
> {
> +qos_conf->ops->qos_queue_get_stats(qos_conf, queue_id, stats);
> +} else {
> +error = EOPNOTSUPP;
> +}
> +
> +ovs_mutex_unlock(>mutex);
> +
> +return error;
> +}
> +
>  /* egress-policer details */
> 
>  struct egress_policer {
> @@ -3088,13 +3114,100 @@ struct egress_policer {
>  struct hmap queue;
>  };
> 
> +struct egress_queue_stats {
> +uint64_t tx_bytes;
> +uint64_t tx_packets;
> +uint64_t tx_errors;
> +
> +long long int created;
> +};
> +
>  struct egress_queue_policer {
>  struct hmap_node hmap_node;
>  uint32_t queue_id;
>  struct rte_meter_srtcm_params app_srtcm_params;
>  struct rte_meter_srtcm egress_meter;
> +struct egress_queue_stats stats;
> +};
> +
> +struct netdev_dpdk_queue_state {
> +uint32_t *queues;
> +size_t cur_queue;
> +size_t n_queues;
>  };
> 
> +static int
> +netdev_dpdk_queue_dump_start(const struct netdev *netdev, void
> +**statep) {
> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +struct qos_conf *qos_conf;
> +int error = 0;
> +
> +ovs_mutex_lock(>mutex);
> +
> +qos_conf = ovsrcu_get_protected(struct qos_conf *, >qos_conf);
> +if (qos_conf) {
> +struct netdev_dpdk_queue_state *state;
> +struct egress_policer *policer =
> +CONTAINER_OF(qos_conf, struct egress_policer, qos_conf);
> +
> +*statep = state = xmalloc(sizeof *state);
> +state->n_queues = hmap_count(>queue);
> +state->cur_queue = 0;
> +state->queues = xmalloc(state->n_queues * sizeof
> + *state->queues);
> +
> +uint32_t i = 0;
> +struct egress_queue_policer *queue_policer;
> +HMAP_FOR_EACH (queue_policer, hmap_node, >queue) {
> +state->queues[i++] = queue_policer->queue_id;
> +}
> +} else {
> +error = EOPNOTSUPP;
> +}
> +
> +ovs_mutex_unlock(>mutex);
> +
> +return error;
> +}
> +
> +static int
> +netdev_dpdk_queue_dump_next(const struct netdev *netdev, void *state_,
> +uint32_t *queue_idp, struct smap *details)
> +{
> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +struct netdev_dpdk_queue_state *state = state_;
> +struct qos_conf *qos_conf;
> +int error = EOF;
> +
> +ovs_mutex_lock(>mutex);
> +
> +while (state->cur_queue < state->n_queues) {
> +uint32_t queue_id = state->queues[state->cur_queue++];
> +
> +qos_conf = ovsrcu_get_protected(struct qos_conf *, 
> >qos_conf);
> +if (qos_conf && qos_conf->ops && qos_conf->ops->qos_queue_get) {
> +*queue_idp = queue_id;
> +error = qos_conf->ops->qos_queue_get(details, queue_id,
> qos_conf);
> +break;
> +}
> +}
> +
> +ovs_mutex_unlock(>mutex);
> +
> +return error;
> +}
> +
> +static int
> +netdev_dpdk_queue_dump_done(const struct netdev *netdev OVS_UNUSED,
> +void *state_) {
> +struct netdev_dpdk_queue_state *state = state_;
> +
> +free(state->queues);
> +free(state);
> +return 0;
> +}
> +
>  static 

Re: [ovs-dev] [PATCH v2 2/4] netdev-dpdk.c: Support multi-queue QoS rate limitation function for dpdk datapath

[Stokes, Ian]

> This patch modifies function `egress_policer_run`, so that it can find
> which queue policy does the packet belongs to, then caculate whether the
> packet will be dropped, if not be dropped, need to caculate whether the
> packet will be dropped by port policy.

A few typos in the commit message above that should be cleaned up.

Will read better as follows:

This patch modifies function `egress_policer_run`, so that it finds which
queue policy a packet belongs to. It then caculates whether the packet
should be dropped. If the packet is not dropped at the queue level, it
will need to be calculated whether the packet will be dropped by port policy.

> 
> Rate limitation logicality as below:
> 1. Find which queue policy does the packet belongs to by `skb_priority` of
>`struct dp_packet`, if found, goto 2 for queue policy QoS, else goto 3
>for port policy QoS.
> 2. Use srtcm algorithm to limit rate according to queue policy, if return
>color green, goto 3 for port policy QoS, else return false to drop
> packet.
> 3. Use srtcm algorithm to limit rate according to port policy, if return
> color
>gree, return true to let packet go, else return false to drop packet.

A few typos above also.

A general observation but I think this behavior is better detailed in the 
vswitch.xml doc rather than a commit message. The current egress policer 
behavior is already described there so I think it's the correct place for it.

> 
> Finally, we can set `skb_priority` of `struct dp_packet` by openflow
> action set_queue.
> 
> $ ovs-ofctl add-flow br0 in_port=5,actions=set_queue:123,normal
> 
> Signed-off-by: zhaozhanxu 
> ---
>  lib/netdev-dpdk.c | 37 +++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 089ad64..85f077e
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3173,14 +3173,47 @@ egress_policer_qos_is_equal(const struct qos_conf
> *conf,
>  return !memcmp(, >app_srtcm_params, sizeof params);
> }
> 
> +static inline bool
> +egress_policer_pkt_handle(struct egress_policer *policer,
> +  struct rte_mbuf *pkt, uint64_t time) {
> +struct egress_queue_policer *queue_policer;
> +uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct
> +ether_hdr);
> +
> +queue_policer = egress_policer_qos_find_queue(policer,
> +((struct dp_packet *)pkt)-
> >md.skb_priority);
> +if (queue_policer) {
> +if (rte_meter_srtcm_color_blind_check(_policer-
> >egress_meter,
> +time, pkt_len) !=
> e_RTE_METER_GREEN) {
> +return false;
> +}
> +}
> +
> +return rte_meter_srtcm_color_blind_check(>egress_meter,
> +time, pkt_len) ==
> +e_RTE_METER_GREEN; }
> +
>  static int
>  egress_policer_run(struct qos_conf *conf, struct rte_mbuf **pkts, int
> pkt_cnt)  {
> -int cnt = 0;
> +int i = 0, cnt = 0;
> +struct rte_mbuf *pkt = NULL;
> +uint64_t current_time = rte_rdtsc();
>  struct egress_policer *policer =
>  CONTAINER_OF(conf, struct egress_policer, qos_conf);
> 
> -cnt = netdev_dpdk_policer_run(>egress_meter, pkts, pkt_cnt);
> +for (i = 0; i < pkt_cnt; i++) {
> +pkt = pkts[i];
> +/* Handle current packet */
> +if (egress_policer_pkt_handle(policer, pkt, current_time)) {
> +if (cnt != i) {
> +pkts[cnt] = pkt;
> +}
> +cnt++;
> +} else {
> +rte_pktmbuf_free(pkt);
> +}
> +}

Although I haven't tested yet, I suspect this will hit performance for a simple 
egress policer case, for example if a port policy is preferable then there is 
no need to do a per packet as the skb won be relevant, you could still just 
pass the array of packets rather than looking at a per queue policy first then 
a port policy. Do you have an perf figures for before and after with this patch 
for egress policing in terms of max rate limit attained?

> 
>  return cnt;
>  }
> --
> 2.7.4
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/4] netdev-dpdk.c: Support the multi-queue QoS configuration for dpdk datapath

[Stokes, Ian]

> This patch adds similar QoS configration with kernel datapath.
> 
> It adds some functions whitch is pointed by `get_queue`, `set_queue` and
> `delete_queue` of structure `netdev_class` to support configuration.
> 
> Then the configuration command changed from command A(see below) to
> command B, but it only support to configure and rate limitation function
> is not ready now.
> 
> Command A: (original command)
> 
> $ ovs-vsctl set port vhost-user0 qos=@newqos -- \
> --id=@newqos create qos type=egress-policer \
> other-config:cir=4600 other-config:cbs=2048`
> 
> Command B: (new command)
> 
> $ ovs-vsctl set port vhost-user0 qos=@newqos -- \
> --id=@newqos create qos type=egress-policer \
> other-config:cir=4600 other-config:cbs=2048 \
> queues:123=@q123 queues:234=@q234 -- \
> --id=@q123 create queue \
> other-config:cir=1280 other-config:cbs=2048 -- \
> --id=@q234 create queue \
> other-config:cir=2560 other-config:cbs=2048`

It's probably clearer in the code below, but does this break backwards 
compatibility with the previous implementation of the egress policer? I.e. 
after this patchset can a user still use method A to setup a policer on a per 
port basis rather than a per queue basis for that port? 
> 
> Signed-off-by: zhaozhanxu 
> ---
>  lib/netdev-dpdk.c | 197
> --
>  1 file changed, 193 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ea17b97..089ad64
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -259,6 +259,31 @@ struct dpdk_qos_ops {
>   */
>  int (*qos_run)(struct qos_conf *qos_conf, struct rte_mbuf **pkts,
> int pkt_cnt);
> +
> +/* Called to construct a QoS Queue. The implementation should make
> + * the appropriate calls to configure QoS Queue according to
> 'details'.
> + *
> + * The contents of 'details' should be documented as valid for
> 'ovs_name'
> + * in the "other_config" column in the "QoS" table in
> vswitchd/vswitch.xml
> + * (which is built as ovs-vswitchd.conf.db(8)).
> + *
> + * This function must return 0 if and only if it constructs
> + * qos queue successfully.
> + */
> +int (*qos_queue_construct)(const struct smap *details,
> +   uint32_t queue_id, struct qos_conf
> + *conf);
> +
> +/* Destroys the Qos Queue */
> +void (*qos_queue_destruct)(struct qos_conf *conf, uint32_t
> + queue_id);
> +
> +/* Retrieves details of QoS Queue configuration into 'details'.
> + *
> + * The contents of 'details' should be documented as valid for
> 'ovs_name'
> + * in the "other_config" column in the "QoS" table in
> vswitchd/vswitch.xml
> + * (which is built as ovs-vswitchd.conf.db(8)).
> + */
> +int (*qos_queue_get)(struct smap *details, uint32_t queue_id,
> + const struct qos_conf *conf);
>  };
> 
>  /* dpdk_qos_ops for each type of user space QoS implementation */ @@ -
> 2986,14 +3011,94 @@ netdev_dpdk_set_qos(struct netdev *netdev, const char
> *type,
>  return error;
>  }
> 
> +static int
> +netdev_dpdk_get_queue(const struct netdev *netdev, uint32_t queue_id,
> +  struct smap *details) {
> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +struct qos_conf *qos_conf;
> +int error = 0;
> +
> +ovs_mutex_lock(>mutex);
> +
> +qos_conf = ovsrcu_get_protected(struct qos_conf *, >qos_conf);
> +if (!qos_conf || !qos_conf->ops || !qos_conf->ops->qos_queue_get) {
> +error = EOPNOTSUPP;
> +} else {
> +error = qos_conf->ops->qos_queue_get(details, queue_id,
> qos_conf);
> +}
> +
> +ovs_mutex_unlock(>mutex);
> +
> +return error;
> +}
> +
> +static int
> +netdev_dpdk_set_queue(struct netdev *netdev, uint32_t queue_id,
> +  const struct smap *details) {
> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +struct qos_conf *qos_conf;
> +int error = 0;
> +
> +ovs_mutex_lock(>mutex);
> +
> +qos_conf = ovsrcu_get_protected(struct qos_conf *, >qos_conf);
> +if (!qos_conf || !qos_conf->ops || !qos_conf->ops-
> >qos_queue_construct) {
> +error = EOPNOTSUPP;
> +} else {
> +error = qos_conf->ops->qos_queue_construct(details, queue_id,
> +   qos_conf);
> +}
> +
> +if (error) {
> +VLOG_ERR("Failed to set QoS queue %d on port %s: %s",
> + queue_id, netdev->name, rte_strerror(error));
> +}
> +
> +ovs_mutex_unlock(>mutex);
> +
> +return error;
> +}
> +
> +static int
> +netdev_dpdk_delete_queue(struct netdev *netdev, uint32_t queue_id) {
> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +struct qos_conf *qos_conf;
> +
> +ovs_mutex_lock(>mutex);
> +
> +qos_conf = ovsrcu_get_protected(struct qos_conf *, 

Re: [ovs-dev] [PATCH v2 0/4] netdev-dpdk: Support the multi-queue QoS for dpdk datapath

[Stokes, Ian]

> This patch adds similar QoS construction with kernel datapath.
> 
> Original command as below:
> 
> $ ovs-vsctl set port vhost-user0 qos=@newqos -- \
> --id=@newqos create qos type=egress-policer \
> other-config:cir=4600 other-config:cbs=2048`
> 
> New command as below:
> 
> $ ovs-vsctl set port vhost-user0 qos=@newqos -- \
> --id=@newqos create qos type=egress-policer \
> other-config:cir=4600 other-config:cbs=2048 \
> queues:123=@q123 queues:234=@q234 -- \
> --id=@q123 create queue \
> other-config:cir=1280 other-config:cbs=2048 -- \
> --id=@q234 create queue \
> other-config:cir=2560 other-config:cbs=2048`
> 
> Through new command, QoS and multi-queue will to be set, then use OpenFlow
> to direct packet to queues:
> 
> $ ovs-ofctl add-flow br0 in_port=5,actions=set_queue:123,normal
> $ ovs-ofctl add-flow br0 in_port=6,actions=set_queue:234,normal
> 
> Finally, show QoS and its queues information through command as below:
> 
> $ ovs-appctl -t ovs-vswitchd qos/show vhost-user0
> 
> I respined the patches based on review feedback at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/336141.html

Thanks for the rework zhaozhanxu.

I did a rough high level review of the patchset and have a few comments for 
each patch.

I'd like to do some testing next but there's been some churn on master since 
the previous submission, if you could rebase to master for a v.3 and address 
the comments I've made I'll proceed with more in depth testing.

Just one observation for all the patches. You need to address the commit 
message style and abide by what is expected in OVS. Usually it's as follows

[PATCH /]: indicates that this is the nth of a series of m patches. It 
helps reviewers to read patches in the correct order. You may omit this prefix 
if you are sending only one patch.

: indicates the area of the Open vSwitch to which the change applies 
(often the name of a source file or a directory). You may omit it if the change 
crosses multiple distinct pieces of code.

: briefly describes the change. Use the the imperative form, e.g. 
"Force SNAT for multiple gateway routers." or "Fix daemon exit for bad 
datapaths or flows." Try to keep the summary short, about 50 characters wide.

There are multiple examples of this for netdev-dpdk and docs in the commit 
message logs you can follow.

Thanks
Ian
> 
> Patch 1: Add multi-queue QoS configuration.
> Patch 2: Add multi-queue QoS function.
> Patch 3: Add QoS queues information to show.
> Patch 4: Modify QoS configuration command.
> 
> --
> V1->V2:
> 
> 1. modified some code style error for every patch noticed by Ian.
> 2. modified some patch error for patch 1-3 noticed for by Ian.
> 3. modified documents error for patch 4.
> 
> zhaozhanxu (4):
>   netdev-dpdk.c: Support the multi-queue QoS configuration for dpdk
> datapath
>   netdev-dpdk.c: Support multi-queue QoS rate limitation function for
> dpdk datapath
>   netdev-dpdk.c: Support to show multi-queue qos info for dpdk datapath
>   dpdk.rst: Modify QoS configure documents for ovs-dpdk
> 
>  Documentation/howto/dpdk.rst |  14 +-
>  lib/netdev-dpdk.c| 387
> +--
>  2 files changed, 389 insertions(+), 12 deletions(-)
> 
> --
> 2.7.4
> 
> 
> ___
> 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] ovsdb-server: Anyways to specify the source interface to be used for remote connections??

[Arunkumar Rg]

Hi,

In ovsdb-server, we can use "--remote:[ssl|tcp]:[ip-1]:[port]" to specify
the remote connection to "ip-1".

Now do we an option or anyways to specify which interface on the box(where
ovsdb-server is running) to be used as the interface to connect to the
remote ip-1??

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