Re: [ovs-dev] [PATCH] OVN: Add NEWS for Policy-based routing

2019-04-16 Thread Ben Pfaff
On Wed, Apr 17, 2019 at 02:05:30AM +, Mary Manohar wrote:
> Signed-off-by: Mary Manohar 

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


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

2019-04-16 Thread Ben Pfaff
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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] OVN: Add NEWS for Policy-based routing

2019-04-16 Thread 0-day Robot
Bleep bloop.  Greetings Mary Manohar, 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:
ERROR: Author Mary Manohar  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Mary Manohar 
WARNING: Line is 80 characters long (recommended limit is 79)
#20 FILE: NEWS:34:
   policies on the logical router. New table(Logical_Router_Policy) added in

Lines checked: 27, Warnings: 2, Errors: 1


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] [ovs-dev, v3, 2/3] OVN ACL: Allow ct_mark and ct_label values to be set from register as well

2019-04-16 Thread 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 80 characters long (recommended limit is 79)
#128 FILE: ovn/lib/actions.c:767:
  ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_MARK), , );

WARNING: Line is 80 characters long (recommended limit is 79)
#145 FILE: ovn/lib/actions.c:782:
  ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_LABEL), >ct_label,

WARNING: Line is 94 characters long (recommended limit is 79)
#162 FILE: ovn/ovn-sb.xml:1183:
ct_commit(ct_mark=(value[/mask] OR 
regX));

WARNING: Line is 97 characters long (recommended limit is 79)
#163 FILE: ovn/ovn-sb.xml:1184:
ct_commit(ct_label=(value[/mask] OR 
xxregX));

WARNING: Line is 151 characters long (recommended limit is 79)
#164 FILE: ovn/ovn-sb.xml:1185:
ct_commit(ct_mark=(value[/mask] OR 
regX), ct_label=(value[/mask] OR 
xxregX));

WARNING: Line is 84 characters long (recommended limit is 79)
#172 FILE: ovn/ovn-sb.xml:1190:
ct_mark=value[/mask] OR xxregX 
and/or

WARNING: Line is 92 characters long (recommended limit is 79)
#173 FILE: ovn/ovn-sb.xml:1191:
ct_label=value[/mask] OR xxregX 
are supplied,

WARNING: Line is 83 characters long (recommended limit is 79)
#178 FILE: ovn/ovn-sb.xml:1193:
values indicated by value[/mask] or 32 bit/128 bit 
registers

WARNING: Line is 84 characters long (recommended limit is 79)
#179 FILE: ovn/ovn-sb.xml:1194:
on the connection tracking entry. ct_mark is a 32-bit 
field

WARNING: Line is 80 characters long (recommended limit is 79)
#180 FILE: ovn/ovn-sb.xml:1195:
and hence will read value only from a 32 bit register (reg0 - reg9).

WARNING: Line is 83 characters long (recommended limit is 79)
#181 FILE: ovn/ovn-sb.xml:1196:
ct_label is a 128-bit field and hence will read value 
only

WARNING: Line is 82 characters long (recommended limit is 79)
#182 FILE: ovn/ovn-sb.xml:1197:
from a 128 bit register (xxreg0 - xxreg1). The 
value[/mask]

Lines checked: 215, Warnings: 12, 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] [ovs-dev, v3, 1/3] OVN ACL: Replace the usage of ct_label with ct_mark

2019-04-16 Thread 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

WARNING: Line is 83 characters long (recommended limit is 79)
#308 FILE: ovn/northd/ovn-northd.c:4161:
  REGBIT_CONNTRACK_COMMIT" == 1", "ct_commit(ct_mark=0/1); 
next;");

WARNING: Line is 83 characters long (recommended limit is 79)
#311 FILE: ovn/northd/ovn-northd.c:4163:
  REGBIT_CONNTRACK_COMMIT" == 1", "ct_commit(ct_mark=0/1); 
next;");

Lines checked: 354, Warnings: 8, 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


[ovs-dev] [PATCH] OVN: Add NEWS for Policy-based routing

2019-04-16 Thread Mary Manohar
Signed-off-by: Mary Manohar 
---
 NEWS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/NEWS b/NEWS
index 5a215b2..f3233d2 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,9 @@ Post-v2.11.0
  * Select IPAM mac_prefix in a random manner if not provided by the user
  * Added the HA chassis group support.
  * Added 'external' logical port support.
+ * 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.
- New QoS type "linux-netem" on Linux.
 
 v2.11.0 - 19 Feb 2019
-- 
1.8.3.1

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


Re: [ovs-dev] [ovs-dev, v3, 3/4] L3 N-S support in ovn, do not replace router port mac on gateway chassis

2019-04-16 Thread 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 82 characters long (recommended limit is 79)
#66 FILE: ovn/controller/physical.c:291:
   /* If a router port's chassisredirect port is resident on this 
chassis,

Lines checked: 302, Warnings: 1, 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] [ovs-dev, v3, 2/4] L3 N-S support in ovn, advertise router port mac from gateway chassis

2019-04-16 Thread 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:
ERROR: C99 style comment
#57 FILE: ovn/controller/pinctrl.c:224:
#define GARP_DEF_REPEAT_INTERVAL_MS   (3 * 60 * 1000) // 3 mins

ERROR: C99 style comment
#126 FILE: ovn/controller/pinctrl.c:2574:
  // Router Port binding without ip and mac configured.

WARNING: Line is 82 characters long (recommended limit is 79)
#134 FILE: ovn/controller/pinctrl.c:2582:
   const char *lrp_name = smap_get(_rec->options, 
"distributed-port");

WARNING: Line is 85 characters long (recommended limit is 79)
#137 FILE: ovn/controller/pinctrl.c:2585:
   distributed_port = lport_lookup_by_name(sbrec_port_binding_by_name, 
lrp_name);

WARNING: Line is 95 characters long (recommended limit is 79)
#146 FILE: ovn/controller/pinctrl.c:2594:
   const char *network_type = smap_get(_port->datapath->external_ids, 
"network-type");

ERROR: C99 style comment
#148 FILE: ovn/controller/pinctrl.c:2596:
   // Advertise GARP only of logical switch is of type vlan.

ERROR: Improper whitespace around control block
#205 FILE: ovn/controller/pinctrl.c:2953:
   SSET_FOR_EACH(gw_port, local_l3gw_ports) {

WARNING: Line is 82 characters long (recommended limit is 79)
#225 FILE: ovn/controller/pinctrl.c:2973:
 cr_port = lport_lookup_by_name(sbrec_port_binding_by_name, 
cr_peer_name);

WARNING: Line is 81 characters long (recommended limit is 79)
#232 FILE: ovn/controller/pinctrl.c:2980:
 is_cr_resident = 
pinctrl_is_chassis_resident(sbrec_port_binding_by_name,

ERROR: C99 style comment
#241 FILE: ovn/controller/pinctrl.c:2989:
// Router Port binding without ip and mac configured.

Lines checked: 386, Warnings: 5, Errors: 5


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] [ovs-dev, v3, 1/4] L3 E-W Support in ovn, replace router port mac with chassis mac.

2019-04-16 Thread 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:
ERROR: C99 style comment
#196 FILE: ovn/controller/chassis.c:343:
// Return the first chassis mac.

WARNING: Line is 81 characters long (recommended limit is 79)
#311 FILE: ovn/controller/physical.c:238:
put_replace_router_port_mac_flows(const struct sbrec_port_binding 
*localnet_port,

WARNING: Line lacks whitespace around operator
#323 FILE: ovn/controller/physical.c:250:
int tag = localnet_port->tag?*localnet_port->tag:0;

ERROR: C99 style comment
#337 FILE: ovn/controller/physical.c:264:
// Get chassis mac

ERROR: C99 style comment
#366 FILE: ovn/controller/physical.c:293:
// Parsing of mac failed.

ERROR: C99 style comment
#373 FILE: ovn/controller/physical.c:300:
// Replace Mac flow

WARNING: Empty return followed by brace, consider omitting
#396 FILE: ovn/controller/physical.c:323:
}

WARNING: Line is 80 characters long (recommended limit is 79)
#408 FILE: ovn/controller/physical.c:796:
put_replace_router_port_mac_flows(binding, chassis, local_datapaths,

ERROR: C99 style comment
#455 FILE: ovn/northd/ovn-northd.c:508:
 // No value in network_type is taken as OVERLAY.

WARNING: Line is 88 characters long (recommended limit is 79)
#539 FILE: ovn/ovn-nb.ovsschema:33:
  "enum": ["set", ["overlay", 
"vlan"]]},

WARNING: Line is 81 characters long (recommended limit is 79)
#576 FILE: ovn/ovn-sb.xml:299:
  column="external_ids:ovn-chassis-mac-mappings"/> column of the 
Open_vSwitch

WARNING: Line is 91 characters long (recommended limit is 79)
WARNING: Line lacks whitespace around operator
#607 FILE: ovn/utilities/ovn-nbctl.c:575:
  ls-add [SWITCH] [TYPE]create a logical switch named SWITCH of TYPE vlan 
or overlay\n\

Lines checked: 1039, Warnings: 8, Errors: 5


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] [PATCH 1/5 V2] datapath: meter: Use struct_size() in kzalloc()

2019-04-16 Thread Gregory Rose


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.


Thanks,

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


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

2019-04-16 Thread 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/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": {
-- 
1.8.3.1

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


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

2019-04-16 Thread 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 | 77 +--
 ovn/ovn-sb.xml| 20 +++--
 tests/ovn.at  | 16 +++
 4 files changed, 99 insertions(+), 17 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..d8b86dc 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,36 @@ 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,
- >ct_label_mask);
+   if 

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

2019-04-16 Thread 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.

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 | 77 +++
 ovn/lib/logical-fields.c  |  3 ++
 ovn/northd/ovn-northd.8.xml   | 14 +++---
 ovn/northd/ovn-northd.c   | 48 +--
 ovn/ovn-nb.ovsschema  |  4 +-
 ovn/ovn-sb.xml| 20 
 tests/ovn.at  | 27 +--
 9 files changed, 147 insertions(+), 61 deletions(-)

-- 
1.8.3.1

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


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

2019-04-16 Thread 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   | 48 +++
 tests/ovn.at  | 11 +++
 5 files changed, 46 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 && 

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

2019-04-16 Thread Gregory Rose



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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v3 4/4] L3 N-S support in ovn, avoid chassis redirection as default for vlan backed networks

2019-04-16 Thread Ankur Sharma
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

This Series:
Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
backed distributed logical router.

This Patch:
a. Add is_chassis_redirect(cr-*) for all VLAN backed logical router
   attached logical switches. This check is done to ensure that
   all the communication with non ovn based endpoints, happens
   only through gateway chassis attached router ports.

b. Return traffic for N-S traffic need not go via redirect chassis
   for VLAN backed networks.
   In the absence of NATing (or any other service provided by a centralized 
chassis),
   we need not redirect the South to North traffic for non overlay traffic.

Signed-off-by: Ankur Sharma 
---
 ovn/northd/ovn-northd.c |  43 +++---
 tests/ovn.at| 204 
 2 files changed, 235 insertions(+), 12 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 411de8e..13260e6 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -6163,6 +6163,20 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
  * from different chassis. */
 ds_put_format(, " && is_chassis_resident(%s)",
   op->od->l3redirect_port->json_key);
+} else if (op->peer &&
+   op->peer->od->network_type == DP_NETWORK_VLAN) {
+
+   /* For a vlan backed router port, we will always have the
+* is_chassis_resident check. This is because there could be
+* vm/server on vlan network, but not on OVN chassis and could
+* end up arping for router port ip.
+*
+* This check works on the assumption that for OVN chassis VMs,
+* logical switch ARP responder will respond to ARP requests
+* for router port IP.
+*/
+   ds_put_format(, " && is_chassis_resident(\"cr-%s\")",
+ op->key);
 }
 
 ds_clear();
@@ -7184,18 +7198,23 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 300,
   REGBIT_DISTRIBUTED_NAT" == 1", "next;");
 
-/* For traffic with outport == l3dgw_port, if the
- * packet did not match any higher priority redirect
- * rule, then the traffic is redirected to the central
- * instance of the l3dgw_port. */
-ds_clear();
-ds_put_format(, "outport == %s",
-  od->l3dgw_port->json_key);
-ds_clear();
-ds_put_format(, "outport = %s; next;",
-  od->l3redirect_port->json_key);
-ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 50,
-  ds_cstr(), ds_cstr());
+   /* For VLAN backed networks, default match will not redirect to
+* chassis redirect port. */
+   if (od->l3dgw_port->peer &&
+   od->l3dgw_port->peer->od->network_type == DP_NETWORK_OVERLAY) {
+  /* For traffic with outport == l3dgw_port, if the
+   * packet did not match any higher priority redirect
+   * rule, then the traffic is redirected to the central
+   * instance of the l3dgw_port. */
+  ds_clear();
+  ds_put_format(, "outport == %s",
+od->l3dgw_port->json_key);
+  ds_clear();
+  ds_put_format(, "outport = %s; next;",
+od->l3redirect_port->json_key);
+  ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 50,
+ds_cstr(), ds_cstr());
+   }
 
 /* If the Ethernet destination has not been resolved,
  * redirect to the central instance of the l3dgw_port.
diff --git a/tests/ovn.at b/tests/ovn.at
index 8da145c..c0c5523 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -13808,6 +13808,7 @@ test_ip() {
 echo "-- OVN dump --"
 ovn-nbctl show
 ovn-sbctl show
+ovn-sbctl list port_binding
 
 echo "-- hv1 dump --"
 as hv1 ovs-vsctl show
@@ -13936,3 +13937,206 @@ AT_CHECK([as hv2 ovs-appctl fdb/show br-phys | grep 
00:00:01:01:02:07 | grep 100
 OVN_CLEANUP([hv1],[hv2])
 
 AT_CLEANUP
+
+
+AT_SETUP([ovn -- 2 HVs, 2 lports/HV, localnet ports, DVR N-S Ping])
+ovn_start
+
+# In this test cases we create 3 switches, all connected to same
+# physical network (through br-phys on each HV). LS1 and LS2 have
+# 1 VIF each. Each HV has 1 VIF port. The first digit
+# of VIF port name indicates the hypervisor it is bound to, e.g.
+# lp23 means VIF 3 on hv2.
+#
+# All the switches are connected to a logical router "router".

[ovs-dev] [PATCH v3 3/4] L3 N-S support in ovn, do not replace router port mac on gateway chassis

2019-04-16 Thread Ankur Sharma
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

This Series:
Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
backed distributed logical router.

This Patch:
a. Do not replace router port mac, if the corrsponding cr- port
   is resident on current chassis.

b. We do not need this, as gateway chassis is where we will advertise
   the router port mac.

Signed-off-by: Ankur Sharma 
---
 ovn/controller/physical.c |  26 ++--
 ovn/controller/pinctrl.c  |  43 +--
 ovn/controller/pinctrl.h  |   6 +++
 tests/ovn.at  | 103 +-
 4 files changed, 150 insertions(+), 28 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index c1ca655..327a12c 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -21,6 +21,7 @@
 #include "lflow.h"
 #include "lport.h"
 #include "chassis.h"
+#include "pinctrl.h"
 #include "lib/bundle.h"
 #include "openvswitch/poll-loop.h"
 #include "lib/uuid.h"
@@ -235,8 +236,12 @@ get_zone_ids(const struct sbrec_port_binding *binding,
 }
 
 static void
-put_replace_router_port_mac_flows(const struct sbrec_port_binding 
*localnet_port,
+put_replace_router_port_mac_flows(struct ovsdb_idl_index
+  *sbrec_port_binding_by_name,
+  const struct sbrec_port_binding
+  *localnet_port,
   const struct sbrec_chassis *chassis,
+  const struct sset *active_tunnels,
   const struct hmap *local_datapaths,
   struct ofpbuf *ofpacts_p,
   ofp_port_t ofport,
@@ -277,8 +282,21 @@ put_replace_router_port_mac_flows(const struct 
sbrec_port_binding *localnet_port
 char *err_str = NULL;
 struct match match;
 struct ofpact_mac *replace_mac;
+char *cr_peer_name = xasprintf("cr-%s", rport_binding->logical_port);
 
-/* Table 65, priority 150.
+
+if (pinctrl_is_chassis_resident(sbrec_port_binding_by_name,
+chassis, active_tunnels,
+cr_peer_name)) {
+   /* If a router port's chassisredirect port is resident on this 
chassis,
+* then we need not do mac replace. */
+   free(cr_peer_name);
+   continue;
+}
+
+free(cr_peer_name);
+
+   /* Table 65, priority 150.
  * ===
  *
  * Implements output to localnet port.
@@ -793,7 +811,9 @@ consider_port_binding(struct ovsdb_idl_index 
*sbrec_port_binding_by_name,
 , ofpacts_p);
 
 if (!strcmp(binding->type, "localnet")) {
-put_replace_router_port_mac_flows(binding, chassis, 
local_datapaths,
+put_replace_router_port_mac_flows(sbrec_port_binding_by_name,
+  binding, chassis,
+  active_tunnels, local_datapaths,
   ofpacts_p, ofport, flow_table);
 }
 
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 86e1e5b..77f6556 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -154,11 +154,6 @@ static void pinctrl_handle_put_mac_binding(const struct 
flow *md,
const struct flow *headers,
bool is_arp)
 OVS_REQUIRES(pinctrl_mutex);
-static bool
-pinctrl_is_chassis_resident(struct ovsdb_idl_index *sbrec_port_binding_by_name,
-const struct sbrec_chassis *chassis,
-const struct sset *active_tunnels,
-const char *port_name);
 
 static void init_put_mac_bindings(void);
 static void destroy_put_mac_bindings(void);
@@ -239,6 +234,25 @@ pinctrl_init(void)
 );
 }
 
+bool
+pinctrl_is_chassis_resident(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+const struct sbrec_chassis *chassis,
+const struct sset *active_tunnels,
+const char *port_name)
+{
+const struct sbrec_port_binding *pb
+= lport_lookup_by_name(sbrec_port_binding_by_name, port_name);
+if (!pb || !pb->chassis) {
+return false;
+}
+if (strcmp(pb->type, "chassisredirect")) {
+return pb->chassis == chassis;
+} else {
+return ha_chassis_group_is_active(pb->ha_chassis_group,
+  active_tunnels, chassis);
+}
+}
+
 static ovs_be32
 queue_msg(struct rconn *swconn, struct ofpbuf 

[ovs-dev] [PATCH v3 2/4] L3 N-S support in ovn, advertise router port mac from gateway chassis

2019-04-16 Thread Ankur Sharma
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

This Series:
Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
backed distributed logical router.

This Patch:
a. For N-S (No NAT) case, we will rely on gateway-chassis construct.
   i.e on a logical router, one router port will be assigned gateway
   chassis(s) and this router port becomes the gateway for north to south
   traffic.

b. This patch adds changes to advertise router port (cr-lrp-*) mac on its
   active gateway chassis, by sending GARP.

c. Additionally, patch adds support for sending the GARP series periodically.
   i.e router port mac will be advertised periodically
   (not just when it is realized on a chassis). This is needed because when
   we add NATing support, then south to north traffic will also go via
   gateway chassis.

Signed-off-by: Ankur Sharma 
---
 ovn/controller/pinctrl.c | 167 ---
 ovn/lib/ovn-util.c   |  31 +
 ovn/lib/ovn-util.h   |   6 ++
 3 files changed, 195 insertions(+), 9 deletions(-)

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 0e49d05..86e1e5b 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -154,6 +154,12 @@ static void pinctrl_handle_put_mac_binding(const struct 
flow *md,
const struct flow *headers,
bool is_arp)
 OVS_REQUIRES(pinctrl_mutex);
+static bool
+pinctrl_is_chassis_resident(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+const struct sbrec_chassis *chassis,
+const struct sset *active_tunnels,
+const char *port_name);
+
 static void init_put_mac_bindings(void);
 static void destroy_put_mac_bindings(void);
 static void run_put_mac_bindings(
@@ -215,6 +221,8 @@ static bool may_inject_pkts(void);
 COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
 COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
 
+#define GARP_DEF_REPEAT_INTERVAL_MS   (3 * 60 * 1000) // 3 mins
+
 void
 pinctrl_init(void)
 {
@@ -2471,6 +2479,8 @@ struct garp_data {
 int backoff; /* Backoff for the next announcement. */
 uint32_t dp_key; /* Datapath used to output this GARP. */
 uint32_t port_key;   /* Port to inject the GARP into. */
+bool is_repeat;  /* Send GARPs continously */
+long long int repeat_interval; /* Interval between GARP bursts in ms */
 };
 
 /* Contains GARPs to be sent. Protected by pinctrl_mutex*/
@@ -2491,7 +2501,8 @@ destroy_send_garps(void)
 /* Runs with in the main ovn-controller thread context. */
 static void
 add_garp(const char *name, const struct eth_addr ea, ovs_be32 ip,
- uint32_t dp_key, uint32_t port_key)
+ uint32_t dp_key, uint32_t port_key, bool is_repeat,
+ long long int repeat_interval)
 {
 struct garp_data *garp = xmalloc(sizeof *garp);
 garp->ea = ea;
@@ -2500,6 +2511,8 @@ add_garp(const char *name, const struct eth_addr ea, 
ovs_be32 ip,
 garp->backoff = 1;
 garp->dp_key = dp_key;
 garp->port_key = port_key;
+garp->is_repeat = is_repeat;
+garp->repeat_interval = repeat_interval;
 shash_add(_garp_data, name, garp);
 
 /* Notify pinctrl_handler so that it can wakeup and process
@@ -2509,7 +2522,8 @@ add_garp(const char *name, const struct eth_addr ea, 
ovs_be32 ip,
 
 /* Add or update a vif for which GARPs need to be announced. */
 static void
-send_garp_update(const struct sbrec_port_binding *binding_rec,
+send_garp_update(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+ const struct sbrec_port_binding *binding_rec,
  struct shash *nat_addresses)
 {
 volatile struct garp_data *garp = NULL;
@@ -2534,7 +2548,7 @@ send_garp_update(const struct sbrec_port_binding 
*binding_rec,
 add_garp(name, laddrs->ea,
  laddrs->ipv4_addrs[i].addr,
  binding_rec->datapath->tunnel_key,
- binding_rec->tunnel_key);
+ binding_rec->tunnel_key, false, 0);
 }
 free(name);
 }
@@ -2544,6 +2558,60 @@ send_garp_update(const struct sbrec_port_binding 
*binding_rec,
 return;
 }
 
+/* Update GARPs for local chassisredirect port, if the peer
+ * layer 2 switch is of type vlan.
+ */
+if (!strcmp(binding_rec->type, "chassisredirect")) {
+   struct eth_addr mac;
+   ovs_be32 ip, mask;
+   uint32_t dp_key = 0;
+   uint32_t port_key = 0;
+   const struct sbrec_port_binding *peer_port = NULL;
+   const struct sbrec_port_binding *distributed_port = NULL;
+
+   if (!ovn_sbrec_get_port_binding_ip_mac(binding_rec, ,

[ovs-dev] [PATCH v3 1/4] L3 E-W Support in ovn, replace router port mac with chassis mac.

2019-04-16 Thread Ankur Sharma
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

This Series:
Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
backed distributed logical router.

This Patch:

L2 :

a. Add 1 more column to the Logical_Switch table in north bound
   schema, to indicate if a logical switch is of type "vlan" or "overlay".
   We will support NULL value in this table as well, to keep it backward
   compatible. NULL value will be treated as "overlay" type by northd.

b. Column name:
   i. network_type: Represents whether network is vlan backed or
   overlay.

c. Update ovn-nbctl ls-add clis to take network type as optional
   input.
   ovn-nbctl ls-add [SWITCH] [TYPE]

d. Add a new ovn-nbctl command to set network type of a logical
   switch.
   ovn-nbctl ls-set-network-type SWITCH vlan|overlay

e. ovn-northd changes to get network_type from northbound database and
   populate the same in external_id column of southbound database as
   following key-value pair:
   network-type = [overlay|vlan]

f. Unit tests to validate above.

L3 :

a. Introduce per physical network chassic mac bindings in ovs.
   For example:
   
ovn-chassis-mac-mappings="physnet1:aa:bb:cc:dd:ee:ff,physnet2:a1:b2:c3:d4:e5:f6"

b. Replace router port mac with a chassis specific mac, by adding a flow
   in table=65, priority=150.
   For example:
   cookie=0x0, duration=67765.830s, table=65, n_packets=0, n_bytes=0,
   idle_age=65534, hard_age=65534, priority=150,reg15=0x1,metadata=0x4,
   dl_src=00:00:01:01:02:03 actions=mod_dl_src:aa:bb:cc:dd:ee:ff,
   mod_vlan_vid:1000,output:16

c. Unit tests to validate above.

Signed-off-by: Ankur Sharma 
---
 ovn/controller/binding.c|  12 +--
 ovn/controller/chassis.c|  66 +++-
 ovn/controller/chassis.h|   4 +
 ovn/controller/ovn-controller.8.xml |   7 ++
 ovn/controller/ovn-controller.c |   2 +-
 ovn/controller/ovn-controller.h |   5 +-
 ovn/controller/physical.c   |  96 ++
 ovn/northd/ovn-northd.c |  40 
 ovn/ovn-architecture.7.xml  |  12 +++
 ovn/ovn-nb.ovsschema|   8 +-
 ovn/ovn-nb.xml  |   9 ++
 ovn/ovn-sb.xml  |  15 +++
 ovn/utilities/ovn-nbctl.c   |  45 ++--
 tests/ovn-nbctl.at  |  48 ++---
 tests/ovn-northd.at |  22 
 tests/ovn.at| 197 
 16 files changed, 556 insertions(+), 32 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 404f0e7..e9461ef 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -159,13 +159,11 @@ add_local_datapath__(struct ovsdb_idl_index 
*sbrec_datapath_binding_by_key,
  sbrec_port_binding_by_name,
  peer->datapath, false,
  depth + 1, local_datapaths);
-ld->n_peer_dps++;
-ld->peer_dps = xrealloc(
-ld->peer_dps,
-ld->n_peer_dps * sizeof *ld->peer_dps);
-ld->peer_dps[ld->n_peer_dps - 1] = datapath_lookup_by_key(
-sbrec_datapath_binding_by_key,
-peer->datapath->tunnel_key);
+ld->n_peer_ports++;
+ld->peer_ports = xrealloc(ld->peer_ports,
+  ld->n_peer_ports *
+  sizeof *ld->peer_ports);
+ld->peer_ports[ld->n_peer_ports - 1] = peer;
 }
 }
 }
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 58d5d49..9730c7a 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -22,6 +22,7 @@
 #include "lib/vswitch-idl.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/ofp-parse.h"
 #include "ovn/lib/chassis-index.h"
 #include "ovn/lib/ovn-sb-idl.h"
 #include "ovn-controller.h"
@@ -68,6 +69,12 @@ get_bridge_mappings(const struct smap *ext_ids)
 }
 
 static const char *
+get_chassis_mac_mappings(const struct smap *ext_ids)
+{
+return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
+}
+
+static const char *
 get_cms_options(const struct smap *ext_ids)
 {
 return smap_get_def(ext_ids, "ovn-cms-options", "");
@@ -140,6 +147,7 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 const char *datapath_type =
 br_int && br_int->datapath_type ? br_int->datapath_type : "";
 const char *cms_options = get_cms_options(>external_ids);
+const char *chassis_macs = get_chassis_mac_mappings(>external_ids);
 
 struct ds iface_types = DS_EMPTY_INITIALIZER;
 

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

2019-04-16 Thread Ankur Sharma
This series is about enhancing the logical router functionality in OVN to work
with vlan backed logical switches.

Intial proposal was discused here:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

This series covers following:
a. L2:
   Associate a type with logical switches. Type value could be vlan or overlay.

b. L3 E-W:
   In the absence of encapsulation, we cannot use router port mac as source mac
   (since it is distributed), hence replace the same with a chassis unique mac.

c. L3 N-S:
   Use gateway-chassis construct to respond to ARP requests for router port,
   so that it becomes entry point for all chassis bound traffic coming from
   "external" network.
   Some additional changes, like no need to redirect south to north traffic
   in the absence of NAT etc.

This series does not cover following:
(will be sent out for review in follow up series once this series is 
reviewed/committed)
a. Network Address Translation.
b. Ensuring VMs mac is learnt in underlay network to avoid flooding of L3 flows.

Ankur Sharma (4):
  L3 E-W Support in ovn, replace router port mac with chassis mac.
  L3 N-S support in ovn, advertise router port mac from gateway chassis
  L3 N-S support in ovn, do not replace router port mac on gateway
chassis
  L3 N-S support in ovn, avoid chassis redirection as default for vlan
backed networks

 ovn/controller/binding.c|  12 +-
 ovn/controller/chassis.c|  66 -
 ovn/controller/chassis.h|   4 +
 ovn/controller/ovn-controller.8.xml |   7 +
 ovn/controller/ovn-controller.c |   2 +-
 ovn/controller/ovn-controller.h |   5 +-
 ovn/controller/physical.c   | 116 +
 ovn/controller/pinctrl.c| 200 --
 ovn/controller/pinctrl.h|   6 +
 ovn/lib/ovn-util.c  |  31 +++
 ovn/lib/ovn-util.h  |   6 +
 ovn/northd/ovn-northd.c |  83 +-
 ovn/ovn-architecture.7.xml  |  12 +
 ovn/ovn-nb.ovsschema|   8 +-
 ovn/ovn-nb.xml  |   9 +
 ovn/ovn-sb.xml  |  15 ++
 ovn/utilities/ovn-nbctl.c   |  45 +++-
 tests/ovn-nbctl.at  |  48 +++-
 tests/ovn-northd.at |  22 ++
 tests/ovn.at| 502 
 20 files changed, 1127 insertions(+), 72 deletions(-)

-- 
1.8.3.1

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


Re: [ovs-dev] [RFC] Introduce "OpenFlow Controller as Shared Library"

2019-04-16 Thread Ben Pfaff
On Sun, Mar 24, 2019 at 09:54:02PM -0700, Ansis Atteka wrote:
> From: Ansis Atteka 
> 
> Currently ovs-vswitchd process can communicate with an OpenFlow
> controller only through tcp, unix and ssl sockets.
> 
> This patch would allow ovs-vswitchd process to communicate
> with an OpenFlow controller by directly calling into its
> code that provides interface similar to a socket (ie
> implements read() and write() functions).
> 
> There are few benefits of using shared library as OpenFlow
> controller:
> 
> 1. Better performance by
>a) avoiding copying OpenFlow messages to socket buffers; AND
>b) reducing context switches.
>The preliminary tests that I did improved performance by ~30% for
>an OpenFlow controller that handles PACKET_INs and resubmits packets
>with PACKET_OUTs.
> 2. Better parallelization in future by distributing the load
>over ovs-vswitchd handler threads (currently only one thread calls into
>the shared library code).
> 3. Eliminate undeterministic thread blocking that may be caused when
>socket buffers are full.
> 4. In some cases better security (e.g. by allowing to confine ovs-vswitchd
>process to a stricter Access Control policy). Although, In some cases
>security may get worse (e.g. because controller would run in the same
>virtual memory space as ovs-vswitchd process).
> 
> While the code is enough to demonstrate PoC, I have left some TODOs.
> Because of that I am sending this code as RFC to hear more feedback
> from community on subjects as
> 1. what should be the API that shared libraries should export.
> 2. if I am possibly missing something critical that makes this approach
>not feasible (e.g. race conditions, something that ovs does behind
>scenes (like vlog module initialization for plugin) and would require
>overhaul in other components, impossible to integrate code with event
>loop, inpractical cleanups that would make it impossible
>to unload plugins)
> 
> In this patch I am proposing an API that requires library to export
> socket functions that mimic socket read() and write() functions.  In some
> cases this allows to easy retrofit existing OpenFlow controllers as
> shared library plugins.  To test out one can set test controller with
> 
> ovs-vsctl add-br brX
> ovs-vsctl set-controller brX dl:/ovs/tests/.libs/libtest-dl.so
> sudo ovs-ofctl add-flow brX "actions=controller"
> 
> And observe that packet-ins get to OpenFlow controller plugin:
> 
> 2019-03-25T04:10:35.615Z|01230|libtestdl|INFO|received 255 byte message from 
> Open vSwitch
> 2019-03-25T04:10:35.615Z|01231|libtestdl|INFO|Received OFPTYPE_PACKET_IN
> 2019-03-25T04:10:36.616Z|01232|libtestdl|INFO|received 255 byte message from 
> Open vSwitch
> 2019-03-25T04:10:36.616Z|01233|libtestdl|INFO|Received OFPTYPE_PACKET_IN
> 2019-03-25T04:10:38.143Z|01234|libtestdl|INFO|received 8 byte message from 
> Open vSwitch
> 
> Another approach would be for shared libraries to export init() and
> finit() functions that would self register and self-unregisters certain class
> implementations.
> 
> Signed-off-by: Ansis Atteka 

This approach seems pretty reasonable overall.  We've had people talk
about plugin interfaces before, but the assumption was always that it
would be essentially a new API with a broad set of definitions.  This
approach is much simpler, API-wise, since OpenFlow is still the core and
it's just a new transport.

You might be able to use a "seq" object (lib/seq.[ch]) for notification
between the controller and the main OVS code.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] rhel ovn: Remove ovn-common rpm

2019-04-16 Thread Ben Pfaff
On Tue, Apr 16, 2019 at 02:31:53PM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> ovn-fedora spec generates the rpms - ovn, ovn-common, ovn-host etc
> in which ovn is an empty package. The ovn fedora spec file here [1]
> has moved all the ovn-common files to the 'ovn' package.
> This patch does the same.
> 
> [1] - https://src.fedoraproject.org/rpms/ovn/blob/master/f/ovn.spec
> 
> Signed-off-by: Numan Siddique 
> CC: Timothy Redaelli 

I'm going to assume that this downstream patch makes sense upstream,
too, so I applied it to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 1/3] sat-math: Add functions for saturating arithmetic on "long long int".

2019-04-16 Thread Ben Pfaff
This series still needs a review.  It still applies cleanly and passes
all the tests.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 2.10 v2 0/2] Fix ovn-nbctl daemon table printing issues.

2019-04-16 Thread Ben Pfaff
On Tue, Feb 26, 2019 at 09:25:58AM -0500, Mark Michelson wrote:
> This is a backport for a fix present in 2.11. ovn-kubernetes encounters
> issues with the daemon mode of ovn-nbctl with 2.10.
> 
> ovn-nbctl when run in daemon mode has two issues:
> 1) An extra newline is prepended to table output
> 2) Table formatting issues are ignored.
> 
> This patch series fixes both issues.

Thanks!  I applied this series to branch-2.10.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 0/2] netdev-linux: fix update via netlink

2019-04-16 Thread Ben Pfaff
On Tue, Mar 26, 2019 at 02:14:58PM -0300, Flavio Leitner wrote:
> The first patch is actually an improvement to avoid reallocation.
> 
> The second patch is a bug fix.
> 
> More details in each patch.
> 
> Those patches need to be applied on master, branch-2.11 and
> branch-2.10.  The same patches apply on those branches, but
> let me know if you need a per branch formal submission.

Thanks.  I applied these patches to all three branches.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3] ovs-tcpdump: add dump_cmd checker before _doexec()

2019-04-16 Thread Ben Pfaff
On Fri, Mar 08, 2019 at 03:32:29PM +0800, txfh2007 via dev wrote:
> Hi Aaron:
> Thanks for your kinkdly suggession. I have tried your proposol and test 
> in my repository. This is the new version.
> 
> Add dump_cmd executable checker in ovs-tcpdump
> 
> Signed-off-by: Liu Chang 

Thanks, I applied this patch to the OVS master branch.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] ofp-monitor: Fixed the usage of 'usable_protocols' variable in 'parse_flow_monitor_request' function.

2019-04-16 Thread Ben Pfaff
On Fri, Mar 15, 2019 at 04:04:19PM -0700, Ashish Varma wrote:
> 'usable_protocols' is now getting set to OFPUTIL_P_OF10_ANY on return from
> 'parse_flow_monitor_request' function. The calling function now checks for the
> value in this variable against the 'allowed_protocols' variable.
> Also a check is added for a match field which is not supported in OpenFlow 1.0
> and return an error.
> Modified the man page of ovs-ofctl to reflect Flow Monitor support as
> OpenFlow 1.0 Nicira extension only.
> 
> Signed-off-by: Ashish Varma 

Thanks for the patch!  (And sorry it took so long to review.)

Would you mind adding tests that trigger the new error conditions?

Thanks,

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


Re: [ovs-dev] [PATCH] stream-ssl: Add support for TLS SNI (Server Name Indication).

2019-04-16 Thread Ben Pfaff
Thanks!  Applied to master.

On Tue, Mar 26, 2019 at 02:03:53PM -0700, Yifeng Sun wrote:
> Looks good to me, thanks.
> 
> Tested-by: Yifeng Sun 
> Reviewed-by: Yifeng Sun 
> 
> On Wed, Mar 20, 2019 at 5:39 PM Ben Pfaff  wrote:
> >
> > This TLS extension, introduced in RFC 3546, allows the server to know what
> > host the client believes it is contacting, the TLS equivalent of the Host:
> > header in HTTP.
> >
> > Requested-by: Shivaram Mysore 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  NEWS   |  2 ++
> >  lib/stream-ssl.c   | 63 ++
> >  m4/openvswitch.m4  | 23 ++---
> >  tests/atlocal.in   |  2 ++
> >  tests/ovs-vsctl.at | 20 +++
> >  5 files changed, 102 insertions(+), 8 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 1e4744dbd244..0f8d02817e61 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -25,6 +25,8 @@ Post-v2.11.0
> > - OVN:
> >   * Select IPAM mac_prefix in a random manner if not provided by the 
> > user
> > - New QoS type "linux-netem" on Linux.
> > +   - Added support for TLS Server Name Indication (SNI).
> > +
> >
> >  v2.11.0 - 19 Feb 2019
> >  -
> > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > index fed71801b823..81f2409965b2 100644
> > --- a/lib/stream-ssl.c
> > +++ b/lib/stream-ssl.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 
> > Nicira, Inc.
> > + * Copyright (c) 2008-2016, 2019 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -220,9 +220,19 @@ want_to_poll_events(int want)
> >  }
> >  }
> >
> > -/* Takes ownership of 'name'. */
> > +/* Creates a new SSL connection based on socket 'fd', as either a client 
> > or a
> > + * server according to 'type', initially in 'state'.  On success, returns 
> > 0 and
> > + * stores the new stream in '*streamp', otherwise returns an errno value 
> > and
> > + * doesn't bother with '*streamp'.
> > + *
> > + * Takes ownership of 'name', which should be the name of the connection 
> > in the
> > + * format that would be used to connect to it, e.g. "ssl:1.2.3.4:5".
> > + *
> > + * For client connections, 'server_name' should be the host name of the 
> > server
> > + * being connected to, for use with SSL SNI (server name indication).  
> > Takes
> > + * ownership of 'server_name'. */
> >  static int
> > -new_ssl_stream(char *name, int fd, enum session_type type,
> > +new_ssl_stream(char *name, char *server_name, int fd, enum session_type 
> > type,
> > enum ssl_state state, struct stream **streamp)
> >  {
> >  struct ssl_stream *sslv;
> > @@ -274,6 +284,14 @@ new_ssl_stream(char *name, int fd, enum session_type 
> > type,
> >  if (!verify_peer_cert || (bootstrap_ca_cert && type == CLIENT)) {
> >  SSL_set_verify(ssl, SSL_VERIFY_NONE, NULL);
> >  }
> > +#if OPENSSL_SUPPORTS_SNI
> > +if (server_name && !SSL_set_tlsext_host_name(ssl, server_name)) {
> > +VLOG_ERR("%s: failed to set server name indication (%s)",
> > + server_name, ERR_error_string(ERR_get_error(), NULL));
> > +retval = ENOPROTOOPT;
> > +goto error;
> > +}
> > +#endif
> >
> >  /* Create and return the ssl_stream. */
> >  sslv = xmalloc(sizeof *sslv);
> > @@ -293,6 +311,7 @@ new_ssl_stream(char *name, int fd, enum session_type 
> > type,
> >  }
> >
> >  *streamp = >stream;
> > +free(server_name);
> >  return 0;
> >
> >  error:
> > @@ -301,6 +320,7 @@ error:
> >  }
> >  closesocket(fd);
> >  free(name);
> > +free(server_name);
> >  return retval;
> >  }
> >
> > @@ -311,6 +331,30 @@ ssl_stream_cast(struct stream *stream)
> >  return CONTAINER_OF(stream, struct ssl_stream, stream);
> >  }
> >
> > +/* Extracts and returns the server name from 'suffix'.  The caller must
> > + * eventually free it.
> > + *
> > + * Returns NULL if there is no server name, and particularly if it is an IP
> > + * address rather than a host name, since RFC 3546 is explicit that IP
> > + * addresses are unsuitable as server name indication (SNI). */
> > +static char *
> > +get_server_name(const char *suffix_)
> > +{
> > +char *suffix = xstrdup(suffix_);
> > +
> > +char *host, *port;
> > +inet_parse_host_port_tokens(suffix, , );
> > +
> > +ovs_be32 ipv4;
> > +struct in6_addr ipv6;
> > +char *server_name = (ip_parse(host, ) || ipv6_parse(host, )
> > + ? NULL : xstrdup(host));
> > +
> > +free(suffix);
> > +
> > +return server_name;
> > +}
> > +
> >  static int
> >  ssl_open(const char *name, char *suffix, struct stream **streamp, uint8_t 
> > dscp)
> >  {
> > @@ -325,7 +369,8 @@ ssl_open(const char *name, char *suffix, struct stream 
> > **streamp, uint8_t dscp)
> >   dscp);
> >  if (fd >= 0) {
> > 

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

2019-04-16 Thread Ben Pfaff
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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCHv2] rhel: if rpms were built without libcapng then let processrs to run as root

2019-04-16 Thread Ansis Atteka
On Tue, 16 Apr 2019 at 12:36, Ben Pfaff  wrote:
>
> On Tue, Apr 16, 2019 at 12:27:59PM -0700, Ansis Atteka wrote:
> > 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 
>
> Acked-by: Ben Pfaff 

Thanks, I pushed it.
>
> s/processrs/processes/ in the subject.
>
> Aaron Conole's comments seem reasonable too but I think that they can be
> treated separately.

Agree about this. Will take a look.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] faq: Add information about git-pw.

2019-04-16 Thread Ben Pfaff
On Thu, Mar 28, 2019 at 05:32:43PM +0100, David Marchand wrote:
> On Thu, Mar 28, 2019 at 5:02 PM Kevin Traynor  wrote:
> 
> > git-pw is similar to pwclient but it can apply series directly.
> >
> > Signed-off-by: Kevin Traynor 
> > ---
> >
> > v2: fix block indent
> >
> >  Documentation/faq/contributing.rst | 19 +++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/Documentation/faq/contributing.rst
> > b/Documentation/faq/contributing.rst
> > index d8de9a49e..9f7ea121d 100644
> > --- a/Documentation/faq/contributing.rst
> > +++ b/Documentation/faq/contributing.rst
> > @@ -171,2 +171,21 @@ Q: How do I apply patches from email?
> > certain patches that contain form-feeds, due to a limitation of the
> > protocol
> > underlying ``pwclient``.)
> > +
> > +   Another way to apply patches directly from patchwork which supports
> > applying
> > +   patch series is to use the ``git-pw`` program. It can be obtained with
> > +   ``pip install git-pw``. Alternative installation instructions and
> > general
> > +   documentation can be found at
> > +   https://patchwork.readthedocs.io/projects/git-pw/en/latest/. You need
> > to
> > +   use your openvswitch patchwork login or create one at
> > +   https://patchwork.ozlabs.org/register/. The following can then be set
> > on
> > +   the command line with ``git config`` or through a ``.gitconfig`` like
> > this::
> > +
> > + [pw]
> > + server=https://patchwork.ozlabs.org/api/1.0
> > + project=openvswitch
> > + username=
> > + password=
> > +
> > +   Patch series can be listed with ``git-pw series list`` and applied with
> > +   ``git-pw series apply #``, where # is the series number. Individual
> > patches
> > +   can be applied with ``git-pw patch apply #``, where # is the patch
> > number.
> > --
> > 2.20.1
> >
> >
> I have been using git-pw for quite some time for ovs and dpdk.
> I personnally use an API token for auth, but apart from that it looks good.
> 
> Reviewed-by: David Marchand 

Thanks, Kevin (and David).  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] OVN: add the possibility to configure a static IPv4/IPv6 address and dynamic MAC

2019-04-16 Thread Ben Pfaff
On Fri, Mar 29, 2019 at 04:58:57PM +0100, Lorenzo Bianconi wrote:
> Add the possibility to configure a static IPv4 and/or IPv6 address
> and get MAC address dynamically allocated. This can be done using the
> following commands:
> 
> $ovn-nbctl ls-add sw0
> $ovn-nbctl set Logical-Switch sw0 other_config:subnet=192.168.0.0/24
> $ovn-nbctl set Logical-switch sw0 other_config:ipv6_prefix=2001::0
> $ovn-nbctl lsp-add sw0 lsp0 -- lsp-set-addresses lsp0 "dynamic 192.168.0.1 
> 2001::1"
> 
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v1:
> - rebase on top of current master branch
> - fix IPv6 address assignment

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


Re: [ovs-dev] [dpdk-latest PATCH v1] dpdk: Update by new color definitions.

2019-04-16 Thread Ian Stokes

On 4/16/2019 9:46 PM, Ophir Munk wrote:

Thanks Ian for accepting this. Please note a review comment from Asaf:
Typo in commit message: Follwoing --> Following



No problem, I had fixed that upon commit also.

Thanks
Ian


Regards,
Ophir


-Original Message-
From: Ian Stokes 
Sent: Tuesday, April 16, 2019 4:00 PM
To: Ophir Munk ; ovs-dev@openvswitch.org
Cc: Olga Shern ; Kevin Traynor
; Ilya Maximets ; Asaf
Penso ; Aaron Conole ;
Jeremy Plsek 
Subject: Re: [dpdk-latest PATCH v1] dpdk: Update by new color definitions.

On 4/16/2019 7:41 AM, Ophir Munk wrote:

Follwoing dpdk new color definitions (see [1]) 'e_RTE_METER_GREEN'
was replaced with 'RTE_COLOR_GREEN'.

[1]
Commit c1656328dbc2: ("meter: replace color definitions")

Signed-off-by: Ophir Munk 


Thanks for this Ophir, this fixes compilation with DPDK master so I've pushed
this to dpdk-latest with a slight modification to the subject line (dpdk: ->
netdev-dpdk).

Thanks
Ian


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


Re: [ovs-dev] [dpdk-latest PATCH v1] dpdk: Update by new color definitions.

2019-04-16 Thread Ophir Munk
Thanks Ian for accepting this. Please note a review comment from Asaf: 
Typo in commit message: Follwoing --> Following

Regards,
Ophir

> -Original Message-
> From: Ian Stokes 
> Sent: Tuesday, April 16, 2019 4:00 PM
> To: Ophir Munk ; ovs-dev@openvswitch.org
> Cc: Olga Shern ; Kevin Traynor
> ; Ilya Maximets ; Asaf
> Penso ; Aaron Conole ;
> Jeremy Plsek 
> Subject: Re: [dpdk-latest PATCH v1] dpdk: Update by new color definitions.
> 
> On 4/16/2019 7:41 AM, Ophir Munk wrote:
> > Follwoing dpdk new color definitions (see [1]) 'e_RTE_METER_GREEN'
> > was replaced with 'RTE_COLOR_GREEN'.
> >
> > [1]
> > Commit c1656328dbc2: ("meter: replace color definitions")
> >
> > Signed-off-by: Ophir Munk 
> 
> Thanks for this Ophir, this fixes compilation with DPDK master so I've pushed
> this to dpdk-latest with a slight modification to the subject line (dpdk: ->
> netdev-dpdk).
> 
> Thanks
> Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [patch v2] conntrack: Fix minimum connections to clean.

2019-04-16 Thread Ben Pfaff
On Fri, Mar 29, 2019 at 02:59:07PM -0700, Darrell Ball wrote:
> If there is low maximum connection count configuration and less than 10
> connections in a bucket, the calculation of the maximum number of
> connections to clean for the bucket could be zero, leading to these
> connections not being cleaned until and if the connection count in the
> bucket increases.
> 
> Fix this by checking for low maximum connection count configuration
> and do this outside of the buckets loop, thereby simplifying the loop.
> 
> Fixes: e6ef6cc6349b ("conntrack: Periodically delete expired connections.")
> CC: Daniele Di Proietto 
> Reported-by: Liujiaxin 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-March/357703.html
> Signed-off-by: Darrell Ball 
> ---
> 
> v2: Fix local variable naming.
> 
> Backport to 2.6.

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


[ovs-dev] Hallo

2019-04-16 Thread mtn
Hallo,
ANBEI IST DIE PROFITIEREN-BENACHRICHTIGUNG.
Mit freundlichen Grüssen
Mary Mkhululi



___
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

2019-04-16 Thread Ben Pfaff
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.
___
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

2019-04-16 Thread Ben Pfaff
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?

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.)

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

2019-04-16 Thread Ansis Atteka
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.
>
> >
> > 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...
> >
> > 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: rpmbuild -bb --without check rhel/openvswitch-dkms.spec
> > > +command: rpmbuild -bb --without check --without libcapng 
> > > rhel/openvswitch-dkms.spec
> > >  args:
> > >  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
> > >
> > > diff --git a/rhel/openvswitch-fedora.spec.in 
> > > b/rhel/openvswitch-fedora.spec.in
> > > index c1cd3f4c6..ce728b4f0 100644
> > > --- a/rhel/openvswitch-fedora.spec.in
> > > +++ b/rhel/openvswitch-fedora.spec.in
> > > @@ -350,6 +350,7 @@ rm -rf $RPM_BUILD_ROOT
> > >  %endif
> > >
> > >  %pre
> > > +%if %{with libcapng}
> > >  getent group openvswitch >/dev/null || groupadd -r openvswitch
> > >  getent passwd openvswitch >/dev/null || \
> > >  useradd -r -g openvswitch -d / -s /sbin/nologin \
> > > @@ -359,9 +360,11 @@ getent passwd openvswitch >/dev/null || \
> > >  getent group hugetlbfs >/dev/null || groupadd -r hugetlbfs
> > >  usermod -a -G hugetlbfs openvswitch
> > >  %endif
> > > +%endif
> > >  exit 0
> > >
> > >  %post
> > > 

Re: [ovs-dev] [PATCH] rhel: if rpms were built without libcapng then let processrs to run as root

2019-04-16 Thread Ansis Atteka
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.

>
> 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.

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...
>
> 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: rpmbuild -bb --without check rhel/openvswitch-dkms.spec
> > +command: rpmbuild -bb --without check --without libcapng 
> > rhel/openvswitch-dkms.spec
> >  args:
> >  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
> >
> > diff --git a/rhel/openvswitch-fedora.spec.in 
> > b/rhel/openvswitch-fedora.spec.in
> > index c1cd3f4c6..ce728b4f0 100644
> > --- a/rhel/openvswitch-fedora.spec.in
> > +++ b/rhel/openvswitch-fedora.spec.in
> > @@ -350,6 +350,7 @@ rm -rf $RPM_BUILD_ROOT
> >  %endif
> >
> >  %pre
> > +%if %{with libcapng}
> >  getent group openvswitch >/dev/null || groupadd -r openvswitch
> >  getent passwd openvswitch >/dev/null || \
> >  useradd -r -g openvswitch -d / -s /sbin/nologin \
> > @@ -359,9 +360,11 @@ getent passwd openvswitch >/dev/null || \
> >  getent group hugetlbfs >/dev/null || groupadd -r hugetlbfs
> >  usermod -a -G hugetlbfs openvswitch
> >  %endif
> > +%endif
> >  exit 0
> >
> >  %post
> > +%if %{with libcapng}
> >  if [ $1 -eq 1 ]; then
> >  sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
> >  sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' 
> > %{_sysconfdir}/logrotate.d/openvswitch
> > @@ -376,6 +379,7 @@ if [ $1 -eq 1 ]; then
> >  chown -R openvswitch:openvswitch 

Re: [ovs-dev] [PATCHv2] rhel: if rpms were built without libcapng then let processrs to run as root

2019-04-16 Thread Ben Pfaff
On Tue, Apr 16, 2019 at 12:27:59PM -0700, Ansis Atteka wrote:
> 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 

Acked-by: Ben Pfaff 

s/processrs/processes/ in the subject.

Aaron Conole's comments seem reasonable too but I think that they can be
treated separately.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] UiPath B2B Contact List

2019-04-16 Thread Lori Wright
Hi,
I was wondering if you are looking for below mentioned technologies.
*   Alteryx Analytics, Win shuttle, Pega Platform, Open Text MBPM, UFTpond and 
many more.
We also provide tech intelligence where you can connect to the decision makers 
from the company using your competitor's product.
Let me know if you have interest so that we can chat further.

Thanks,
Kristine Merritt

To opt out kindly reply back with 'unsubscribe' or 'leave out'

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


Re: [ovs-dev] [PATCH v4 2/2] chassis.c: Return chassis record whenever available in chassis_run().

2019-04-16 Thread Ben Pfaff
On Tue, Apr 16, 2019 at 11:42:04AM -0700, Han Zhou wrote:
> From: Han Zhou 
> 
> The ovn-controller main loop relies on the return value of chassis_run().
> When ovnsb_idl_txn is NULL (i.e. there is a pending transaction for SB),
> chasssis_run() returns NULL, which blocks functions to be executed in
> the main loop unnecessarily. This patch updates chassis_run() so that
> it returns chassis record whenever it is available.

Thanks, I applied this series to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCHv2] rhel: if rpms were built without libcapng then let processrs to run as root

2019-04-16 Thread Ansis Atteka
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 
---
 rhel/openvswitch-fedora.spec.in | 8 
 1 file changed, 8 insertions(+)

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index c1cd3f4c6..ce728b4f0 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -350,6 +350,7 @@ rm -rf $RPM_BUILD_ROOT
 %endif
 
 %pre
+%if %{with libcapng}
 getent group openvswitch >/dev/null || groupadd -r openvswitch
 getent passwd openvswitch >/dev/null || \
 useradd -r -g openvswitch -d / -s /sbin/nologin \
@@ -359,9 +360,11 @@ getent passwd openvswitch >/dev/null || \
 getent group hugetlbfs >/dev/null || groupadd -r hugetlbfs
 usermod -a -G hugetlbfs openvswitch
 %endif
+%endif
 exit 0
 
 %post
+%if %{with libcapng}
 if [ $1 -eq 1 ]; then
 sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
 sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' 
%{_sysconfdir}/logrotate.d/openvswitch
@@ -376,6 +379,7 @@ if [ $1 -eq 1 ]; then
 chown -R openvswitch:openvswitch /etc/openvswitch
 chown -R openvswitch:openvswitch /var/log/openvswitch
 fi
+%endif
 
 %if 0%{?systemd_post:1}
 %systemd_post %{name}.service
@@ -445,7 +449,11 @@ fi
 %endif
 
 %files
+%if %{with libcapng}
 %defattr(-,openvswitch,openvswitch)
+%else
+%defattr(-,root,root)
+%endif
 %dir %{_sysconfdir}/openvswitch
 %{_sysconfdir}/openvswitch/default.conf
 %config %ghost %{_sysconfdir}/openvswitch/conf.db
-- 
2.14.1

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


Re: [ovs-dev] [PATCH] datapath: Revert "datapath: Fix template leak in error cases."

2019-04-16 Thread Ben Pfaff
OK, I backported as far as 2.6.  On 2.5, the original patch hadn't been
applied, so there was nothing to revert.

On Tue, Apr 16, 2019 at 11:12:11AM -0700, Yifeng Sun wrote:
> The more appropriate patch (7664423) mentioned in the revert message
> was introduced in ovs-2.5.
> So I think this revert patch should be backported to 2.5.
> 
> Thanks,
> Yifeng
> 
> On Mon, Apr 15, 2019 at 4:08 PM Ben Pfaff  wrote:
> >
> > On Wed, Apr 03, 2019 at 03:34:34PM -0300, Flavio Leitner via dev wrote:
> > > On Wed, Apr 03, 2019 at 09:49:13AM -0700, Yifeng Sun wrote:
> > > > From: Flavio Leitner 
> > > >
> > > > Upstream commit:
> > > > commit 7f6d6558ae44bc193eb28df3617c364d3bb6df39
> > > > Author: Flavio Leitner 
> > > > Date:   Fri Sep 28 14:55:34 2018 -0300
> > > >
> > > > Revert "openvswitch: Fix template leak in error cases."
> > > > This reverts commit 90c7afc.
> > > >
> > > > When the commit was merged, the code used nf_ct_put() to free
> > > > the entry, but later on commit 7664423 ("openvswitch: Free
> > > > tmpl with tmpl_free.") replaced that with nf_ct_tmpl_free which
> > > > is a more appropriate. Now the original problem is removed.
> > > >
> > > > Then 44d6e2f ("net: Replace NF_CT_ASSERT() with WARN_ON().")
> > > > replaced a debug assert with a WARN_ON() which is trigged now.
> > > >
> > > > Signed-off-by: Flavio Leitner 
> > > > Acked-by: Joe Stringer 
> > > > Signed-off-by: David S. Miller 
> > > >
> > > > This patch backports this upstream patch to OVS.
> > > >
> > > > Cc: Flavio Leitner 
> > > > Signed-off-by: Yifeng Sun 
> > > > ---
> > >
> > > LGTM
> > > Acked-by: Flavio Leitner 
> >
> > I applied this to master.  I would appreciate advice on backporting.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3 1/2] ovn-controller: Fix busy loop when sb disconnected.

2019-04-16 Thread Han Zhou
On Tue, Apr 16, 2019 at 10:43 AM Ben Pfaff  wrote:
>
> On Wed, Apr 03, 2019 at 05:49:12PM -0700, Han Zhou wrote:
> > From: Han Zhou 
> >
> > In the main loop, if the SB DB is disconnected when there is a pending
> > transaction, there can be busy loop causing 100% CPU of ovn-controller,
> > until SB DB is connected again.
> >
> > The root cause is that when a transaction is pending,
ovsdb_idl_loop_run()
> > will return NULL for ovnsb_idl_txn, and chassis_run() returns NULL when
> > ovnsb_idl_txn is NULL, so the condition if (br_int && chassis) is not
> > satisfied and so ofctrl_run() is not executed in the main loop. If there
> > is any message pending from br-int.mgmt, such as OFPTYPE_BARRIER_REPLY
or
> > OFPTYPE_ECHO_REQUEST, the main loop will be woken up again and again
> > because those messages are not processed because ofctrl_run() is not
> > invoked.
> >
> > This patch fixes the problem by moving ofctrl_run() above and run it
> > whenever br_int is not NULL, and not care about chassis because this
> > function doesn't depend on it.
> >
> > It also moves out sbrec_chassis_set_nb_cfg() from the "if (ovs_idl_txn)"
> > just to avoid adding more indentation of the whole block to avoid >79
> > line length.
> >
> > Note: the changes of this patch is better to be shown with "-w" because
> > most of them are indent changes.
> >
> > Acked-by: Numan Siddique 
> > Signed-off-by: Han Zhou 
> > ---
> >
> > Notes:
> > v2->v3: fix >79 line found by 0-day robot
>
> This has a patch reject against current master, so if it's still
> relevant would you mind rebasing and reposting?
>
Hi Ben, I just rebased and sent out v4.

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


[ovs-dev] [PATCH v4 2/2] chassis.c: Return chassis record whenever available in chassis_run().

2019-04-16 Thread Han Zhou
From: Han Zhou 

The ovn-controller main loop relies on the return value of chassis_run().
When ovnsb_idl_txn is NULL (i.e. there is a pending transaction for SB),
chasssis_run() returns NULL, which blocks functions to be executed in
the main loop unnecessarily. This patch updates chassis_run() so that
it returns chassis record whenever it is available.

This changes allows xxx_run() functions being executed whenever
br_int and chassis are not NULL. For functions that need to update
SB DB, there are already additional checks making sure ovnsb_idl_txn
is not NULL.

Acked-by: Numan Siddique 
Acked-by: Mark Michelson 
Signed-off-by: Han Zhou 
---
v3->v4: rebase on master

 ovn/controller/chassis.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 3ea908d..58d5d49 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -73,8 +73,7 @@ get_cms_options(const struct smap *ext_ids)
 return smap_get_def(ext_ids, "ovn-cms-options", "");
 }
 
-/* Returns this chassis's Chassis record, if it is available and is currently
- * amenable to a transaction. */
+/* Returns this chassis's Chassis record, if it is available. */
 const struct sbrec_chassis *
 chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 struct ovsdb_idl_index *sbrec_chassis_by_name,
@@ -82,8 +81,10 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 const char *chassis_id,
 const struct ovsrec_bridge *br_int)
 {
+const struct sbrec_chassis *chassis_rec
+= chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
 if (!ovnsb_idl_txn) {
-return NULL;
+return chassis_rec;
 }
 
 const struct ovsrec_open_vswitch *cfg;
@@ -148,8 +149,6 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 ds_chomp(_types, ',');
 const char *iface_types_str = ds_cstr(_types);
 
-const struct sbrec_chassis *chassis_rec
-= chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
 const char *encap_csum = smap_get_def(>external_ids,
   "ovn-encap-csum", "true");
 int n_encaps = count_1bits(req_tunnels);
-- 
2.1.0

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


[ovs-dev] [PATCH v4 1/2] ovn-controller: Fix busy loop when sb disconnected.

2019-04-16 Thread Han Zhou
From: Han Zhou 

In the main loop, if the SB DB is disconnected when there is a pending
transaction, there can be busy loop causing 100% CPU of ovn-controller,
until SB DB is connected again.

The root cause is that when a transaction is pending, ovsdb_idl_loop_run()
will return NULL for ovnsb_idl_txn, and chassis_run() returns NULL when
ovnsb_idl_txn is NULL, so the condition if (br_int && chassis) is not
satisfied and so ofctrl_run() is not executed in the main loop. If there
is any message pending from br-int.mgmt, such as OFPTYPE_BARRIER_REPLY or
OFPTYPE_ECHO_REQUEST, the main loop will be woken up again and again
because those messages are not processed because ofctrl_run() is not
invoked.

This patch fixes the problem by moving ofctrl_run() above and run it
whenever br_int is not NULL, and not care about chassis because this
function doesn't depend on it.

It also moves out sbrec_chassis_set_nb_cfg() from the "if (ovs_idl_txn)"
just to avoid adding more indentation of the whole block to avoid >79
line length.

Note: the changes of this patch is better to be shown with "-w" because
most of them are indent changes.

Acked-by: Numan Siddique 
Acked-by: Mark Michelson 
Signed-off-by: Han Zhou 
---
v3->v4: rebase on master

 ovn/controller/ovn-controller.c | 97 ++---
 1 file changed, 51 insertions(+), 46 deletions(-)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index ad626f6..cf4907d 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -722,38 +722,40 @@ main(int argc, char *argv[])
 _tunnels, _datapaths,
 _lports, _lport_ids);
 }
-if (br_int && chassis) {
-struct shash addr_sets = SHASH_INITIALIZER(_sets);
-addr_sets_init(sbrec_address_set_table_get(ovnsb_idl_loop.idl),
-   _sets);
-struct shash port_groups = SHASH_INITIALIZER(_groups);
-port_groups_init(
-sbrec_port_group_table_get(ovnsb_idl_loop.idl),
-_groups);
-
-patch_run(ovs_idl_txn,
-  ovsrec_bridge_table_get(ovs_idl_loop.idl),
-  ovsrec_open_vswitch_table_get(ovs_idl_loop.idl),
-  ovsrec_port_table_get(ovs_idl_loop.idl),
-  sbrec_port_binding_table_get(ovnsb_idl_loop.idl),
-  br_int, chassis);
 
+if (br_int) {
 enum mf_field_id mff_ovn_geneve = ofctrl_run(
 br_int, _ct_zones);
 
-pinctrl_run(ovnsb_idl_txn,
-sbrec_datapath_binding_by_key,
-sbrec_port_binding_by_datapath,
-sbrec_port_binding_by_key,
-sbrec_port_binding_by_name,
-sbrec_mac_binding_by_lport_ip,
-sbrec_dns_table_get(ovnsb_idl_loop.idl),
-br_int, chassis,
-_datapaths, _tunnels);
-update_ct_zones(_lports, _datapaths, _zones,
-ct_zone_bitmap, _ct_zones);
-if (ovs_idl_txn) {
-if (ofctrl_can_put()) {
+if (chassis) {
+struct shash addr_sets = SHASH_INITIALIZER(_sets);
+addr_sets_init(
+sbrec_address_set_table_get(ovnsb_idl_loop.idl),
+_sets);
+struct shash port_groups = SHASH_INITIALIZER(_groups);
+port_groups_init(
+sbrec_port_group_table_get(ovnsb_idl_loop.idl),
+_groups);
+
+patch_run(ovs_idl_txn,
+  ovsrec_bridge_table_get(ovs_idl_loop.idl),
+  ovsrec_open_vswitch_table_get(ovs_idl_loop.idl),
+  ovsrec_port_table_get(ovs_idl_loop.idl),
+  sbrec_port_binding_table_get(ovnsb_idl_loop.idl),
+  br_int, chassis);
+
+pinctrl_run(ovnsb_idl_txn,
+sbrec_datapath_binding_by_key,
+sbrec_port_binding_by_datapath,
+sbrec_port_binding_by_key,
+sbrec_port_binding_by_name,
+sbrec_mac_binding_by_lport_ip,
+sbrec_dns_table_get(ovnsb_idl_loop.idl),
+br_int, chassis,
+_datapaths, _tunnels);
+update_ct_zones(_lports, _datapaths, _zones,
+ct_zone_bitmap, _ct_zones);
+if (ovs_idl_txn && ofctrl_can_put()) {

Re: [ovs-dev] [PATCHv2] datapath: Add support for kernel 4.19.x and 4.20.x

2019-04-16 Thread Yifeng Sun
Thanks Yi-Hung! I will look at it and come up with a new version.
Yifeng

On Mon, Apr 15, 2019 at 5:39 PM Yi-Hung Wei  wrote:
>
> On Fri, Apr 12, 2019 at 10:23 AM Yifeng Sun  wrote:
> >
> > This patch introduces changes needed by OVS to support latest
> > Linux kernels (4.19.x and 4.20.x). Recent kernels changed many
> > APIs that are being used by OVS. One major change is that
> > struct nf_conntrack_l3proto became invisible outside of kernel, so
> > get_l4proto function is added in file compact/nf_conntrack_core.c to
> > accommodate this issue.
> >
> > In addition, if kernel is not compiled with CONFIG_NF_NAT_IPV4
> > or CONFIG_NF_NAT_IPV6, flow action 'ct(nat)' can cause kernel
> > to crash. This patch handles this condition.
> >
> > This patch doesn't introduce new failed tests when running
> > 'make check-kmod' for kernels listed below:
> > 3.10.0-957.5.1.el7.x86_64
> > 4.4.0-142-generic
> > 4.17.14
> > 4.18.0-16-generic
> > 4.19.34
> > 4.20.17
> >
> > Travis passed at
> > https://travis-ci.org/yifsun/ovs-travis/builds/519011670
> >
> > Signed-off-by: Yifeng Sun 
> > v1->v2: Fixed the CONFIG_NF_NAT_IPV4 bug by using Greg's config
> > file. Thanks Greg!
> > ---
> Hi Yifeng,
>
> Thanks for the patch.
>
> I think this patch mixes a couple upstream patches backport, 4.19,
> 4.20 compilation issues, and the nf_nat issue together so that it may
> be hard to keep track of the kernel backport.  IMHO, it would be
> easier to break this patch down to a couple of them, so that it would
> be easier to maintain and review. My detailed comments are as below.
>
> > diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> > index 52208bad3029..ce36a8ddea50 100644
> > --- a/datapath/conntrack.c
> > +++ b/datapath/conntrack.c
> > @@ -38,6 +38,10 @@
> >  #include 
> >  #endif
> >
> > +#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6) && defined(HAVE_IPV6_FRAG_H)
> > +#include 
> > +#endif
> > +
> I think this is related to an upstream change 70b095c843266 ("ipv6:
> remove dependency of nf_defrag_ipv6 on ipv6 module"). Should we split
> this out in anther patch? We may be able to hide the following in the
> compat layer.
> +#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6) && defined(HAVE_IPV6_FRAG_H)
> +#endif
>
>
> >  #include "datapath.h"
> >  #include "conntrack.h"
> >  #include "flow.h"
> > @@ -645,32 +649,62 @@ static struct nf_conn *
> >  ovs_ct_find_existing(struct net *net, const struct nf_conntrack_zone *zone,
> >  u8 l3num, struct sk_buff *skb, bool natted)
> >  {
> > -   const struct nf_conntrack_l3proto *l3proto;
> > const struct nf_conntrack_l4proto *l4proto;
> > struct nf_conntrack_tuple tuple;
> > struct nf_conntrack_tuple_hash *h;
> > struct nf_conn *ct;
> > -   unsigned int dataoff;
> > u8 protonum;
> >
> > +#ifdef HAVE_NF_CT_INVERT_TUPLE_TAKES_L3PROTO
> > +   const struct nf_conntrack_l3proto *l3proto;
> > +   unsigned int dataoff;
> > +
> > l3proto = __nf_ct_l3proto_find(l3num);
> > if (l3proto->get_l4proto(skb, skb_network_offset(skb), ,
> >  ) <= 0) {
> > pr_debug("ovs_ct_find_existing: Can't get protonum\n");
> > return NULL;
> > }
> > +#else
> > +   int protooff;
> > +
> > +   protooff = get_l4proto(skb, skb_network_offset(skb),
> > +  l3num, );
> > +   if (protooff <= 0) {
> > +   pr_warn("ovs_ct_find_existing: Can't get protonum\n");
> > +   return NULL;
> > +   }
> > +#endif
> > +
> > +#ifdef HAVE_NF_CT_L4PROTO_FIND_TAKES_L3PROTO
> > l4proto = __nf_ct_l4proto_find(l3num, protonum);
> > -   if (!nf_ct_get_tuple(skb, skb_network_offset(skb), dataoff, l3num,
> > -protonum, net, , l3proto, l4proto)) {
> > +#else
> > +   l4proto = __nf_ct_l4proto_find(protonum);
> > +#endif
> > +
> > +#ifdef HAVE_NF_CT_GET_TUPLE
> > +   if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
> > +  l3num, net, )) {
> > +   pr_debug("ovs_ct_find_existing: Can't get tuple\n");
> > +   return NULL;
> > +   }
> > +#else
> > +   if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
> > +  l3num, net, )) {
> > pr_debug("ovs_ct_find_existing: Can't get tuple\n");
> > return NULL;
> > }
> > +#endif
> >
> > /* Must invert the tuple if skb has been transformed by NAT. */
> > if (natted) {
> > struct nf_conntrack_tuple inverse;
> >
> > +#ifdef HAVE_NF_CT_INVERT_TUPLE_TAKES_L3PROTO
> > if (!nf_ct_invert_tuple(, , l3proto, 
> > l4proto)) {
> > +#else
> > +   if (!nf_ct_invert_tuple(, , l4proto)) {
> > +#endif
> > pr_debug("ovs_ct_find_existing: Inversion 
> > failed!\n");
> > return NULL;
> > 

Re: [ovs-dev] [PATCH] datapath: Revert "datapath: Fix template leak in error cases."

2019-04-16 Thread Yifeng Sun
The more appropriate patch (7664423) mentioned in the revert message
was introduced in ovs-2.5.
So I think this revert patch should be backported to 2.5.

Thanks,
Yifeng

On Mon, Apr 15, 2019 at 4:08 PM Ben Pfaff  wrote:
>
> On Wed, Apr 03, 2019 at 03:34:34PM -0300, Flavio Leitner via dev wrote:
> > On Wed, Apr 03, 2019 at 09:49:13AM -0700, Yifeng Sun wrote:
> > > From: Flavio Leitner 
> > >
> > > Upstream commit:
> > > commit 7f6d6558ae44bc193eb28df3617c364d3bb6df39
> > > Author: Flavio Leitner 
> > > Date:   Fri Sep 28 14:55:34 2018 -0300
> > >
> > > Revert "openvswitch: Fix template leak in error cases."
> > > This reverts commit 90c7afc.
> > >
> > > When the commit was merged, the code used nf_ct_put() to free
> > > the entry, but later on commit 7664423 ("openvswitch: Free
> > > tmpl with tmpl_free.") replaced that with nf_ct_tmpl_free which
> > > is a more appropriate. Now the original problem is removed.
> > >
> > > Then 44d6e2f ("net: Replace NF_CT_ASSERT() with WARN_ON().")
> > > replaced a debug assert with a WARN_ON() which is trigged now.
> > >
> > > Signed-off-by: Flavio Leitner 
> > > Acked-by: Joe Stringer 
> > > Signed-off-by: David S. Miller 
> > >
> > > This patch backports this upstream patch to OVS.
> > >
> > > Cc: Flavio Leitner 
> > > Signed-off-by: Yifeng Sun 
> > > ---
> >
> > LGTM
> > Acked-by: Flavio Leitner 
>
> I applied this to master.  I would appreciate advice on backporting.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v4] Policy-based routing (PBR) in OVN.

2019-04-16 Thread Ben Pfaff
On Wed, Apr 03, 2019 at 11:27:56PM +, Mary Manohar wrote:
> PBR provides a mechanism to configure permit/deny and reroute policies on the
> router. Permit/deny policies are similar to OVN ACLs, but exist on the
> logical-router. Reroute policies are needed for service-insertion and
> service-chaining. Currently, policies are stateless.

Thanks, I applied this to master.

I'd appreciate it if you'd submit a followup patch that adds a NEWS item
so that users know about the new feature.
___
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

2019-04-16 Thread Aaron Conole
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.

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?

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: rpmbuild -bb --without check rhel/openvswitch-dkms.spec
> +command: rpmbuild -bb --without check --without libcapng 
> rhel/openvswitch-dkms.spec
>  args:
>  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
>  
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index c1cd3f4c6..ce728b4f0 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -350,6 +350,7 @@ rm -rf $RPM_BUILD_ROOT
>  %endif
>  
>  %pre
> +%if %{with libcapng}
>  getent group openvswitch >/dev/null || groupadd -r openvswitch
>  getent passwd openvswitch >/dev/null || \
>  useradd -r -g openvswitch -d / -s /sbin/nologin \
> @@ -359,9 +360,11 @@ getent passwd openvswitch >/dev/null || \
>  getent group hugetlbfs >/dev/null || groupadd -r hugetlbfs
>  usermod -a -G hugetlbfs openvswitch
>  %endif
> +%endif
>  exit 0
>  
>  %post
> +%if %{with libcapng}
>  if [ $1 -eq 1 ]; then
>  sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
>  sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' 
> %{_sysconfdir}/logrotate.d/openvswitch
> @@ -376,6 +379,7 @@ if [ $1 -eq 1 ]; then
>  chown -R openvswitch:openvswitch /etc/openvswitch
>  chown -R openvswitch:openvswitch /var/log/openvswitch
>  fi
> +%endif
>  
>  %if 0%{?systemd_post:1}
>  %systemd_post %{name}.service
> @@ -445,7 +449,11 @@ fi
>  %endif
>  
>  %files
> +%if %{with libcapng}
>  %defattr(-,openvswitch,openvswitch)
> +%else
> +%defattr(-,root,root)
> +%endif
>  %dir %{_sysconfdir}/openvswitch
>  %{_sysconfdir}/openvswitch/default.conf
>  %config %ghost %{_sysconfdir}/openvswitch/conf.db
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3 1/2] ovn-controller: Fix busy loop when sb disconnected.

2019-04-16 Thread Ben Pfaff
On Wed, Apr 03, 2019 at 05:49:12PM -0700, Han Zhou wrote:
> From: Han Zhou 
> 
> In the main loop, if the SB DB is disconnected when there is a pending
> transaction, there can be busy loop causing 100% CPU of ovn-controller,
> until SB DB is connected again.
> 
> The root cause is that when a transaction is pending, ovsdb_idl_loop_run()
> will return NULL for ovnsb_idl_txn, and chassis_run() returns NULL when
> ovnsb_idl_txn is NULL, so the condition if (br_int && chassis) is not
> satisfied and so ofctrl_run() is not executed in the main loop. If there
> is any message pending from br-int.mgmt, such as OFPTYPE_BARRIER_REPLY or
> OFPTYPE_ECHO_REQUEST, the main loop will be woken up again and again
> because those messages are not processed because ofctrl_run() is not
> invoked.
> 
> This patch fixes the problem by moving ofctrl_run() above and run it
> whenever br_int is not NULL, and not care about chassis because this
> function doesn't depend on it.
> 
> It also moves out sbrec_chassis_set_nb_cfg() from the "if (ovs_idl_txn)"
> just to avoid adding more indentation of the whole block to avoid >79
> line length.
> 
> Note: the changes of this patch is better to be shown with "-w" because
> most of them are indent changes.
> 
> Acked-by: Numan Siddique 
> Signed-off-by: Han Zhou 
> ---
> 
> Notes:
> v2->v3: fix >79 line found by 0-day robot

This has a patch reject against current master, so if it's still
relevant would you mind rebasing and reposting?

Thanks,

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


Re: [ovs-dev] [PATCH v2] OVN: fix DVR Floating IP support

2019-04-16 Thread Ben Pfaff
On Sat, Apr 06, 2019 at 05:42:52PM +0200, Lorenzo Bianconi wrote:
> When DVR is enabled FIP traffic need to be forwarded directly using
> external connection to the underlay network and not be distributed
> through geneve tunnels.
> Fix this adding new logical flows to take care of distributed DNAT/SNAT
> 
> Acked-by: Mark Michelson 
> Signed-off-by: Lorenzo Bianconi 

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


Re: [ovs-dev] [PATCH v7 0/5] ovn: Add HA chassis group and 'external' port

2019-04-16 Thread Numan Siddique
On Tue, Apr 16, 2019 at 10:39 PM Ben Pfaff  wrote:

> On Thu, Mar 28, 2019 at 11:39:17AM +0530, nusid...@redhat.com wrote:
> > From: Numan Siddique 
> >
> > This patch series adds a generic HA chassis group support in OVN
> > deprecating the existing Gateway chassis support. The final patch
> > of the series adds the 'external' port support in OVN.
> > The 'external' port patch addresses the review comments from Han Zhou
> > which he provided when 'external' port patch was submitted without
> > the HA support.
> >
> > A generic HA chassis group support is added so that both the distributed
> > logical router ports (providing gateway functionality) and 'external'
> > ports can use it for HA and also to simplify the existing HA code
> > (which seems to be a bit complicated).
> >
> > To support HA, BFD is configured on tunnel ports. And even though
> > 'external' ports are expected to be used with the logical
> > switches having localnet ports (representing physical networks),
> > BFD is used for now since each chassis uses geneve tunnels with
> > all other chassis in the OVN cluster.
>
> Thanks.  I applied this to master.
>
> I am uncertain whether this introduces upgrade issues.  If so, then we
> should add some explanation for users to explain how to upgrade safely.
>
>
Thanks for applying the series.

I think it would have some upgrade issues if ovn-controllers are upgraded
first. Since it will no
longer look into the 'Gateway_Chassis' table in SB DB. And this would
result in some downtime until
ovn-northd and ovn db servers are upgraded.

I will send a patch to explain this.

Thanks for pointing this out.

Regards
Numan




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


Re: [ovs-dev] [PATCH] checkpatch: Fix handling of line endings.

2019-04-16 Thread Ben Pfaff
On Tue, Apr 16, 2019 at 10:31:20AM +0300, Ilya Maximets wrote:
> On 15.04.2019 22:35, Ben Pfaff wrote:
> > On Mon, Apr 15, 2019 at 04:36:54PM +0300, Ilya Maximets wrote:
> >> Unlike manual splitting, 'splitlines' correctly handles different
> >> line endings. Without this change script fails to check files with
> >> '\r\n' endings treating the whole patch as a header.
> >>
> >> Signed-off-by: Ilya Maximets 
> > 
> > Thanks, I applied this to master.
> > 
> > I don't think OVS legitimately has any text files that contain carriage
> > returns, so it might be worth warning about them.
> 
> 'git am' by default strips '\r' symbols. OTOH, some mail clients adds them.
> There might be a lot of false positives if we'll add this kind of check
> to checkpatch.

OK, never mind then.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v7 0/5] ovn: Add HA chassis group and 'external' port

2019-04-16 Thread Ben Pfaff
On Thu, Mar 28, 2019 at 11:39:17AM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> This patch series adds a generic HA chassis group support in OVN
> deprecating the existing Gateway chassis support. The final patch
> of the series adds the 'external' port support in OVN.
> The 'external' port patch addresses the review comments from Han Zhou
> which he provided when 'external' port patch was submitted without
> the HA support.
> 
> A generic HA chassis group support is added so that both the distributed
> logical router ports (providing gateway functionality) and 'external'
> ports can use it for HA and also to simplify the existing HA code
> (which seems to be a bit complicated).
> 
> To support HA, BFD is configured on tunnel ports. And even though
> 'external' ports are expected to be used with the logical
> switches having localnet ports (representing physical networks),
> BFD is used for now since each chassis uses geneve tunnels with
> all other chassis in the OVN cluster.

Thanks.  I applied this to master.

I am uncertain whether this introduces upgrade issues.  If so, then we
should add some explanation for users to explain how to upgrade safely.

Thanks,

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


[ovs-dev] KPIS de Recursos Humanos

2019-04-16 Thread Indicadores que debes de medir
Cursos TOP - Webinar Interactivo – Viernes 22 de Mayo
 
KPIS de Recursos Humanos - Para mejorar resultados de tu departamento -  

Los KPIS adecuados deben ser concisos, simples, fáciles de procesar y 
equiparables. Para cualquier área resultan de 
mucha utilidad ya que nos proporcionan mucha información. Nuestro webinar te 
enseñará a reconocer los indicadores 
que debes de medir en tu departamento de RRHH para evaluar el desempeño del 
departamento. 

Entenderemos la importancia que tienen los KPIS en una organización y la manera 
de integrarlos en el departamento de RRHH. 

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


NOMBRE:
TELÉFONO:
EMPRESA:
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 v2] datapath-windows: Do not send out nbls when cloned nbls are being accessed

2019-04-16 Thread Alin Gabriel Serdean


> On 11 Apr 2019, at 19:14, Anand Kumar via dev  wrote:
> 
> As per MSDN documentation, "As soon as a filter driver calls the
> NdisFSendNetBufferLists function, it relinquishes ownership of
> the NET_BUFFER_LIST structures and all associated resources.
> A filter driver should never try to examine the NET_BUFFER_LIST
> structures or any associated data after calling NdisFSendNetBufferLists".
> 
> https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/ndis/nf-ndis-ndisfsendnetbufferlists
> 
> When freeing up memory of a cloned nbl, parent's nbl and context
> is being accessed, which is incorrect can cause BSOD.
> With this patch, original nbl is sent out only when cloned nbl is done
> with packet processing and its memory is freed.
> 
> Signed-off-by: Anand Kumar 
> ---
> v1->v2:
> - Remove the else block and by default try to send the packet out.
> —
> 

Acked-by: Alin Gabriel Serdean mailto:aserd...@ovn.org>>


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


Re: [ovs-dev] [PATCH v7 0/5] dpcls func ptrs & optimizations

2019-04-16 Thread Ilya Maximets
On 15.04.2019 21:50, Van Haaren, Harry wrote:
>> -Original Message-
>> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>> Sent: Monday, April 15, 2019 9:33 AM
>> To: Van Haaren, Harry ; ovs-dev@openvswitch.org
>> Cc: Stokes, Ian ; acon...@redhat.com;
>> echau...@redhat.com
>> Subject: Re: [PATCH v7 0/5] dpcls func ptrs & optimizations
>>
>> On 11.04.2019 15:55, Harry van Haaren wrote:
> 
>>> Harry van Haaren (5):
>>>   dpif-netdev: implement function pointers/subtable
>>>   dpif-netdev: move dpcls lookup structures to .h
>>>   dpif-netdev: split out generic lookup function
>>>   dpif-netdev: refactor generic implementation
>>>   dpif-netdev: add specialized generic scalar functions
>>>
>>>  lib/automake.mk  |   1 +
>>>  lib/dpif-netdev-lookup-generic.c | 342 +++
>>>  lib/dpif-netdev.c| 128 +++-
>>>  lib/dpif-netdev.h|  83 
>>>  4 files changed, 456 insertions(+), 98 deletions(-)
>>>  create mode 100644 lib/dpif-netdev-lookup-generic.c
>>>
>>
>> Hi. This patch-set fails the sparse build:
>>
>> libtool: compile:  env "REAL_CC=gcc -std=gnu99" "CHECK=sparse  -I
>> ./include/sparse -m64 -I /usr/local/include -I /usr/include/x86_64-linux-gnu
>> " cgcc -target=x86_64 -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 -Wsparse-error -g -O2 -MT lib/dpif-netdev-lookup-generic.lo -MD -MP -
>> MF lib/.deps/dpif-netdev-lookup-generic.Tpo -c lib/dpif-netdev-lookup-
>> generic.c -o lib/dpif-netdev-lookup-generic.o
>> lib/dpif-netdev-lookup-generic.c:203:43: error: Variable length array is
>> used.
>> lib/dpif-netdev-lookup-generic.c:210:23: error: Variable length array is
>> used.
>> make[2]: *** [lib/dpif-netdev-lookup-generic.lo] Error 1
>>
>> https://travis-ci.org/ovsrobot/ovs/jobs/518798999
>>
>> Sparse and MSVC doesn't like variable length arrays.
> 
> Thanks for the heads up - I'll add sparse to my test builds here, and add 
> #ifdefs around the VLAs (like other places in the code). Will fix in V+1.

If you have a github, you may allow travis to make most of sanity checks for 
you. 

> 
> Any performance numbers/gains there?

Sorry, had no chance to run perf tests yet.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] compiler: Fix compilation when using VStudio 2015/2017

2019-04-16 Thread Alin Gabriel Serdean
Thanks both! I applied this patch as far back as branch-2.9.

> On 16 Apr 2019, at 02:16, Anand Kumar via dev  wrote:
> 
> Acked-by: Anand Kumar 
> 
> Thanks,
> Anand Kumar
> 
> 

___
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

2019-04-16 Thread Darrell Ball
For '3' comment below:

s/Regarding playbook-fedora-builder.yml in general, there is issue with
playbook-fedora-builder.yml, assuming I use "as is"./
Regarding POC in general, there is an issue with POC for Fedora usage,
assuming I use "as is"./

On Mon, Apr 15, 2019 at 11:21 PM Darrell Ball  wrote:

> Thanks for the fix
>
> 1/ Main changes to openvswitch-fedora.spec.in look ok to me, but we
> should probably also see if there is any specific use
> case concerns from others.
>
> 2/ Couple comments inline
>
> 3/ Regarding playbook-fedora-builder.yml in general, there is issue with
> playbook-fedora-builder.yml, assuming I use "as is".
>
>
> dball@ubuntu:~/ovs/poc/builders$ sudo vagrant up
> DEPRECATION: The 'sudo' option for the Ansible provisioner is deprecated.
> Please use the 'become' option instead.
> The 'sudo' option will be removed in a future release of Vagrant.
>
> Bringing machine 'fedorabuilder' up with 'virtualbox' provider...
> ==> fedorabuilder: Box 'fedora/27-cloud-base' could not be found.
> Attempting to find and install...
> fedorabuilder: Box Provider: virtualbox
> fedorabuilder: Box Version: >= 0
> ==> fedorabuilder: Loading metadata for box 'fedora/27-cloud-base'
> fedorabuilder: URL: https://vagrantcloud.com/fedora/27-cloud-base
> ==> fedorabuilder: Adding box 'fedora/27-cloud-base' (v20171105) for
> provider: virtualbox
> fedorabuilder: Downloading:
> https://vagrantcloud.com/fedora/boxes/27-cloud-base/versions/20171105/providers/virtualbox.box
> fedorabuilder: Download redirected to host: download.fedoraproject.org
> An error occurred while downloading the remote file. The error
> message, if any, is reproduced below. Please fix this error and try
> again.
>
> The requested URL returned error: 404 Not Found
>
> On Mon, Apr 15, 2019 at 6:26 PM Ansis Atteka  wrote:
>
>> 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 
>> ---
>>  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
>>
>
> Is the correct spec file openvswitch-kmod-fedora.spec ?
> Hence, do we need a change here ?
>
>
>>  args:
>>  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
>>
>>- name: Build Open vSwitch dkms rpm
>> -command: rpmbuild -bb --without check rhel/openvswitch-dkms.spec
>> +command: rpmbuild -bb --without check --without libcapng
>> rhel/openvswitch-dkms.spec
>>
>
> Do you need this line changed ?
>
>
>
>>  args:
>>  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
>>
>> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
>> openvswitch-fedora.spec.in
>> index c1cd3f4c6..ce728b4f0 100644
>> --- a/rhel/openvswitch-fedora.spec.in
>> +++ b/rhel/openvswitch-fedora.spec.in
>> @@ -350,6 +350,7 @@ rm -rf $RPM_BUILD_ROOT
>>  %endif
>>
>>  %pre
>> +%if %{with libcapng}
>>  getent group openvswitch >/dev/null || groupadd -r openvswitch
>>  getent passwd openvswitch >/dev/null || \
>>  useradd -r -g openvswitch -d / -s /sbin/nologin \
>> @@ -359,9 +360,11 @@ getent passwd openvswitch >/dev/null || \
>>  getent group hugetlbfs >/dev/null || groupadd -r hugetlbfs
>>  usermod -a -G hugetlbfs openvswitch
>>  %endif
>> +%endif
>>  exit 0
>>
>>  %post
>> +%if %{with libcapng}
>>  if [ $1 -eq 1 ]; then
>>  sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
>>  sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:'
>> %{_sysconfdir}/logrotate.d/openvswitch
>> @@ -376,6 +379,7 @@ if [ $1 -eq 1 ]; then
>>  chown -R openvswitch:openvswitch /etc/openvswitch
>>  chown -R openvswitch:openvswitch /var/log/openvswitch
>>  fi
>> +%endif
>>
>>  %if 0%{?systemd_post:1}
>>  %systemd_post %{name}.service
>> @@ -445,7 +449,11 @@ fi
>>  %endif
>>
>>  %files
>> +%if %{with libcapng}
>>  %defattr(-,openvswitch,openvswitch)
>> +%else
>> 

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

2019-04-16 Thread Ilya Maximets
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.

> +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.

> +/* Restore the number of queue pairs to default. */
> +if (dev->requested_n_txq != qp_num
> +|| dev->requested_n_rxq != qp_num) {
> +dev->requested_n_rxq = qp_num;
> +dev->requested_n_txq = qp_num;
> +netdev_request_reconfigure(>up);
> +}
> +ovs_mutex_unlock(>mutex);
> +break;
> +}
> +ovs_mutex_unlock(>mutex);
> +}
> +ovs_mutex_unlock(_mutex);
> +}
> +
>  /*
>   * Retrieve the DPDK virtio device ID (vid) associated with a vhostuser
>   * or vhostuserclient netdev.
> 
___
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

2019-04-16 Thread Ilya Maximets
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.

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.

>  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;
> 
___
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

2019-04-16 Thread Ilya Maximets
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.

More comments inline.

Best regards, Ilya Maximets.

On 16.04.2019 12: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;

What do you think about renaming to 'rxq_enabled'? It seems more clear.
Having both 'emc_enabled' and just 'enabled' is a bit confusing.

> +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");
> +}
>  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;
> +}
> +
>  if (dp_netdev_process_rxq_port(non_pmd,
> >rxqs[i],
> port->port_no)) {
> @@ -5371,6 +5381,9 @@ pmd_load_queues_and_ports(struct dp_netdev_pmd_thread 
> *pmd,
>  poll_list[i].rxq = poll->rxq;
>  poll_list[i].port_no = poll->rxq->port->port_no;
>  poll_list[i].emc_enabled = 

[ovs-dev] (no subject)

2019-04-16 Thread carex martine
Hello Gentlemen & Ladies,
Looking for a commercial loan, personal loan, home loan, auto loan, student
loans, debt consolidation loans, unsecured loan, venture capital, or was
denied a loan by a bank or financial institution for either reason? We
offer all types of loans ranging from €1000 to €500,000 to anyone able to
meet the requirements. We are by a bank and do not require a lot of
documents to trust you, but you must be a fair, honest, wise and
trustworthy person. We give loans to people living across Europe and around
the world. Our interest rate is 3% per annum. If you need money for other
reasons, please feel free to contact us for more information. We are
available to respond to our customers immediately after receiving your
application form.
Here is our E-mail: hohmanntho...@hotmail.com
Thank you and good to you.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [dpdk-latest PATCH v1] dpdk: Update by new color definitions.

2019-04-16 Thread Ian Stokes

On 4/16/2019 7:41 AM, Ophir Munk wrote:

Follwoing dpdk new color definitions (see [1]) 'e_RTE_METER_GREEN'
was replaced with 'RTE_COLOR_GREEN'.

[1]
Commit c1656328dbc2: ("meter: replace color definitions")

Signed-off-by: Ophir Munk 


Thanks for this Ophir, this fixes compilation with DPDK master so I've 
pushed this to dpdk-latest with a slight modification to the subject 
line (dpdk: -> netdev-dpdk).


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


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

2019-04-16 Thread David Marchand
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. */
+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;
+
+/* Restore the number of queue pairs to default. */
+if (dev->requested_n_txq != qp_num
+|| dev->requested_n_rxq != qp_num) {
+dev->requested_n_rxq = qp_num;
+dev->requested_n_txq = qp_num;
+netdev_request_reconfigure(>up);
+}
+ovs_mutex_unlock(>mutex);
+break;
+}
+ovs_mutex_unlock(>mutex);
+}
+ovs_mutex_unlock(_mutex);
+}
+
 /*
  * Retrieve the DPDK virtio device ID (vid) associated with a vhostuser
  * or vhostuserclient netdev.
-- 
1.8.3.1

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


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

2019-04-16 Thread David Marchand
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;
-- 
1.8.3.1

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


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

2019-04-16 Thread David Marchand
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");
+}
 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;
+}
+
 if (dp_netdev_process_rxq_port(non_pmd,
>rxqs[i],
port->port_no)) {
@@ -5371,6 +5381,9 @@ pmd_load_queues_and_ports(struct dp_netdev_pmd_thread 
*pmd,
 poll_list[i].rxq = poll->rxq;
 poll_list[i].port_no = poll->rxq->port->port_no;
 poll_list[i].emc_enabled = poll->rxq->port->emc_enabled;
+poll_list[i].enabled = netdev_rxq_enabled(poll->rxq->rx);
+poll_list[i].change_seq =
+ netdev_get_change_seq(poll->rxq->port->netdev);
 i++;
 }
 
@@ -5436,6 +5449,10 @@ reload:
 
 for (i = 0; i < poll_cnt; i++) {
 
+if (!poll_list[i].enabled) {
+continue;
+}
+
 if (poll_list[i].emc_enabled) {
 atomic_read_relaxed(>dp->emc_insert_min,
 >ctx.emc_insert_min);
@@ -5472,6 +5489,16 @@ reload:
 if (reload) {
 break;
 }
+
+for (i = 0; i < poll_cnt; i++) {
+uint64_t current_seq =
+ 

[ovs-dev] [PATCH] rhel ovn: Remove ovn-common rpm

2019-04-16 Thread nusiddiq
From: Numan Siddique 

ovn-fedora spec generates the rpms - ovn, ovn-common, ovn-host etc
in which ovn is an empty package. The ovn fedora spec file here [1]
has moved all the ovn-common files to the 'ovn' package.
This patch does the same.

[1] - https://src.fedoraproject.org/rpms/ovn/blob/master/f/ovn.spec

Signed-off-by: Numan Siddique 
CC: Timothy Redaelli 
---
 rhel/ovn-fedora.spec.in | 30 ++
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in
index 8940d0581..2ecc629f2 100644
--- a/rhel/ovn-fedora.spec.in
+++ b/rhel/ovn-fedora.spec.in
@@ -45,6 +45,8 @@ Summary: Open Virtual Network support
 Group: System Environment/Daemons
 URL: http://www.openvswitch.org/
 Version: @VERSION@
+Obsoletes: openvswitch-ovn-common < %{?epoch:%{epoch}:}%{version}-%{release}
+Provides: openvswitch-ovn-common = %{?epoch:%{epoch}:}%{version}-%{release}
 
 # Nearly all of openvswitch is ASL 2.0.  The bugtool is LGPLv2+, and the
 # lib/sflow*.[ch] files are SISSL
@@ -89,7 +91,7 @@ overlays and security groups.
 %package central
 Summary: Open Virtual Network support
 License: ASL 2.0
-Requires: ovn ovn-common
+Requires: ovn
 Requires: firewalld-filesystem
 Obsoletes: openvswitch-ovn-central
 Provides: openvswitch-ovn-central = %{?epoch:%{epoch}:}%{version}-%{release}
@@ -100,7 +102,7 @@ OVN DB servers and ovn-northd running on a central node.
 %package host
 Summary: Open Virtual Network support
 License: ASL 2.0
-Requires: ovn ovn-common
+Requires: ovn
 Requires: firewalld-filesystem
 Obsoletes: openvswitch-ovn-host
 Provides: openvswitch-ovn-host = %{?epoch:%{epoch}:}%{version}-%{release}
@@ -111,27 +113,17 @@ OVN controller running on each host.
 %package vtep
 Summary: Open Virtual Network support
 License: ASL 2.0
-Requires: ovn ovn-common
+Requires: ovn
 Obsoletes: openvswitch-ovn-vtep
 Provides: openvswitch-ovn-vtep = %{?epoch:%{epoch}:}%{version}-%{release}
 
 %description vtep
 OVN vtep controller
 
-%package common
-Summary: Open Virtual Network support
-License: ASL 2.0
-Requires: ovn
-Obsoletes: openvswitch-ovn-common
-Provides: openvswitch-ovn-common = %{?epoch:%{epoch}:}%{version}-%{release}
-
-%description common
-Utilities that are use to diagnose and manage the OVN components.
-
 %package docker
 Summary: Open Virtual Network support
 License: ASL 2.0
-Requires: ovn ovn-common  %{_py2}-openvswitch
+Requires: ovn %{_py2}-openvswitch
 Obsoletes: openvswitch-ovn-docker
 Provides: openvswitch-ovn-docker = %{?epoch:%{epoch}:}%{version}-%{release}
 
@@ -393,12 +385,6 @@ if [ $1 -eq 1 ]; then
 fi
 
 %files
-
-%files docker
-%{_bindir}/ovn-docker-overlay-driver
-%{_bindir}/ovn-docker-underlay-driver
-
-%files common
 %{_bindir}/ovn-nbctl
 %{_bindir}/ovn-sbctl
 %{_bindir}/ovn-trace
@@ -418,6 +404,10 @@ fi
 %{_mandir}/man5/ovn-sb.5*
 %{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers
 
+%files docker
+%{_bindir}/ovn-docker-overlay-driver
+%{_bindir}/ovn-docker-underlay-driver
+
 %files central
 %{_bindir}/ovn-northd
 %{_mandir}/man8/ovn-northd.8*
-- 
2.20.1

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


[ovs-dev] [PATCH v3 6/6] ovn: Generate ICMPv4 packet in router pipeline for larger packets

2019-04-16 Thread nusiddiq
From: Numan Siddique 

This patch adds 2 stages in router pipeline after ARP_RESOLVE
and adds the logical flows to check the packet length and
generate ICMPv4 packet.

   * S_ROUTER_IN_CHK_PKT_LEN - Which checks the packet length using
   check_pkt_larger OVN action

   * S_ROUTER_IN_LARGER_PKTS - Which generates icmp packet with
   type 3 (Destination Unreachable),
   code 4 (Frag Needed and DF was Set)
   icmp4.frag_mtu = gw_mtu

In order to add these logical flows, CMS should set the
option 'gateway_mtu' for the distributed logical router port.

Signed-off-by: Numan Siddique 
Acked-by: Mark Michelson 
---
 ovn/northd/ovn-northd.8.xml |  83 +-
 ovn/northd/ovn-northd.c |  92 +++-
 tests/ovn.at| 167 
 3 files changed, 336 insertions(+), 6 deletions(-)

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 392a5efc9..345023eb4 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -2002,7 +2002,86 @@ next;
   
 
 
-Ingress Table 9: Gateway Redirect
+Ingress Table 9: Check packet length
+
+
+  For distributed logical routers with distributed gateway port configured
+  with options:gateway_mtu to a valid integer value, this
+  table adds a priority-50 logical flow with the match
+  ip4  outport == GW_PORT where
+  GW_PORT is the distributed gateway router port and applies the
+  action check_pkt_larger and advances the packet to the
+  next table.
+
+
+
+REGBIT_PKT_LARGER = check_pkt_larger(L); next;
+
+
+
+  where L is the packet length to check for. If the packet
+  is larger than L, it stores 1 in the register bit
+  REGBIT_PKT_LARGER. The value of
+  L is taken from  column of
+   row.
+
+
+
+  This table adds one priority-0 fallback flow that matches all packets
+  and advances to the next table.
+
+
+Ingress Table 10: Handle larger packets
+
+
+  For distributed logical routers with distributed gateway port configured
+  with options:gateway_mtu to a valid integer value, this
+  table adds the following priority-50 logical flow for each
+  logical router port with the match ip4 
+  inport == LRP  outport == GW_PORT
+   REGBIT_PKT_LARGER, where LRP is the logical
+  router port and GW_PORT is the distributed gateway router port
+  and applies the following action
+
+
+
+icmp4 {
+icmp4.type = 3; /* Destination Unreachable. */
+icmp4.code = 4;  /* Frag Needed and DF was Set. */
+icmp4.frag_mtu = M;
+eth.dst = E;
+ip4.dst = ip4.src;
+ip4.src = I;
+ip.ttl = 255;
+REGBIT_EGRESS_LOOPBACK = 1;
+next(pipeline=ingress, table=0);
+};
+
+
+
+  
+Where M is the (fragment MTU - 58) whose value is taken from
+ column of
+ row.
+  
+
+  
+E is the Ethernet address of the logical router port.
+  
+
+  
+I is the IPv4 address of the logical router port.
+  
+
+
+
+  This table adds one priority-0 fallback flow that matches all packets
+  and advances to the next table.
+
+
+Ingress Table 11: Gateway Redirect
 
 
   For distributed logical routers where one of the logical router
@@ -2059,7 +2138,7 @@ next;
   
 
 
-Ingress Table 10: ARP Request
+Ingress Table 12: ARP Request
 
 
   In the common case where the Ethernet destination has been resolved, this
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 5614f9fa3..92e0e9c9d 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -142,8 +142,10 @@ enum ovn_stage {
 PIPELINE_STAGE(ROUTER, IN,  ND_RA_RESPONSE, 6, "lr_in_nd_ra_response") \
 PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING, 7, "lr_in_ip_routing")   \
 PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE,8, "lr_in_arp_resolve")  \
-PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT,9, "lr_in_gw_redirect")  \
-PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST,10, "lr_in_arp_request")  \
+PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN   , 9, "lr_in_chk_pkt_len")   \
+PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS,10,"lr_in_larger_pkts")   \
+PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT,11, "lr_in_gw_redirect")  \
+PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST,12, "lr_in_arp_request")  \
   \
 /* Logical router egress stages. */   \
 PIPELINE_STAGE(ROUTER, OUT, UNDNAT,0, "lr_out_undnat")\
@@ -179,6 +181,8 @@ enum ovn_stage {
  * logical router dropping packets with source IP address equals
  * one of the logical router's own IP addresses. */
 #define REGBIT_EGRESS_LOOPBACK  "reg9[1]"
+/* Register to store the result of 

[ovs-dev] [PATCH v3 5/6] ovn: Support OVS action 'check_pkt_larger' in OVN

2019-04-16 Thread nusiddiq
From: Numan Siddique 

Previous commit added a new OVS action 'check_pkt_larger'. This
patch supports that action in OVN. The syntax to use this would be

reg0[0] = check_pkt_larger(LEN)

Upcoming commit will make use of this action in ovn-northd and
will generate an ICMPv4 packet if the packet length is greater than
the specified length.

Signed-off-by: Numan Siddique 
Acked-by: Mark Michelson 
---
 include/ovn/actions.h | 10 +++-
 ovn/lib/actions.c | 53 +++
 ovn/ovn-sb.xml| 21 
 ovn/utilities/ovn-trace.c |  4 ++-
 tests/ovn.at  | 10 
 5 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index af8b0a4a0..e07ad9aa3 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -82,7 +82,8 @@ struct ovn_extend_table;
 OVNACT(PUT_ND_RA_OPTS,ovnact_put_opts)\
 OVNACT(ND_NS, ovnact_nest)\
 OVNACT(SET_METER, ovnact_set_meter)   \
-OVNACT(OVNFIELD_LOAD, ovnact_load)
+OVNACT(OVNFIELD_LOAD, ovnact_load)\
+OVNACT(CHECK_PKT_LARGER,  ovnact_check_pkt_larger)
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -310,6 +311,13 @@ struct ovnact_set_meter {
 uint64_t burst;  /* burst rate field, in kbps. */
 };
 
+/* OVNACT_CHECK_IP_PKT_LARGER. */
+struct ovnact_check_pkt_larger {
+struct ovnact ovnact;
+uint16_t pkt_len;
+struct expr_field dst;  /* 1-bit destination field. */
+};
+
 /* Internal use by the helpers below. */
 void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
 void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index f78e6ffcb..a37e115e5 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -2359,6 +2359,55 @@ encode_OVNFIELD_LOAD(const struct ovnact_load *load,
 }
 }
 
+static void
+parse_check_pkt_larger(struct action_context *ctx,
+   const struct expr_field *dst,
+   struct ovnact_check_pkt_larger *cipl)
+{
+int pkt_len;
+
+lexer_get(ctx->lexer); /* Skip check_pkt_len. */
+if (!lexer_force_match(ctx->lexer, LEX_T_LPAREN)
+|| !lexer_get_int(ctx->lexer, _len)
+|| !lexer_force_match(ctx->lexer, LEX_T_RPAREN)) {
+return;
+}
+
+/* Validate that the destination is a 1-bit, modifiable field. */
+char *error = expr_type_check(dst, 1, true);
+if (error) {
+lexer_error(ctx->lexer, "%s", error);
+free(error);
+return;
+}
+cipl->dst = *dst;
+cipl->pkt_len = pkt_len;
+}
+
+static void
+format_CHECK_PKT_LARGER(const struct ovnact_check_pkt_larger *cipl,
+struct ds *s)
+{
+expr_field_format(>dst, s);
+ds_put_format(s, " = check_pkt_larger(%d);", cipl->pkt_len);
+}
+
+static void
+encode_CHECK_PKT_LARGER(const struct ovnact_check_pkt_larger *cipl,
+const struct ovnact_encode_params *ep OVS_UNUSED,
+struct ofpbuf *ofpacts)
+{
+struct ofpact_check_pkt_larger *check_pkt_larger =
+ofpact_put_CHECK_PKT_LARGER(ofpacts);
+check_pkt_larger->pkt_len = cipl->pkt_len;
+check_pkt_larger->dst = expr_resolve_field(>dst);
+}
+
+static void
+ovnact_check_pkt_larger_free(struct ovnact_check_pkt_larger *cipl OVS_UNUSED)
+{
+}
+
 /* Parses an assignment or exchange or put_dhcp_opts action. */
 static void
 parse_set_action(struct action_context *ctx)
@@ -2388,6 +2437,10 @@ parse_set_action(struct action_context *ctx)
 && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
 parse_put_nd_ra_opts(ctx, ,
  ovnact_put_PUT_ND_RA_OPTS(ctx->ovnacts));
+} else if (!strcmp(ctx->lexer->token.s, "check_pkt_larger")
+&& lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
+parse_check_pkt_larger(ctx, ,
+   ovnact_put_CHECK_PKT_LARGER(ctx->ovnacts));
 } else {
 parse_assignment_action(ctx, false, );
 }
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 0d35fe4fc..9d611c9ab 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1766,6 +1766,27 @@
 
   Example: set_meter(100, 1000);
 
+
+R = check_pkt_larger(L)
+
+  
+Parameters: packet length L to check for
+in bytes.
+  
+
+  
+Result: stored to a 1-bit subfield R.
+  
+
+  
+This is a logical equivalent of the OpenFlow
+check_pkt_larger action. If the packet is larger
+than the length specified in L, it stores 1 in the
+subfield R.
+  
+
+  Example: reg0[6] = check_pkt_larger(1000);
+
   
 
   
diff --git 

[ovs-dev] [PATCH v3 4/6] ovn: Add a new OVN action 'icmp4_error'

2019-04-16 Thread nusiddiq
From: Numan Siddique 

This action is similar to the existing 'icmp4' OVN action except that
that this action is expected to be used to generate an ICMPv4 packet
in response to an error in original IP packet. When this action
injects the icmpv4 packet, it also copies the original IP datagram
following the icmp4 header as per RFC 1122: 3.2.2

Signed-off-by: Numan Siddique 
Acked-by: Mark Michelson 
---
 include/ovn/actions.h |  7 ++
 ovn/controller/pinctrl.c  | 47 +--
 ovn/lib/actions.c | 22 ++
 ovn/ovn-sb.xml| 20 -
 ovn/utilities/ovn-trace.c |  5 +
 tests/ovn.at  | 15 +
 6 files changed, 109 insertions(+), 7 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 89e28c50c..af8b0a4a0 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -65,6 +65,7 @@ struct ovn_extend_table;
 OVNACT(CLONE, ovnact_nest)\
 OVNACT(ARP,   ovnact_nest)\
 OVNACT(ICMP4, ovnact_nest)\
+OVNACT(ICMP4_ERROR,   ovnact_nest)\
 OVNACT(ICMP6, ovnact_nest)\
 OVNACT(TCP_RESET, ovnact_nest)\
 OVNACT(ND_NA, ovnact_nest)\
@@ -471,6 +472,12 @@ enum action_opcode {
  /* MTU value (to put in the icmp4 header field - frag_mtu) follow the
  * action header. */
 ACTION_OPCODE_PUT_ICMP4_FRAG_MTU,
+
+/* "icmp4_error { ...actions... }".
+ *
+ * The actions, in OpenFlow 1.3 format, follow the action_header.
+ */
+ACTION_OPCODE_ICMP4_ERROR,
 };
 
 /* Header. */
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 63202e6d4..2f22a57b2 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -531,7 +531,8 @@ pinctrl_handle_arp(struct rconn *swconn, const struct flow 
*ip_flow,
 static void
 pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow,
 struct dp_packet *pkt_in,
-const struct match *md, struct ofpbuf *userdata)
+const struct match *md, struct ofpbuf *userdata,
+bool include_orig_ip_datagram)
 {
 /* This action only works for IP packets, and the switch should only send
  * us IP packets this way, but check here just to be sure. */
@@ -556,6 +557,16 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct 
flow *ip_flow,
 eh->eth_src = ip_flow->dl_src;
 
 if (get_dl_type(ip_flow) == htons(ETH_TYPE_IP)) {
+struct ip_header *in_ip = dp_packet_l3(pkt_in);
+uint16_t in_ip_len = ntohs(in_ip->ip_tot_len);
+if (in_ip_len < IP_HEADER_LEN) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(,
+"ICMP action on IP packet with invalid length (%u)",
+in_ip_len);
+return;
+}
+
 struct ip_header *nh = dp_packet_put_zeros(, sizeof *nh);
 
 eh->eth_type = htons(ETH_TYPE_IP);
@@ -571,6 +582,33 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct 
flow *ip_flow,
 struct icmp_header *ih = dp_packet_put_zeros(, sizeof *ih);
 dp_packet_set_l4(, ih);
 packet_set_icmp(, ICMP4_DST_UNREACH, 1);
+
+if (include_orig_ip_datagram) {
+/* RFC 1122: 3.2.2 MUST send at least the IP header and 8 bytes
+ * of header. MAY send more.
+ * RFC says return as much as we can without exceeding 576
+ * bytes.
+ * So, lets return as much as we can. */
+
+/* Calculate available room to include the original IP + data. */
+nh = dp_packet_l3();
+uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len));
+if (in_ip_len > room) {
+in_ip_len = room;
+}
+dp_packet_put(, in_ip, in_ip_len);
+
+/* dp_packet_put may reallocate the buffer. Get the l3 and l4
+ * header pointers again. */
+nh = dp_packet_l3();
+ih = dp_packet_l4();
+uint16_t ip_total_len = ntohs(nh->ip_tot_len) + in_ip_len;
+nh->ip_tot_len = htons(ip_total_len);
+ih->icmp_csum = 0;
+ih->icmp_csum = csum(ih, sizeof *ih + in_ip_len);
+nh->ip_csum = 0;
+nh->ip_csum = csum(nh, sizeof *nh);
+}
 } else {
 struct ip6_hdr *nh = dp_packet_put_zeros(, sizeof *nh);
 struct icmp6_error_header *ih;
@@ -1674,7 +1712,12 @@ process_packet_in(struct rconn *swconn, const struct 
ofp_header *msg)
 
 case ACTION_OPCODE_ICMP:
 pinctrl_handle_icmp(swconn, , , _metadata,
-);
+, false);
+break;
+
+case ACTION_OPCODE_ICMP4_ERROR:
+pinctrl_handle_icmp(swconn, , , _metadata,
+ 

[ovs-dev] [PATCH v3 2/6] datapath: Add a new action check_pkt_len

2019-04-16 Thread nusiddiq
From: Numan Siddique 

Upstream commit:
commit 4d5ec89fc8d14dcdab7214a0c13a1c7321dc6ea9
Author: Numan Siddique 
Date:   Tue Mar 26 06:13:46 2019 +0530

net: openvswitch: Add a new action check_pkt_len

This patch adds a new action - 'check_pkt_len' which checks the
packet length and executes a set of actions if the packet
length is greater than the specified length or executes
another set of actions if the packet length is lesser or equal to.

This action takes below nlattrs
  * OVS_CHECK_PKT_LEN_ATTR_PKT_LEN - 'pkt_len' to check for

  * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER - Nested actions
to apply if the packet length is greater than the specified 'pkt_len'

  * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested
actions to apply if the packet length is lesser or equal to the
specified 'pkt_len'.

The main use case for adding this action is to solve the packet
drops because of MTU mismatch in OVN virtual networking solution.
When a VM (which belongs to a logical switch of OVN) sends a packet
destined to go via the gateway router and if the nic which provides
external connectivity, has a lesser MTU, OVS drops the packet
if the packet length is greater than this MTU.

With the help of this action, OVN will check the packet length
and if it is greater than the MTU size, it will generate an
ICMP packet (type 3, code 4) and includes the next hop mtu in it
so that the sender can fragment the packets.

Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047039.html
Suggested-by: Ben Pfaff 
Signed-off-by: Numan Siddique 
CC: Gregory Rose 
CC: Pravin B Shelar 
Acked-by: Pravin B Shelar 
Tested-by: Greg Rose 
Reviewed-by: Greg Rose 
Signed-off-by: David S. Miller 

Use of 'nla_parse_strict()' (in validate_and_copy_check_len()) is available
only in recent kernels. So changed it to 'nla_parse_nested()'.

Signed-off-by: Numan Siddique 
CC: Gregory Rose 
CC: Pravin B Shelar 
---
 datapath/actions.c|  44 +
 datapath/flow_netlink.c   | 171 ++
 .../linux/compat/include/linux/openvswitch.h  |  17 ++
 3 files changed, 232 insertions(+)

diff --git a/datapath/actions.c b/datapath/actions.c
index 8abe70aa5..f5db12389 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -1219,6 +1219,40 @@ static int execute_recirc(struct datapath *dp, struct 
sk_buff *skb,
return clone_execute(dp, skb, key, recirc_id, NULL, 0, last, true);
 }
 
+static int execute_check_pkt_len(struct datapath *dp, struct sk_buff *skb,
+struct sw_flow_key *key,
+const struct nlattr *attr, bool last)
+{
+   const struct nlattr *actions, *cpl_arg;
+   const struct check_pkt_len_arg *arg;
+   int rem = nla_len(attr);
+   bool clone_flow_key;
+
+   /* The first netlink attribute in 'attr' is always
+* 'OVS_CHECK_PKT_LEN_ATTR_ARG'.
+*/
+   cpl_arg = nla_data(attr);
+   arg = nla_data(cpl_arg);
+
+   if (skb->len <= arg->pkt_len) {
+   /* Second netlink attribute in 'attr' is always
+* 'OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL'.
+*/
+   actions = nla_next(cpl_arg, );
+   clone_flow_key = !arg->exec_for_lesser_equal;
+   } else {
+   /* Third netlink attribute in 'attr' is always
+* 'OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER'.
+*/
+   actions = nla_next(cpl_arg, );
+   actions = nla_next(actions, );
+   clone_flow_key = !arg->exec_for_greater;
+   }
+
+   return clone_execute(dp, skb, key, 0, nla_data(actions),
+nla_len(actions), last, clone_flow_key);
+}
+
 /* Execute a list of actions against 'skb'. */
 static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
  struct sw_flow_key *key,
@@ -1379,6 +1413,16 @@ static int do_execute_actions(struct datapath *dp, 
struct sk_buff *skb,
return err;
break;
}
+
+   case OVS_ACTION_ATTR_CHECK_PKT_LEN: {
+bool last = nla_is_last(a, rem);
+
+err = execute_check_pkt_len(dp, skb, key, a, last);
+if (last)
+return err;
+
+break;
+}
}
 
if (unlikely(err)) {
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 13e6e3399..c4704a5b3 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -93,6 +93,7 @@ static bool actions_may_change_flow(const struct nlattr 
*actions)
case OVS_ACTION_ATTR_SET:
case 

[ovs-dev] [PATCH v3 3/6] ovn: Add a new OVN field icmp4.frag_mtu

2019-04-16 Thread nusiddiq
From: Numan Siddique 

In order to support OVN specific fields (which are not yet
supported in OpenvSwitch to set or modify values) a generic
OVN field support is added in this patch. These OVN fields
gets translated to controller actions.

This patch adds only one field for now - icmp4.frag_mtu.
It should be fairly straightforward to add similar fields in the
near future.

Example usage.
action=(icmp4 {"eth.dst <-> eth.src; "
"icmp4.type = 3; /* Destination Unreachable */ "
"icmp4.code = 4; /* Fragmentation Needed */ "
 icmp4.frag_mtu = 1442;
 ...
 "next; };")

action=(icmp4.frag_mtu = 1500; ..)

pinctrl module of ovn-controller will set the specified value
in the the low-order 16 bits of the ICMP4 header field that is
labelled "unused" in the ICMP specification as defined in the RFC 1191.

Upcoming patch will use it to send an icmp4 packet if the
source IPv4 packet destined to go via external gateway needs to
be fragmented.

Signed-off-by: Numan Siddique 
Acked-by: Mark Michelson 
---
 include/ovn/actions.h | 27 +-
 include/ovn/automake.mk   |  3 +-
 include/ovn/expr.h|  5 ++
 {ovn/lib => include/ovn}/logical-fields.h | 39 ++
 ovn/controller/lflow.c|  1 +
 ovn/controller/lflow.h|  2 +-
 ovn/controller/pinctrl.c  | 62 ++-
 ovn/lib/actions.c | 46 -
 ovn/lib/automake.mk   |  3 +-
 ovn/lib/expr.c| 17 ++-
 ovn/lib/logical-fields.c  | 36 -
 ovn/northd/ovn-northd.c   |  2 +-
 ovn/utilities/ovn-trace.c | 25 -
 tests/ovn.at  |  8 +++
 tests/test-ovn.c  |  2 +-
 15 files changed, 264 insertions(+), 14 deletions(-)
 rename {ovn/lib => include/ovn}/logical-fields.h (73%)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 1c0c67ce6..89e28c50c 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -80,7 +80,8 @@ struct ovn_extend_table;
 OVNACT(LOG,   ovnact_log) \
 OVNACT(PUT_ND_RA_OPTS,ovnact_put_opts)\
 OVNACT(ND_NS, ovnact_nest)\
-OVNACT(SET_METER, ovnact_set_meter)
+OVNACT(SET_METER, ovnact_set_meter)   \
+OVNACT(OVNFIELD_LOAD, ovnact_load)
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -142,6 +143,20 @@ ovnact_end(const struct ovnact *ovnacts, size_t 
ovnacts_len)
 #define OVNACT_FOR_EACH(POS, OVNACTS, OVNACTS_LEN)  \
 for ((POS) = (OVNACTS); (POS) < ovnact_end(OVNACTS, OVNACTS_LEN);  \
  (POS) = ovnact_next(POS))
+
+static inline int
+ovnacts_count(const struct ovnact *ovnacts, size_t ovnacts_len)
+{
+uint8_t n_ovnacts = 0;
+if (ovnacts) {
+const struct ovnact *a;
+
+OVNACT_FOR_EACH (a, ovnacts, ovnacts_len) {
+n_ovnacts++;
+}
+}
+return n_ovnacts;
+}
 
 /* Action structure for each OVNACT_*. */
 
@@ -452,6 +467,10 @@ enum action_opcode {
 * The actions, in OpenFlow 1.3 format, follow the action_header.
 */
 ACTION_OPCODE_ND_NA_ROUTER,
+
+ /* MTU value (to put in the icmp4 header field - frag_mtu) follow the
+ * action header. */
+ACTION_OPCODE_PUT_ICMP4_FRAG_MTU,
 };
 
 /* Header. */
@@ -461,6 +480,12 @@ struct action_header {
 };
 BUILD_ASSERT_DECL(sizeof(struct action_header) == 8);
 
+OVS_PACKED(
+struct ovnfield_act_header {
+ovs_be16 id; /* one of enum ovnfield_id. */
+ovs_be16 len; /* Length of the ovnfield data. */
+});
+
 struct ovnact_parse_params {
 /* A table of "struct expr_symbol"s to support (as one would provide to
  * expr_parse()). */
diff --git a/include/ovn/automake.mk b/include/ovn/automake.mk
index d2924c2f6..54b0e2c0e 100644
--- a/include/ovn/automake.mk
+++ b/include/ovn/automake.mk
@@ -2,4 +2,5 @@ ovnincludedir = $(includedir)/ovn
 ovninclude_HEADERS = \
include/ovn/actions.h \
include/ovn/expr.h \
-   include/ovn/lex.h
+   include/ovn/lex.h  \
+   include/ovn/logical-fields.h
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 3995e62f0..a68fd6e1c 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -58,6 +58,7 @@
 #include "openvswitch/list.h"
 #include "openvswitch/match.h"
 #include "openvswitch/meta-flow.h"
+#include "logical-fields.h"
 
 struct ds;
 struct expr;
@@ -244,6 +245,7 @@ struct expr_symbol {
 int width;
 
 const struct mf_field *field; /* Fields only, otherwise NULL. */
+const struct ovn_field *ovn_field;  /* OVN Fields only, otherwise NULL. */
 const struct expr_symbol *parent; /* Subfields only, otherwise NULL. */
 int parent_ofs;   /* Subfields only, otherwise 0. */
 

[ovs-dev] [PATCH v3 1/6] Add a new OVS action check_pkt_larger

2019-04-16 Thread nusiddiq
From: Numan Siddique 

This patch adds a new action 'check_pkt_larger' which checks if the
packet is larger than the given size and stores the result in the
destination register.

Usage: check_pkt_larger(len)->REGISTER
Eg. match=...,actions=check_pkt_larger(1442)->NXM_NX_REG0[0],next;

This patch makes use of the new datapath action - 'check_pkt_len'
which was recently added in the commit [1].
At the start of ovs-vswitchd, datapath is probed for this action.
If the datapath action is present, then 'check_pkt_larger'
makes use of this datapath action.

Datapath action 'check_pkt_len' takes these nlattrs
  * OVS_CHECK_PKT_LEN_ATTR_PKT_LEN - 'pkt_len' to check for
  * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER (optional) - Nested actions
to apply if the packet length is greater than the specified 'pkt_len'
  * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL (optional) - Nested
actions to apply if the packet length is lesser or equal to the
specified 'pkt_len'.

Let's say we have these flows added to an OVS bridge br-int

table=0, priority=100 
in_port=1,ip,actions=check_pkt_larger:100->NXM_NX_REG0[0],resubmit(,1)
table=1, priority=200,in_port=1,ip,reg0=0x1/0x1 actions=output:3
table=1, priority=100,in_port=1,ip,actions=output:4

Then the action 'check_pkt_larger' will be translated as
  - check_pkt_len(size=100,gt(3),le(4))

datapath will check the packet length and if the packet length is greater than 
100,
it will output to port 3, else it will output to port 4.

In case, datapath doesn't support 'check_pkt_len' action, the OVS action
'check_pkt_larger' sets SLOW_ACTION so that datapath flow is not added.

This OVS action is intended to be used by OVN to check the packet length
and generate an ICMP packet with type 3, code 4 and next hop mtu
in the logical router pipeline if the MTU of the physical interface
is lesser than the packet length. More information can be found here [2]

[1] - 
https://kernel.googlesource.com/pub/scm/linux/kernel/git/davem/net-next/+/4d5ec89fc8d14dcdab7214a0c13a1c7321dc6ea9
[2] - https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047039.html

Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047039.html
Suggested-by: Ben Pfaff 
Signed-off-by: Numan Siddique 
CC: Ben Pfaff 
CC: Gregory Rose 
Acked-by: Mark Michelson 
---
 NEWS  |   2 +
 .../linux/compat/include/linux/openvswitch.h  |  22 +++
 include/openvswitch/ofp-actions.h |  19 ++
 lib/dpif-netdev.c |   1 +
 lib/dpif.c|   1 +
 lib/odp-execute.c |  72 
 lib/odp-util.c|  86 +
 lib/ofp-actions.c | 121 -
 lib/ofp-parse.c   |  10 ++
 lib/ovs-actions.xml   |  31 
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  | 131 ++
 ofproto/ofproto-dpif.c|  43 +
 ofproto/ofproto-dpif.h|   5 +-
 tests/odp.at  |   5 +
 tests/ofp-actions.at  |   6 +
 tests/ofproto-dpif.at | 163 ++
 18 files changed, 718 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 1e4744dbd..7a9b3aeec 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,7 @@ Post-v2.11.0
hugepage memory that can be used by DPDK.
- OpenFlow:
  * Removed support for OpenFlow 1.6 (draft), which ONF abandoned.
+ * New action "check_pkt_larger".
- Userspace datapath:
  * ICMPv6 ND enhancements: support for match and set ND options type
and reserved fields.
@@ -17,6 +18,7 @@ Post-v2.11.0
conntrack fragmentation support.
  * New "ovs-appctl dpctl/ipf-get-status" command for userspace datapath
conntrack fragmentation support.
+ * New action "check_pkt_len".
- OVSDB:
  * OVSDB clients can now resynchronize with clustered servers much more
quickly after a brief disconnection, saving bandwidth and CPU time.
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index d5aa09d8d..ce364e577 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -863,6 +863,24 @@ enum ovs_nat_attr {
 
 #define OVS_NAT_ATTR_MAX (__OVS_NAT_ATTR_MAX - 1)
 
+/*
+ * enum ovs_check_pkt_len_attr - Attributes for %OVS_ACTION_ATTR_CHECK_PKT_LEN.
+ *
+ * @OVS_CHECK_PKT_LEN_ATTR_PKT_LEN: u16 Packet length to check for.
+ * @OVS_CHECK_PKT_LEN_ATTR_USERSPACE_COND: u8 comparison condition to send
+ * the packet to userspace. One of OVS_CHECK_PKT_LEN_COND_*.
+ * @OVS_CHECK_PKT_LEN_ATTR_USERPACE - Nested OVS_USERSPACE_ATTR_* 

[ovs-dev] [PATCH v3 0/6] Address MTU issue for larger packets in OVN

2019-04-16 Thread nusiddiq
From: Numan Siddique 

This series addresses the MTU issues for OVN reported
here [1].

This series has 6 patches
 
Patch 1 adds a new OVS action - check_pkt_larger. It makes use
of the datapath action - 'check_pkt_len' which was added
recently to net-next [2].

Patch 2 is the datapath patch from [1] for the OVS in tree datapath
code.

Patch 2 is submitted after patch 1 to avoid the compilation issues
since we are adding a new action attr - OVS_ACTION_ATTR_CHECK_PKT_LEN.

Patch 3 to 6 are OVN patches.

This patch series addressed most of the comments received for the RFC
series.

Patch 1 still hasn't addressed the comment for the usage of
'check_pkt_larger' action i.e check_pkt_larger(len)->REG_BIT.

[1] - https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047039.html
[2] - 
https://kernel.googlesource.com/pub/scm/linux/kernel/git/davem/net-next/+/4d5ec89fc8d14dcdab7214a0c13a1c7321dc6ea9


v2 -> v3
-
  * There was error in ovn/lib/automake.mk in patch 3 because of which "make 
rpm-fedora" was failing.
Fixed it by deleting the unnecessary '/'.

v1 -> v2
-- 
  * Updated the commit message of patch 2 to include the net-next commit
id, author and date.

No other changes.

Numan Siddique (6):
  Add a new OVS action check_pkt_larger
  datapath: Add a new action check_pkt_len
  ovn: Add a new OVN field icmp4.frag_mtu
  ovn: Add a new OVN action 'icmp4_error'
  ovn: Support OVS action 'check_pkt_larger' in OVN
  ovn: Generate ICMPv4 packet in router pipeline for larger packets

 NEWS  |   2 +
 datapath/actions.c|  44 
 datapath/flow_netlink.c   | 171 +++
 .../linux/compat/include/linux/openvswitch.h  |  39 
 include/openvswitch/ofp-actions.h |  19 ++
 include/ovn/actions.h |  42 +++-
 include/ovn/automake.mk   |   3 +-
 include/ovn/expr.h|   5 +
 {ovn/lib => include/ovn}/logical-fields.h |  39 
 lib/dpif-netdev.c |   1 +
 lib/dpif.c|   1 +
 lib/odp-execute.c |  72 +++
 lib/odp-util.c|  86 
 lib/ofp-actions.c | 121 ++-
 lib/ofp-parse.c   |  10 +
 lib/ovs-actions.xml   |  31 +++
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  | 131 
 ofproto/ofproto-dpif.c|  43 
 ofproto/ofproto-dpif.h|   5 +-
 ovn/controller/lflow.c|   1 +
 ovn/controller/lflow.h|   2 +-
 ovn/controller/pinctrl.c  | 109 +-
 ovn/lib/actions.c | 121 ++-
 ovn/lib/automake.mk   |   3 +-
 ovn/lib/expr.c|  17 +-
 ovn/lib/logical-fields.c  |  36 +++-
 ovn/northd/ovn-northd.8.xml   |  83 +++-
 ovn/northd/ovn-northd.c   |  94 +++-
 ovn/ovn-sb.xml|  41 +++-
 ovn/utilities/ovn-trace.c |  34 ++-
 tests/odp.at  |   5 +
 tests/ofp-actions.at  |   6 +
 tests/ofproto-dpif.at | 163 ++
 tests/ovn.at  | 200 ++
 tests/test-ovn.c  |   2 +-
 37 files changed, 1754 insertions(+), 30 deletions(-)
 rename {ovn/lib => include/ovn}/logical-fields.h (73%)

-- 
2.20.1

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


Re: [ovs-dev] [PATCH] checkpatch: Fix handling of line endings.

2019-04-16 Thread Ilya Maximets
On 15.04.2019 22:35, Ben Pfaff wrote:
> On Mon, Apr 15, 2019 at 04:36:54PM +0300, Ilya Maximets wrote:
>> Unlike manual splitting, 'splitlines' correctly handles different
>> line endings. Without this change script fails to check files with
>> '\r\n' endings treating the whole patch as a header.
>>
>> Signed-off-by: Ilya Maximets 
> 
> Thanks, I applied this to master.
> 
> I don't think OVS legitimately has any text files that contain carriage
> returns, so it might be worth warning about them.

'git am' by default strips '\r' symbols. OTOH, some mail clients adds them.
There might be a lot of false positives if we'll add this kind of check
to checkpatch.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [dpdk-latest PATCH v1] dpdk: Update by new color definitions.

2019-04-16 Thread Ophir Munk
Follwoing dpdk new color definitions (see [1]) 'e_RTE_METER_GREEN'
was replaced with 'RTE_COLOR_GREEN'.

[1]
Commit c1656328dbc2: ("meter: replace color definitions")

Signed-off-by: Ophir Munk 
---
 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 47153dc..f129b99 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2031,7 +2031,7 @@ netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm 
*meter,
 uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct ether_hdr);
 
 return rte_meter_srtcm_color_blind_check(meter, profile, time, pkt_len) ==
- e_RTE_METER_GREEN;
+ RTE_COLOR_GREEN;
 }
 
 static int
-- 
1.8.3.1

___
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

2019-04-16 Thread Darrell Ball
Thanks for the fix

1/ Main changes to openvswitch-fedora.spec.in look ok to me, but we should
probably also see if there is any specific use
case concerns from others.

2/ Couple comments inline

3/ Regarding playbook-fedora-builder.yml in general, there is issue with
playbook-fedora-builder.yml, assuming I use "as is".


dball@ubuntu:~/ovs/poc/builders$ sudo vagrant up
DEPRECATION: The 'sudo' option for the Ansible provisioner is deprecated.
Please use the 'become' option instead.
The 'sudo' option will be removed in a future release of Vagrant.

Bringing machine 'fedorabuilder' up with 'virtualbox' provider...
==> fedorabuilder: Box 'fedora/27-cloud-base' could not be found.
Attempting to find and install...
fedorabuilder: Box Provider: virtualbox
fedorabuilder: Box Version: >= 0
==> fedorabuilder: Loading metadata for box 'fedora/27-cloud-base'
fedorabuilder: URL: https://vagrantcloud.com/fedora/27-cloud-base
==> fedorabuilder: Adding box 'fedora/27-cloud-base' (v20171105) for
provider: virtualbox
fedorabuilder: Downloading:
https://vagrantcloud.com/fedora/boxes/27-cloud-base/versions/20171105/providers/virtualbox.box
fedorabuilder: Download redirected to host: download.fedoraproject.org
An error occurred while downloading the remote file. The error
message, if any, is reproduced below. Please fix this error and try
again.

The requested URL returned error: 404 Not Found

On Mon, Apr 15, 2019 at 6:26 PM Ansis Atteka  wrote:

> 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 
> ---
>  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
>

Is the correct spec file openvswitch-kmod-fedora.spec ?
Hence, do we need a change here ?


>  args:
>  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
>
>- name: Build Open vSwitch dkms rpm
> -command: rpmbuild -bb --without check rhel/openvswitch-dkms.spec
> +command: rpmbuild -bb --without check --without libcapng
> rhel/openvswitch-dkms.spec
>

Do you need this line changed ?



>  args:
>  chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}"
>
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
> openvswitch-fedora.spec.in
> index c1cd3f4c6..ce728b4f0 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -350,6 +350,7 @@ rm -rf $RPM_BUILD_ROOT
>  %endif
>
>  %pre
> +%if %{with libcapng}
>  getent group openvswitch >/dev/null || groupadd -r openvswitch
>  getent passwd openvswitch >/dev/null || \
>  useradd -r -g openvswitch -d / -s /sbin/nologin \
> @@ -359,9 +360,11 @@ getent passwd openvswitch >/dev/null || \
>  getent group hugetlbfs >/dev/null || groupadd -r hugetlbfs
>  usermod -a -G hugetlbfs openvswitch
>  %endif
> +%endif
>  exit 0
>
>  %post
> +%if %{with libcapng}
>  if [ $1 -eq 1 ]; then
>  sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
>  sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:'
> %{_sysconfdir}/logrotate.d/openvswitch
> @@ -376,6 +379,7 @@ if [ $1 -eq 1 ]; then
>  chown -R openvswitch:openvswitch /etc/openvswitch
>  chown -R openvswitch:openvswitch /var/log/openvswitch
>  fi
> +%endif
>
>  %if 0%{?systemd_post:1}
>  %systemd_post %{name}.service
> @@ -445,7 +449,11 @@ fi
>  %endif
>
>  %files
> +%if %{with libcapng}
>  %defattr(-,openvswitch,openvswitch)
> +%else
> +%defattr(-,root,root)
> +%endif
>  %dir %{_sysconfdir}/openvswitch
>  %{_sysconfdir}/openvswitch/default.conf
>  %config %ghost %{_sysconfdir}/openvswitch/conf.db
> --
> 2.14.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev