Re: [ovs-dev] [ovs-dev, v4, 1/3] OVN ACL: Replace the usage of ct_label with ct_mark

[0-day Robot]

Bleep bloop.  Greetings Ankur Sharma, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 141 characters long (recommended limit is 79)
#33 FILE: Documentation/tutorials/ovn-openstack.rst:1204:
   6. ls_in_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && ct.blocked 
== 0 && (inport == "ap" && ip4), priority 2002, uuid a12b39f0

WARNING: Line is 202 characters long (recommended limit is 79)
#42 FILE: Documentation/tutorials/ovn-openstack.rst:1273:
   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct.blocked == 0 && (outport == "cp" && ip4 && ip4.src == 
$as_ip4_0fc1b6cf_f925_49e6_8f00_6dd13beca9dc), priority 2002, uuid a746fa0d

WARNING: Line is 176 characters long (recommended limit is 79)
#51 FILE: Documentation/tutorials/ovn-openstack.rst:1500:
   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct.blocked == 0 && (outport == "dp" && ip4 && ip4.src == 0.0.0.0/0 && icmp4), 
priority 2002, uuid b860fc9f

WARNING: Line is 141 characters long (recommended limit is 79)
#60 FILE: Documentation/tutorials/ovn-openstack.rst:1612:
   6. ls_in_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && ct.blocked 
== 0 && (inport == "ap" && ip6), priority 2002, uuid 7fdd607e

WARNING: Line is 202 characters long (recommended limit is 79)
#69 FILE: Documentation/tutorials/ovn-openstack.rst:1670:
   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct.blocked == 0 && (outport == "cp" && ip6 && ip6.src == 
$as_ip6_0fc1b6cf_f925_49e6_8f00_6dd13beca9dc), priority 2002, uuid 12fc96f9

WARNING: Line is 227 characters long (recommended limit is 79)
#78 FILE: Documentation/tutorials/ovn-openstack.rst:1861:
   6. ls_in_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && ct.blocked 
== 0 && (inport == "ap" && ip4 && ip4.dst == {255.255.255.255, 10.1.1.0/24} && 
udp && udp.src == 68 && udp.dst == 67), priority 2002, uuid 9c90245d

Lines checked: 356, Warnings: 6, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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

Re: [ovs-dev] OVN/OVS Split POC: version 2

[Ben Pfaff]

On Wed, Apr 17, 2019 at 05:07:34PM -0300, Flavio Leitner via dev wrote:
> On Tue, Mar 26, 2019 at 04:15:07PM -0400, Mark Michelson wrote:
> > I've once again rolled another OVN/OVS split version. It can be found at
> > https://github.com/putnopvut/ovn_mk2.git
> > 
> > The main changes between this and the old split POC are as follows:
> > 
> > * This is based on a much newer build of OVS master. Therefore, build errors
> > people had with dhparams.c *should* be cleared up.
> > 
> > * This fixes errors with manpages.mk generation/checking, so there is no
> > need to do a pointless `touch` of manpages.mk during the build process.
> > 
> > * In many cases, rather than hard-coding paths to OVS, we use variables.
> > This isn't universally applied, but it's used for the locations of C
> > headers, libraries, and manpages.
> > 
> > Please give this a look and let me know what you think.
> 
> I see OVS binaries, headers, man-pages and libraries being built
> and installed. Not sure what the plans are, but this could cause
> some confusion to users once they start using the repo.

It's good to fix this issue, and anything else we find before we do the
split, but I want to avoid the idea that it has to be perfect before we
pull the trigger.  There will be problems.  We'll fix them.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVN/OVS Split POC: version 2

[Ben Pfaff]

On Wed, Apr 17, 2019 at 04:31:33PM -0400, Mark Michelson wrote:
> On 4/12/19 6:02 PM, Ben Pfaff wrote:
> > On Tue, Mar 26, 2019 at 04:15:07PM -0400, Mark Michelson wrote:
> > > I've once again rolled another OVN/OVS split version. It can be found at
> > > https://github.com/putnopvut/ovn_mk2.git
> > > 
> > > The main changes between this and the old split POC are as follows:
> > > 
> > > * This is based on a much newer build of OVS master. Therefore, build 
> > > errors
> > > people had with dhparams.c *should* be cleared up.
> > > 
> > > * This fixes errors with manpages.mk generation/checking, so there is no
> > > need to do a pointless `touch` of manpages.mk during the build process.
> > > 
> > > * In many cases, rather than hard-coding paths to OVS, we use variables.
> > > This isn't universally applied, but it's used for the locations of C
> > > headers, libraries, and manpages.
> > > 
> > > Please give this a look and let me know what you think.
> > 
> As far as next steps go, I think that we need to do these changes in an
> official capacity. What does this entail?
> 
> First, we need an official repo to perform the changes in. I suspect that
> you would need to provide this. Whether this is in the openvswitch github
> project or a new one is a potential matter of discussion.

We have an OVN organization that would be suitable for the repo:
https://github.com/ovn-org.  Justin is currently the only "owner" for
this organization, so he is the only one who can create a repo.  I'd
suggest that he should do that and give you access to it.

(Justin is out of the office until tomorrow.)

> Next, we need to declare a date/time to officially switch over to performing
> OVN development in the new repo. There are a couple of reasons for this:
> * Merges to OVN code should halt while the work is done. This way, there is
> no chance of losing OVN changes as part of the merge.
> * Contributors need to be aware of when they need to switch over to using
> the new repo for development.
> * Contributors may want to get changes they are working on up for review
> prior to the switchover to prevent having to re-do their changes in the new
> repo.
> 
> I think those are the big things that need to be done next. After that, we
> can make incremental changes in the new repo, even if they are somewhat
> large. The other major things to discuss are administrative/policy changes.
> Those can also wait until the code split is complete.

OK.  We should set a date soon.  How about May 1?  We want enough time
between then and the soft-freeze for the 2.12 release, which by the book
should be July 1.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 3/3] OVN ACL: Allow a user to input ct.label value for an acl

[Ankur Sharma]

Hi Ben,

Sorry, somehow messed up the 3rd patch.
Submitted a v4 of this series.

Thanks

Regards,
Ankur

-Original Message-
From: Ben Pfaff  
Sent: Wednesday, April 17, 2019 10:46 AM
To: Ankur Sharma 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v3 3/3] OVN ACL: Allow a user to input ct.label 
value for an acl

On Tue, Apr 16, 2019 at 11:58:10PM +, Ankur Sharma wrote:
> This patch allows user to associate a value with acl, which will be 
> assigned to ct.label of the corresponding connection tracking entry.
> 
> This value can be used to map a ct entry with corresponding OVN ACL or 
> higher level constructs like security group.
> 
> Signed-off-by: Ankur Sharma 
> ---
>  ovn/ovn-nb.ovsschema | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema index 
> 2c87cbb..4391e3b 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
> -"version": "5.16.0",
> -"cksum": "923459061 23095",
> +"version": "5.17.0",
> +"cksum": "3491001412 23095",
>  "tables": {
>  "NB_Global": {
>  "columns": {

This patch seems like it's a mistake since it only updates the version and 
checksum.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4 3/3] OVN ACL: Allow a user to input ct.label value for an acl

[Ankur Sharma]

This patch allows user to associate a value with acl,
which will be assigned to ct.label of the corresponding
connection tracking entry.

This value can be used to map a ct entry with corresponding
OVN ACL or higher level constructs like security group.

Signed-off-by: Ankur Sharma 
---
 ovn/northd/ovn-northd.c   | 37 --
 ovn/ovn-nb.ovsschema  |  5 +++--
 ovn/ovn-nb.xml| 12 ++
 ovn/utilities/ovn-nbctl.c | 24 +++-
 tests/ovn-nbctl.at| 12 --
 tests/ovn.at  | 57 +++
 6 files changed, 135 insertions(+), 12 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 3254a61..3c73f36 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -168,12 +168,13 @@ enum ovn_stage {
 #define OVN_ACL_PRI_OFFSET 1000
 
 /* Register definitions specific to switches. */
-#define REGBIT_CONNTRACK_DEFRAG  "reg0[0]"
-#define REGBIT_CONNTRACK_COMMIT  "reg0[1]"
-#define REGBIT_CONNTRACK_NAT "reg0[2]"
-#define REGBIT_DHCP_OPTS_RESULT  "reg0[3]"
-#define REGBIT_DNS_LOOKUP_RESULT "reg0[4]"
-#define REGBIT_ND_RA_OPTS_RESULT "reg0[5]"
+#define REGBIT_CONNTRACK_DEFRAG "reg0[0]"
+#define REGBIT_CONNTRACK_COMMIT "reg0[1]"
+#define REGBIT_CONNTRACK_NAT"reg0[2]"
+#define REGBIT_CONNTRACK_SET_LABEL  "reg0[3]"
+#define REGBIT_DHCP_OPTS_RESULT "reg0[4]"
+#define REGBIT_DNS_LOOKUP_RESULT"reg0[5]"
+#define REGBIT_ND_RA_OPTS_RESULT"reg0[6]"
 
 /* Register definitions for switches and routers. */
 #define REGBIT_NAT_REDIRECT "reg9[0]"
@@ -3715,7 +3716,14 @@ consider_acl(struct hmap *lflows, struct ovn_datapath 
*od,
   " || (!ct.new && ct.est && !ct.rpl "
"&& ct.blocked == 1)) "
   "&& (%s)", acl->match);
-ds_put_cstr(, REGBIT_CONNTRACK_COMMIT" = 1; ");
+if (acl->label) {
+   ds_put_format(, REGBIT_CONNTRACK_COMMIT" = 1; "
+ ""REGBIT_CONNTRACK_SET_LABEL" = 1; "
+ "xxreg1 = %s; ", acl->label);
+} else {
+   ds_put_cstr(, REGBIT_CONNTRACK_COMMIT" = 1; ");
+}
+
 build_acl_log(, acl);
 ds_put_cstr(, "next;");
 ovn_lflow_add_with_hint(lflows, od, stage,
@@ -4153,6 +4161,21 @@ build_stateful(struct ovn_datapath *od, struct hmap 
*lflows)
 ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 0, "1", "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 0, "1", "next;");
 
+/* If REGBIT_CONNTRACK_COMMIT is set as 1 and
+ * REGBIT_CONNTRACK_SET_LABEL is set to 1, then the packets should be
+ * committed to conntrack.
+ * We always set ct_mark.blocked to 0 here as
+ * any packet that makes it this far is part of a connection we
+ * want to allow to continue. */
+ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 150,
+  REGBIT_CONNTRACK_COMMIT" == 1 && "
+  ""REGBIT_CONNTRACK_SET_LABEL" == 1",
+  "ct_commit(ct_mark=0/1, ct_label=xxreg1); next;");
+ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 150,
+  REGBIT_CONNTRACK_COMMIT" == 1 && "
+  ""REGBIT_CONNTRACK_SET_LABEL" == 1",
+  "ct_commit(ct_mark=0/1, ct_label=xxreg1); next;");
+
 /* If REGBIT_CONNTRACK_COMMIT is set as 1, then the packets should be
  * committed to conntrack. We always set ct.blocked to 0 here as
  * any packet that makes it this far is part of a connection we
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 2c87cbb..75f34c8 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Northbound",
-"version": "5.16.0",
-"cksum": "923459061 23095",
+"version": "5.17.0",
+"cksum": "1043090930 23169",
 "tables": {
 "NB_Global": {
 "columns": {
@@ -177,6 +177,7 @@
 "debug"]]},
   "min": 0, "max": 1}},
 "meter": {"type": {"key": "string", "min": 0, "max": 1}},
+"label": {"type": {"key": "string", "min": 0, "max": 1}},
 "external_ids": {
 "type": {"key": "string", "value": "string",
  "min": 0, "max": "unlimited"}}},
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index cbaa949..7c10278 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -1299,6 +1299,18 @@
 default, log messages are not rate-limited.
 
   
+
+  
+
+Associates an identifier with the ACL.
+Same value will be written to corresponding connection
+tracker entry. Value should be in hex, for example: 0x1234.
+This value can help in debugging from connection 

[ovs-dev] [PATCH v4 2/3] OVN ACL: Allow ct_mark and ct_label values to be set from register as well

[Ankur Sharma]

OVN allows only an integer (or masked integer) to be assigned to
ct_mark and ct_label.

This patch, enhances the parser code to allow ct_mark and ct_label
to be assigned from 32 bit registers (MFF_REG0 - MFF_REG15) and  128
bit registers (MFF_XXREG0 - MFF_XXREG3) respectively.

Signed-off-by: Ankur Sharma 
---
 include/ovn/actions.h |  3 ++
 ovn/lib/actions.c | 78 +--
 ovn/ovn-sb.xml| 39 ++
 tests/ovn.at  | 16 +++
 4 files changed, 117 insertions(+), 19 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 1c0c67c..58b96a1 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -24,6 +24,7 @@
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/hmap.h"
 #include "openvswitch/uuid.h"
+#include "openvswitch/meta-flow.h"
 #include "util.h"
 
 struct expr;
@@ -196,8 +197,10 @@ struct ovnact_ct_next {
 /* OVNACT_CT_COMMIT. */
 struct ovnact_ct_commit {
 struct ovnact ovnact;
+bool is_ct_mark_reg, is_ct_label_reg; /* If the value is from a register */
 uint32_t ct_mark, ct_mark_mask;
 ovs_be128 ct_label, ct_label_mask;
+enum mf_field_id ct_mark_reg, ct_label_reg;
 };
 
 /* OVNACT_CT_DNAT, OVNACT_CT_SNAT. */
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index eb7e5ba..c153dd9 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -627,8 +627,28 @@ parse_ct_commit_arg(struct action_context *ctx,
 } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) {
 cc->ct_mark = ntohll(ctx->lexer->token.value.integer);
 cc->ct_mark_mask = ntohll(ctx->lexer->token.mask.integer);
+} else if (ctx->lexer->token.type == LEX_T_ID) {
+
+   cc->ct_mark_mask = UINT32_MAX;
+
+   const struct mf_field *mf = mf_from_name(ctx->lexer->token.s);
+   if (mf) {
+
+  if (mf->id >= MFF_REG0 && mf->id <= MFF_REG15) {
+ cc->is_ct_mark_reg = true;
+ cc->ct_mark_reg = mf->id;
+  } else {
+ lexer_syntax_error(ctx->lexer, "input: %s,  not a 32 bit "
+"register", mf->name);
+ return;
+  }
+   } else {
+  lexer_syntax_error(ctx->lexer, "invalid field name: %s",
+ ctx->lexer->token.s);
+  return;
+   }
 } else {
-lexer_syntax_error(ctx->lexer, "expecting integer");
+lexer_syntax_error(ctx->lexer, "invalid token type");
 return;
 }
 lexer_get(ctx->lexer);
@@ -642,9 +662,28 @@ parse_ct_commit_arg(struct action_context *ctx,
 } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) {
 cc->ct_label = ctx->lexer->token.value.be128_int;
 cc->ct_label_mask = ctx->lexer->token.mask.be128_int;
+} else if (ctx->lexer->token.type == LEX_T_ID) {
+
+   cc->ct_label_mask = OVS_BE128_MAX;
+   const struct mf_field *mf = mf_from_name(ctx->lexer->token.s);
+   if (mf) {
+  if (mf->id >= MFF_XXREG0 && mf->id <= MFF_XXREG3) {
+ cc->is_ct_label_reg = true;
+ cc->ct_label_reg = mf->id;
+  } else {
+ lexer_syntax_error(ctx->lexer, "input: %s,  not a 128 bit "
+"register", mf->name);
+ return;
+  }
+   } else {
+  lexer_syntax_error(ctx->lexer, "invalid field name: %s",
+ ctx->lexer->token.s);
+  return;
+   }
+
 } else {
-lexer_syntax_error(ctx->lexer, "expecting integer");
-return;
+   lexer_syntax_error(ctx->lexer, "invalid token type");
+   return;
 }
 lexer_get(ctx->lexer);
 } else {
@@ -713,14 +752,37 @@ encode_CT_COMMIT(const struct ovnact_ct_commit *cc,
 ofpbuf_pull(ofpacts, set_field_offset);
 
 if (cc->ct_mark_mask) {
-const ovs_be32 value = htonl(cc->ct_mark);
-const ovs_be32 mask = htonl(cc->ct_mark_mask);
-ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_MARK), , );
+   if (cc->is_ct_mark_reg) {
+  struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts);
+
+  move->src.field = mf_from_id(cc->ct_mark_reg);
+  move->src.ofs = 0;
+  move->src.n_bits = 32;
+  move->dst.field = mf_from_id(MFF_CT_MARK);
+  move->dst.ofs = 0;
+  move->dst.n_bits = 32;
+   } else {
+  const ovs_be32 value = htonl(cc->ct_mark);
+  const ovs_be32 mask = htonl(cc->ct_mark_mask);
+  ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_MARK), ,
+   );
+   }
 }
 
 if (!ovs_be128_is_zero(cc->ct_label_mask)) {
-ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_LABEL), >ct_label,
-   

[ovs-dev] [PATCH v4 1/3] OVN ACL: Replace the usage of ct_label with ct_mark

[Ankur Sharma]

OVN ACL implementation used ct_label to indicate if a previosuly
allowed connection shoudl not be allowed anymore and vice versa.

However, ct_label is a 128 bit value and we should rather leverage
on ct_mark which is a 32 bit value.

Using ct_mark for this purpose, allows us to use ct_label for storing
other values like, identifier for corresponidng OVN ACL/Security group etc.

Signed-off-by: Ankur Sharma 
---
 Documentation/tutorials/ovn-openstack.rst | 12 
 ovn/lib/logical-fields.c  |  3 ++
 ovn/northd/ovn-northd.8.xml   | 14 -
 ovn/northd/ovn-northd.c   | 50 ---
 tests/ovn.at  | 11 +++
 5 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/Documentation/tutorials/ovn-openstack.rst 
b/Documentation/tutorials/ovn-openstack.rst
index c6dff5e..dfd18da 100644
--- a/Documentation/tutorials/ovn-openstack.rst
+++ b/Documentation/tutorials/ovn-openstack.rst
@@ -1201,7 +1201,7 @@ as the output port::
 
   ct_next(ct_state=est|trk /* default (use --ct to customize) */)
   ---
-   6. ls_in_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct_label.blocked == 0 && (inport == "ap" && ip4), priority 2002, uuid a12b39f0
+   6. ls_in_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct.blocked == 0 && (inport == "ap" && ip4), priority 2002, uuid a12b39f0
   next;
   13. ls_in_l2_lkup (ovn-northd.c:3529): eth.dst == fa:16:3e:f6:e2:8f, 
priority 50, uuid c43ead31
   outport = "17d870";
@@ -1270,7 +1270,7 @@ Finally the logical switch for ``n2`` runs through the 
same logic as
 
   ct_next(ct_state=est|trk /* default (use --ct to customize) */)
   ---
-   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct_label.blocked == 0 && (outport == "cp" && ip4 && ip4.src == 
$as_ip4_0fc1b6cf_f925_49e6_8f00_6dd13beca9dc), priority 2002, uuid a746fa0d
+   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct.blocked == 0 && (outport == "cp" && ip4 && ip4.src == 
$as_ip4_0fc1b6cf_f925_49e6_8f00_6dd13beca9dc), priority 2002, uuid a746fa0d
   next;
7. ls_out_port_sec_ip (ovn-northd.c:2364): outport == "cp" && eth.dst == 
fa:16:3e:89:f2:36 && ip4.dst == {255.255.255.255, 224.0.0.0/4, 10.1.2.7}, 
priority 90, uuid 4d9862b5
   next;
@@ -1497,7 +1497,7 @@ firewall and is output to ``d``::
 
   ct_next(ct_state=est|trk /* default (use --ct to customize) */)
   ---
-   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct_label.blocked == 0 && (outport == "dp" && ip4 && ip4.src == 0.0.0.0/0 && 
icmp4), priority 2002, uuid b860fc9f
+   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct.blocked == 0 && (outport == "dp" && ip4 && ip4.src == 0.0.0.0/0 && icmp4), 
priority 2002, uuid b860fc9f
   next;
7. ls_out_port_sec_ip (ovn-northd.c:2364): outport == "dp" && eth.dst == 
fa:16:3e:c1:f5:a2 && ip4.dst == {255.255.255.255, 224.0.0.0/4, 10.0.0.6}, 
priority 90, uuid 15655a98
   next;
@@ -1609,7 +1609,7 @@ closely to those for IPv4 which we already discussed back 
under
 
   ct_next(ct_state=est|trk /* default (use --ct to customize) */)
   ---
-   6. ls_in_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct_label.blocked == 0 && (inport == "ap" && ip6), priority 2002, uuid 7fdd607e
+   6. ls_in_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct.blocked == 0 && (inport == "ap" && ip6), priority 2002, uuid 7fdd607e
   next;
   13. ls_in_l2_lkup (ovn-northd.c:3529): eth.dst == fa:16:3e:ef:2f:8b, 
priority 50, uuid e1d87fc5
   outport = "ad952e";
@@ -1667,7 +1667,7 @@ closely to those for IPv4 which we already discussed back 
under
 
   ct_next(ct_state=est|trk /* default (use --ct to customize) */)
   ---
-   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct_label.blocked == 0 && (outport == "cp" && ip6 && ip6.src == 
$as_ip6_0fc1b6cf_f925_49e6_8f00_6dd13beca9dc), priority 2002, uuid 12fc96f9
+   4. ls_out_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 
ct.blocked == 0 && (outport == "cp" && ip6 && ip6.src == 
$as_ip6_0fc1b6cf_f925_49e6_8f00_6dd13beca9dc), priority 2002, uuid 12fc96f9
   next;
7. ls_out_port_sec_ip (ovn-northd.c:2390): outport == "cp" && eth.dst == 
fa:16:3e:89:f2:36 && ip6.dst == {fe80::f816:3eff:fe89:f236, ff00::/8, fc22::7}, 
priority 90, uuid c622596a
   next;
@@ -1858,7 +1858,7 @@ action replaces a DHCPDISCOVER or DHCPREQUEST packet by a
 reply.  Table 12 flips the packet's source and destination and sends
 it back the way it came in::
 
-   6. ls_in_acl (ovn-northd.c:2925): !ct.new && ct.est && !ct.rpl && 

[ovs-dev] [PATCH v4 0/3] Associate identifier with OVN ACL connection tracking entry

[Ankur Sharma]

What:

a. Goal is to be able to associate some identifier with a connection tracking
entry.

b. This identifier can be used to map OVN ACL which added this entry or
higher level constructs like openstack security group etc.

c. There are 2 connection tracking fields which can be used for it.
ct.mark (32 bits) and ct.label (128 bits).

d. Patch intends to use ct.label, as this is a longer field and
hence would be put to a better use, if it stores the identifier.

Why:

a. Adding an identifier would help in debugging.
b. Now, we can map a connection tracking entry to corresponding
   acl, security group etc.
c. One of the use cases for this mapping would be to identify
   ACLs which added corresponding connection tracker entry, which
   is causing unexpected drops/leaks.

How:

Following is the sequence of changes:

Patch 1:
   i.  Current implementation uses a bit ct.label to handle policy update cases,
   where we use a bit in ct.label to indicate that reply traffic should
   be dropped now.
  ii. Swap the usage of ct.label in current implementation with ct.mark.

Patch 2:
   i. Add support in parser to allow ct.label and mark to be set from registers
  as well (as of now only integer/masked integer is allowed).

Patch 3:
   i. Add a new column (named 'label') to Table ACL in northbound schema.
  ii. ovn-northd changes to enhance logical flows to set ct.label to acl->label.
  For example:
  table=4 (ls_out_acl ),  action=(reg0[1] = 1; reg0[3] = 1; 
xxreg1 = 0x1234; next;)
  .
  .
  .
  table=7 (ls_out_stateful), ... match=(reg0[1] == 1 && reg0[3] == 1),
 action=(ct_commit(ct_mark=0/1, 
ct_label=xxreg1); next;)

Ankur Sharma (3):
  OVN ACL: Replace the usage of ct_label with ct_mark
  OVN ACL: Allow ct_mark and ct_label values to be set from register as
well
  OVN ACL: Allow a user to input ct.label value for an acl

 Documentation/tutorials/ovn-openstack.rst | 12 ++---
 include/ovn/actions.h |  3 ++
 ovn/lib/actions.c | 78 ---
 ovn/lib/logical-fields.c  |  3 ++
 ovn/northd/ovn-northd.8.xml   | 14 ++---
 ovn/northd/ovn-northd.c   | 87 ---
 ovn/ovn-nb.ovsschema  |  5 +-
 ovn/ovn-nb.xml| 12 +
 ovn/ovn-sb.xml| 39 ++
 ovn/utilities/ovn-nbctl.c | 24 -
 tests/ovn-nbctl.at| 12 -
 tests/ovn.at  | 84 +++--
 12 files changed, 300 insertions(+), 73 deletions(-)

--
1.8.3.1

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

Re: [ovs-dev] OVN/OVS Split POC: version 2

[Mark Michelson]

On 4/12/19 6:02 PM, Ben Pfaff wrote:

On Tue, Mar 26, 2019 at 04:15:07PM -0400, Mark Michelson wrote:

I've once again rolled another OVN/OVS split version. It can be found at
https://github.com/putnopvut/ovn_mk2.git

The main changes between this and the old split POC are as follows:

* This is based on a much newer build of OVS master. Therefore, build errors
people had with dhparams.c *should* be cleared up.

* This fixes errors with manpages.mk generation/checking, so there is no
need to do a pointless `touch` of manpages.mk during the build process.

* In many cases, rather than hard-coding paths to OVS, we use variables.
This isn't universally applied, but it's used for the locations of C
headers, libraries, and manpages.

Please give this a look and let me know what you think.




Sorry for the late reply Ben. Your response was filtered to a folder I 
wasn't expecting.



This builds cleanly for me when srcdir==builddir.  I get a failure if I
use a separate builddir, as is my usual habit, e.g.:

 blp@sigill:~/nicira/ovn_mk2/_build(2)$ make -j10
 make  all-recursive
 make[1]: Entering directory '/home/blp/nicira/ovn_mk2/_build'
 Making all in ovs
 make[2]: Entering directory '/home/blp/nicira/ovn_mk2/_build/ovs'
 make  all-recursive
 make[3]: Entering directory '/home/blp/nicira/ovn_mk2/_build/ovs'
 Making all in datapath
 make[4]: Entering directory '/home/blp/nicira/ovn_mk2/_build/ovs/datapath'
 make[5]: Entering directory '/home/blp/nicira/ovn_mk2/_build/ovs/datapath'
 make[5]: Leaving directory '/home/blp/nicira/ovn_mk2/_build/ovs/datapath'
 make[4]: Leaving directory '/home/blp/nicira/ovn_mk2/_build/ovs/datapath'
 make[4]: Entering directory '/home/blp/nicira/ovn_mk2/_build/ovs'
 make[4]: Leaving directory '/home/blp/nicira/ovn_mk2/_build/ovs'
 make[3]: Leaving directory '/home/blp/nicira/ovn_mk2/_build/ovs'
 make[2]: Leaving directory '/home/blp/nicira/ovn_mk2/_build/ovs'
 make[2]: Entering directory '/home/blp/nicira/ovn_mk2/_build'
 make[2]: *** No rule to make target '../ovs/ovsdb/libovsdb.la', needed by 
'utilities/ovn-nbctl'.  Stop.
 make[2]: *** Waiting for unfinished jobs
 make[2]: Leaving directory '/home/blp/nicira/ovn_mk2/_build'
 make[1]: *** [Makefile:3335: all-recursive] Error 1
 make[1]: Leaving directory '/home/blp/nicira/ovn_mk2/_build'
 make: *** [Makefile:2205: all] Error 2
 blp@sigill:~/nicira/ovn_mk2/_build(2)$

But that's a minor thing and I expect you can fix it easily.  I think
this is pretty much ready to go.  What do you think should be the next
step?


You're correct that this can be fixed easily. It just requires a few 
extra lines to be added to the configure script.


As far as next steps go, I think that we need to do these changes in an 
official capacity. What does this entail?


First, we need an official repo to perform the changes in. I suspect 
that you would need to provide this. Whether this is in the openvswitch 
github project or a new one is a potential matter of discussion.


Next, we need to declare a date/time to officially switch over to 
performing OVN development in the new repo. There are a couple of 
reasons for this:
* Merges to OVN code should halt while the work is done. This way, there 
is no chance of losing OVN changes as part of the merge.
* Contributors need to be aware of when they need to switch over to 
using the new repo for development.
* Contributors may want to get changes they are working on up for review 
prior to the switchover to prevent having to re-do their changes in the 
new repo.


I think those are the big things that need to be done next. After that, 
we can make incremental changes in the new repo, even if they are 
somewhat large. The other major things to discuss are 
administrative/policy changes. Those can also wait until the code split 
is complete.






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

Re: [ovs-dev] OVN/OVS Split POC: version 2

[Flavio Leitner via dev]

On Tue, Mar 26, 2019 at 04:15:07PM -0400, Mark Michelson wrote:
> I've once again rolled another OVN/OVS split version. It can be found at
> https://github.com/putnopvut/ovn_mk2.git
> 
> The main changes between this and the old split POC are as follows:
> 
> * This is based on a much newer build of OVS master. Therefore, build errors
> people had with dhparams.c *should* be cleared up.
> 
> * This fixes errors with manpages.mk generation/checking, so there is no
> need to do a pointless `touch` of manpages.mk during the build process.
> 
> * In many cases, rather than hard-coding paths to OVS, we use variables.
> This isn't universally applied, but it's used for the locations of C
> headers, libraries, and manpages.
> 
> Please give this a look and let me know what you think.

I see OVS binaries, headers, man-pages and libraries being built
and installed. Not sure what the plans are, but this could cause
some confusion to users once they start using the repo.

Other than that, it builds. It does without openvswitch-devel
package installed or direct install (unpackaged) of OVS, which\
brings my point for the previous iteration back. It's okay to do
later in my opinion, but somehow this will need to be cleaned up
or you might end up with two OVSs to maintain. Also that it
becomes less clear of which fixes we need to do in OVS itself
to allow OVN to be build.

The Makefile target rpm-fedora should build OVN packages, and
there should be no rpm-fedora-kmod. The rpm-fedora-ovn becomes
the rpm-fedora as already pointed.

The make rpm-fedora-ovn works though, super cool!

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

[ovs-dev] [PATCH] selinux: update for netlink socket types

[Aaron Conole]

These are used for interfacing with conntrack, as well as by some
DPDK PMDs

Signed-off-by: Aaron Conole 
---
 selinux/openvswitch-custom.te.in | 8 
 1 file changed, 8 insertions(+)

diff --git a/selinux/openvswitch-custom.te.in b/selinux/openvswitch-custom.te.in
index 26495828a..2adaf231f 100644
--- a/selinux/openvswitch-custom.te.in
+++ b/selinux/openvswitch-custom.te.in
@@ -49,6 +49,10 @@ require {
 class filesystem getattr;
 class lnk_file { read open };
 class netlink_audit_socket { create nlmsg_relay audit_write read write 
};
+class netlink_netfilter_socket { create nlmsg_relay audit_write read 
write };
+@begin_dpdk@
+class netlink_rdma_socket { setopt bind create };
+@end_dpdk@
 class netlink_socket { setopt getopt create connect getattr write read 
};
 class sock_file { write };
 class system { module_load module_request };
@@ -75,6 +79,10 @@ domtrans_pattern(openvswitch_t, 
openvswitch_load_module_exec_t, openvswitch_load
 #= openvswitch_t ==
 allow openvswitch_t self:capability { dac_override audit_write net_broadcast 
net_raw };
 allow openvswitch_t self:netlink_audit_socket { create nlmsg_relay audit_write 
read write };
+allow openvswitch_t self:netlink_netfilter_socket { create nlmsg_relay 
audit_write read write };
+@begin_dpdk@
+allow openvswitch_t self:netlink_rdma_socket { setopt bind create };
+@end_dpdk@
 allow openvswitch_t self:netlink_socket { setopt getopt create connect getattr 
write read };
 
 allow openvswitch_t hostname_exec_t:file { read getattr open execute 
execute_no_trans };
-- 
2.19.1

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

Re: [ovs-dev] OVN/OVS Split POC: version 2

[Mark Michelson]

On 4/12/19 3:38 PM, Aaron Conole wrote:

Hi Mark,

Mark Michelson  writes:


I've once again rolled another OVN/OVS split version. It can be found
at https://github.com/putnopvut/ovn_mk2.git

The main changes between this and the old split POC are as follows:

* This is based on a much newer build of OVS master. Therefore, build
errors people had with dhparams.c *should* be cleared up.

* This fixes errors with manpages.mk generation/checking, so there is
no need to do a pointless `touch` of manpages.mk during the build
process.

* In many cases, rather than hard-coding paths to OVS, we use
variables. This isn't universally applied, but it's used for the
locations of C headers, libraries, and manpages.




Sorry for the late reply Aaron. Your reply got filtered to a folder I 
wasn't expecting.



A logical extension of these variables might be to move to incorporate
the pkgconfig files that OVS installs.  We may need to generate some
additional pkgconfig files for some components (maybe..) in OvS.

But something like:

+PKG_CHECK_MODULES([OVS], [libopenvswitch])
+ >
in your configure.ac at OVN (with openvswitch being installed) means we
get an even weaker coupling between the projects.  This would give all
the flags, options, etc. that are needed.


I'll have to do some research into pkgconfig since I'm not familiar with 
it, but based on what you're saying, this could be a very good idea.




I think there may be some internal headers that need to be exposed
externally (moved to include/openvswitch/).  Some of the common manpage
includes will need to be shared (installed somewhere that they can be
referenced) by the openvswitch installer.


Yes, right now some headers are in lib/ and others are in 
include/openvswitch/ . I'm not sure what the current rules are regarding 
which headers go in which directory. But it probably makes sense to have 
all of them (if possible) in include/openvswitch/ .


Agreed on the common manpage includes as well.



WDYT?  I have started a quick mock-up.  If I can spend more time next
week on it I will, but can't promise anything.


Sure, can you point me to the mock-up? I'd love to take a look at it.




Please give this a look and let me know what you think.


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

Re: [ovs-dev] [PATCH] rhel: Include all header files in the Fedora's devel package

[Ansis Atteka]

On Wed, 17 Apr 2019 at 08:49, Aaron Conole  wrote:
>
> Ben Pfaff  writes:
>
> > On Mon, Apr 01, 2019 at 09:26:31AM -0700, Ansis Atteka wrote:
> >> From: Ansis Atteka 
> >>
> >> While the header files added by this patch into Fedora's devel
> >> rpm package can be considered private, the other devel packages
> >> for RHEL/CentOS and Debian/Ubuntu distros include them.
> >>
> >> So this patch simply makes the Fedora devel package consistent with
> >> the other devel packages.
> >>
> >> Signed-off-by: Ansis Atteka 
> >
> > I support consistency here.  I didn't test the patch.
>
> In case it hasn't been applied yet:
>
> Acked-by: Aaron Conole 
>
>
> Actually, I think it helps with the ovn / ovs split.

Thanks for reviews. I already pushed this.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 0/4] OVN: Distributed Virtual Router for Vlan Backed Networks

[Ben Pfaff]

On Tue, Apr 16, 2019 at 11:37:14PM +, Ankur Sharma wrote:
> This series is about enhancing the logical router functionality in OVN to work
> with vlan backed logical switches.

This series has a lot of style and cosmetic issues reported by the
robot.  Please take a look at them.  I also noticed that it tends to use
3-space indents in many places rather than the 4-space standard adopted
in OVS.

The patch titles should begin with "ovn: ".

The commit messages for this series are written mostly as bullet points
that in some cases just say in words the same thing that the patch does
in code.  This is not the usual style for OVS/OVN.  While it is useful
to say at a high level what a patch does, generally we expect commit
messages to focus on rationale.  I do really appreciate that the patches
contain references to the overall goal of the series.

The documentation should give more details.  The documentation for
network_type, for example, just says this:

+
+  
+Whether logical switch is VLAN backed or Overlay.
+  
+
+  
+Default is overlay.
+  
+

which formats in the manpage as:

   network_type: optional string, either overlay or vlan
  Whether logical switch is VLAN backed or Overlay.

  Default is overlay.

So, the first paragraph just repeats what the manpage will say anyway
in the autogenerated text, and thus it can be removed.  The second
paragraph is useful.  But it leaves out all the important information on
what are VLAN backed or overlay networks and how the user can decide
which one is appropriate in a given situation.  Maybe this is not the
exact place where that should be explained--but, if not, then it should
be explained somewhere else, such as the ovn-architecture document, and
this should refer the reader to wherever that is.

It looks like "ovn-nbctl ls-add" now will fail to work with older
databases, since it sets the network_type unconditionally.  I think it
would be a good idea to have it set the network_type only if the user
specifies one.  (Maybe it should validate that it is "vlan" or
"overlay"?)

Thanks,

Ben.

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

Re: [ovs-dev] [PATCH] ovn-nbctl: Fix 32-bit build with gcc.

[Ben Pfaff]

On Wed, Apr 17, 2019 at 08:22:06PM +0300, Ilya Maximets wrote:
> ovn/utilities/ovn-nbctl.c: In function 'print_routing_policy':
> ovn/utilities/ovn-nbctl.c:3620:23: error: format '%ld' expects argument
> of type 'long int', but argument 3 has type 'int64_t'
>policy->match, policy->action, next_hop);
>^
> ovn/utilities/ovn-nbctl.c:3624:23: error: format '%ld' expects argument
> of type 'long int', but argument 3 has type 'int64_t'
>policy->match, policy->action);
>^
> ovn/utilities/ovn-nbctl.c: In function 'cmd_ha_ch_grp_list':
> ovn/utilities/ovn-nbctl.c:5056:27: error: format '%lu' expects argument
> of type 'long unsigned int', but argument 10 has type 'int64_t'
>ha_ch->priority);
>^
> cc1: all warnings being treated as errors
> make[2]: *** [ovn/utilities/ovn-nbctl.o] Error 1
> 
> https://travis-ci.org/openvswitch/ovs/jobs/521015912

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

Re: [ovs-dev] [PATCH v3 3/3] OVN ACL: Allow a user to input ct.label value for an acl

[Ben Pfaff]

On Tue, Apr 16, 2019 at 11:58:10PM +, Ankur Sharma wrote:
> This patch allows user to associate a value with acl,
> which will be assigned to ct.label of the corresponding
> connection tracking entry.
> 
> This value can be used to map a ct entry with corresponding
> OVN ACL or higher level constructs like security group.
> 
> Signed-off-by: Ankur Sharma 
> ---
>  ovn/ovn-nb.ovsschema | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 2c87cbb..4391e3b 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
> -"version": "5.16.0",
> -"cksum": "923459061 23095",
> +"version": "5.17.0",
> +"cksum": "3491001412 23095",
>  "tables": {
>  "NB_Global": {
>  "columns": {

This patch seems like it's a mistake since it only updates the version
and checksum.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/3] OVN ACL: Allow ct_mark and ct_label values to be set from register as well

[Ben Pfaff]

On Tue, Apr 16, 2019 at 11:58:08PM +, Ankur Sharma wrote:
> OVN allows only an integer (or masked integer) to be assigned to
> ct_mark and ct_label.
> 
> This patch, enhances the parser code to allow ct_mark and ct_label
> to be assigned from 32 bit registers (MFF_REG0 - MFF_REG15) and  128
> bit registers (MFF_XXREG0 - MFF_XXREG3) respectively.
> 
> Signed-off-by: Ankur Sharma 

Thanks for the patch!

When we add new errors to the ovn action parser, we add tests to provoke
each of them.  I don't see any new test for some of the errors here,
such as the "invalid token type" error.

We try to make the OVN action parser error messages grammatically
correct English sentences or phrases.  For example:
Syntax error at `reg1' expecting integer
The new message
Syntax error at `xxreg1' input: xxreg1,  not a 32 bit register.
is not in this category, and it unnecessarily repeats the xxreg1 part.
Something like
Syntax error at `xxreg1' expecting a 32-bit register.
would be better.

This patch seems to add unnecessary restrictions.  Why restrict the
source fields to registers, for example?  It's also a bit of an
unnecessary restriction to force the entire ct_mark or ct_label field to
be set.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[William Tu]

On Wed, Apr 17, 2019 at 9:48 AM Ben Pfaff  wrote:
>
> On Wed, Apr 17, 2019 at 12:16:59PM +0200, Eelco Chaudron wrote:
> > One other thing that popped up in my head is how (will) it work together
> > with DPDK enabled on the same system?
>
> Why not?

It works OK with OVS-DPDK.
For example, I can create a br0, attach a af_xdp port and also attach
a dpdk port to it. (I tested using dpdk vhost port, not physical one).

The performance is lower than using two dpdk ports due to some packet
copying from one to another.

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

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[William Tu]

On Wed, Apr 17, 2019 at 9:47 AM Ben Pfaff  wrote:
>
> On Wed, Apr 17, 2019 at 10:09:53AM +0200, Eelco Chaudron wrote:
> > On 16 Apr 2019, at 21:55, Ben Pfaff wrote:
> > > AF_XDP is a faster way to access the existing kernel devices.  If we
> > > take that point of view, then it would be ideal if AF_XDP were
> > > automatically used when it was available, instead of adding a new
> > > network device type.  Is there a reason that this point of view is
> > > wrong?  That is, when AF_XDP is available, is there a reason not to use
> > > it?
> >
> > This needs support by all the ingress and egress ports in the system, and
> > currently, there is no API to check this.
>
> Do you mean for performance or for some other reason?  I would suspect
> that, if AF_XDP was not available, then everything would still work OK
> via AF_PACKET, just slower.
>
> > There are also features like traffic shaping that will not work. Maybe it
> > will be worth adding the table for AF_XDP in
> > http://docs.openvswitch.org/en/latest/faq/releases/
>
> AF_XDP is comparable to DPDK/userspace, not to the Linux kernel
> datapath.
>
> The table currently conflates the userspace datapath with the DPDK
> network device.  I believe that the only entry there that depends on the
> DPDK network device is the one for policing.  It could be replaced by a
> [*] with a note like this:
>
> YES - for DPDK network devices.
> NO - for system or AF_XDP network devices.
>
> > > You said that your goal for the next version is to improve performance
> > > and add optimizations.  Do you think that is important before we merge
> > > the series?  We can continue to improve performance after it is merged.
> >
> > The previous patch was rather unstable and I could not get it running with
> > the PVP test without crashing. I think this patchset should get some proper
> > testing and reviews by others. Especially for all the features being marked
> > as supported in the above-mentioned table.
>
> If it's unstable, we should fix that before adding it in.

Agree.
My first goal is to make sure people can at least run
$ make check-afxdp
This uses the virtual device, veth XDP skb-mode, to run various
OVS test case. The performance will be bad, but it makes sure the
correctness.

Regards,
William

>
> However, the bar is lower for new features that don't break existing
> features, especially optional ones and ones that can be easily be
> removed if they don't work out in the end.  DPDK support was considered
> "experimental" for a long time, it's possible that AF_XDP would be in
> the same boat for a while.
>
> > > If we set performance aside, do you have a reason to want to wait to
> > > merge this?  (I wasn't able to easily apply this series to current
> > > master, so it'll need at least a rebase before we apply it.  And I have
> > > only skimmed it, not fully reviewed it.)
> >
> > Other than the items above, do we really need another datapath?
>
> It's less than a new datapath.  It's a new network device
> implementation.
>
> > With this, we use two or more cores for processing packets. If we poll
> > two physical ports it could be 300%, which is a typical use case with
> > bonding. What about multiple queue support, does it work? Both in
> > kernel and DPDK mode we use multiple queues to distribute the load,
> > with this scenario does it double the number of CPUs used? Can we use
> > the poll() mode as explained here,
> > https://linuxplumbersconf.org/event/2/contributions/99/, and how will
> > it work with multiple queues/pmd threads? What about any latency
> > tests, is it worse or better than kernel/dpdk? Also with the AF_XDP
> > datapath, there is no to leverage hardware offload, like DPDK and
> > TC. And then there is the part that it only works on the most recent
> > kernels.
>
> These are good questions.  William will have some of the answers.
>
> > To me looking at this I would say it’s far from being ready to be merged
> > into OVS. However, if others decide to go ahead I think it should be
> > disabled, not compiled in by default.
>
> Yes, that seems reasonable to me.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS-DPDK public meeting

[Kevin Traynor]

On 17/04/2019 18:24, Kevin Traynor wrote:
> Next meeting April 15th 1600 UTC
> 

oops, should be May 15th, time travel is still an experimental feature.

> Attendees: Aaron, Alejandro, David, Fouad, Flavio, Ariel, Pieter, Mark,
> Ian, Scott, Kevin.
> 
> ===
> GENERAL
> ===
> 
> - UNH lab testing (Aaron)
> -- Integrating ovs_perf test scripts
> -- Testing dpdk-latest OVS branch and dpdk master + patches from dpdk-dev
> -- Would prefer a stable OVS base to avoid multiple moving targets
> -- Dashboard https://lab.dpdk.org/results/dashboard/
> -- Results mailing list https://mails.dpdk.org/listinfo/test-report
> 
> - DPDK stable releases
> - DPDK 18.11.1 (OVS 2.11, OVS Master)
> - Recently available, validated.
> - DPDK 17.11.5 (OVS 2.10, OVS 2.9)
> - Available but unit test failures flagged in DPDK validation.
> - 17.11.6 will be released soon, so might be better to wait and
> validate with it
> - DPDK 16.11.9 (OVS 2.7)
> - Final release for 16.11, 2 year support cycle ended.
> - Probably worth it? as it's the last one
> 
> - DPDK 19.05 release
> - dpdk-latest branch patched to compile with DPDK master (Kudos Ophir)
> - dpdk-latest branch process
> - if this is to be used to detect regressions with DPDK patches in
> UNH, then it should not be updated much. However, then it loses some
> effectiveness that the latest OVS is functional with the latest dpdk.
> Maybe there is need for 2 branches with different purposes. Ian to send
> a mail to discuss.
> 
> - Spring cleaning
> - Deprecated features
> - General question about deprecation and time frames.
> - vhostuser
> -- Not causing a maintenance burden as it's mostly shared code with
> vhostuserclient
> -- Aware of some cases where users want to upgrade from 2.6 -> newer
> release but don't want to switch to vhostuserclient at upgrade time as
> it's an additional risk.
> 
> - Experimental Features
> - vhost zero copy
> - SMC
> - HWOL
> - Experimental Tag process?
> -  Should there be a set path to removing experimental tag for each
> feature, even if that is different based on the type of feature.
> 
> 
> 
> PERFORMANCE/FEATURES
> 
> - Features Targeting OVS 2.12
> - HWOL vport offloads
> - DPCLS function pointers + scalar optimizations
> - RFC4115 Egress policer
> -- the required DPDK bits are not available in DPDK 18.11
> 
> On 25/07/2017 14:25, Kevin Traynor wrote:
>> Hi All,
>>
>> The OVS-DPDK public meetings have moved to Wednesday's at the same time.
>> Everyone is welcome, it's a chance to share status/plans etc.
>>
>> It's scheduled for every 2 weeks starting 26th July. Meeting time is
>> currently @ 4pm UTC.
>>
>> You can connect through Bluejeans or through dial in:
>>
>> https://bluejeans.com/139318596
>>
>> US: +1.408.740.7256
>> UK: +44.203.608.5256
>> Germany: +49.32.221.091256
>> Ireland: +353.1.697.1256
>> Other numbers at https://www.bluejeans.com/numbers
>> Meeting ID: 139318596
>>
>> thanks,
>> Kevin.
>>
> 
> ___
> 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] OVS-DPDK public meeting

[Kevin Traynor]

Next meeting April 15th 1600 UTC

Attendees: Aaron, Alejandro, David, Fouad, Flavio, Ariel, Pieter, Mark,
Ian, Scott, Kevin.

===
GENERAL
===

- UNH lab testing (Aaron)
-- Integrating ovs_perf test scripts
-- Testing dpdk-latest OVS branch and dpdk master + patches from dpdk-dev
-- Would prefer a stable OVS base to avoid multiple moving targets
-- Dashboard https://lab.dpdk.org/results/dashboard/
-- Results mailing list https://mails.dpdk.org/listinfo/test-report

- DPDK stable releases
- DPDK 18.11.1 (OVS 2.11, OVS Master)
- Recently available, validated.
- DPDK 17.11.5 (OVS 2.10, OVS 2.9)
- Available but unit test failures flagged in DPDK validation.
- 17.11.6 will be released soon, so might be better to wait and
validate with it
- DPDK 16.11.9 (OVS 2.7)
- Final release for 16.11, 2 year support cycle ended.
- Probably worth it? as it's the last one

- DPDK 19.05 release
- dpdk-latest branch patched to compile with DPDK master (Kudos Ophir)
- dpdk-latest branch process
- if this is to be used to detect regressions with DPDK patches in
UNH, then it should not be updated much. However, then it loses some
effectiveness that the latest OVS is functional with the latest dpdk.
Maybe there is need for 2 branches with different purposes. Ian to send
a mail to discuss.

- Spring cleaning
- Deprecated features
- General question about deprecation and time frames.
- vhostuser
-- Not causing a maintenance burden as it's mostly shared code with
vhostuserclient
-- Aware of some cases where users want to upgrade from 2.6 -> newer
release but don't want to switch to vhostuserclient at upgrade time as
it's an additional risk.

- Experimental Features
- vhost zero copy
- SMC
- HWOL
- Experimental Tag process?
-  Should there be a set path to removing experimental tag for each
feature, even if that is different based on the type of feature.



PERFORMANCE/FEATURES

- Features Targeting OVS 2.12
- HWOL vport offloads
- DPCLS function pointers + scalar optimizations
- RFC4115 Egress policer
-- the required DPDK bits are not available in DPDK 18.11

On 25/07/2017 14:25, Kevin Traynor wrote:
> Hi All,
> 
> The OVS-DPDK public meetings have moved to Wednesday's at the same time.
> Everyone is welcome, it's a chance to share status/plans etc.
> 
> It's scheduled for every 2 weeks starting 26th July. Meeting time is
> currently @ 4pm UTC.
> 
> You can connect through Bluejeans or through dial in:
> 
> https://bluejeans.com/139318596
> 
> US: +1.408.740.7256
> UK: +44.203.608.5256
> Germany: +49.32.221.091256
> Ireland: +353.1.697.1256
> Other numbers at https://www.bluejeans.com/numbers
> Meeting ID: 139318596
> 
> thanks,
> Kevin.
> 

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

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[William Tu]

Hi Eelco,
Thanks for trying this patchset!

On Wed, Apr 17, 2019 at 7:26 AM Eelco Chaudron  wrote:
>
>
>
> On 17 Apr 2019, at 14:01, Eelco Chaudron wrote:
>
> > Hi William,
> >
> > I think you applied the following patch to get it to compile? Or did
> > you copy in the kernel headers?
> >
> > https://www.spinics.net/lists/netdev/msg563507.html
>
Right. I apply the patch to get it compile.
I should document it better in next version about how to install.

> I noticed you duplicated the macros, which resulted in all kind of
> compile errors. So I removed them, applied the two patches above, which
> would get me to the next step.
>
> I’m building it with DPDK enabled and it was causing all kind of
> duplicate definition errors as the kernel and DPDK re-use some structure
> names.

Sorry about that. I will fix it in next version.

>
> To get it all compiled and working I had top make the following changes:
>
> $ git diff
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index b3bf2f044..47fb3342a 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -295,7 +295,7 @@ netdev_linux_rxq_xsk(struct xsk_socket_info *xsk,
>   uint32_t idx_rx = 0, idx_fq = 0;
>   int ret = 0;
>
> -unsigned int non_afxdp;
> +unsigned int non_afxdp = 0;
>
>   /* See if there is any packet on RX queue,
>* if yes, idx_rx is the index having the packet.
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 47153dc60..77f2150ab 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -24,7 +24,7 @@
>   #include 
>   #include 
>   #include 
> -#include 
> +//#include 
>
>   #include 
>   #include 
> diff --git a/lib/xdpsock.h b/lib/xdpsock.h
> index 8df8fa451..a2ed1a136 100644
> --- a/lib/xdpsock.h
> +++ b/lib/xdpsock.h
> @@ -28,7 +28,7 @@
>   #include 
>   #include 
>   #include 
> -#include 
> +//#include 
>   #include 
>   #include 
>   #include 
> @@ -43,14 +43,6 @@
>   #include "ovs-atomic.h"
>   #include "openvswitch/thread.h"
>
> -/* bpf/xsk.h uses the following macros not defined in OVS,
> - * so re-define them before include.
> - */
> -#define unlikely OVS_UNLIKELY
> -#define likely OVS_LIKELY
> -#define barrier() __asm__ __volatile__("": : :"memory")
> -#define smp_rmb() barrier()
> -#define smp_wmb() barrier()
>   #include 
>
> In addition you need to do “make install_headers” from kernel libbpf
> and copy the libbpf_util.h manually.
>
> I was able to do a simple physical port in same physical port out test
> without crashing, but the numbers seem low:

This probably is due to some log printing.
I have a couple of optimizations, will share it soon.

Regards,
William
>
> $ ovs-ofctl dump-flows ovs_pvp_br0
>   cookie=0x0, duration=210.344s, table=0, n_packets=1784692,
> n_bytes=2694884920, in_port=eno1 actions=IN_PORT
>
> "Physical loopback test, L3 flows[port redirect]"
> ,Packet size
> Number of flows,64,128,256,512,768,1024,1514
> 100,77574,77329,76605,76417,75539,75252,74617
>
> The above is using two cores, but with a single DPDK core I get the
> following (on the same machine):
>
> "Physical loopback test, L3 flows[port redirect]"
> ,Packet size
> Number of flows,64,128,256,512,768,1024,1514
> 100,9527075,8445852,4528935,2349597,1586276,1197304,814854
>
> For the kernel datapath the numbers are:
>
> "Physical loopback test, L3 flows[port redirect]"
> ,Packet size
> Number of flows,64,128,256,512,768,1024,1514
> 100,4862995,5521870,4528872,2349596,1586277,1197305,814854
>
> But keep in mind it uses roughly 550/610/520/380/180/140/110% of the CPU
> for the respective packet size.
>
> > On 2 Apr 2019, at 0:46, William Tu wrote:
> >
> >> The patch series introduces AF_XDP support for OVS netdev.
> >> AF_XDP is a new address family working together with eBPF.
> >> In short, a socket with AF_XDP family can receive and send
> >> packets from an eBPF/XDP program attached to the netdev.
> >> For more details about AF_XDP, please see linux kernel's
> >> Documentation/networking/af_xdp.rst
> >>
> >> OVS has a couple of netdev types, i.e., system, tap, or
> >> internal.  The patch first adds a new netdev types called
> >> "afxdp", and implement its configuration, packet reception,
> >> and transmit functions.  Since the AF_XDP socket, xsk,
> >> operates in userspace, once ovs-vswitchd receives packets
> >> from xsk, the proposed architecture re-uses the existing
> >> userspace dpif-netdev datapath.  As a result, most of
> >> the packet processing happens at the userspace instead of
> >> linux kernel.
> >>
> >> Architecure
> >> ===
> >>_
> >>   |   +---+
> >>   |   |ovs-vswitchd   |<-->ovsdb-server
> >>   |   +---+
> >>   |   |  ofproto  |<-->OpenFlow controllers
> >>   |   ++-++
> >>   |   | netdev | |ofproto-|
> >> userspace |   ++ |  dpif  |
> >>   |   | netdev | ++
> >>   |   

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[William Tu]

On Wed, Apr 17, 2019 at 1:09 AM Eelco Chaudron  wrote:
>
>
>
> On 16 Apr 2019, at 21:55, Ben Pfaff wrote:
>
> > On Mon, Apr 01, 2019 at 03:46:48PM -0700, William Tu wrote:
> >> The patch series introduces AF_XDP support for OVS netdev.
> >> AF_XDP is a new address family working together with eBPF.
> >> In short, a socket with AF_XDP family can receive and send
> >> packets from an eBPF/XDP program attached to the netdev.
> >> For more details about AF_XDP, please see linux kernel's
> >> Documentation/networking/af_xdp.rst
> >
> > I'm glad to see some more revisions of this series!
>
> I’m planning on reviewing and testing this patch, I’ll try to start
> it this week, or else when I get back from PTO.
>
> > AF_XDP is a faster way to access the existing kernel devices.  If we
> > take that point of view, then it would be ideal if AF_XDP were
> > automatically used when it was available, instead of adding a new
> > network device type.  Is there a reason that this point of view is
> > wrong?  That is, when AF_XDP is available, is there a reason not to
> > use
> > it?
>
> This needs support by all the ingress and egress ports in the system,
> and currently, there is no API to check this.

Not necessary all ports.
On a OVS switch, you can have some ports supporting AF_XDP,
and some ports are other types, ex: DPDK vhost, or tap.

>
> There are also features like traffic shaping that will not work. Maybe
> it will be worth adding the table for AF_XDP in
> http://docs.openvswitch.org/en/latest/faq/releases/

Right, when using AF_XDP, we don't have QoS support.
If people want to do rate limiting on a AF_XDP port, another
way is to use OpenFlow meter actions.

>
> > You said that your goal for the next version is to improve performance
> > and add optimizations.  Do you think that is important before we merge
> > the series?  We can continue to improve performance after it is
> > merged.
>
> The previous patch was rather unstable and I could not get it running
> with the PVP test without crashing. I think this patchset should get
> some proper testing and reviews by others. Especially for all the
> features being marked as supported in the above-mentioned table.
>

Yes, Tim has been helping a lot to test this and I have a couple of
new fixes. I will incorporate into next version.

> > If we set performance aside, do you have a reason to want to wait to
> > merge this?  (I wasn't able to easily apply this series to current
> > master, so it'll need at least a rebase before we apply it.  And I
> > have
> > only skimmed it, not fully reviewed it.)
>
> Other than the items above, do we really need another datapath? With

This is using the same datapath, the userspace datapath, as OVS-DPDK.
So we don't introduce another datapath, we introduce a new netdev type.

> this, we use two or more cores for processing packets. If we poll two
> physical ports it could be 300%, which is a typical use case with
> bonding. What about multiple queue support, does it work? Both in kernel

Yes, this patchset only allows 1 pmd and 1 queue.
I'm adding the multiqueue support.

> and DPDK mode we use multiple queues to distribute the load, with this
> scenario does it double the number of CPUs used? Can we use the poll()
> mode as explained here,
> https://linuxplumbersconf.org/event/2/contributions/99/, and how will it
> work with multiple queues/pmd threads? What about any latency tests, is
> it worse or better than kernel/dpdk? Also with the AF_XDP datapath,
> there is no to leverage hardware offload, like DPDK and TC. And then
> there is the part that it only works on the most recent kernels.

You have lots of good points here.
My experiments show that it's slower than DPDK, but much faster than
kernel.

>
> To me looking at this I would say it’s far from being ready to be
> merged into OVS. However, if others decide to go ahead I think it should
> be disabled, not compiled in by default.
>
I agree. This should be experimental feature and we're adding s.t like
#./configure --enable-afxdp
so not compiled in by default

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

Re: [ovs-dev] [PATCH v5] OVN: Add support for Transport Zones

[Ben Pfaff]

On Wed, Apr 17, 2019 at 04:55:24PM +0100, lmart...@redhat.com wrote:
> From: Lucas Alvares Gomes 
> 
> This patch is adding support for Transport Zones. Transport zones (a.k.a
> TZs) is way to enable users of OVN to separate Chassis into different
> logical groups that will only form tunnels between members of the same
> groups. Each Chassis can belong to one or more Transport Zones. If
> not set, the Chassis will be considered part of a default group.

[...]

> v4 -> v5
>  * Bumped the version of the ovn-sb.ovsschema to 2.2.1.

The version should become 2.3.0, see ovsdb.7:

An OVSDB schema also has a version of the form ``x.y.z``
e.g. ``1.2.3``.  Schemas managed within the Open vSwitch project
manage version numbering in the following way (but OVSDB does not
mandate this approach).  Whenever we change the database schema in a
non-backward compatible way (e.g. when we delete a column or a
table), we increment  and set  and  to 0.  When we change
the database schema in a backward compatible way (e.g. when we add a
new column), we increment  and set  to 0.  When we change the
database schema cosmetically (e.g. we reindent its syntax), we
increment .  The ``ovsdb-tool`` commands ``schema-version`` and
``db-version`` extract the schema version from a schema or database
file, respectively.

But the main issue I see here is that I get a consistent test failure in
the new test:

# -*- compilation -*-
2693. ovn.at:13656: testing ovn -- test transport zones ...
creating ovn-sb database
creating ovn-nb database
starting ovn-northd
starting backup ovn-northd
adding simulator 'main'
adding simulator 'hv1'
adding simulator 'hv2'
adding simulator 'hv3'
adding simulator 'hv4'
adding simulator 'hv5'
../../tests/ovn.at:13669: ovs-vsctl --bare --columns=name find interface 
type="geneve" | awk NF | sort
--- -   2019-04-17 10:02:31.402675315 -0700
+++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/2693/stdout   
2019-04-17 10:02:31.397247995 -0700
@@ -1,5 +1,4 @@
 ovn-hv2-0
 ovn-hv3-0
 ovn-hv4-0
-ovn-hv5-0
 
2693. ovn.at:13656: 2693. ovn -- test transport zones (ovn.at:13656): FAILED 
(ovn.at:13669)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, v1] netdev-rte-offloads: Reassign vport netdev functions.

[Ben Pfaff]

On Wed, Apr 17, 2019 at 09:39:45AM -0700, Gregory Rose wrote:
> 
> 
> On 4/17/2019 9:34 AM, Ben Pfaff wrote:
> > On Wed, Apr 17, 2019 at 11:45:33AM -0400, Aaron Conole wrote:
> > > rte comes from dpdk as an acronym for Run Time Environment.  Maybe even
> > > just dropping the 'rte_' portion?
> > *That* is what rte stands for?  What a ridiculously generic name.  It's
> > like naming a library Operating System.
> 
> Like libOS?
> 
> https://lwn.net/Articles/637658/

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

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[William Tu]

Thanks for the feedbacks.

On Tue, Apr 16, 2019 at 12:55 PM Ben Pfaff  wrote:
>
> On Mon, Apr 01, 2019 at 03:46:48PM -0700, William Tu wrote:
> > The patch series introduces AF_XDP support for OVS netdev.
> > AF_XDP is a new address family working together with eBPF.
> > In short, a socket with AF_XDP family can receive and send
> > packets from an eBPF/XDP program attached to the netdev.
> > For more details about AF_XDP, please see linux kernel's
> > Documentation/networking/af_xdp.rst
>
> I'm glad to see some more revisions of this series!
>
> AF_XDP is a faster way to access the existing kernel devices.  If we
> take that point of view, then it would be ideal if AF_XDP were
> automatically used when it was available, instead of adding a new
> network device type.  Is there a reason that this point of view is
> wrong?  That is, when AF_XDP is available, is there a reason not to use
> it?

I think we should use it if it is available. However, now only ixgbe/i40e
driver support AF_XDP mode. But I think more vendors are working on
this feature.

>
> You said that your goal for the next version is to improve performance
> and add optimizations.  Do you think that is important before we merge
> the series?  We can continue to improve performance after it is merged.
>
> If we set performance aside, do you have a reason to want to wait to
> merge this?  (I wasn't able to easily apply this series to current
> master, so it'll need at least a rebase before we apply it.  And I have
> only skimmed it, not fully reviewed it.)

OK Thanks.
I have been working on measuring the performance and adding some
optimizations. I will consider submit another version.

>
> It might make sense to squash all of these into a single patch.  I am
> not sure that they are really distinct conceptually.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[Ben Pfaff]

On Wed, Apr 17, 2019 at 12:16:59PM +0200, Eelco Chaudron wrote:
> One other thing that popped up in my head is how (will) it work together
> with DPDK enabled on the same system?

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

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[Ben Pfaff]

On Wed, Apr 17, 2019 at 10:09:53AM +0200, Eelco Chaudron wrote:
> On 16 Apr 2019, at 21:55, Ben Pfaff wrote:
> > AF_XDP is a faster way to access the existing kernel devices.  If we
> > take that point of view, then it would be ideal if AF_XDP were
> > automatically used when it was available, instead of adding a new
> > network device type.  Is there a reason that this point of view is
> > wrong?  That is, when AF_XDP is available, is there a reason not to use
> > it?
> 
> This needs support by all the ingress and egress ports in the system, and
> currently, there is no API to check this.

Do you mean for performance or for some other reason?  I would suspect
that, if AF_XDP was not available, then everything would still work OK
via AF_PACKET, just slower.

> There are also features like traffic shaping that will not work. Maybe it
> will be worth adding the table for AF_XDP in
> http://docs.openvswitch.org/en/latest/faq/releases/

AF_XDP is comparable to DPDK/userspace, not to the Linux kernel
datapath.

The table currently conflates the userspace datapath with the DPDK
network device.  I believe that the only entry there that depends on the
DPDK network device is the one for policing.  It could be replaced by a
[*] with a note like this:

YES - for DPDK network devices.
NO - for system or AF_XDP network devices.

> > You said that your goal for the next version is to improve performance
> > and add optimizations.  Do you think that is important before we merge
> > the series?  We can continue to improve performance after it is merged.
> 
> The previous patch was rather unstable and I could not get it running with
> the PVP test without crashing. I think this patchset should get some proper
> testing and reviews by others. Especially for all the features being marked
> as supported in the above-mentioned table.

If it's unstable, we should fix that before adding it in.

However, the bar is lower for new features that don't break existing
features, especially optional ones and ones that can be easily be
removed if they don't work out in the end.  DPDK support was considered
"experimental" for a long time, it's possible that AF_XDP would be in
the same boat for a while.

> > If we set performance aside, do you have a reason to want to wait to
> > merge this?  (I wasn't able to easily apply this series to current
> > master, so it'll need at least a rebase before we apply it.  And I have
> > only skimmed it, not fully reviewed it.)
> 
> Other than the items above, do we really need another datapath? 

It's less than a new datapath.  It's a new network device
implementation.

> With this, we use two or more cores for processing packets. If we poll
> two physical ports it could be 300%, which is a typical use case with
> bonding. What about multiple queue support, does it work? Both in
> kernel and DPDK mode we use multiple queues to distribute the load,
> with this scenario does it double the number of CPUs used? Can we use
> the poll() mode as explained here,
> https://linuxplumbersconf.org/event/2/contributions/99/, and how will
> it work with multiple queues/pmd threads? What about any latency
> tests, is it worse or better than kernel/dpdk? Also with the AF_XDP
> datapath, there is no to leverage hardware offload, like DPDK and
> TC. And then there is the part that it only works on the most recent
> kernels.

These are good questions.  William will have some of the answers.

> To me looking at this I would say it’s far from being ready to be merged
> into OVS. However, if others decide to go ahead I think it should be
> disabled, not compiled in by default.

Yes, that seems reasonable to me.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, v1] netdev-rte-offloads: Reassign vport netdev functions.

[Gregory Rose]

On 4/17/2019 9:34 AM, Ben Pfaff wrote:

On Wed, Apr 17, 2019 at 11:45:33AM -0400, Aaron Conole wrote:

rte comes from dpdk as an acronym for Run Time Environment.  Maybe even
just dropping the 'rte_' portion?

*That* is what rte stands for?  What a ridiculously generic name.  It's
like naming a library Operating System.


Like libOS?

https://lwn.net/Articles/637658/

;^)


___
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] [ovs-dev, v1] netdev-rte-offloads: Reassign vport netdev functions.

[Ben Pfaff]

On Wed, Apr 17, 2019 at 11:45:33AM -0400, Aaron Conole wrote:
> rte comes from dpdk as an acronym for Run Time Environment.  Maybe even
> just dropping the 'rte_' portion?

*That* is what rte stands for?  What a ridiculously generic name.  It's
like naming a library Operating System.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] OVN: Add support for Transport Zones

[Ben Pfaff]

On Wed, Apr 17, 2019 at 09:15:33PM +0530, Numan Siddique wrote:
> On Wed, Apr 17, 2019 at 8:21 PM  wrote:
> > diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
> > index d87a02e7b..98007cb6e 100644
> > --- a/ovn/ovn-sb.ovsschema
> > +++ b/ovn/ovn-sb.ovsschema
> > @@ -1,7 +1,7 @@
> >  {
> >  "name": "OVN_Southbound",
> >  "version": "2.2.0",
> >
> 
> Since you are adding a new column, can you please bump the version to
> "2.2.1" ?
> 
> With this change
> Acked-by: Numan Siddique 

New columns should bump the middle digit, e.g. 2.2.0 -> 2.3.0.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Cómo vender por whatsapp

[WhatsApp cómo poderosa herramienta]

Cursos TOP - Webinar Interactivo – Viernes 03 de Mayo

¿Cómo vender por whatsapp ? 

Las ventas se logran el 90% con una estrategia comunicacional y el 10% con 
tecnología. En nuestro webinar podrás conocer todo lo que debes saber sobre 
WhatsApp, esta poderosa herramienta que todos utilizamos de manera personal y 
que tiene un potencial increíble para los negocios, en específico, las ventas. 

Conoceremos las novedosas prácticas que te ofrece la aplicación WhatsApp cómo 
herramienta para impulsar ventas.  

Ejes Temáticos:

• WhatsApp cómo poderosa herramienta de ventas.
• Errores comunes al utilizar WhatsApp cómo herramienta de Marketing.
• Atención al cliente como estrategia de fidelización.
• Software para envíos masivos. 


Para mayor información, responder sobre este correo con la palabra Whatsapp + 
los siguientes datos:


NOMBRE:

TELÉFONO:

EMPRES

EMAIL ALTERNO: 
 

Llamanos al (045) 55 1554 6630
www.Innovalearn.mx 


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

Re: [ovs-dev] [PATCH] dpif-netdev: Update comment about flow installation race.

[Gregory Rose]

On 4/17/2019 1:43 AM, Ilya Maximets wrote:

Userspace datapath uses per-PMD flow tables/classifiers for a long
time. However, it was decided to keep this race window to not block
revalidators. Comment should be updated to reflect the current state.

Fixes: 1c1e46ed8457 ("dpif-netdev: Add per-pmd flow-table/classifier.")
Signed-off-by: Ilya Maximets 
---
  lib/dpif-netdev.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bd9718824..645981c38 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6535,8 +6535,7 @@ handle_packet_upcall(struct dp_netdev_pmd_thread *pmd,
   * could have already been installed since we last did the flow
   * lookup before upcall.  This could be solved by moving the
   * mutex lock outside the loop, but that's an awful long time
- * to be locking everyone out of making flow installs.  If we
- * move to a per-core classifier, it would be reasonable. */
+ * to be locking revalidators out of making flow modifications. */
  ovs_mutex_lock(>flow_mutex);
  netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
  if (OVS_LIKELY(!netdev_flow)) {


LGTM
Reviewed-by: Greg Rose 

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

Re: [ovs-dev] [PATCH v1] netdev-rte-offloads: Reserve mark space.

[Gregory Rose]

On 4/17/2019 1:42 AM, Ophir Munk wrote:

From: Roni Bar Yanai 

Reserve the first 64 intergers (0-63) for special processing.
For example, if a packet can't complete its procssesing in hardware,
we will make sure that the packet is marked with a special mark to
continue its processing in software.

s/intergers/integers/

What size integers?

From the patch it appears to be 32 bit integers but since integer size 
isn't fixed across CPU types it should

be called out I think.

- Greg



Co-authored-by: Asaf Penso 
Signed-off-by: Asaf Penso 
Co-authored-by: Ophir Munk 
Signed-off-by: Ophir Munk 
Signed-off-by: Roni Bar Yanai 
---
  lib/dpif-netdev.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9a48038..ea0ace7 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2096,8 +2096,11 @@ dp_netdev_pmd_find_dpcls(struct dp_netdev_pmd_thread 
*pmd,
  return cls;
  }
  
-#define MAX_FLOW_MARK   (UINT32_MAX - 1)

-#define INVALID_FLOW_MARK   (UINT32_MAX)
+#define INVALID_FLOW_MARK(UINT32_MAX)
+#define MAX_FLOW_MARK(UINT32_MAX - 1)
+#define RESERVED_FLOW_MARK_SIZE  (64)
+#define MIN_FLOW_MARKRESERVED_FLOW_MARK_SIZE
+#define AVAILABLE_FLOW_MARK_SIZE (MAX_FLOW_MARK - MIN_FLOW_MARK + 1)
  
  struct megaflow_to_mark_data {

  const struct cmap_node node;
@@ -2123,7 +2126,8 @@ flow_mark_alloc(void)
  
  if (!flow_mark.pool) {

  /* Haven't initiated yet, do it here */
-flow_mark.pool = id_pool_create(0, MAX_FLOW_MARK);
+flow_mark.pool = id_pool_create(MIN_FLOW_MARK,
+AVAILABLE_FLOW_MARK_SIZE);
  }
  
  if (id_pool_alloc_id(flow_mark.pool, )) {


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

[ovs-dev] [PATCH v5] OVN: Add support for Transport Zones

[lmartins]

From: Lucas Alvares Gomes 

This patch is adding support for Transport Zones. Transport zones (a.k.a
TZs) is way to enable users of OVN to separate Chassis into different
logical groups that will only form tunnels between members of the same
groups. Each Chassis can belong to one or more Transport Zones. If
not set, the Chassis will be considered part of a default group.

Configuring Transport Zones is done by creating a key called
"ovn-transport-zones" in the external_ids column of the Open_vSwitch
table from the local OVS instance. The value is a string with the name
of the Transport Zone that this instance is part of. Multiple TZs can
be specified with a comma-separated list. For example:

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1

or

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3

This configuration is also exposed in the Chassis table of the OVN
Southbound Database in a new column called "transport_zones".

The use for Transport Zones includes but are not limited to:

* Edge computing: As a way to preventing edge sites from trying to create
  tunnels with every node on every other edge site while still allowing
  these sites to create tunnels with the central node.

* Extra security layer: Where users wants to create "trust zones"
  and prevent computes in a more secure zone to communicate with a less
  secure zone.

This patch is also backward compatible so the upgrade guide for OVN [0]
is still valid and the ovn-controller service can be upgraded before the
OVSDBs.

[0] http://docs.openvswitch.org/en/latest/intro/install/ovn-upgrades/

Reported-by: Daniel Alvarez Sanchez 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
Signed-off-by: Lucas Alvares Gomes 
Acked-by: Numan Siddique 
---
v4 -> v5
 * Bumped the version of the ovn-sb.ovsschema to 2.2.1.

v3 -> v4
 * Stopped using the "external_ids" column of the Chassis table and
   instead added a new column called "transport_zones" to hold the set of
   transport zones that the Chassis is part of.

v2 -> v3
 * Enhanced the test to include two more Chassis and assert the case
   where Chassis with no TZs set will have tunnels formed between them.
 * Rebased the patch on top of the latest master branch.

v1 -> v2   
 * Rename the function check_chassis_tzones to chassis_tzones_overlap.
 * Fix a memory leak in chassis_tzones_overlap.
 * Pass the transport_zones to encaps_run() as a "const char *"
   instead of "struct sbrec_chassis". With this we can also avoid not
   running the function in case the Chassis entry is not yet created.


 NEWS|   3 +
 ovn/controller/chassis.c|  26 -
 ovn/controller/chassis.h|   4 +-
 ovn/controller/encaps.c |  35 ++-
 ovn/controller/encaps.h |   4 +-
 ovn/controller/ovn-controller.8.xml |   9 ++
 ovn/controller/ovn-controller.c |  19 +++-
 ovn/ovn-sb.ovsschema|   9 +-
 ovn/ovn-sb.xml  |   8 ++
 tests/ovn.at| 151 
 10 files changed, 258 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index c7440b476..e09f59d64 100644
--- a/NEWS
+++ b/NEWS
@@ -33,6 +33,9 @@ Post-v2.11.0
  * Added Policy-based routing(PBR) support to create permit/deny/reroute
policies on the logical router. New table(Logical_Router_Policy) added 
in
OVN-NB schema. New "ovn-nbctl" commands to add/delete/list PBR policies.
+ * Support for Transport Zones, a way to separate chassis into
+   logical groups which results in tunnels only been formed between
+   members of the same transport zone(s).
- New QoS type "linux-netem" on Linux.
- Added support for TLS Server Name Indication (SNI).
 
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 58d5d49d5..0f537f1f7 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -19,6 +19,7 @@
 #include "chassis.h"
 
 #include "lib/smap.h"
+#include "lib/sset.h"
 #include "lib/vswitch-idl.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
@@ -73,13 +74,34 @@ get_cms_options(const struct smap *ext_ids)
 return smap_get_def(ext_ids, "ovn-cms-options", "");
 }
 
+static void
+update_chassis_transport_zones(const struct sset *transport_zones,
+   const struct sbrec_chassis *chassis_rec)
+{
+struct sset chassis_tzones_set = SSET_INITIALIZER(_tzones_set);
+for (int i = 0; i < chassis_rec->n_transport_zones; i++) {
+sset_add(_tzones_set, chassis_rec->transport_zones[i]);
+}
+
+/* Only update the transport zones if something changed */
+if (!sset_equals(transport_zones, _tzones_set)) {
+const char **ls_arr = sset_array(transport_zones);
+sbrec_chassis_set_transport_zones(chassis_rec, ls_arr,
+  

Re: [ovs-dev] [PATCH 1/5 V2] datapath: meter: Use struct_size() in kzalloc()

[Gregory Rose]

On 4/16/2019 9:17 PM, Ben Pfaff wrote:

On Tue, Apr 16, 2019 at 04:58:55PM -0700, Gregory Rose wrote:

On 4/16/2019 4:56 PM, Gregory Rose wrote:


On 4/16/2019 2:04 PM, Ben Pfaff wrote:

On Wed, Mar 27, 2019 at 10:14:00AM -0700, Greg Rose wrote:

From: "Gustavo A. R. Silva" 

Upstream commit:
  commit c5c3899de09e307e3a0999ab8d620ab0ede05aa1
  Author: Gustavo A. R. Silva 
  Date:   Tue Jan 15 15:19:17 2019 -0600

  openvswitch: meter: Use struct_size() in kzalloc()

Thanks, applied to master.  Advice on backporting is appreciated.

No worries about backporting, master is fine.  If these patches are fine
on the master branch for a while then
we may decide to backport them.  As it is they're new kernel features
and not of interest right now to older
branches.

Thanks!

- Greg

While I'm looking,  patches 2 through 5 of the series are still awaiting
action.  They've been reviewed.

Oh, somehow I missed the rest.

I applied them to master now too.


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

Re: [ovs-dev] [PATCH v4] OVN: Add support for Transport Zones

[Lucas Alvares Gomes]

On Wed, Apr 17, 2019 at 4:46 PM Numan Siddique  wrote:
>
> On Wed, Apr 17, 2019 at 8:21 PM  wrote:
>
> > From: Lucas Alvares Gomes 
> >
> > This patch is adding support for Transport Zones. Transport zones (a.k.a
> > TZs) is way to enable users of OVN to separate Chassis into different
> > logical groups that will only form tunnels between members of the same
> > groups. Each Chassis can belong to one or more Transport Zones. If
> > not set, the Chassis will be considered part of a default group.
> >
> > Configuring Transport Zones is done by creating a key called
> > "ovn-transport-zones" in the external_ids column of the Open_vSwitch
> > table from the local OVS instance. The value is a string with the name
> > of the Transport Zone that this instance is part of. Multiple TZs can
> > be specified with a comma-separated list. For example:
> >
> > $ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1
> >
> > or
> >
> > $ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3
> >
> > This configuration is also exposed in the Chassis table of the OVN
> > Southbound Database in a new column called "transport_zones".
> >
> > The use for Transport Zones includes but are not limited to:
> >
> > * Edge computing: As a way to preventing edge sites from trying to create
> >   tunnels with every node on every other edge site while still allowing
> >   these sites to create tunnels with the central node.
> >
> > * Extra security layer: Where users wants to create "trust zones"
> >   and prevent computes in a more secure zone to communicate with a less
> >   secure zone.
> >
> > This patch is also backward compatible so the upgrade guide for OVN [0]
> > is still valid and the ovn-controller service can be upgraded before the
> > OVSDBs.
> >
> > [0] http://docs.openvswitch.org/en/latest/intro/install/ovn-upgrades/
> >
> > Reported-by: Daniel Alvarez Sanchez 
> > Reported-at:
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
> > Signed-off-by: Lucas Alvares Gomes 
> > ---
> > v3 -> v4
> >  * Stopped using the "external_ids" column of the Chassis table and
> >instead added a new column called "transport_zones" to hold the set of
> >transport zones that the Chassis is part of.
> >
> > v2 -> v3
> >  * Enhanced the test to include two more Chassis and assert the case
> >where Chassis with no TZs set will have tunnels formed between them.
> >  * Rebased the patch on top of the latest master branch.
> >
> > v1 -> v2
> >
> >  * Rename the function check_chassis_tzones to chassis_tzones_overlap.
> >  * Fix a memory leak in chassis_tzones_overlap.
> >  * Pass the transport_zones to encaps_run() as a "const char *"
> >instead of "struct sbrec_chassis". With this we can also avoid not
> >running the function in case the Chassis entry is not yet created.
> >
> >  NEWS|   3 +
> >  ovn/controller/chassis.c|  26 -
> >  ovn/controller/chassis.h|   4 +-
> >  ovn/controller/encaps.c |  35 ++-
> >  ovn/controller/encaps.h |   4 +-
> >  ovn/controller/ovn-controller.8.xml |   9 ++
> >  ovn/controller/ovn-controller.c |  19 +++-
> >  ovn/ovn-sb.ovsschema|   7 +-
> >  ovn/ovn-sb.xml  |   8 ++
> >  tests/ovn.at| 151 
> >  10 files changed, 257 insertions(+), 9 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index c7440b476..e09f59d64 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -33,6 +33,9 @@ Post-v2.11.0
> >   * Added Policy-based routing(PBR) support to create
> > permit/deny/reroute
> > policies on the logical router. New table(Logical_Router_Policy)
> > added in
> > OVN-NB schema. New "ovn-nbctl" commands to add/delete/list PBR
> > policies.
> > + * Support for Transport Zones, a way to separate chassis into
> > +   logical groups which results in tunnels only been formed between
> > +   members of the same transport zone(s).
> > - New QoS type "linux-netem" on Linux.
> > - Added support for TLS Server Name Indication (SNI).
> >
> > diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
> > index 58d5d49d5..0f537f1f7 100644
> > --- a/ovn/controller/chassis.c
> > +++ b/ovn/controller/chassis.c
> > @@ -19,6 +19,7 @@
> >  #include "chassis.h"
> >
> >  #include "lib/smap.h"
> > +#include "lib/sset.h"
> >  #include "lib/vswitch-idl.h"
> >  #include "openvswitch/dynamic-string.h"
> >  #include "openvswitch/vlog.h"
> > @@ -73,13 +74,34 @@ get_cms_options(const struct smap *ext_ids)
> >  return smap_get_def(ext_ids, "ovn-cms-options", "");
> >  }
> >
> > +static void
> > +update_chassis_transport_zones(const struct sset *transport_zones,
> > +   const struct sbrec_chassis *chassis_rec)
> > +{
> > +struct sset chassis_tzones_set =
> > SSET_INITIALIZER(_tzones_set);
> > +for (int i = 0; i < 

Re: [ovs-dev] [PATCH] rhel: Include all header files in the Fedora's devel package

[Aaron Conole]

Ben Pfaff  writes:

> On Mon, Apr 01, 2019 at 09:26:31AM -0700, Ansis Atteka wrote:
>> From: Ansis Atteka 
>> 
>> While the header files added by this patch into Fedora's devel
>> rpm package can be considered private, the other devel packages
>> for RHEL/CentOS and Debian/Ubuntu distros include them.
>> 
>> So this patch simply makes the Fedora devel package consistent with
>> the other devel packages.
>> 
>> Signed-off-by: Ansis Atteka 
>
> I support consistency here.  I didn't test the patch.

In case it hasn't been applied yet:

Acked-by: Aaron Conole 


Actually, I think it helps with the ovn / ovs split.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] OVN: Add support for Transport Zones

[Numan Siddique]

On Wed, Apr 17, 2019 at 8:21 PM  wrote:

> From: Lucas Alvares Gomes 
>
> This patch is adding support for Transport Zones. Transport zones (a.k.a
> TZs) is way to enable users of OVN to separate Chassis into different
> logical groups that will only form tunnels between members of the same
> groups. Each Chassis can belong to one or more Transport Zones. If
> not set, the Chassis will be considered part of a default group.
>
> Configuring Transport Zones is done by creating a key called
> "ovn-transport-zones" in the external_ids column of the Open_vSwitch
> table from the local OVS instance. The value is a string with the name
> of the Transport Zone that this instance is part of. Multiple TZs can
> be specified with a comma-separated list. For example:
>
> $ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1
>
> or
>
> $ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3
>
> This configuration is also exposed in the Chassis table of the OVN
> Southbound Database in a new column called "transport_zones".
>
> The use for Transport Zones includes but are not limited to:
>
> * Edge computing: As a way to preventing edge sites from trying to create
>   tunnels with every node on every other edge site while still allowing
>   these sites to create tunnels with the central node.
>
> * Extra security layer: Where users wants to create "trust zones"
>   and prevent computes in a more secure zone to communicate with a less
>   secure zone.
>
> This patch is also backward compatible so the upgrade guide for OVN [0]
> is still valid and the ovn-controller service can be upgraded before the
> OVSDBs.
>
> [0] http://docs.openvswitch.org/en/latest/intro/install/ovn-upgrades/
>
> Reported-by: Daniel Alvarez Sanchez 
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
> Signed-off-by: Lucas Alvares Gomes 
> ---
> v3 -> v4
>  * Stopped using the "external_ids" column of the Chassis table and
>instead added a new column called "transport_zones" to hold the set of
>transport zones that the Chassis is part of.
>
> v2 -> v3
>  * Enhanced the test to include two more Chassis and assert the case
>where Chassis with no TZs set will have tunnels formed between them.
>  * Rebased the patch on top of the latest master branch.
>
> v1 -> v2
>
>  * Rename the function check_chassis_tzones to chassis_tzones_overlap.
>  * Fix a memory leak in chassis_tzones_overlap.
>  * Pass the transport_zones to encaps_run() as a "const char *"
>instead of "struct sbrec_chassis". With this we can also avoid not
>running the function in case the Chassis entry is not yet created.
>
>  NEWS|   3 +
>  ovn/controller/chassis.c|  26 -
>  ovn/controller/chassis.h|   4 +-
>  ovn/controller/encaps.c |  35 ++-
>  ovn/controller/encaps.h |   4 +-
>  ovn/controller/ovn-controller.8.xml |   9 ++
>  ovn/controller/ovn-controller.c |  19 +++-
>  ovn/ovn-sb.ovsschema|   7 +-
>  ovn/ovn-sb.xml  |   8 ++
>  tests/ovn.at| 151 
>  10 files changed, 257 insertions(+), 9 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index c7440b476..e09f59d64 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -33,6 +33,9 @@ Post-v2.11.0
>   * Added Policy-based routing(PBR) support to create
> permit/deny/reroute
> policies on the logical router. New table(Logical_Router_Policy)
> added in
> OVN-NB schema. New "ovn-nbctl" commands to add/delete/list PBR
> policies.
> + * Support for Transport Zones, a way to separate chassis into
> +   logical groups which results in tunnels only been formed between
> +   members of the same transport zone(s).
> - New QoS type "linux-netem" on Linux.
> - Added support for TLS Server Name Indication (SNI).
>
> diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
> index 58d5d49d5..0f537f1f7 100644
> --- a/ovn/controller/chassis.c
> +++ b/ovn/controller/chassis.c
> @@ -19,6 +19,7 @@
>  #include "chassis.h"
>
>  #include "lib/smap.h"
> +#include "lib/sset.h"
>  #include "lib/vswitch-idl.h"
>  #include "openvswitch/dynamic-string.h"
>  #include "openvswitch/vlog.h"
> @@ -73,13 +74,34 @@ get_cms_options(const struct smap *ext_ids)
>  return smap_get_def(ext_ids, "ovn-cms-options", "");
>  }
>
> +static void
> +update_chassis_transport_zones(const struct sset *transport_zones,
> +   const struct sbrec_chassis *chassis_rec)
> +{
> +struct sset chassis_tzones_set =
> SSET_INITIALIZER(_tzones_set);
> +for (int i = 0; i < chassis_rec->n_transport_zones; i++) {
> +sset_add(_tzones_set, chassis_rec->transport_zones[i]);
> +}
> +
> +/* Only update the transport zones if something changed */
> +if (!sset_equals(transport_zones, _tzones_set)) {
> +const char **ls_arr = 

Re: [ovs-dev] [ovs-dev, v1] netdev-rte-offloads: Reassign vport netdev functions.

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Ophir Munk, I am a robot and I have tried out your 
> patch.
> Thanks for your contribution.
>
> I encountered some error that I wasn't expecting.  See the details below.
>
>
> build:
...
> libtool: link: gcc -std=gnu99 -Wstrict-prototypes -Wall -Wextra
> -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security
> -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align
> -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes
> -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror
> -Werror -g -O2 -o utilities/ovs-testcontroller
> utilities/ovs-testcontroller.o lib/.libs/libopenvswitch.a -lcap-ng
> -lssl -lcrypto -lpthread -lrt -lm -lunbound
> lib/.libs/libopenvswitch.a(dpif-netdev.o): In function `dpif_netdev_port_add':
> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/lib/dpif-netdev.c:1869:
> undefined reference to `netdev_rte_offloads_port_add'
> collect2: error: ld returned 1 exit status
> make[2]: *** [utilities/ovs-testcontroller] Error 1
> make[2]: Leaving directory 
> `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory 
> `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
> make: *** [all] Error 2

I guess you'll need to provide an implementation for the non-dpdk
builds.

I also suggest a different name than netdev_rte_offloads_port_add

rte comes from dpdk as an acronym for Run Time Environment.  Maybe even
just dropping the 'rte_' portion?

>
> Thanks,
> 0-day Robot
> ___
> 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] rhel: if rpms were built without libcapng then let processrs to run as root

[Aaron Conole]

Ansis Atteka  writes:

> On Tue, 16 Apr 2019 at 12:36, Ansis Atteka  wrote:
>>
>> On Tue, 16 Apr 2019 at 10:46, Aaron Conole  wrote:
>> >
>> > Ansis Atteka  writes:
>> >
>> > > Otherwise, Open vSwitch will fail to start with the following
>> > > error "libcap-ng is not configured at compile time" when it
>> > > attempts to downgrade to Open vSwitch user.
>> > >
>> > > Also, if packages were built in a way where processes are
>> > > supposed to be running only as root, then there is no point
>> > > in creating "openvswitch" user in the first place.
>> > >
>> > > Signed-off-by: Ansis Atteka 
>> > > ---
>> >
>> > It seems strange to not provide a user-downgrade option just because we
>> > cannot drop capabilities via libcap-ng.
>>
>> I did not look too close in the daemon-unix.c, but I believe in
>> current design "linux capabilities" and "linux user downgrade" go hand
>> in hand (i.e. you either do both or neither). At least for Linux
>> Kernel datapath. Not sure about other datapaths.

I guess this is because we need cap_net_admin, cap_net_raw, etc.,
but it's possible to keep this even without using libcap-ng (or even
needing programmatic idiom, iirc).  For example, it is possible to
attach these capabilities to the ovs-vswitchd binary on the filesystem
(for filesystems which support) and thereby provide this functionality
without needing libcap-ng.

Then we retain the ability to run as a (mostly) unprivileged user, and
still provide the control and slow-path for the kernel space side.

>> >
>> > Maybe it's possible instead to change daemon-unix.c to provide an
>> > alternative fallback when building on linux without libcapng?  Something
>> > like the untested code here.  I have no idea if all of the capabilities
>> > will get dropped when the user/group ids are changed, but we might be
>> > able to deal with that as well.  WDYT?
>>
>> I can take a look at that and if there is a valid use-case I can send
>> another patch addressing that specific issue. However, I think we can
>> at this time proceed with this patch, because
>> 1. it fixes fatal bug when OVS was built without libcap-ng support; AND
>> 2. it does break anything when OVS was built with libcap-ng support.
>
> meant to say "it does NOT break anything"
>>
>> Let me know what you think and if I am missing something? Is your main
>> concern that if there turns out to be a valid use case to not have
>> libcap-ng support but still user downgrade feature, then portions of
>> this patch will need to be reverted? If so, I am fine with that...

RHEL systems run as non-root user by default.  I think it's a valid
use-case.  I agree, we need to address the issue when building without
libcap-ng.  Can you check if it is possible to update the RPM build so
that the file capabilities are attached to the ovs-vswitchd executable
directly?  If it works, it would allow us to achieve the same end result
in an alternative path.

>> >
>> > diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
>> > index 6169763c2..cd2f66295 100644
>> > --- a/lib/daemon-unix.c
>> > +++ b/lib/daemon-unix.c
>> > @@ -859,9 +859,10 @@ daemon_become_new_user__(bool access_datapath)
>> >  if (LIBCAPNG) {
>> >  daemon_become_new_user_linux(access_datapath);
>> >  } else {
>> > -VLOG_FATAL("%s: fail to downgrade user using libcap-ng. "
>> > -   "(libcap-ng is not configured at compile time), "
>> > -   "aborting.", pidfile);
>> > +VLOG_INFO("%s: fail to downgrade user using libcap-ng. "
>> > +  "(libcap-ng is not configured at compile time).",
>> > +  pidfile);
>> > +daemon_become_new_user_unix();
>> >  }
>> >  } else {
>> >  daemon_become_new_user_unix();
>> >
>> >
>> > >  poc/playbook-fedora-builder.yml | 6 +++---
>> > >  rhel/openvswitch-fedora.spec.in | 8 
>> > >  2 files changed, 11 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/poc/playbook-fedora-builder.yml 
>> > > b/poc/playbook-fedora-builder.yml
>> > > index 70f0b6ff2..b955714fc 100644
>> > > --- a/poc/playbook-fedora-builder.yml
>> > > +++ b/poc/playbook-fedora-builder.yml
>> > > @@ -99,17 +99,17 @@
>> > >- openvswitch-dkms.spec
>> > >
>> > >- name: Build Open vSwitch user space rpms
>> > > -command: rpmbuild -bb --without check rhel/openvswitch-fedora.spec
>> > > +command: rpmbuild -bb --without check --without libcapng 
>> > > rhel/openvswitch-fedora.spec
>> > >  args:
>> > >  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
>> > >
>> > >- name: Build Open vSwitch kmod rpm
>> > > -command: rpmbuild -bb --without check rhel/openvswitch-fedora.spec
>> > > +command: rpmbuild -bb --without check --without libcapng 
>> > > rhel/openvswitch-fedora.spec
>> > >  args:
>> > >  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
>> > >
>> > >- name: Build Open vSwitch dkms rpm
>> > > -command: 

[ovs-dev] [PATCH v4] OVN: Add support for Transport Zones

[lmartins]

From: Lucas Alvares Gomes 

This patch is adding support for Transport Zones. Transport zones (a.k.a
TZs) is way to enable users of OVN to separate Chassis into different
logical groups that will only form tunnels between members of the same
groups. Each Chassis can belong to one or more Transport Zones. If
not set, the Chassis will be considered part of a default group.

Configuring Transport Zones is done by creating a key called
"ovn-transport-zones" in the external_ids column of the Open_vSwitch
table from the local OVS instance. The value is a string with the name
of the Transport Zone that this instance is part of. Multiple TZs can
be specified with a comma-separated list. For example:

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1

or

$ sudo ovs-vsctl set open . external-ids:ovn-transport-zones=tz1,tz2,tz3

This configuration is also exposed in the Chassis table of the OVN
Southbound Database in a new column called "transport_zones".

The use for Transport Zones includes but are not limited to:

* Edge computing: As a way to preventing edge sites from trying to create
  tunnels with every node on every other edge site while still allowing
  these sites to create tunnels with the central node.

* Extra security layer: Where users wants to create "trust zones"
  and prevent computes in a more secure zone to communicate with a less
  secure zone.

This patch is also backward compatible so the upgrade guide for OVN [0]
is still valid and the ovn-controller service can be upgraded before the
OVSDBs.

[0] http://docs.openvswitch.org/en/latest/intro/install/ovn-upgrades/

Reported-by: Daniel Alvarez Sanchez 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048255.html
Signed-off-by: Lucas Alvares Gomes 
---
v3 -> v4
 * Stopped using the "external_ids" column of the Chassis table and
   instead added a new column called "transport_zones" to hold the set of
   transport zones that the Chassis is part of.

v2 -> v3
 * Enhanced the test to include two more Chassis and assert the case
   where Chassis with no TZs set will have tunnels formed between them.
 * Rebased the patch on top of the latest master branch.

v1 -> v2   
 * Rename the function check_chassis_tzones to chassis_tzones_overlap.
 * Fix a memory leak in chassis_tzones_overlap.
 * Pass the transport_zones to encaps_run() as a "const char *"
   instead of "struct sbrec_chassis". With this we can also avoid not
   running the function in case the Chassis entry is not yet created.

 NEWS|   3 +
 ovn/controller/chassis.c|  26 -
 ovn/controller/chassis.h|   4 +-
 ovn/controller/encaps.c |  35 ++-
 ovn/controller/encaps.h |   4 +-
 ovn/controller/ovn-controller.8.xml |   9 ++
 ovn/controller/ovn-controller.c |  19 +++-
 ovn/ovn-sb.ovsschema|   7 +-
 ovn/ovn-sb.xml  |   8 ++
 tests/ovn.at| 151 
 10 files changed, 257 insertions(+), 9 deletions(-)

diff --git a/NEWS b/NEWS
index c7440b476..e09f59d64 100644
--- a/NEWS
+++ b/NEWS
@@ -33,6 +33,9 @@ Post-v2.11.0
  * Added Policy-based routing(PBR) support to create permit/deny/reroute
policies on the logical router. New table(Logical_Router_Policy) added 
in
OVN-NB schema. New "ovn-nbctl" commands to add/delete/list PBR policies.
+ * Support for Transport Zones, a way to separate chassis into
+   logical groups which results in tunnels only been formed between
+   members of the same transport zone(s).
- New QoS type "linux-netem" on Linux.
- Added support for TLS Server Name Indication (SNI).
 
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 58d5d49d5..0f537f1f7 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -19,6 +19,7 @@
 #include "chassis.h"
 
 #include "lib/smap.h"
+#include "lib/sset.h"
 #include "lib/vswitch-idl.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
@@ -73,13 +74,34 @@ get_cms_options(const struct smap *ext_ids)
 return smap_get_def(ext_ids, "ovn-cms-options", "");
 }
 
+static void
+update_chassis_transport_zones(const struct sset *transport_zones,
+   const struct sbrec_chassis *chassis_rec)
+{
+struct sset chassis_tzones_set = SSET_INITIALIZER(_tzones_set);
+for (int i = 0; i < chassis_rec->n_transport_zones; i++) {
+sset_add(_tzones_set, chassis_rec->transport_zones[i]);
+}
+
+/* Only update the transport zones if something changed */
+if (!sset_equals(transport_zones, _tzones_set)) {
+const char **ls_arr = sset_array(transport_zones);
+sbrec_chassis_set_transport_zones(chassis_rec, ls_arr,
+  sset_count(transport_zones));
+free(ls_arr);
+}
+
+

[ovs-dev] [PATCH net-next v3 4/4] openvswitch: load and reference the NAT helper.

[Flavio Leitner]

This improves the original commit 17c357efe5ec ("openvswitch: load
NAT helper") where it unconditionally tries to load the module for
every flow using NAT, so not efficient when loading multiple flows.
It also doesn't hold any references to the NAT module while the
flow is active.

This change fixes those problems. It will try to load the module
only if it's not present. It grabs a reference to the NAT module
and holds it while the flow is active. Finally, an error message
shows up if either actions above fails.

Fixes: 17c357efe5ec ("openvswitch: load NAT helper")
Signed-off-by: Flavio Leitner 
---
 net/openvswitch/conntrack.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

v3
   - replaced 'err' with 'error' in the msg.
v2
   - updated with new functions names.

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 0be3ab5bde26..4cc4dd948969 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1307,6 +1307,7 @@ static int ovs_ct_add_helper(struct ovs_conntrack_info 
*info, const char *name,
 {
struct nf_conntrack_helper *helper;
struct nf_conn_help *help;
+   int ret = 0;
 
helper = nf_conntrack_helper_try_module_get(name, info->family,
key->ip.proto);
@@ -1321,13 +1322,21 @@ static int ovs_ct_add_helper(struct ovs_conntrack_info 
*info, const char *name,
return -ENOMEM;
}
 
+#ifdef CONFIG_NF_NAT_NEEDED
+   if (info->nat) {
+   ret = nf_nat_helper_try_module_get(name, info->family,
+  key->ip.proto);
+   if (ret) {
+   nf_conntrack_helper_put(helper);
+   OVS_NLERR(log, "Failed to load \"%s\" NAT helper, 
error: %d",
+ name, ret);
+   return ret;
+   }
+   }
+#endif
rcu_assign_pointer(help->helper, helper);
info->helper = helper;
-
-   if (info->nat)
-   request_module("ip_nat_%s", name);
-
-   return 0;
+   return ret;
 }
 
 #ifdef CONFIG_NF_NAT_NEEDED
@@ -1801,8 +1810,13 @@ void ovs_ct_free_action(const struct nlattr *a)
 
 static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info)
 {
-   if (ct_info->helper)
+   if (ct_info->helper) {
+#ifdef CONFIG_NF_NAT_NEEDED
+   if (ct_info->nat)
+   nf_nat_helper_put(ct_info->helper);
+#endif
nf_conntrack_helper_put(ct_info->helper);
+   }
if (ct_info->ct) {
if (ct_info->timeout[0])
nf_ct_destroy_timeout(ct_info->ct);
-- 
2.20.1

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

[ovs-dev] [PATCH net-next v3 3/4] netfilter: nf_nat: register NAT helpers.

[Flavio Leitner]

Register amanda, ftp, irc, sip and tftp NAT helpers.

Signed-off-by: Flavio Leitner 
---
 net/netfilter/nf_nat_amanda.c | 9 -
 net/netfilter/nf_nat_ftp.c| 9 -
 net/netfilter/nf_nat_irc.c| 9 -
 net/netfilter/nf_nat_sip.c| 9 +++--
 net/netfilter/nf_nat_tftp.c   | 9 -
 5 files changed, 39 insertions(+), 6 deletions(-)

v3
  - grouped all NAT helpers changes into a single patch.

v2
  - defined NAT_HELPER_NAME for consistency.
  - C99 static change.
  - renamed the variables to be nat_helper.*

diff --git a/net/netfilter/nf_nat_amanda.c b/net/netfilter/nf_nat_amanda.c
index 6b729a897c5f..4e59416ea709 100644
--- a/net/netfilter/nf_nat_amanda.c
+++ b/net/netfilter/nf_nat_amanda.c
@@ -19,10 +19,15 @@
 #include 
 #include 
 
+#define NAT_HELPER_NAME "amanda"
+
 MODULE_AUTHOR("Brian J. Murrell ");
 MODULE_DESCRIPTION("Amanda NAT helper");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_NF_NAT_HELPER("amanda");
+MODULE_ALIAS_NF_NAT_HELPER(NAT_HELPER_NAME);
+
+static struct nf_conntrack_nat_helper nat_helper_amanda =
+   NF_CT_NAT_HELPER_INIT(NAT_HELPER_NAME);
 
 static unsigned int help(struct sk_buff *skb,
 enum ip_conntrack_info ctinfo,
@@ -74,6 +79,7 @@ static unsigned int help(struct sk_buff *skb,
 
 static void __exit nf_nat_amanda_fini(void)
 {
+   nf_nat_helper_unregister(_helper_amanda);
RCU_INIT_POINTER(nf_nat_amanda_hook, NULL);
synchronize_rcu();
 }
@@ -81,6 +87,7 @@ static void __exit nf_nat_amanda_fini(void)
 static int __init nf_nat_amanda_init(void)
 {
BUG_ON(nf_nat_amanda_hook != NULL);
+   nf_nat_helper_register(_helper_amanda);
RCU_INIT_POINTER(nf_nat_amanda_hook, help);
return 0;
 }
diff --git a/net/netfilter/nf_nat_ftp.c b/net/netfilter/nf_nat_ftp.c
index 0e93b1f19432..0ea6b1bc52de 100644
--- a/net/netfilter/nf_nat_ftp.c
+++ b/net/netfilter/nf_nat_ftp.c
@@ -21,13 +21,18 @@
 #include 
 #include 
 
+#define NAT_HELPER_NAME "ftp"
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Rusty Russell ");
 MODULE_DESCRIPTION("ftp NAT helper");
-MODULE_ALIAS_NF_NAT_HELPER("ftp");
+MODULE_ALIAS_NF_NAT_HELPER(NAT_HELPER_NAME);
 
 /* FIXME: Time out? --RR */
 
+static struct nf_conntrack_nat_helper nat_helper_ftp =
+   NF_CT_NAT_HELPER_INIT(NAT_HELPER_NAME);
+
 static int nf_nat_ftp_fmt_cmd(struct nf_conn *ct, enum nf_ct_ftp_type type,
  char *buffer, size_t buflen,
  union nf_inet_addr *addr, u16 port)
@@ -124,6 +129,7 @@ static unsigned int nf_nat_ftp(struct sk_buff *skb,
 
 static void __exit nf_nat_ftp_fini(void)
 {
+   nf_nat_helper_unregister(_helper_ftp);
RCU_INIT_POINTER(nf_nat_ftp_hook, NULL);
synchronize_rcu();
 }
@@ -131,6 +137,7 @@ static void __exit nf_nat_ftp_fini(void)
 static int __init nf_nat_ftp_init(void)
 {
BUG_ON(nf_nat_ftp_hook != NULL);
+   nf_nat_helper_register(_helper_ftp);
RCU_INIT_POINTER(nf_nat_ftp_hook, nf_nat_ftp);
return 0;
 }
diff --git a/net/netfilter/nf_nat_irc.c b/net/netfilter/nf_nat_irc.c
index 6c06e997395f..d87cbe5e03ec 100644
--- a/net/netfilter/nf_nat_irc.c
+++ b/net/netfilter/nf_nat_irc.c
@@ -23,10 +23,15 @@
 #include 
 #include 
 
+#define NAT_HELPER_NAME "irc"
+
 MODULE_AUTHOR("Harald Welte ");
 MODULE_DESCRIPTION("IRC (DCC) NAT helper");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_NF_NAT_HELPER("irc");
+MODULE_ALIAS_NF_NAT_HELPER(NAT_HELPER_NAME);
+
+static struct nf_conntrack_nat_helper nat_helper_irc =
+   NF_CT_NAT_HELPER_INIT(NAT_HELPER_NAME);
 
 static unsigned int help(struct sk_buff *skb,
 enum ip_conntrack_info ctinfo,
@@ -96,6 +101,7 @@ static unsigned int help(struct sk_buff *skb,
 
 static void __exit nf_nat_irc_fini(void)
 {
+   nf_nat_helper_unregister(_helper_irc);
RCU_INIT_POINTER(nf_nat_irc_hook, NULL);
synchronize_rcu();
 }
@@ -103,6 +109,7 @@ static void __exit nf_nat_irc_fini(void)
 static int __init nf_nat_irc_init(void)
 {
BUG_ON(nf_nat_irc_hook != NULL);
+   nf_nat_helper_register(_helper_irc);
RCU_INIT_POINTER(nf_nat_irc_hook, help);
return 0;
 }
diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c
index f1f007d9484c..464387b3600f 100644
--- a/net/netfilter/nf_nat_sip.c
+++ b/net/netfilter/nf_nat_sip.c
@@ -24,11 +24,15 @@
 #include 
 #include 
 
+#define NAT_HELPER_NAME "sip"
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Christian Hentschel ");
 MODULE_DESCRIPTION("SIP NAT helper");
-MODULE_ALIAS_NF_NAT_HELPER("sip");
+MODULE_ALIAS_NF_NAT_HELPER(NAT_HELPER_NAME);
 
+static struct nf_conntrack_nat_helper nat_helper_sip =
+   NF_CT_NAT_HELPER_INIT(NAT_HELPER_NAME);
 
 static unsigned int mangle_packet(struct sk_buff *skb, unsigned int protoff,
  unsigned int dataoff,
@@ -656,8 +660,8 @@ static struct nf_ct_helper_expectfn sip_nat = {
 
 static void __exit nf_nat_sip_fini(void)
 {
+   

[ovs-dev] [PATCH net-next v3 2/4] netfilter: add API to manage NAT helpers.

[Flavio Leitner]

The API allows a conntrack helper to indicate its corresponding
NAT helper which then can be loaded and reference counted.

Signed-off-by: Flavio Leitner 
---
 include/net/netfilter/nf_conntrack_helper.h | 22 +-
 net/netfilter/nf_conntrack_amanda.c |  8 +-
 net/netfilter/nf_conntrack_ftp.c| 18 +++--
 net/netfilter/nf_conntrack_helper.c | 86 +
 net/netfilter/nf_conntrack_irc.c|  6 +-
 net/netfilter/nf_conntrack_sane.c   | 12 +--
 net/netfilter/nf_conntrack_sip.c| 28 +++
 net/netfilter/nf_conntrack_tftp.c   | 18 +++--
 8 files changed, 161 insertions(+), 37 deletions(-)

v3
   - respected 80 columns.
   - changed checks comparing with NULL to !.
   - fixed return codes to ENOENT.
   - removed request_module return error checking.
   - replaced BUG_ON() with WARN_ON_ONCE() at nf_nat_helper_put().
   - removed other BUG_ON() to allow modules to be built-in.

v2
   - renamed functions names as suggested by Pablo
   - renamed structs and other variables accordingly.
   - replaced the spinlock with mutex as suggested
 by Pablo.
   - used structure in C99 as static in the NAT helper
 module as suggested by Pablo.
   - defined a HELPER_NAME for consistency on each NAT
 helper module.

diff --git a/include/net/netfilter/nf_conntrack_helper.h 
b/include/net/netfilter/nf_conntrack_helper.h
index 28bd4569aa64..44b5a00a9c64 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -15,7 +15,8 @@
 #include 
 #include 
 
-#define NF_NAT_HELPER_NAME(name)   "ip_nat_" name
+#define NF_NAT_HELPER_PREFIX   "ip_nat_"
+#define NF_NAT_HELPER_NAME(name)   NF_NAT_HELPER_PREFIX name
 #define MODULE_ALIAS_NF_NAT_HELPER(name) \
MODULE_ALIAS(NF_NAT_HELPER_NAME(name))
 
@@ -58,6 +59,8 @@ struct nf_conntrack_helper {
unsigned int queue_num;
/* length of userspace private data stored in nf_conn_help->data */
u16 data_len;
+   /* name of NAT helper module */
+   char nat_mod_name[NF_CT_HELPER_NAME_LEN];
 };
 
 /* Must be kept in sync with the classes defined by helpers */
@@ -157,4 +160,21 @@ nf_ct_helper_expectfn_find_by_symbol(const void *symbol);
 extern struct hlist_head *nf_ct_helper_hash;
 extern unsigned int nf_ct_helper_hsize;
 
+struct nf_conntrack_nat_helper {
+   struct list_head list;
+   char mod_name[NF_CT_HELPER_NAME_LEN];   /* module name */
+   struct module *module;  /* pointer to self */
+};
+
+#define NF_CT_NAT_HELPER_INIT(name) \
+   { \
+   .mod_name = NF_NAT_HELPER_NAME(name), \
+   .module = THIS_MODULE \
+   }
+
+void nf_nat_helper_register(struct nf_conntrack_nat_helper *nat);
+void nf_nat_helper_unregister(struct nf_conntrack_nat_helper *nat);
+int nf_nat_helper_try_module_get(const char *name, u16 l3num,
+u8 protonum);
+void nf_nat_helper_put(struct nf_conntrack_helper *helper);
 #endif /*_NF_CONNTRACK_HELPER_H*/
diff --git a/net/netfilter/nf_conntrack_amanda.c 
b/net/netfilter/nf_conntrack_amanda.c
index f2681ec5b5f6..dbec6fca0d9e 100644
--- a/net/netfilter/nf_conntrack_amanda.c
+++ b/net/netfilter/nf_conntrack_amanda.c
@@ -28,11 +28,13 @@
 static unsigned int master_timeout __read_mostly = 300;
 static char *ts_algo = "kmp";
 
+#define HELPER_NAME "amanda"
+
 MODULE_AUTHOR("Brian J. Murrell ");
 MODULE_DESCRIPTION("Amanda connection tracking module");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ip_conntrack_amanda");
-MODULE_ALIAS_NFCT_HELPER("amanda");
+MODULE_ALIAS_NFCT_HELPER(HELPER_NAME);
 
 module_param(master_timeout, uint, 0600);
 MODULE_PARM_DESC(master_timeout, "timeout for the master connection");
@@ -179,13 +181,14 @@ static const struct nf_conntrack_expect_policy 
amanda_exp_policy = {
 
 static struct nf_conntrack_helper amanda_helper[2] __read_mostly = {
{
-   .name   = "amanda",
+   .name   = HELPER_NAME,
.me = THIS_MODULE,
.help   = amanda_help,
.tuple.src.l3num= AF_INET,
.tuple.src.u.udp.port   = cpu_to_be16(10080),
.tuple.dst.protonum = IPPROTO_UDP,
.expect_policy  = _exp_policy,
+   .nat_mod_name   = NF_NAT_HELPER_NAME(HELPER_NAME),
},
{
.name   = "amanda",
@@ -195,6 +198,7 @@ static struct nf_conntrack_helper amanda_helper[2] 
__read_mostly = {
.tuple.src.u.udp.port   = cpu_to_be16(10080),
.tuple.dst.protonum = IPPROTO_UDP,
.expect_policy  = _exp_policy,
+   .nat_mod_name   = NF_NAT_HELPER_NAME(HELPER_NAME),
},
 };
 
diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index a11c304fb771..32aeac1c4760 100644
--- 

[ovs-dev] [PATCH net-next v3 1/4] netfilter: use macros to create module aliases.

[Flavio Leitner]

Each NAT helper creates a module alias which follows a pattern.
Use macros for consistency.

Signed-off-by: Flavio Leitner 
---
 include/net/netfilter/nf_conntrack_helper.h | 4 
 net/ipv4/netfilter/nf_nat_h323.c| 2 +-
 net/ipv4/netfilter/nf_nat_pptp.c| 2 +-
 net/netfilter/nf_nat_amanda.c   | 2 +-
 net/netfilter/nf_nat_ftp.c  | 2 +-
 net/netfilter/nf_nat_irc.c  | 2 +-
 net/netfilter/nf_nat_sip.c  | 2 +-
 net/netfilter/nf_nat_tftp.c | 2 +-
 8 files changed, 11 insertions(+), 7 deletions(-)

v3
  - no changes.
v2
  - renamed the defines as suggested by Pablo.

diff --git a/include/net/netfilter/nf_conntrack_helper.h 
b/include/net/netfilter/nf_conntrack_helper.h
index ec52a8dc32fd..28bd4569aa64 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -15,6 +15,10 @@
 #include 
 #include 
 
+#define NF_NAT_HELPER_NAME(name)   "ip_nat_" name
+#define MODULE_ALIAS_NF_NAT_HELPER(name) \
+   MODULE_ALIAS(NF_NAT_HELPER_NAME(name))
+
 struct module;
 
 enum nf_ct_helper_flags {
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index 4e6b53ab6c33..7875c98072eb 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -631,4 +631,4 @@ module_exit(fini);
 MODULE_AUTHOR("Jing Min Zhao ");
 MODULE_DESCRIPTION("H.323 NAT helper");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("ip_nat_h323");
+MODULE_ALIAS_NF_NAT_HELPER("h323");
diff --git a/net/ipv4/netfilter/nf_nat_pptp.c b/net/ipv4/netfilter/nf_nat_pptp.c
index 68b4d450391b..e17b4ee7604c 100644
--- a/net/ipv4/netfilter/nf_nat_pptp.c
+++ b/net/ipv4/netfilter/nf_nat_pptp.c
@@ -37,7 +37,7 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Harald Welte ");
 MODULE_DESCRIPTION("Netfilter NAT helper module for PPTP");
-MODULE_ALIAS("ip_nat_pptp");
+MODULE_ALIAS_NF_NAT_HELPER("pptp");
 
 static void pptp_nat_expected(struct nf_conn *ct,
  struct nf_conntrack_expect *exp)
diff --git a/net/netfilter/nf_nat_amanda.c b/net/netfilter/nf_nat_amanda.c
index e4d61a7a5258..6b729a897c5f 100644
--- a/net/netfilter/nf_nat_amanda.c
+++ b/net/netfilter/nf_nat_amanda.c
@@ -22,7 +22,7 @@
 MODULE_AUTHOR("Brian J. Murrell ");
 MODULE_DESCRIPTION("Amanda NAT helper");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("ip_nat_amanda");
+MODULE_ALIAS_NF_NAT_HELPER("amanda");
 
 static unsigned int help(struct sk_buff *skb,
 enum ip_conntrack_info ctinfo,
diff --git a/net/netfilter/nf_nat_ftp.c b/net/netfilter/nf_nat_ftp.c
index 5063cbf1689c..0e93b1f19432 100644
--- a/net/netfilter/nf_nat_ftp.c
+++ b/net/netfilter/nf_nat_ftp.c
@@ -24,7 +24,7 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Rusty Russell ");
 MODULE_DESCRIPTION("ftp NAT helper");
-MODULE_ALIAS("ip_nat_ftp");
+MODULE_ALIAS_NF_NAT_HELPER("ftp");
 
 /* FIXME: Time out? --RR */
 
diff --git a/net/netfilter/nf_nat_irc.c b/net/netfilter/nf_nat_irc.c
index 3aa35a43100d..6c06e997395f 100644
--- a/net/netfilter/nf_nat_irc.c
+++ b/net/netfilter/nf_nat_irc.c
@@ -26,7 +26,7 @@
 MODULE_AUTHOR("Harald Welte ");
 MODULE_DESCRIPTION("IRC (DCC) NAT helper");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("ip_nat_irc");
+MODULE_ALIAS_NF_NAT_HELPER("irc");
 
 static unsigned int help(struct sk_buff *skb,
 enum ip_conntrack_info ctinfo,
diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c
index aa1be643d7a0..f1f007d9484c 100644
--- a/net/netfilter/nf_nat_sip.c
+++ b/net/netfilter/nf_nat_sip.c
@@ -27,7 +27,7 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Christian Hentschel ");
 MODULE_DESCRIPTION("SIP NAT helper");
-MODULE_ALIAS("ip_nat_sip");
+MODULE_ALIAS_NF_NAT_HELPER("sip");
 
 
 static unsigned int mangle_packet(struct sk_buff *skb, unsigned int protoff,
diff --git a/net/netfilter/nf_nat_tftp.c b/net/netfilter/nf_nat_tftp.c
index 7f67e1d5310d..dd3a835c111d 100644
--- a/net/netfilter/nf_nat_tftp.c
+++ b/net/netfilter/nf_nat_tftp.c
@@ -16,7 +16,7 @@
 MODULE_AUTHOR("Magnus Boden ");
 MODULE_DESCRIPTION("TFTP NAT helper");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("ip_nat_tftp");
+MODULE_ALIAS_NF_NAT_HELPER("tftp");
 
 static unsigned int help(struct sk_buff *skb,
 enum ip_conntrack_info ctinfo,
-- 
2.20.1

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

[ovs-dev] [PATCH net-next v3 0/4] openvswitch: load and reference the NAT helper

[Flavio Leitner]

The request_module() is quite expensive and triggers the
usermode helper in userspace. Instead, load only if the
module is not present and keep module references to avoid
problems.

The first patch standardize the module alias which is already
there, but not in a formal way.

The second patch adds an API to point to the NAT helper.

The third patch will register each NAT helper using the
new API.

The last patch fixes openvswitch to use the new API to
load and reference the NAT helper and also report an error
if the operation fails.


Flavio Leitner (4):
  netfilter: use macros to create module aliases.
  netfilter: add API to manage NAT helpers.
  netfilter: nf_nat: register NAT helpers.
  openvswitch: load and reference the NAT helper.

 include/net/netfilter/nf_conntrack_helper.h | 24 ++
 net/ipv4/netfilter/nf_nat_h323.c|  2 +-
 net/ipv4/netfilter/nf_nat_pptp.c|  2 +-
 net/netfilter/nf_conntrack_amanda.c |  8 +-
 net/netfilter/nf_conntrack_ftp.c| 18 +++--
 net/netfilter/nf_conntrack_helper.c | 86 +
 net/netfilter/nf_conntrack_irc.c|  6 +-
 net/netfilter/nf_conntrack_sane.c   | 12 +--
 net/netfilter/nf_conntrack_sip.c| 28 +++
 net/netfilter/nf_conntrack_tftp.c   | 18 +++--
 net/netfilter/nf_nat_amanda.c   |  9 ++-
 net/netfilter/nf_nat_ftp.c  |  9 ++-
 net/netfilter/nf_nat_irc.c  |  9 ++-
 net/netfilter/nf_nat_sip.c  |  9 ++-
 net/netfilter/nf_nat_tftp.c |  9 ++-
 net/openvswitch/conntrack.c | 26 +--
 16 files changed, 225 insertions(+), 50 deletions(-)

-- 
2.20.1

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

Re: [ovs-dev] [ovs-discuss] Does OVS support Infiniband?

[Ben Pfaff]

OVS does not support Infiniband.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 3/3] netdev-dpdk: reset queue number for vhost devices on vm shutdown

[Kevin Traynor]

On 16/04/2019 10:45, David Marchand wrote:
> Rather than poll all disabled queues and waste some memory for vms that
> have been shutdown, we can reconfigure when receiving a destroy
> connection notification from the vhost library.
> 
> $ while true; do
>   ovs-appctl dpif-netdev/pmd-rxq-show |awk '
>   /port: / {
> tot++;
> if ($NF == "disabled") {
>   dis++;
> }
>   }
>   END {
> print "total: " tot ", enabled: " (tot - dis)
>   }'
>   sleep 1
> done
> 
> total: 66, enabled: 66
> total: 6, enabled: 2
> 
> Note: this patch requires a fix for the vhost library submitted here:
> http://patchwork.dpdk.org/patch/52680/
> 
> Without it, this change will do nothing but have openvswitch complain
> that the vhost device is unknown:
> 
> dpdk|INFO|VHOST_CONFIG: vhost peer closed
> dpdk|ERR|VHOST_CONFIG: (0) device not found.
> 
> dpdk|INFO|VHOST_CONFIG: vhost peer closed
> dpdk|ERR|VHOST_CONFIG: (1) device not found.
> 
> Signed-off-by: David Marchand 

I think this probably shouldn't be merged until OVS is using a version
of DPDK with the linked patch because it is introducing errors in the
logs which can be alarming for a user. The DPDK fix should be part of
DPDK 18.11.2.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[Eelco Chaudron]

On 17 Apr 2019, at 14:01, Eelco Chaudron wrote:


Hi William,

I think you applied the following patch to get it to compile? Or did 
you copy in the kernel headers?


https://www.spinics.net/lists/netdev/msg563507.html


I noticed you duplicated the macros, which resulted in all kind of 
compile errors. So I removed them, applied the two patches above, which 
would get me to the next step.


I’m building it with DPDK enabled and it was causing all kind of 
duplicate definition errors as the kernel and DPDK re-use some structure 
names.


To get it all compiled and working I had top make the following changes:

$ git diff
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index b3bf2f044..47fb3342a 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -295,7 +295,7 @@ netdev_linux_rxq_xsk(struct xsk_socket_info *xsk,
 uint32_t idx_rx = 0, idx_fq = 0;
 int ret = 0;

-unsigned int non_afxdp;
+unsigned int non_afxdp = 0;

 /* See if there is any packet on RX queue,
  * if yes, idx_rx is the index having the packet.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 47153dc60..77f2150ab 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -24,7 +24,7 @@
 #include 
 #include 
 #include 
-#include 
+//#include 

 #include 
 #include 
diff --git a/lib/xdpsock.h b/lib/xdpsock.h
index 8df8fa451..a2ed1a136 100644
--- a/lib/xdpsock.h
+++ b/lib/xdpsock.h
@@ -28,7 +28,7 @@
 #include 
 #include 
 #include 
-#include 
+//#include 
 #include 
 #include 
 #include 
@@ -43,14 +43,6 @@
 #include "ovs-atomic.h"
 #include "openvswitch/thread.h"

-/* bpf/xsk.h uses the following macros not defined in OVS,
- * so re-define them before include.
- */
-#define unlikely OVS_UNLIKELY
-#define likely OVS_LIKELY
-#define barrier() __asm__ __volatile__("": : :"memory")
-#define smp_rmb() barrier()
-#define smp_wmb() barrier()
 #include 

In addition you need to do “make install_headers” from kernel libbpf 
and copy the libbpf_util.h manually.


I was able to do a simple physical port in same physical port out test 
without crashing, but the numbers seem low:


$ ovs-ofctl dump-flows ovs_pvp_br0
 cookie=0x0, duration=210.344s, table=0, n_packets=1784692, 
n_bytes=2694884920, in_port=eno1 actions=IN_PORT


"Physical loopback test, L3 flows[port redirect]"
,Packet size
Number of flows,64,128,256,512,768,1024,1514
100,77574,77329,76605,76417,75539,75252,74617

The above is using two cores, but with a single DPDK core I get the 
following (on the same machine):


"Physical loopback test, L3 flows[port redirect]"
,Packet size
Number of flows,64,128,256,512,768,1024,1514
100,9527075,8445852,4528935,2349597,1586276,1197304,814854

For the kernel datapath the numbers are:

"Physical loopback test, L3 flows[port redirect]"
,Packet size
Number of flows,64,128,256,512,768,1024,1514
100,4862995,5521870,4528872,2349596,1586277,1197305,814854

But keep in mind it uses roughly 550/610/520/380/180/140/110% of the CPU 
for the respective packet size.



On 2 Apr 2019, at 0:46, William Tu wrote:


The patch series introduces AF_XDP support for OVS netdev.
AF_XDP is a new address family working together with eBPF.
In short, a socket with AF_XDP family can receive and send
packets from an eBPF/XDP program attached to the netdev.
For more details about AF_XDP, please see linux kernel's
Documentation/networking/af_xdp.rst

OVS has a couple of netdev types, i.e., system, tap, or
internal.  The patch first adds a new netdev types called
"afxdp", and implement its configuration, packet reception,
and transmit functions.  Since the AF_XDP socket, xsk,
operates in userspace, once ovs-vswitchd receives packets
from xsk, the proposed architecture re-uses the existing
userspace dpif-netdev datapath.  As a result, most of
the packet processing happens at the userspace instead of
linux kernel.

Architecure
===
   _
  |   +---+
  |   |ovs-vswitchd   |<-->ovsdb-server
  |   +---+
  |   |  ofproto  |<-->OpenFlow controllers
  |   ++-++
  |   | netdev | |ofproto-|
userspace |   ++ |  dpif  |
  |   | netdev | ++
  |   |provider| |  dpif  |
  |   +---||---+ ++
  |   || |  dpif- |
  |   || | netdev |
  |_  || ++
  ||
   _  +---||-++
  |   | af_xdp prog + |
   kernel |   |   xsk_map |
  |_  +||-+
   ||
physical
   NIC

To simply start, create a ovs userspace bridge using dpif-netdev
by setting the datapath_type to netdev:
  # ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev

And attach a linux netdev with type afxdp:
  # ovs-vsctl add-port br0 afxdp-p0 -- \
  set 

Re: [ovs-dev] [PATCH v2 2/3] netdev-dpdk: avoid reconfiguration on VIRTIO_NET_F_MQ changes

[Kevin Traynor]

On 16/04/2019 10:45, David Marchand wrote:
> At the moment, a malicious guest might negotiate VIRTIO_NET_F_MQ and
> !VIRTIO_NET_F_MQ in a loop which would be seen as qp_num going from 1 to
> n and n to 1 continuously, triggering datapath reconfigurations at each
> transition.
> 
> Limit this by only reconfiguring on increased qp_num.
> The previous patch reduced the observed cost of polling disabled queues,
> so the only cost is memory.
> 
> Signed-off-by: David Marchand 
> ---
>  lib/netdev-dpdk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 9ba8e67..fc554db 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3510,8 +3510,8 @@ new_device(int vid)
>  newnode = dev->socket_id;
>  }
>  
> -if (dev->requested_n_txq != qp_num
> -|| dev->requested_n_rxq != qp_num
> +if (dev->requested_n_txq < qp_num
> +|| dev->requested_n_rxq < qp_num
>  || dev->requested_socket_id != newnode) {
>  dev->requested_socket_id = newnode;
>  dev->requested_n_rxq = qp_num;
> 

LGTM
Acked-by: Kevin Traynor 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/3] dpif-netdev: only poll enabled vhost queues

[Kevin Traynor]

On 16/04/2019 10:45, David Marchand wrote:
> We currently poll all available queues based on the max queue count
> exchanged with the vhost peer and rely on the vhost library in DPDK to
> check the vring status beneath.
> This can lead to some overhead when we have a lot of unused queues.
> 
> To enhance the situation, we can skip the disabled queues.
> On rxq notifications, we make use of the netdev's change_seq number so
> that the pmd thread main loop can cache the queue state periodically.
> 
> $ ovs-appctl dpif-netdev/pmd-rxq-show
> pmd thread numa_id 0 core_id 1:
>   isolated : true
>   port: dpdk0 queue-id:  0  pmd usage:  0 %
> pmd thread numa_id 0 core_id 2:
>   isolated : true
>   port: vhost1queue-id:  0  pmd usage:  0 %
>   port: vhost3queue-id:  0  pmd usage:  0 %
> pmd thread numa_id 0 core_id 15:
>   isolated : true
>   port: dpdk1 queue-id:  0  pmd usage:  0 %
> pmd thread numa_id 0 core_id 16:
>   isolated : true
>   port: vhost0queue-id:  0  pmd usage:  0 %
>   port: vhost2queue-id:  0  pmd usage:  0 %
> 
> $ while true; do
>   ovs-appctl dpif-netdev/pmd-rxq-show |awk '
>   /port: / {
> tot++;
> if ($NF == "disabled") {
>   dis++;
> }
>   }
>   END {
> print "total: " tot ", enabled: " (tot - dis)
>   }'
>   sleep 1
> done
> 
> total: 6, enabled: 2
> total: 6, enabled: 2
> ...
> 
>  # Started vm, virtio devices are bound to kernel driver which enables
>  # F_MQ + all queue pairs
> total: 6, enabled: 2
> total: 66, enabled: 66
> ...
> 
>  # Unbound vhost0 and vhost1 from the kernel driver
> total: 66, enabled: 66
> total: 66, enabled: 34
> ...
> 
>  # Configured kernel bound devices to use only 1 queue pair
> total: 66, enabled: 34
> total: 66, enabled: 19
> total: 66, enabled: 4
> ...
> 
>  # While rebooting the vm
> total: 66, enabled: 4
> total: 66, enabled: 2
> ...
> total: 66, enabled: 66
> ...
> 
>  # After shutting down the vm
> total: 66, enabled: 66
> total: 66, enabled: 2
> 
> Signed-off-by: David Marchand 
> ---
> 
> Changes since v1:
> - only indicate disabled queues in dpif-netdev/pmd-rxq-show output
> - Ilya comments
>   - no need for a struct as we only need a boolean per rxq
>   - "rx_q" is generic, while we only care for this in vhost case,
> renamed as "vhost_rxq_enabled",
>   - add missing rte_free on allocation error,
>   - vhost_rxq_enabled is freed in vhost destruct only,
>   - rxq0 is enabled at the virtio device activation to accomodate
> legacy implementations which would not report per queue states
> later,
>   - do not mix boolean with integer,
>   - do not use bit operand on boolean,
> 
> ---
>  lib/dpif-netdev.c | 27 
>  lib/netdev-dpdk.c | 58 
> +++
>  lib/netdev-provider.h |  5 +
>  lib/netdev.c  | 10 +
>  lib/netdev.h  |  1 +
>  5 files changed, 88 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 4d6d0c3..5bfa6ad 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -591,6 +591,8 @@ struct polled_queue {
>  struct dp_netdev_rxq *rxq;
>  odp_port_t port_no;
>  bool emc_enabled;
> +bool enabled;
> +uint64_t change_seq;
>  };
>  
>  /* Contained by struct dp_netdev_pmd_thread's 'poll_list' member. */
> @@ -1171,6 +1173,9 @@ pmd_info_show_rxq(struct ds *reply, struct 
> dp_netdev_pmd_thread *pmd)
>  } else {
>  ds_put_format(reply, "%s", "NOT AVAIL");
>  }
> +if (!netdev_rxq_enabled(list[i].rxq->rx)) {
> +ds_put_cstr(reply, "  polling: disabled");
> +}

It's just a personal preference but I'm not crazy about the additional
columns appearing/disappearing. Also it seems like it's more fundamental
than the % usage and should be closer to the queue-id. It's currently

port: v0queue-id:  0  pmd usage: 13 %
port: v0queue-id:  1  pmd usage:  0 %  polling: disabled
port: v1queue-id:  0  pmd usage: 13 %
port: v1queue-id:  1  pmd usage:  0 %  polling: disabled

As suggestion, could be:

port: v0queue-id:  0   enabled  pmd usage: 13 %
port: v0queue-id:  1  disabled  pmd usage:  0 %
port: v1queue-id:  0   enabled  pmd usage: 13 %
port: v1queue-id:  1  disabled  pmd usage:  0 %

or else just remove the % usage if a queue is disabled:

port: v0queue-id:  0  pmd usage: 13 %
port: v0queue-id:  1  disabled
port: v1queue-id:  0  pmd usage: 13 %
port: v1queue-id:  1  disabled

>  ds_put_cstr(reply, "\n");
>  }
>  ovs_mutex_unlock(>port_mutex);
> @@ -5198,6 +5203,11 @@ dpif_netdev_run(struct dpif *dpif)
>  }
>  
>  for (i = 0; i < port->n_rxq; i++) {
> +
> +if (!netdev_rxq_enabled(port->rxqs[i].rx)) {
> +continue;
> +   

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[Eelco Chaudron]

Hi William,

I think you applied the following patch to get it to compile? Or did you 
copy in the kernel headers?


https://www.spinics.net/lists/netdev/msg563507.html

//Eelco

On 2 Apr 2019, at 0:46, William Tu wrote:


The patch series introduces AF_XDP support for OVS netdev.
AF_XDP is a new address family working together with eBPF.
In short, a socket with AF_XDP family can receive and send
packets from an eBPF/XDP program attached to the netdev.
For more details about AF_XDP, please see linux kernel's
Documentation/networking/af_xdp.rst

OVS has a couple of netdev types, i.e., system, tap, or
internal.  The patch first adds a new netdev types called
"afxdp", and implement its configuration, packet reception,
and transmit functions.  Since the AF_XDP socket, xsk,
operates in userspace, once ovs-vswitchd receives packets
from xsk, the proposed architecture re-uses the existing
userspace dpif-netdev datapath.  As a result, most of
the packet processing happens at the userspace instead of
linux kernel.

Architecure
===
   _
  |   +---+
  |   |ovs-vswitchd   |<-->ovsdb-server
  |   +---+
  |   |  ofproto  |<-->OpenFlow controllers
  |   ++-++
  |   | netdev | |ofproto-|
userspace |   ++ |  dpif  |
  |   | netdev | ++
  |   |provider| |  dpif  |
  |   +---||---+ ++
  |   || |  dpif- |
  |   || | netdev |
  |_  || ++
  ||
   _  +---||-++
  |   | af_xdp prog + |
   kernel |   |   xsk_map |
  |_  +||-+
   ||
physical
   NIC

To simply start, create a ovs userspace bridge using dpif-netdev
by setting the datapath_type to netdev:
  # ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev

And attach a linux netdev with type afxdp:
  # ovs-vsctl add-port br0 afxdp-p0 -- \
  set interface afxdp-p0 type="afxdp"

Performance
===
For this version, v4, I mainly focus on making the features right with
libbpf AF_XDP API and use the AF_XDP SKB mode, which is the slower 
set-up.

My next version is to measure the performance and add optimizations.

Documentation
=
Most of the design details are described in the paper presetned at
Linux Plumber 2018, "Bringing the Power of eBPF to Open vSwitch"[1],
section 4, and slides[2].
This path uses a not-yet upstreamed feature called XDP_ATTACH[3],
described in section 3.1, which is a built-in XDP program for the 
AF_XDP.

This greatly simplifies the management of XDP/eBPF programs.

[1] http://vger.kernel.org/lpc_net2018_talks/ovs-ebpf-afxdp.pdf
[2] 
http://vger.kernel.org/lpc_net2018_talks/ovs-ebpf-lpc18-presentation.pdf
[3] 
http://vger.kernel.org/lpc_net2018_talks/lpc18_paper_af_xdp_perf-v2.pdf


For installation and configuration guide, see
  # Documentation/intro/install/bpf.rst

Test Cases
==
Test cases are created using namespaces and veth peer, with AF_XDP 
socket
attached to the veth (thus the SKB_MODE).  By issuing "make 
check-afxdp",

the patch shows the following:

AF_XDP netdev datapath-sanity

  1: datapath - ping between two ports   ok
  2: datapath - ping between two ports on vlan   ok
  3: datapath - ping6 between two ports  ok
  4: datapath - ping6 between two ports on vlan  ok
  5: datapath - ping over vxlan tunnel   ok
  6: datapath - ping over vxlan6 tunnel  ok
  7: datapath - ping over gre tunnel ok
  8: datapath - ping over erspan v1 tunnel   ok
  9: datapath - ping over erspan v2 tunnel   ok
 10: datapath - ping over ip6erspan v1 tunnelok
 11: datapath - ping over ip6erspan v2 tunnelok
 12: datapath - ping over geneve tunnel  ok
 13: datapath - ping over geneve6 tunnel ok
 14: datapath - clone action ok
 15: datapath - basic truncate actionok

conntrack

 16: conntrack - controller  ok
 17: conntrack - force commitok
 18: conntrack - ct flush by 5-tuple ok
 19: conntrack - IPv4 ping   ok
 20: conntrack - get_nconns and get/set_maxconns ok
 21: conntrack - IPv6 ping   ok

system-ovn

 22: ovn -- 2 LRs connected via LS, gateway router, SNAT and DNAT ok
 23: ovn -- 2 LRs connected via LS, gateway router, easy SNAT ok
 24: ovn -- multiple gateway routers, SNAT and DNAT  ok
 25: ovn -- load-balancing   ok
 26: ovn -- load-balancing - same subnet.ok
 27: ovn -- load balancing in gateway router ok
 28: ovn -- multiple gateway routers, load-balancing ok

Re: [ovs-dev] [PATCH v2 3/3] netdev-dpdk: reset queue number for vhost devices on vm shutdown

[Ilya Maximets]

On 17.04.2019 11:37, David Marchand wrote:
> 
> 
> 
> On Tue, Apr 16, 2019 at 4:01 PM Ilya Maximets  > wrote:
> 
> On 16.04.2019 12:45, David Marchand wrote:
> > Rather than poll all disabled queues and waste some memory for vms that
> > have been shutdown, we can reconfigure when receiving a destroy
> > connection notification from the vhost library.
> >
> > $ while true; do
> >   ovs-appctl dpif-netdev/pmd-rxq-show |awk '
> >   /port: / {
> >     tot++;
> >     if ($NF == "disabled") {
> >       dis++;
> >     }
> >   }
> >   END {
> >     print "total: " tot ", enabled: " (tot - dis)
> >   }'
> >   sleep 1
> > done
> >
> > total: 66, enabled: 66
> > total: 6, enabled: 2
> >
> > Note: this patch requires a fix for the vhost library submitted here:
> > http://patchwork.dpdk.org/patch/52680/
> >
> > Without it, this change will do nothing but have openvswitch complain
> > that the vhost device is unknown:
> >
> > dpdk|INFO|VHOST_CONFIG: vhost peer closed
> > dpdk|ERR|VHOST_CONFIG: (0) device not found.
> >
> > dpdk|INFO|VHOST_CONFIG: vhost peer closed
> > dpdk|ERR|VHOST_CONFIG: (1) device not found.
> >
> > Signed-off-by: David Marchand  >
> > ---
> >  lib/netdev-dpdk.c | 35 ++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index fc554db..a7fae4f 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -186,12 +186,15 @@ static const struct rte_eth_conf port_conf = {
> >  static int new_device(int vid);
> >  static void destroy_device(int vid);
> >  static int vring_state_changed(int vid, uint16_t queue_id, int enable);
> > +static void destroy_connection(int vid);
> >  static const struct vhost_device_ops virtio_net_device_ops =
> >  {
> >      .new_device =  new_device,
> >      .destroy_device = destroy_device,
> >      .vring_state_changed = vring_state_changed,
> > -    .features_changed = NULL
> > +    .features_changed = NULL,
> > +    .new_connection = NULL,
> > +    .destroy_connection = destroy_connection,
> >  };
> > 
> >  enum { DPDK_RING_SIZE = 256 };
> > @@ -3661,6 +3664,36 @@ vring_state_changed(int vid, uint16_t queue_id, 
> int enable)
> >      return 0;
> >  }
> > 
> > +static void
> > +destroy_connection(int vid)
> > +{
> > +    struct netdev_dpdk *dev;
> > +    char ifname[IF_NAME_SZ];
> > +
> > +    rte_vhost_get_ifname(vid, ifname, sizeof ifname);
> > +
> > +    ovs_mutex_lock(_mutex);
> > +    /* Add device to the vhost port with the same name as that passed 
> down. */
> 
> This comment is a leftover from the 'new_device'. It could be dropped.
> 
> 
> Yep.
> 
> 
> 
> > +    LIST_FOR_EACH (dev, list_node, _list) {
> > +        ovs_mutex_lock(>mutex);
> > +        if (nullable_string_is_equal(ifname, dev->vhost_id)) {
> > +            uint32_t qp_num = NR_QUEUE;
> > +
> 
> It might be good to check if device is destroyed here and log an error
> otherwise.
> 
> 
> So that we have a log similar to what we have in destroy_device() ?

Not sure if I understand the question correctly. I meant something like:

if (netdev_dpdk_get_vid(dev) >= 0) {
VLOG_ERR(Connection on socket '%s' destroyed while vhost device still 
attached.",
 dev->vhost_id);
}

But it's probably a good idea to log an error if we didn't find any matching
device in 'dpdk_list' too.

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

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[Eelco Chaudron]

On 17 Apr 2019, at 10:09, Eelco Chaudron wrote:


On 16 Apr 2019, at 21:55, Ben Pfaff wrote:


On Mon, Apr 01, 2019 at 03:46:48PM -0700, William Tu wrote:

The patch series introduces AF_XDP support for OVS netdev.
AF_XDP is a new address family working together with eBPF.
In short, a socket with AF_XDP family can receive and send
packets from an eBPF/XDP program attached to the netdev.
For more details about AF_XDP, please see linux kernel's
Documentation/networking/af_xdp.rst


I'm glad to see some more revisions of this series!


I’m planning on reviewing and testing this patch, I’ll try to 
start it this week, or else when I get back from PTO.



AF_XDP is a faster way to access the existing kernel devices.  If we
take that point of view, then it would be ideal if AF_XDP were
automatically used when it was available, instead of adding a new
network device type.  Is there a reason that this point of view is
wrong?  That is, when AF_XDP is available, is there a reason not to 
use

it?


This needs support by all the ingress and egress ports in the system, 
and currently, there is no API to check this.


There are also features like traffic shaping that will not work. Maybe 
it will be worth adding the table for AF_XDP in 
http://docs.openvswitch.org/en/latest/faq/releases/


You said that your goal for the next version is to improve 
performance
and add optimizations.  Do you think that is important before we 
merge
the series?  We can continue to improve performance after it is 
merged.


The previous patch was rather unstable and I could not get it running 
with the PVP test without crashing. I think this patchset should get 
some proper testing and reviews by others. Especially for all the 
features being marked as supported in the above-mentioned table.



If we set performance aside, do you have a reason to want to wait to
merge this?  (I wasn't able to easily apply this series to current
master, so it'll need at least a rebase before we apply it.  And I 
have

only skimmed it, not fully reviewed it.)


Other than the items above, do we really need another datapath? With 
this, we use two or more cores for processing packets. If we poll two 
physical ports it could be 300%, which is a typical use case with 
bonding. What about multiple queue support, does it work? Both in 
kernel and DPDK mode we use multiple queues to distribute the load, 
with this scenario does it double the number of CPUs used? Can we use 
the poll() mode as explained here, 
https://linuxplumbersconf.org/event/2/contributions/99/, and how will 
it work with multiple queues/pmd threads? What about any latency 
tests, is it worse or better than kernel/dpdk? Also with the AF_XDP 
datapath, there is no to leverage hardware offload, like DPDK and TC. 
And then there is the part that it only works on the most recent 
kernels.


One other thing that popped up in my head is how (will) it work together 
with DPDK enabled on the same system?


To me looking at this I would say it’s far from being ready to be 
merged into OVS. However, if others decide to go ahead I think it 
should be disabled, not compiled in by default.



It might make sense to squash all of these into a single patch.  I am
not sure that they are really distinct conceptually.

___
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/3] dpif-netdev: only poll enabled vhost queues

[Ilya Maximets]

On 17.04.2019 11:35, David Marchand wrote:
> Hello Ilya,
> 
> 
> On Tue, Apr 16, 2019 at 3:41 PM Ilya Maximets  > wrote:
> 
> Hi.
> 
> One comment for the patch names. It's not a rule, but it's better
> to make the  part of the subject a complete sentence.
> i.e. start with a capital letter and end with a period.
> 
> Same rule is applicable to the comments in the code, but this is
> documented in coding-style.
> 
> 
> Another thing different from my dpdk habits.
> Need to focus when hacking ovs :-).
> 
> 
> On 16.04.2019 12:45, David Marchand wrote:
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 4d6d0c3..5bfa6ad 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -591,6 +591,8 @@ struct polled_queue {
> >      struct dp_netdev_rxq *rxq;
> >      odp_port_t port_no;
> >      bool emc_enabled;
> > +    bool enabled;
> 
> What do you think about renaming to 'rxq_enabled'? It seems more clear.
> Having both 'emc_enabled' and just 'enabled' is a bit confusing.
> 
> 
> Yes, this object is not a rxq itself, so a short enabled is confusing.
> Ok for rxq_enabled.
> 
>  
> 
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 47153dc..9ba8e67 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -424,6 +424,9 @@ struct netdev_dpdk {
> >          OVSRCU_TYPE(struct ingress_policer *) ingress_policer;
> >          uint32_t policer_rate;
> >          uint32_t policer_burst;
> > +
> > +        /* Array of vhost rxq states, see vring_state_changed */
> 
> Should end with a period.
> 
> 
> Yes.
> 
> 
> > +        bool *vhost_rxq_enabled;
> >      );
> > 
> >      PADDED_MEMBERS(CACHE_LINE_SIZE,
> > @@ -1235,8 +1238,14 @@ vhost_common_construct(struct netdev *netdev)
> >      int socket_id = rte_lcore_to_socket_id(rte_get_master_lcore());
> >      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> > 
> > +    dev->vhost_rxq_enabled = dpdk_rte_mzalloc(OVS_VHOST_MAX_QUEUE_NUM *
> > +                                              sizeof 
> *dev->vhost_rxq_enabled);
> > +    if (!dev->vhost_rxq_enabled) {
> > +        return ENOMEM;
> > +    }
> >      dev->tx_q = netdev_dpdk_alloc_txq(OVS_VHOST_MAX_QUEUE_NUM);
> >      if (!dev->tx_q) {
> > +        rte_free(dev->vhost_rxq_enabled);
> >          return ENOMEM;
> >      }
> > 
> > @@ -1448,6 +1457,7 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
> >      dev->vhost_id = NULL;
> > 
> >      common_destruct(dev);
> > +    rte_free(dev->vhost_rxq_enabled);
> 
> Logically, 'common_destruct' should go after class-specific things.
> 
> 
> Indeed.
> 
> 
> 
> > 
> >      ovs_mutex_unlock(_mutex);
> > 
> > @@ -2200,6 +2210,14 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq 
> *rxq,
> >      return 0;
> >  }
> > 
> > +static bool
> > +netdev_dpdk_vhost_rxq_enabled(struct netdev_rxq *rxq)
> > +{
> > +    struct netdev_dpdk *dev = netdev_dpdk_cast(rxq->netdev);
> > +
> > +    return dev->vhost_rxq_enabled[rxq->queue_id];
> > +}
> > +
> >  static int
> >  netdev_dpdk_rxq_recv(struct netdev_rxq *rxq, struct dp_packet_batch 
> *batch,
> >                       int *qfill)
> > @@ -3563,6 +3581,8 @@ destroy_device(int vid)
> >              ovs_mutex_lock(>mutex);
> >              dev->vhost_reconfigured = false;
> >              ovsrcu_index_set(>vid, -1);
> > +            memset(dev->vhost_rxq_enabled, 0,
> > +                   OVS_VHOST_MAX_QUEUE_NUM * sizeof 
> *dev->vhost_rxq_enabled);
> 
> We need to clear only first 'dev->up.n_rxq' queues.
> 
> 
> Would not hurt, but yes only clearing this part is required.
> 
> 
> 
> >              netdev_dpdk_txq_map_clear(dev);
> > 
> >              netdev_change_seq_changed(>up);
> > @@ -3597,24 +3617,30 @@ vring_state_changed(int vid, uint16_t queue_id, 
> int enable)
> >      struct netdev_dpdk *dev;
> >      bool exists = false;
> >      int qid = queue_id / VIRTIO_QNUM;
> > +    bool is_rx = (queue_id % VIRTIO_QNUM) == VIRTIO_TXQ;
> >      char ifname[IF_NAME_SZ];
> > 
> >      rte_vhost_get_ifname(vid, ifname, sizeof ifname);
> > 
> > -    if (queue_id % VIRTIO_QNUM == VIRTIO_TXQ) {
> > -        return 0;
> > -    }
> > -
> >      ovs_mutex_lock(_mutex);
> >      LIST_FOR_EACH (dev, list_node, _list) {
> >          ovs_mutex_lock(>mutex);
> >          if (nullable_string_is_equal(ifname, dev->vhost_id)) {
> > -            if (enable) {
> > -                dev->tx_q[qid].map = qid;
> > +            if (is_rx) {
> > +                bool enabled = dev->vhost_rxq_enabled[qid];
> 
> This is also confusing to have 'enable' and 'enabled' 

Re: [ovs-dev] [ovs-dev, v1] netdev-rte-offloads: Reassign vport netdev functions.

[0-day Robot]

Bleep bloop.  Greetings Ophir Munk, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


build:
/bin/sh ./libtool  --tag=CXX   --mode=compile g++ -std=gnu++11 -DHAVE_CONFIG_H 
-I.-I ./include -I ./include -I ./lib -I ./lib -g -O2 -MT 
include/openvswitch/cxxtest.lo -MD -MP -MF $depbase.Tpo -c -o 
include/openvswitch/cxxtest.lo include/openvswitch/cxxtest.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  g++ -std=gnu++11 -DHAVE_CONFIG_H -I. -I ./include -I 
./include -I ./lib -I ./lib -g -O2 -MT include/openvswitch/cxxtest.lo -MD -MP 
-MF include/openvswitch/.deps/cxxtest.Tpo -c include/openvswitch/cxxtest.cc -o 
include/openvswitch/cxxtest.o
/bin/sh ./libtool  --tag=CXX   --mode=link g++ -std=gnu++11  -g -O2 -o 
include/openvswitch/libcxxtest.la  include/openvswitch/cxxtest.lo  -lpthread 
-lrt -lm  -lunbound
libtool: link: ar cru include/openvswitch/.libs/libcxxtest.a  
include/openvswitch/cxxtest.o
libtool: link: ranlib include/openvswitch/.libs/libcxxtest.a
libtool: link: ( cd "include/openvswitch/.libs" && rm -f "libcxxtest.la" && ln 
-s "../libcxxtest.la" "libcxxtest.la" )
depbase=`echo utilities/ovs-appctl.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I.-I ./include -I ./include -I ./lib -I 
./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith 
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast 
-Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT utilities/ovs-appctl.o -MD -MP -MF $depbase.Tpo -c -o 
utilities/ovs-appctl.o utilities/ovs-appctl.c &&\
mv -f $depbase.Tpo $depbase.Po
/bin/sh ./libtool  --tag=CC   --mode=link gcc -std=gnu99 -Wstrict-prototypes 
-Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -o utilities/ovs-appctl utilities/ovs-appctl.o lib/libopenvswitch.la 
-lpthread -lrt -lm  -lunbound
libtool: link: gcc -std=gnu99 -Wstrict-prototypes -Wall -Wextra 
-Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum 
-Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes 
-Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers 
-fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -o utilities/ovs-appctl 
utilities/ovs-appctl.o  lib/.libs/libopenvswitch.a -lssl -lcrypto -lcap-ng 
-lpthread -lrt -lm -lunbound
depbase=`echo utilities/ovs-testcontroller.o | sed 
's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I.-I ./include -I ./include -I ./lib -I 
./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith 
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast 
-Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT utilities/ovs-testcontroller.o -MD -MP -MF $depbase.Tpo -c -o 
utilities/ovs-testcontroller.o utilities/ovs-testcontroller.c &&\
mv -f $depbase.Tpo $depbase.Po
/bin/sh ./libtool  --tag=CC   --mode=link gcc -std=gnu99 -Wstrict-prototypes 
-Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -o utilities/ovs-testcontroller utilities/ovs-testcontroller.o 
lib/libopenvswitch.la -lssl -lcrypto   -lpthread -lrt -lm  -lunbound
libtool: link: gcc -std=gnu99 -Wstrict-prototypes -Wall -Wextra 
-Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum 
-Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes 
-Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers 
-fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -o 
utilities/ovs-testcontroller utilities/ovs-testcontroller.o  
lib/.libs/libopenvswitch.a -lcap-ng -lssl -lcrypto -lpthread -lrt -lm -lunbound
lib/.libs/libopenvswitch.a(dpif-netdev.o): In function `dpif_netdev_port_add':
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/lib/dpif-netdev.c:1869: 
undefined reference to `netdev_rte_offloads_port_add'
collect2: error: ld returned 1 exit status
make[2]: *** [utilities/ovs-testcontroller] Error 1
make[2]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


Please check this out.  If you feel 

[ovs-dev] [PATCH] dpif-netdev: Update comment about flow installation race.

[Ilya Maximets]

Userspace datapath uses per-PMD flow tables/classifiers for a long
time. However, it was decided to keep this race window to not block
revalidators. Comment should be updated to reflect the current state.

Fixes: 1c1e46ed8457 ("dpif-netdev: Add per-pmd flow-table/classifier.")
Signed-off-by: Ilya Maximets 
---
 lib/dpif-netdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bd9718824..645981c38 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6535,8 +6535,7 @@ handle_packet_upcall(struct dp_netdev_pmd_thread *pmd,
  * could have already been installed since we last did the flow
  * lookup before upcall.  This could be solved by moving the
  * mutex lock outside the loop, but that's an awful long time
- * to be locking everyone out of making flow installs.  If we
- * move to a per-core classifier, it would be reasonable. */
+ * to be locking revalidators out of making flow modifications. */
 ovs_mutex_lock(>flow_mutex);
 netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
 if (OVS_LIKELY(!netdev_flow)) {
-- 
2.17.1

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

[ovs-dev] [PATCH v1] netdev-rte-offloads: Reserve mark space.

[Ophir Munk]

From: Roni Bar Yanai 

Reserve the first 64 intergers (0-63) for special processing.
For example, if a packet can't complete its procssesing in hardware,
we will make sure that the packet is marked with a special mark to
continue its processing in software.

Co-authored-by: Asaf Penso 
Signed-off-by: Asaf Penso 
Co-authored-by: Ophir Munk 
Signed-off-by: Ophir Munk 
Signed-off-by: Roni Bar Yanai 
---
 lib/dpif-netdev.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9a48038..ea0ace7 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2096,8 +2096,11 @@ dp_netdev_pmd_find_dpcls(struct dp_netdev_pmd_thread 
*pmd,
 return cls;
 }
 
-#define MAX_FLOW_MARK   (UINT32_MAX - 1)
-#define INVALID_FLOW_MARK   (UINT32_MAX)
+#define INVALID_FLOW_MARK(UINT32_MAX)
+#define MAX_FLOW_MARK(UINT32_MAX - 1)
+#define RESERVED_FLOW_MARK_SIZE  (64)
+#define MIN_FLOW_MARKRESERVED_FLOW_MARK_SIZE
+#define AVAILABLE_FLOW_MARK_SIZE (MAX_FLOW_MARK - MIN_FLOW_MARK + 1)
 
 struct megaflow_to_mark_data {
 const struct cmap_node node;
@@ -2123,7 +2126,8 @@ flow_mark_alloc(void)
 
 if (!flow_mark.pool) {
 /* Haven't initiated yet, do it here */
-flow_mark.pool = id_pool_create(0, MAX_FLOW_MARK);
+flow_mark.pool = id_pool_create(MIN_FLOW_MARK,
+AVAILABLE_FLOW_MARK_SIZE);
 }
 
 if (id_pool_alloc_id(flow_mark.pool, )) {
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH v2 3/3] netdev-dpdk: reset queue number for vhost devices on vm shutdown

[David Marchand]

On Tue, Apr 16, 2019 at 4:01 PM Ilya Maximets 
wrote:

> On 16.04.2019 12:45, David Marchand wrote:
> > Rather than poll all disabled queues and waste some memory for vms that
> > have been shutdown, we can reconfigure when receiving a destroy
> > connection notification from the vhost library.
> >
> > $ while true; do
> >   ovs-appctl dpif-netdev/pmd-rxq-show |awk '
> >   /port: / {
> > tot++;
> > if ($NF == "disabled") {
> >   dis++;
> > }
> >   }
> >   END {
> > print "total: " tot ", enabled: " (tot - dis)
> >   }'
> >   sleep 1
> > done
> >
> > total: 66, enabled: 66
> > total: 6, enabled: 2
> >
> > Note: this patch requires a fix for the vhost library submitted here:
> > http://patchwork.dpdk.org/patch/52680/
> >
> > Without it, this change will do nothing but have openvswitch complain
> > that the vhost device is unknown:
> >
> > dpdk|INFO|VHOST_CONFIG: vhost peer closed
> > dpdk|ERR|VHOST_CONFIG: (0) device not found.
> >
> > dpdk|INFO|VHOST_CONFIG: vhost peer closed
> > dpdk|ERR|VHOST_CONFIG: (1) device not found.
> >
> > Signed-off-by: David Marchand 
> > ---
> >  lib/netdev-dpdk.c | 35 ++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index fc554db..a7fae4f 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -186,12 +186,15 @@ static const struct rte_eth_conf port_conf = {
> >  static int new_device(int vid);
> >  static void destroy_device(int vid);
> >  static int vring_state_changed(int vid, uint16_t queue_id, int enable);
> > +static void destroy_connection(int vid);
> >  static const struct vhost_device_ops virtio_net_device_ops =
> >  {
> >  .new_device =  new_device,
> >  .destroy_device = destroy_device,
> >  .vring_state_changed = vring_state_changed,
> > -.features_changed = NULL
> > +.features_changed = NULL,
> > +.new_connection = NULL,
> > +.destroy_connection = destroy_connection,
> >  };
> >
> >  enum { DPDK_RING_SIZE = 256 };
> > @@ -3661,6 +3664,36 @@ vring_state_changed(int vid, uint16_t queue_id,
> int enable)
> >  return 0;
> >  }
> >
> > +static void
> > +destroy_connection(int vid)
> > +{
> > +struct netdev_dpdk *dev;
> > +char ifname[IF_NAME_SZ];
> > +
> > +rte_vhost_get_ifname(vid, ifname, sizeof ifname);
> > +
> > +ovs_mutex_lock(_mutex);
> > +/* Add device to the vhost port with the same name as that passed
> down. */
>
> This comment is a leftover from the 'new_device'. It could be dropped.
>

Yep.



> > +LIST_FOR_EACH (dev, list_node, _list) {
> > +ovs_mutex_lock(>mutex);
> > +if (nullable_string_is_equal(ifname, dev->vhost_id)) {
> > +uint32_t qp_num = NR_QUEUE;
> > +
>
> It might be good to check if device is destroyed here and log an error
> otherwise.
>

So that we have a log similar to what we have in destroy_device() ?


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

Re: [ovs-dev] [PATCH v2 2/3] netdev-dpdk: avoid reconfiguration on VIRTIO_NET_F_MQ changes

[David Marchand]

On Tue, Apr 16, 2019 at 3:51 PM Ilya Maximets 
wrote:

> On 16.04.2019 12:45, David Marchand wrote:
> > At the moment, a malicious guest might negotiate VIRTIO_NET_F_MQ and
> > !VIRTIO_NET_F_MQ in a loop which would be seen as qp_num going from 1 to
> > n and n to 1 continuously, triggering datapath reconfigurations at each
> > transition.
> >
> > Limit this by only reconfiguring on increased qp_num.
> > The previous patch reduced the observed cost of polling disabled queues,
> > so the only cost is memory.
> >
> > Signed-off-by: David Marchand 
> > ---
>
> It seems weird to ACK my own code. You may credit me in some other way
> like From/Suggested/Co-authored tags. Any of them is OK for me.
> Please, refer the
> Documentation/internals/contributing/submitting-patches.rst
> for details and additional constraints (like additional sign-offs).
> Same for the next patch.
>

I did not want to take all the glory for me :-)
The code is mainly yours, so Co-authored-by + SoB.


> BTW, It's completely unnecessary to send new versions of the patch-set in
> reply to the previous. This is specific to DPDK mail-list and not a
> common practice. Complicates reading the list and searching for the right
> patch versions.
>

Ok, so I won't chain the new version.


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

Re: [ovs-dev] [PATCH v2 1/3] dpif-netdev: only poll enabled vhost queues

[David Marchand]

Hello Ilya,


On Tue, Apr 16, 2019 at 3:41 PM Ilya Maximets 
wrote:

> Hi.
>
> One comment for the patch names. It's not a rule, but it's better
> to make the  part of the subject a complete sentence.
> i.e. start with a capital letter and end with a period.
>
> Same rule is applicable to the comments in the code, but this is
> documented in coding-style.
>

Another thing different from my dpdk habits.
Need to focus when hacking ovs :-).


On 16.04.2019 12:45, David Marchand wrote:
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 4d6d0c3..5bfa6ad 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -591,6 +591,8 @@ struct polled_queue {
> >  struct dp_netdev_rxq *rxq;
> >  odp_port_t port_no;
> >  bool emc_enabled;
> > +bool enabled;
>
> What do you think about renaming to 'rxq_enabled'? It seems more clear.
> Having both 'emc_enabled' and just 'enabled' is a bit confusing.
>

Yes, this object is not a rxq itself, so a short enabled is confusing.
Ok for rxq_enabled.



> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 47153dc..9ba8e67 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -424,6 +424,9 @@ struct netdev_dpdk {
> >  OVSRCU_TYPE(struct ingress_policer *) ingress_policer;
> >  uint32_t policer_rate;
> >  uint32_t policer_burst;
> > +
> > +/* Array of vhost rxq states, see vring_state_changed */
>
> Should end with a period.
>

Yes.


> > +bool *vhost_rxq_enabled;
> >  );
> >
> >  PADDED_MEMBERS(CACHE_LINE_SIZE,
> > @@ -1235,8 +1238,14 @@ vhost_common_construct(struct netdev *netdev)
> >  int socket_id = rte_lcore_to_socket_id(rte_get_master_lcore());
> >  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> >
> > +dev->vhost_rxq_enabled = dpdk_rte_mzalloc(OVS_VHOST_MAX_QUEUE_NUM *
> > +  sizeof
> *dev->vhost_rxq_enabled);
> > +if (!dev->vhost_rxq_enabled) {
> > +return ENOMEM;
> > +}
> >  dev->tx_q = netdev_dpdk_alloc_txq(OVS_VHOST_MAX_QUEUE_NUM);
> >  if (!dev->tx_q) {
> > +rte_free(dev->vhost_rxq_enabled);
> >  return ENOMEM;
> >  }
> >
> > @@ -1448,6 +1457,7 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
> >  dev->vhost_id = NULL;
> >
> >  common_destruct(dev);
> > +rte_free(dev->vhost_rxq_enabled);
>
> Logically, 'common_destruct' should go after class-specific things.
>

Indeed.



> >
> >  ovs_mutex_unlock(_mutex);
> >
> > @@ -2200,6 +2210,14 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
> >  return 0;
> >  }
> >
> > +static bool
> > +netdev_dpdk_vhost_rxq_enabled(struct netdev_rxq *rxq)
> > +{
> > +struct netdev_dpdk *dev = netdev_dpdk_cast(rxq->netdev);
> > +
> > +return dev->vhost_rxq_enabled[rxq->queue_id];
> > +}
> > +
> >  static int
> >  netdev_dpdk_rxq_recv(struct netdev_rxq *rxq, struct dp_packet_batch
> *batch,
> >   int *qfill)
> > @@ -3563,6 +3581,8 @@ destroy_device(int vid)
> >  ovs_mutex_lock(>mutex);
> >  dev->vhost_reconfigured = false;
> >  ovsrcu_index_set(>vid, -1);
> > +memset(dev->vhost_rxq_enabled, 0,
> > +   OVS_VHOST_MAX_QUEUE_NUM * sizeof
> *dev->vhost_rxq_enabled);
>
> We need to clear only first 'dev->up.n_rxq' queues.
>

Would not hurt, but yes only clearing this part is required.



> >  netdev_dpdk_txq_map_clear(dev);
> >
> >  netdev_change_seq_changed(>up);
> > @@ -3597,24 +3617,30 @@ vring_state_changed(int vid, uint16_t queue_id,
> int enable)
> >  struct netdev_dpdk *dev;
> >  bool exists = false;
> >  int qid = queue_id / VIRTIO_QNUM;
> > +bool is_rx = (queue_id % VIRTIO_QNUM) == VIRTIO_TXQ;
> >  char ifname[IF_NAME_SZ];
> >
> >  rte_vhost_get_ifname(vid, ifname, sizeof ifname);
> >
> > -if (queue_id % VIRTIO_QNUM == VIRTIO_TXQ) {
> > -return 0;
> > -}
> > -
> >  ovs_mutex_lock(_mutex);
> >  LIST_FOR_EACH (dev, list_node, _list) {
> >  ovs_mutex_lock(>mutex);
> >  if (nullable_string_is_equal(ifname, dev->vhost_id)) {
> > -if (enable) {
> > -dev->tx_q[qid].map = qid;
> > +if (is_rx) {
> > +bool enabled = dev->vhost_rxq_enabled[qid];
>
> This is also confusing to have 'enable' and 'enabled' in a same scope.
> What do you think about renaming 'enabled' --> 'old_state'?
>

Ok.


>
> > +
> > +dev->vhost_rxq_enabled[qid] = enable != 0;
> > +if (enabled != dev->vhost_rxq_enabled[qid]) {
> > +netdev_change_seq_changed(>up);
> > +}
>
>


> > diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
> > index fb0c27e..5faae0d 100644
> > --- a/lib/netdev-provider.h
> > +++ b/lib/netdev-provider.h
> > @@ -789,6 +789,11 @@ struct netdev_class {
> >  void (*rxq_destruct)(struct netdev_rxq *);
> >  

[ovs-dev] [PATCH v1] netdev-rte-offloads: Reassign vport netdev functions.

[Ophir Munk]

From: Roni Bar Yanai 

vport offloaded functions should have a different implementation for
kernel based OVS versus dpdk based OVS. Currently there is an unconditional
execution of a kernel based calls even if the vport was added by dpif-netdev
rather than by dpif-netlink. Before this commit and in case hw-offload=true
adding a netdev datapath vport resulted in an error since the vport kernel
based APIs were called. In practice the API returned immediately on a
get_ifindex() failure (see [1]), which caused no harm, but it is required
to avoid such scenarios in advance. In case of a netdev datapath vport flow
functions must be updated at runtime. This commit reassigns flow functions
to NULL when such a vport is added. It uses a duplicated vport class to
keep the original default kernel vport class intact. This enables using
vports in a mixed environment of kernel and netdev (userspace) bridges at
the same time.

[1]
ovs|2|netdev_tc_offloads(dp_netdev_flow_5)|ERR|flow_put: failed to
get ifindex for vxlan_sys_4789: No such device

Reviewed-by: Asaf Penso 
Co-authored-by: Ophir Munk 
Signed-off-by: Ophir Munk 
Signed-off-by: Roni Bar Yanai 
---
 lib/dpif-netdev.c |   2 +
 lib/netdev-rte-offloads.c |  47 +++
 lib/netdev-rte-offloads.h |   5 ++
 lib/netdev-vport.c| 211 --
 lib/netdev-vport.h|   4 +
 5 files changed, 188 insertions(+), 81 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4d6d0c3..9a48038 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -51,6 +51,7 @@
 #include "latch.h"
 #include "netdev.h"
 #include "netdev-provider.h"
+#include "netdev-rte-offloads.h"
 #include "netdev-vport.h"
 #include "netlink.h"
 #include "odp-execute.h"
@@ -1865,6 +1866,7 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev 
*netdev,
 if (!error) {
 *port_nop = port_no;
 error = do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no);
+netdev_rte_offloads_port_add(netdev);
 }
 ovs_mutex_unlock(>port_mutex);
 
diff --git a/lib/netdev-rte-offloads.c b/lib/netdev-rte-offloads.c
index e9ab086..85f01e5 100644
--- a/lib/netdev-rte-offloads.c
+++ b/lib/netdev-rte-offloads.c
@@ -22,6 +22,7 @@
 #include "cmap.h"
 #include "dpif-netdev.h"
 #include "netdev-provider.h"
+#include "netdev-vport.h"
 #include "openvswitch/match.h"
 #include "openvswitch/vlog.h"
 #include "packets.h"
@@ -731,3 +732,49 @@ netdev_rte_offloads_flow_del(struct netdev *netdev, const 
ovs_u128 *ufid,
 
 return netdev_rte_offloads_destroy_flow(netdev, ufid, rte_flow);
 }
+
+/*
+ * Vport netdev flow pointers are initialized by default to kernel calls.
+ * They should be nullified or be set to a valid netdev (userspace) calls.
+ */
+#define NULLIFY(f) (ndc->f = NULL)
+static void
+netdev_rte_offloads_vxlan_init(struct netdev *netdev)
+{
+/*
+ * Clone default function pointers some
+ * of which may be kernel flow pointers.
+ */
+struct netdev_class *ndc = netdev_vport_dup_class_once(netdev);
+if (!ndc) {
+VLOG_DBG("Vport netdev_class offload api cannot be updated.");
+return;
+}
+
+/* Override kernel flow pointers. */
+NULLIFY(flow_put);
+NULLIFY(flow_flush);
+NULLIFY(flow_dump_create);
+NULLIFY(flow_dump_destroy);
+NULLIFY(flow_dump_next);
+NULLIFY(flow_put);
+NULLIFY(flow_get);
+NULLIFY(flow_del);
+NULLIFY(init_flow_api);
+
+netdev_vport_update_class(netdev, ndc);
+}
+
+/*
+ * This function is called as part of adding a new dpif netdev port.
+ * In case of vport class of "vxlan" type we update it to match netdev
+ * datapath apis.
+ */
+void
+netdev_rte_offloads_port_add(struct netdev *netdev)
+{
+const char *type = netdev_get_type(netdev);
+if (!strcmp("vxlan", type)) {
+netdev_rte_offloads_vxlan_init(netdev);
+}
+}
diff --git a/lib/netdev-rte-offloads.h b/lib/netdev-rte-offloads.h
index 18c8a75..7fbe414 100644
--- a/lib/netdev-rte-offloads.h
+++ b/lib/netdev-rte-offloads.h
@@ -50,6 +50,11 @@ int netdev_rte_offloads_flow_put(struct netdev *netdev, 
struct match *match,
 int netdev_rte_offloads_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
  struct dpif_flow_stats *stats);
 
+/*
+ * Called by dpif netdev when a port is added
+ */
+void netdev_rte_offloads_port_add(struct netdev *netdev);
+
 #define DPDK_FLOW_OFFLOAD_API   \
 .flow_put = netdev_rte_offloads_flow_put,   \
 .flow_del = netdev_rte_offloads_flow_del
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 808a43f..e27bc5e 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -1131,90 +1131,95 @@ netdev_vport_get_ifindex(const struct netdev *netdev_)
 .get_tunnel_config = get_netdev_tunnel_config,  \
 .get_status = tunnel_get_status
 
+/* The name of the dpif_port should be short enough to accomodate adding
+ * a port number to the end if one is necessary. */

Re: [ovs-dev] [PATCH RFCv4 0/4] AF_XDP netdev support for OVS

[Eelco Chaudron]

On 16 Apr 2019, at 21:55, Ben Pfaff wrote:


On Mon, Apr 01, 2019 at 03:46:48PM -0700, William Tu wrote:

The patch series introduces AF_XDP support for OVS netdev.
AF_XDP is a new address family working together with eBPF.
In short, a socket with AF_XDP family can receive and send
packets from an eBPF/XDP program attached to the netdev.
For more details about AF_XDP, please see linux kernel's
Documentation/networking/af_xdp.rst


I'm glad to see some more revisions of this series!


I’m planning on reviewing and testing this patch, I’ll try to start 
it this week, or else when I get back from PTO.



AF_XDP is a faster way to access the existing kernel devices.  If we
take that point of view, then it would be ideal if AF_XDP were
automatically used when it was available, instead of adding a new
network device type.  Is there a reason that this point of view is
wrong?  That is, when AF_XDP is available, is there a reason not to 
use

it?


This needs support by all the ingress and egress ports in the system, 
and currently, there is no API to check this.


There are also features like traffic shaping that will not work. Maybe 
it will be worth adding the table for AF_XDP in 
http://docs.openvswitch.org/en/latest/faq/releases/



You said that your goal for the next version is to improve performance
and add optimizations.  Do you think that is important before we merge
the series?  We can continue to improve performance after it is 
merged.


The previous patch was rather unstable and I could not get it running 
with the PVP test without crashing. I think this patchset should get 
some proper testing and reviews by others. Especially for all the 
features being marked as supported in the above-mentioned table.



If we set performance aside, do you have a reason to want to wait to
merge this?  (I wasn't able to easily apply this series to current
master, so it'll need at least a rebase before we apply it.  And I 
have

only skimmed it, not fully reviewed it.)


Other than the items above, do we really need another datapath? With 
this, we use two or more cores for processing packets. If we poll two 
physical ports it could be 300%, which is a typical use case with 
bonding. What about multiple queue support, does it work? Both in kernel 
and DPDK mode we use multiple queues to distribute the load, with this 
scenario does it double the number of CPUs used? Can we use the poll() 
mode as explained here, 
https://linuxplumbersconf.org/event/2/contributions/99/, and how will it 
work with multiple queues/pmd threads? What about any latency tests, is 
it worse or better than kernel/dpdk? Also with the AF_XDP datapath, 
there is no to leverage hardware offload, like DPDK and TC. And then 
there is the part that it only works on the most recent kernels.


To me looking at this I would say it’s far from being ready to be 
merged into OVS. However, if others decide to go ahead I think it should 
be disabled, not compiled in by default.



It might make sense to squash all of these into a single patch.  I am
not sure that they are really distinct conceptually.

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

[ovs-dev] recherche médicale

[Association 1901]

  

  

  

  

  

  

  
  
  Vous 
  aussi tlchargez sur votre 
tlphonel'application 
  GRATUITE : . 
  S . A . U . 
  V 
  . Lifedestine  sauver des 
  vies
  
  Il 
  s'agit d'une application compose d'une 
  communaut de citoyens 
  volontaires  la disposition des SAMU et des 
  POMPIERS.Les secours 
  organiss mettent en moyenne 13 minutes 
  pour arriver, sachant que 
  chaqueminute sans massage 
cardiaque/dfibrillation c'est 
  10 % de survie en mois, notre objectif est de rduire 
ce 
  dlai en faisant intervenir des "Citoyens 
  Sauveteurs". 
  Form au nom, tout le 
  monde peut agir et devenir un"Citoyen Qauveteur" 
  tout simplement en tlchargeant notre 
application 
  GRATUITE. 
  

40 000  
60 000 victimes de mort subite par an en France soit 
environ 130 dcs par jour. 


1/3 
des victimes ont 
moins de 55 ans 


Reprsente presque 10% 
des dcs par an en France (10 fois 
plus 
que les accidents de la route) 

En France, un taux de survietrs 
faible :  peine 5% (contre 
20  40 % dans les pays anglo-saxons et 
scandinaves, 
ou par exemple plus de 70 %  Las Vegas) 

70% 
des arrts cardiaques se passent devant 
des tmoins 

80% 
des survivants le sont car un tmoin a 
proximit a agit trs rapidement 

Au-del de 
3 
minutes sans massage 
cardiaque, les lsions 
crbrales sont 
irrversibles


  

  

  

  

  


  

  

  

  


   

  Aprs 
  avoirtlcharg 
gratuitementnotre 
  application Smartphone, en cas d'arrt 
  cardiaque signal par les secours  
  moins de dix minutes de l o vous vous 
  trouvez, notre application vous prvient.
  Vous tes alors invit  
  intervenir sur place pour pratiquer un massage 
  cardiaqueou pour aller chercher le 
  dfibrillateur 
  le plus proche. 
  Une quipe de 
  Sapeurs-Pompiers et du Samu sont envoys 
simultanment vers la 
  victime. 
  
  


  

Nous esperons ne pas vous avoir 
importun avec ce mail, 
  si vous ne souhaitez plus recevoir de mails de notre part, 
  vous pouvez rpondre tout simplement  ce mail 
en mettant 
  NE PLUS dans l'objet. Merci de votre 
  comprhension