Re: [ovs-dev] [patch v1] conntrack: Support global drop statistics.

[Darrell Ball]

On Tue, Jul 10, 2018 at 10:38 AM, Ben Pfaff  wrote:

> On Tue, Jun 26, 2018 at 04:16:42PM -0700, Darrell Ball wrote:
> > Signed-off-by: Darrell Ball 
>
> I wonder whether coverage counters would be a better alternative?  They
> use thread-local data so incrementing them is cheaper than atomic
> increments, at least if they are accessed from multiple CPUs.
>


Thanks for the pointer on the coverage counters; since the counters are
for sanity failures, the performance does not matter much.

That being said, I have recently made some changes to the Fragmentation
series where some same sanity failures also need counting, since some now
bypass
conntrack. It is probably best to do both together, after the Fragmentation
series.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 00/14] ovn-controller incremental processing.

[Han Zhou]

On Mon, Aug 6, 2018 at 3:13 AM, Mark Michelson  wrote:
>
> Hi Han,
>
> I thought about this more over the weekend, and I was hoping I'd get to
respond to my own e-mail before you saw it, because I realized I had a
fundamental misunderstanding of the scope and nature of change handlers.
I'll reply to your comments in-line below.
>
> On 08/05/2018 03:11 PM, Han Zhou wrote:
>>
>> Hi Mark,
>>
>> Thanks for the review and very valuable comments! (I was on vacation
last week so sorry for slow response).
>>
>> On Tue, Jul 31, 2018 at 3:47 AM, Mark Michelson mailto:mmich...@redhat.com>> wrote:
>>  >
>>  > Hi Han,
>>  >
>>  > I've given this patchset a look, and I was following along pretty
well until I got to about patch 11. From that point on, I had to re-read
the code more times than I care to admit before I finally understood what
was going on :)
>>  >
>>  > What you have is a structure (lflow_ref_list_node) that is
simultaneously in two lists. These two lists are each the data in separate
hmap nodes. The hmap nodes are in two separate hmaps. One hmap uses the
reference type and name as a key, and the other uses the lflow UUID as a
key. This way given an address set name, you can find the associated
logical flow UUIDs in the ref_lflow_table. Or given a logical flow UUID,
you can find address sets.
>>  >
>>  > I'm wondering if this can be simplified somehow.
>>  >
>>  > Right now, if logical flows change, the change handler has to update
the ref_lflow_table so that address sets no longer reference that logical
flow. If address sets change, then the lflow_ref_table is updated. In both
cases consider_logical_flow() gets called and realigns the tables as
appropriate.
>>  >
>>  > The problem with this is that it reeks of cross-cutting concerns, and
it seems like it wouldn't scale well (consider a 3- or 4-chain of
dependencies). Ideally, the dependency chain would make sure that the
change handler for logical flows only deals with logical flows, and the
change handler for address sets only deals with address sets.
>>
>> I agree that maintaining the cross reference has some overhead, but I
don't see a scaling issue in this case. Adding entries to the cross
reference table is a by-product of the consider_logical_flow() when parsing
the lflow, and deleting the entries is also efficient with O(N), N = number
of address sets used by a lflow, which in most cases should be a very small
number (correct me if I am wrong). As to memory consumption, it maintains
only the mapping between resource names and lflow uuids, so I don't expect
it to be a significant cost either. Could you explain a little more about
the "3- or 4-chain of dependencies" example?
>
>
> My thought was along the lines that table A references table B, which
references table C. A change in table A might result in a change to table
B, which then results in a change to table C. In my head, I thought this
would mean having to maintain a large series of hashmaps that
cross-referenced each other. I realize this isn't correct, though, and that
as long as the dependency chain is linear, this isn't any different from
what you already have proposed here.
>
> In reality, I can't think of any example changes in the southbound
database that would cause such a series of events, but I may not be
thinking hard enough :)
>
>>
>> For the "cross-cutting concern", I don't see it that way. I see it as a
pattern of change handler implementation. In general, output of an engine
node is the result of a "join" operation of its inputs. When there are
multiple inputs and one of them changes, for a change handler to compute
the output incrementally, we will need to use the changed row to probe all
the other inputs to update the final output. For the address-set and
port-group handlers, it is join between two tables only, and the cross
reference table is built to make the probing efficient in change handler.
The cross reference table is also generalized so that any resources
referenced by logical flows can reuse the same data structure and
interfaces, and now it is reused by both address-sets and port-groups. We
can make it more generalized to be used for other mappings if needed.
>
>
> Yes, and this sort of thinking is what I had over the weekend that made
me have a "Eureka!" moment and realize what I had been missing here.
>
> I had been looking at the address set change handler and thinking of the
change handler as being an "owner" of address set data. The reason is that
the engine node is tied directly to updates to the address set table. It
felt like it was overstepping its boundaries by then stepping through data
that I thought was owned by the logical flow change handler. The fact that
both change handlers acted on the same data just struck me as wrong.
>
> However, I realized I need to stop thinking about data ownership in that
sort of way when it comes to change handlers. Engine nodes do have
ownership like I was imagining in their run() method, but change handlers
are very 

[ovs-dev] [PATCH 3/3] ovn-trace: Fix warnings when port is found but not in current datapath.

[Han Zhou]

When port group is used, ovn-trace may print warnings like this:

$ ovn-trace ls1 'inport == "lp111" && eth.src == f0:00:00:00:01:11 && eth.dst 
== f0:00:00:00:01:12  && ip4.src == 192.168.11.1 && ip4.dst == 192.168.11.2 && 
ip.ttl == 10'
2018-08-02T01:43:23Z|1|ovntrace|WARN|lp211: not in datapath ls1
2018-08-02T01:43:23Z|2|ovntrace|WARN|lp211: unknown logical port
2018-08-02T01:43:23Z|3|ovntrace|WARN|lp221: not in datapath ls1
2018-08-02T01:43:23Z|4|ovntrace|WARN|lp221: unknown logical port
2018-08-02T01:43:23Z|5|ovntrace|WARN|lp231: not in datapath ls1
2018-08-02T01:43:23Z|6|ovntrace|WARN|lp231: unknown logical port

There are 2 warnings:

For the first one, it might be reasonable
before port group is supported, but now since ports in a port group
can span across multiple datapaths, this situation is normal, and
warning should not be printed.

For the second one, it is misleading, and it should not be printed
in this situation even before port group is supported. It should be
printed only if the port is not found at all.

This patch fixes both.

Signed-off-by: Han Zhou 
---
 ovn/utilities/ovn-trace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 1fd48f2..7ca3d97 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -1020,7 +1020,9 @@ ovntrace_lookup_port(const void *dp_, const char 
*port_name,
 *portp = port->tunnel_key;
 return true;
 }
-VLOG_WARN("%s: not in datapath %s", port_name, dp->name);
+/* The port is found but not in this datapath. It happens when port
+ * group is used in match conditions. */
+return false;
 }
 
 const struct ovntrace_mcgroup *mcgroup = ovntrace_mcgroup_find_by_name(dp, 
port_name);
-- 
2.1.0

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

[ovs-dev] [PATCH 2/3] ovn-northd: Improve efficiency of stateful checking for ACLs on port groups.

[Han Zhou]

Currently in has_stateful_acl(), to check if a datapath has stateful ACLs,
it needs to iterate all port groups and check if the current datapath is
related to each port group, and then iterate the ACLs on the port group. This
is inefficient if there are a lot of port groups. A typical scenario is in
OpenStack each tenant will have a default security group which will be mapped
as a port group, and the default security group is supposed to contain ports
of the tenant only, so most likely only the logical switches belonging to the
tenant should be related to the port group, but we are checking all the port
groups belonging to all tenants for each datapath.

To improve this, a reverse direction of hmap is built from logical switch to
port group, so that the iteration is avoided. The time complexity of this
function improves from O(P * A) to O(PL * A), P = total number of port groups
in NB, PL = number of port groups related to the logical switch, A = number
of ACLs.

Signed-off-by: Han Zhou 
---
 ovn/northd/ovn-northd.c | 60 +
 1 file changed, 46 insertions(+), 14 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index d2a777f..ba86bf5 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -439,6 +439,9 @@ struct ovn_datapath {
  * the "redirect-chassis". */
 struct ovn_port *l3redirect_port;
 struct ovn_port *localnet_port;
+
+/* Port groups related to the datapath, used only when nbs is NOT NULL. */
+struct hmap nb_pgs;
 };
 
 struct macam_node {
@@ -467,11 +470,14 @@ ovn_datapath_create(struct hmap *datapaths, const struct 
uuid *key,
 od->nbs = nbs;
 od->nbr = nbr;
 hmap_init(>port_tnlids);
+hmap_init(>nb_pgs);
 od->port_key_hint = 0;
 hmap_insert(datapaths, >key_node, uuid_hash(>key));
 return od;
 }
 
+static void ovn_ls_port_group_destroy(struct hmap *nb_pgs);
+
 static void
 ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od)
 {
@@ -483,6 +489,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct 
ovn_datapath *od)
 destroy_tnlids(>port_tnlids);
 bitmap_free(od->ipam_info.allocated_ipv4s);
 free(od->router_ports);
+ovn_ls_port_group_destroy(>nb_pgs);
 free(od);
 }
 }
@@ -3053,8 +3060,34 @@ ovn_port_group_ls_find(struct ovn_port_group *pg, const 
struct uuid *ls_uuid)
 return NULL;
 }
 
+struct ovn_ls_port_group {
+struct hmap_node key_node;  /* Index on 'key'. */
+struct uuid key;/* nb_pg->header_.uuid. */
+const struct nbrec_port_group *nb_pg;
+};
+
+static void
+ovn_ls_port_group_add(struct hmap *nb_pgs,
+  const struct nbrec_port_group *nb_pg)
+{
+struct ovn_ls_port_group *ls_pg = xzalloc(sizeof *ls_pg);
+ls_pg->key = nb_pg->header_.uuid;
+ls_pg->nb_pg = nb_pg;
+hmap_insert(nb_pgs, _pg->key_node, uuid_hash(_pg->key));
+}
+
+static void
+ovn_ls_port_group_destroy(struct hmap *nb_pgs)
+{
+struct ovn_ls_port_group *ls_pg;
+HMAP_FOR_EACH_POP (ls_pg, key_node, nb_pgs) {
+free(ls_pg);
+}
+hmap_destroy(nb_pgs);
+}
+
 static bool
-has_stateful_acl(struct ovn_datapath *od, struct hmap *port_groups)
+has_stateful_acl(struct ovn_datapath *od)
 {
 for (size_t i = 0; i < od->nbs->n_acls; i++) {
 struct nbrec_acl *acl = od->nbs->acls[i];
@@ -3063,25 +3096,23 @@ has_stateful_acl(struct ovn_datapath *od, struct hmap 
*port_groups)
 }
 }
 
-struct ovn_port_group *pg;
-HMAP_FOR_EACH (pg, key_node, port_groups) {
-if (ovn_port_group_ls_find(pg, >nbs->header_.uuid)) {
-for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
-struct nbrec_acl *acl = pg->nb_pg->acls[i];
-if (!strcmp(acl->action, "allow-related")) {
-return true;
-}
+struct ovn_ls_port_group *ls_pg;
+HMAP_FOR_EACH (ls_pg, key_node, >nb_pgs) {
+for (size_t i = 0; i < ls_pg->nb_pg->n_acls; i++) {
+struct nbrec_acl *acl = ls_pg->nb_pg->acls[i];
+if (!strcmp(acl->action, "allow-related")) {
+return true;
 }
 }
 }
+
 return false;
 }
 
 static void
-build_pre_acls(struct ovn_datapath *od, struct hmap *lflows,
-   struct hmap *port_groups)
+build_pre_acls(struct ovn_datapath *od, struct hmap *lflows)
 {
-bool has_stateful = has_stateful_acl(od, port_groups);
+bool has_stateful = has_stateful_acl(od);
 
 /* Ingress and Egress Pre-ACL Table (Priority 0): Packets are
  * allowed by default. */
@@ -3606,6 +3637,7 @@ build_port_group_lswitches(struct northd_context *ctx, 
struct hmap *pgs,
 ovn_port_group_ls_find(pg, >od->nbs->header_.uuid);
 if (!pg_ls) {
 ovn_port_group_ls_add(pg, op->od->nbs);
+ovn_ls_port_group_add(>od->nb_pgs, nb_pg);
 }
 }
 }
@@ -3615,7 +3647,7 @@ static 

[ovs-dev] [PATCH 1/3] ovn-northd: Simplify struct ovn_port_group.

[Han Zhou]

Remove the redundant members that's already in nb_pg.

Signed-off-by: Han Zhou 
---
 ovn/northd/ovn-northd.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 067d52d..d2a777f 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3027,8 +3027,6 @@ struct ovn_port_group {
 struct uuid key;/* nb_pg->header_.uuid. */
 const struct nbrec_port_group *nb_pg;
 struct hmap nb_lswitches;   /* NB lswitches related to the port group */
-size_t n_acls;  /* Number of ACLs applied to the port group */
-struct nbrec_acl **acls;/* ACLs applied to the port group */
 };
 
 static void
@@ -3068,8 +3066,8 @@ has_stateful_acl(struct ovn_datapath *od, struct hmap 
*port_groups)
 struct ovn_port_group *pg;
 HMAP_FOR_EACH (pg, key_node, port_groups) {
 if (ovn_port_group_ls_find(pg, >nbs->header_.uuid)) {
-for (size_t i = 0; i < pg->n_acls; i++) {
-struct nbrec_acl *acl = pg->acls[i];
+for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
+struct nbrec_acl *acl = pg->nb_pg->acls[i];
 if (!strcmp(acl->action, "allow-related")) {
 return true;
 }
@@ -3558,8 +3556,6 @@ ovn_port_group_create(struct hmap *pgs,
 struct ovn_port_group *pg = xzalloc(sizeof *pg);
 pg->key = nb_pg->header_.uuid;
 pg->nb_pg = nb_pg;
-pg->n_acls = nb_pg->n_acls;
-pg->acls = nb_pg->acls;
 hmap_init(>nb_lswitches);
 hmap_insert(pgs, >key_node, uuid_hash(>key));
 return pg;
@@ -3723,8 +3719,8 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
 struct ovn_port_group *pg;
 HMAP_FOR_EACH (pg, key_node, port_groups) {
 if (ovn_port_group_ls_find(pg, >nbs->header_.uuid)) {
-for (size_t i = 0; i < pg->n_acls; i++) {
-consider_acl(lflows, od, pg->acls[i], has_stateful);
+for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
+consider_acl(lflows, od, pg->nb_pg->acls[i], has_stateful);
 }
 }
 }
-- 
2.1.0

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

[ovs-dev] [PATCH 0/3] Port group related enhancements.

[Han Zhou]

These patches are related to port groups, but the patch 3/3 is
independent from the others.

Han Zhou (3):
  ovn-northd: Simplify struct ovn_port_group.
  ovn-northd: Improve efficiency of stateful checking for ACLs on port
groups.
  ovn-trace: Fix warnings when port is found but not in current
datapath.

 ovn/northd/ovn-northd.c   | 68 +--
 ovn/utilities/ovn-trace.c |  4 ++-
 2 files changed, 51 insertions(+), 21 deletions(-)

-- 
2.1.0

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

Re: [ovs-dev] [patch v4] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

On 8/6/18, 4:15 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

On Mon, Aug 06, 2018 at 03:55:55PM -0700, Darrell Ball wrote:
> By making opt_dpif_open() more general, it can be used effectively
> by all potential callers. Also, the error handling is improved by
> having more specific errors.
> 
> Signed-off-by: Darrell Ball 

I'm not sure I understand the improvement yet.  It looks to me like this
only provides value if the user specifies a dp name that contains @.
I'm not sure that's something that is commonly done; I only do it in
extremis.

Right, name syntax is not enforced; not sure if that is good per se, but 
admittedly,
I had not given this much thought b4. I made a correction.


It looks like the algorithm enumerates all the datapaths and opens them
in turn and then checks whether the name is what the user asked for.  Is
this better than just trying to open what the user asked for and seeing
if it worked?

Overall, this allows for more correct error handling since we don’t try to open
bogus datapaths provided by the user as b4. The new code tries to open 
datapaths that we expect
exist. However, upon revisiting the code, I see no reason to be opening (and 
closing) existing
datapaths in dp_exists(). I just removed this useless part.

Thanks,

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

https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Cdball%40vmware.com%7C61732b29c57c426ba09908d5fbf1c309%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636691941526057539sdata=4zj6MzDxfExCkO%2F%2F8ER534ur0142%2Fm3MvPntexUmF8c%3Dreserved=0






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

[ovs-dev] [patch v5] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

By making opt_dpif_open() more general, it can be used effectively
by all potential callers and avoids trying to open potentially bogus
datapaths provided by the user. Also, the error handling is improved by
having more specific errors.

Signed-off-by: Darrell Ball 
---
 lib/dpctl.c | 61 +
 tests/system-traffic.at |  8 +++
 2 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index c600eeb..b8a8dbf 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -187,18 +187,69 @@ parsed_dpif_open(const char *arg_, bool create, struct 
dpif **dpifp)
 return result;
 }
 
+static bool
+dp_exists(const char *queried_dp)
+{
+struct sset dpif_names = SSET_INITIALIZER(_names),
+dpif_types = SSET_INITIALIZER(_types);
+bool found = false;
+char *queried_name, *queried_type;
+
+dp_parse_name(queried_dp, _name, _type);
+dp_enumerate_types(_types);
+
+const char *type;
+SSET_FOR_EACH (type, _types) {
+int error = dp_enumerate_names(type, _names);
+if (!error) {
+const char *name;
+SSET_FOR_EACH (name, _names) {
+if (!strcmp(type, queried_type) &&
+!strcmp(name, queried_name)) {
+found = true;
+goto out;
+}
+}
+}
+}
+
+out:
+sset_destroy(_names);
+sset_destroy(_types);
+return found;
+}
+
+static bool
+dp_arg_exists(int argc, const char *argv[])
+{
+if (argc > 1 && dp_exists(argv[1])) {
+return true;
+}
+return false;
+}
+
 /* Open a dpif with an optional name argument.
  *
- * The datapath name is not a mandatory parameter for this command.  If
- * it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
- * the current setup, assuming only one exists.  On success stores the
- * opened dpif in '*dpifp'. */
+ * The datapath name is not a mandatory parameter for this command.  If it is
+ * not specified, we retrieve it from the current setup, assuming only one
+ * exists.  On success stores the opened dpif in '*dpifp'. */
 static int
 opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
   uint8_t max_args, struct dpif **dpifp)
 {
+char *dpname;
+if (dp_arg_exists(argc, argv)) {
+dpname = xstrdup(argv[1]);
+} else if (argc != max_args) {
+dpname = get_one_dp(dpctl_p);
+} else {
+/* If the arguments are the maximum possible number and there is no
+ * valid datapath argument, then we fall into the case of dpname is
+ * NULL, since this is an error. */
+dpname = NULL;
+}
+
 int error = 0;
-char *dpname = argc >= max_args ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
 if (!dpname) {
 error = EINVAL;
 dpctl_error(dpctl_p, error, "datapath not found");
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cbd9542..d43d2ee 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1124,17 +1124,17 @@ ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-set-maxconns one-bad-dp 10], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-maxconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-nconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
@@ -1164,7 +1164,7 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
 10
 ])
 
-OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath one-bad-dp of unknown 
type system/d"])
+OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - IPv6 ping])
-- 
1.9.1

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

Re: [ovs-dev] [ovs-dev,v3] tests: Test for ovs-ofctl snoop command

[0-day Robot]

Bleep bloop.  Greetings Ashish Varma, 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.


git-am:
Failed to merge in the changes.
Patch failed at 0001 tests: Test for ovs-ofctl snoop command
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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] project idea for someone: hash function sensitivity

[0-day Robot]

Bleep bloop.  Greetings Ben Pfaff, 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: No signatures found.
Lines checked: 101, Warnings: 0, 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] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[0-day Robot]

Bleep bloop.  Greetings Ben Pfaff, 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.


git-am:
Failed to merge in the changes.
Patch failed at 0001 stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions 
that lack it.
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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, v2] stream-ssl: Tweak recent change to fix travis builds.

[0-day Robot]

Bleep bloop.  Greetings Darrell Ball, 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.


git-am:
Failed to merge in the changes.
Patch failed at 0001 stream-ssl: Tweak recent change to fix travis builds.
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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 v1] ovs-testcontroller: Added section for runtime managment commands

[Ashish Varma]

Even though there are no runtime management commands supported by
ovs-testcontroller, the '--unixctl' section of the man page refers to
'RUNTIME MANAGEMENT COMMANDS'. This message which refers to the runtime
management commands section is common for all '--unixctl' option.
(via 'lib/unixctl.man')

Added an empty section in the man page for 'RUNTIME MANAGEMENT COMMANDS' to
avoid confusion to the reader of the man page.

Signed-off-by: Ashish Varma 
---
 utilities/ovs-testcontroller.8.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/utilities/ovs-testcontroller.8.in 
b/utilities/ovs-testcontroller.8.in
index df3c35f..5ea325f 100644
--- a/utilities/ovs-testcontroller.8.in
+++ b/utilities/ovs-testcontroller.8.in
@@ -146,6 +146,9 @@ Use this option more than once to add flows from multiple 
files.
 .so lib/common.man
 .so lib/ofp-version.man
 .
+.SH "RUNTIME MANAGEMENT COMMANDS"
+There are no runtime management commands defined for \fBovs\-testcontroller\fR.
+.
 .SH EXAMPLES
 .PP
 To bind locally to port 6653 (the default) and wait for incoming
-- 
2.7.4

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

Re: [ovs-dev] [PATCH 1/2] ofp-port: Fix buffer overread parsing Intel custom statistics.

[Ben Pfaff]

On Fri, Jul 27, 2018 at 11:14:43AM -0700, Ben Pfaff wrote:
> CC: Michal Weglicki 
> Fixes: 971f4b394c6e ("netdev: Custom statistics.")
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9445
> Signed-off-by: Ben Pfaff 

Still needs a review.

Thanks,

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

Re: [ovs-dev] [PATCH] checkpatch: warn on possible bare return

[Ben Pfaff]

On Tue, Jul 31, 2018 at 04:37:38PM -0400, Bala Sankaran wrote:
> void functions do not need to have a return statement, because
> such statements are redundant. Warn the user of such instances.
> 
> An interim line check is added to allow gathering additional
> context for each line that is being processed.
> 
> Signed-off-by: Bala Sankaran 

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

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

[Ben Pfaff]

On Wed, Aug 01, 2018 at 07:29:06AM +0530, Aravind Prasad wrote:
>  > > Currently, rule_insert() API does not have return value. There are
> some possible
> > > scenarios where rule insertions can fail at run-time even though the
> static
> > > checks during rule_construct() had passed previously.
> > > Some possible scenarios for failure of rule insertions:
> > > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> > > Normal switch functioning coexist) where the CAM space could get
> suddenly
> > > filled up by Normal switch functioning and Openflow gets devoid of
> > > available space.
> > > **) Some deployments could have separate independent layers for HW rule
> > > insertions and application layer to interact with OVS. HW layer
> > > could face any dynamic issue during rule handling which application
> could
> > > not have predicted/captured in rule-construction phase.
> > > Rule-insert errors for bundles are handled too in this pull-request.
> > >
> > > Signed-off-by: Aravind Prasad S 
> >
> > >Which switches does this help?
> >
> > Hi Ben, These type of errors are possible in actual Hardware
> > implementations of OVS. It is possible that ofproto and netdev providers
> be
> > implemented for an actual HW/NPU. Usually, in such cases, in the rule
> > construct phase, all the static checks like verifying the qualifiers/
> > actions, CAM full checks could be done and the other related
> verifications.
> > But during the rule insert phase, it is possible that the rule insertion
> > may get failed in HW (runtime errors, HW errors and so on). Also, since HW
> > switches may support Hybrid mode (coexistence of Normal and Openflow
> > functioning), the possibility of this issue could be even more. Further,
> > when rule-insertion fails, it results in a stale state where the
> Controller
> > and Switch Flow-DB differ. Hence, we need a way to rollback for
> rule-insert
> > phase also. Kindly let me know your views. And sorry for re-sending the
> > review requests many times over. Will avoid in future.
> 
> > >Which switches does this help?
> 
> Hi Ben,
> 
> By "which" switches, you mean to ask to which vendor switches ?
> In general, this patch will be very useful for all Switches which
> implement OVS for their HW implementations, specifically,
> will be useful for Switches which had implemented ofproto
> and netdev provider APIs for their respective HW/NPU and
> Kernel. And, in case of HW, the possibility of dynamic rule
> insertion failures are usually higher. Static checks during
> rule-construct phase are not sufficient and predicting
> future failure predictions in construct phase may not be possible.
> Hence, this patch will make OVS more flexible to handle such
> rule insertion failures.

So, basically, you're not willing to name any switches.  I'm tired of
the vendor secrecy game.  They get to see all of our code but they're
never willing even to name themselves.

Sorry, I'm not going to willingly take this patch.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs-vtep: Pass log level arguments to underlying utils.

[Ben Pfaff]

On Wed, Aug 01, 2018 at 06:54:22PM +0300, Ilya Maximets wrote:
> Control utils should be called with the same verbose level
> at least to manage output to system logs. For example, to
> disable unwanted syslog messages in unit tests or to enable
> higher debug levels if needed.
> New arguments added before '-vconsole:off' because it's
> still incovinient to have console output.
> 
> Signed-off-by: Ilya Maximets 
> ---
> 
> This could be a part of syslog clean up patch-set [1], but it looks
> more like a separate independent feature.
> 
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/350425.html

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

Re: [ovs-dev] [PATCH] json: Use unnamed embedded union.

[Ben Pfaff]

On Thu, Aug 02, 2018 at 06:52:55PM -0300, Flavio Leitner wrote:
> On Thu, Aug 02, 2018 at 06:44:10PM -0300, Flavio Leitner wrote:
> > Otherwise the code does not build.
> > 
> > Fixes: fa37affad362 ("Embrace anonymous unions.")
> > Signed-off-by: Flavio Leitner 
> 
> This needs to go in branch-2.10 as well.

Apparently I never build this code (oops!).

I applied this to master and branch-2.10.  I didn't actually build or
test it or anything though.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] 答复: [PATCH v2] netlink-notifier: support blacklist

[Ben Pfaff]

What is the cost of the notification?

On Sat, Aug 04, 2018 at 03:58:57AM +, Linhaifeng wrote:
> 
> @@ -1015,6 +1016,8 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>  
>  rte_eth_dev_info_get(dev->port_id, );
>  
> +rtnetlink_blacklist_add(dev->up.name);   // add a ovs-dpdk 
> port to blacklist
> +
> 
> 
> Some DPDK port even use PMD they also have linux interface (like mellanox 
> ethernet)but they don't need 'if change' notification.
> 
> 
> -邮件原件-
> 发件人: Ben Pfaff [mailto:b...@ovn.org] 
> 发送时间: 2018年8月4日 8:25
> 收件人: Linhaifeng 
> 抄送: d...@openvswitch.org
> 主题: Re: [PATCH v2] netlink-notifier: support blacklist
> 
> On Fri, Aug 03, 2018 at 02:10:06PM +0800, Haifeng Lin wrote:
> > in dpdk ovs mode some ether not need rtnetlink notifier so we can use 
> > blacklist remove ethernet from rtnetlink notifier
> > 
> > Signed-off-by: Haifeng Lin 
> 
> Can you explain the benefit of the patch?
> 
> I don't see any way for a port to be removed from the blacklist.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] tests: Test for ovs-ofctl snoop command

[Ben Pfaff]

On Mon, Aug 06, 2018 at 03:53:37PM -0700, Ashish Varma wrote:
> Added test for snoop command to check for the initial handshake messages
> when a bridge connects to a controller via 'unix' connection method.
> 
> Signed-off-by: Ashish Varma 
> ---
> v2-v3:
> 
> Moved 'on_exit kill" command before the start of the command to avoid the 
> race 
> condition between start of the process and registering to have it killed on
> interrupt.
> Removed deletion of snoopbr0.txt to debug any failure.
> Cleaner shell re-direct.

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

Re: [ovs-dev] [PATCH v5 6/9] Documentation: IPsec tunnel tutorial and documentation.

[Qiuyu Xiao]

Yes. This describes the real situation. I will apply this to the next series.

Thanks,
Qiuyu

On Mon, Aug 6, 2018 at 4:25 PM, Ben Pfaff  wrote:
> On Mon, Aug 06, 2018 at 11:04:36AM -0700, Qiuyu Xiao wrote:
>> tutorials/index.rst gives a step-by-setp guide to set up OVS IPsec
>> tunnel.
>>
>> tutorials/ipsec.rst gives detailed explanation on the IPsec tunnel
>> configuration methods and forwarding modes.
>>
>> Signed-off-by: Qiuyu Xiao 
>> Signed-off-by: Ansis Atteka 
>> Co-authored-by: Ansis Atteka 
>
> Following our in-person discussion today, does the following correctly
> reflect the real situation?
>
> Thanks,
>
> Ben.
>
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 3caef4f79539..e9a8d20feece 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -823,11 +823,15 @@
>
>
>  
> -  After an IPsec tunnel is configured, it takes a few round trips to
> -  negotiate details of the encryption with the remote host.  In the
> -  meantime, packets sent by the local host over the tunnel can be
> -  transmitted in plaintext.  This setting controls the behavior in 
> this
> -  situation.
> +  When an IPsec tunnel is configured in this database, multiple
> +  independent components take responsibility for implementing it.
> +  ovs-vswitchd and its datapath handle packet forwarding
> +  to the tunnel and a separate daemon pushes the tunnel's IPsec 
> policy
> +  configuration to the kernel or other entity that implements it.
> +  There is a race: if the former configuration completes before the
> +  latter, then packets sent by the local host over the tunnel can be
> +  transmitted in plaintext.  Using this setting, OVS users can avoid
> +  this undesirable situation.
>  
>type='{"type": "string"}'>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 04:07:03PM -0700, Darrell Ball wrote:
> On Mon, Aug 6, 2018 at 3:53 PM, Ben Pfaff  wrote:
> 
> > On Mon, Aug 06, 2018 at 12:47:39PM -1000, Han Zhou wrote:
> > > On Mon, Aug 6, 2018 at 12:39 PM, Ben Pfaff  wrote:
> > > >
> > > > 10 of the travis builds are failing such as
> > > > TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
> > > >
> > > > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
> > > default")
> > > > CC: Timothy Redaelli 
> > > > Signed-off-by: Darrell Ball 
> > > > Signed-off-by: Ben Pfaff 
> > > > ---
> > > > v1->v2: Add SSL_OP_NO_SSLv2 (thanks Han!).
> > > >
> > > >  lib/stream-ssl.c | 6 ++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > > > index f3d623c035f8..fed71801b823 100644
> > > > --- a/lib/stream-ssl.c
> > > > +++ b/lib/stream-ssl.c
> > > > @@ -1188,6 +1188,12 @@ stream_ssl_set_protocols(const char *arg)
> > > >  }
> > > >
> > > >  /* Start with all the flags off and turn them on as requested. */
> > > > +#ifndef SSL_OP_NO_SSL_MASK
> > > > +/* For old OpenSSL without this macro, this is the correct
> > value.  */
> > > > +#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | \
> > > > +SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1 | \
> > > > +SSL_OP_NO_TLSv1_2)
> > > > +#endif
> > > >  long protocol_flags = SSL_OP_NO_SSL_MASK;
> > > >
> > > >  char *s = xstrdup(arg);
> > > > --
> > > > 2.16.1
> > > >
> > > > ___
> > > > dev mailing list
> > > > d...@openvswitch.org
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> > > Acked-by: Han Zhou 
> >
> > Thanks.
> >
> > Darrell, does this make sense to you?
> >
> 
> 
> It looks fine and works locally; assuming you checked Travis (which I
> expect should be fixed)
> Acked-by: Darrell Ball 

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

Re: [ovs-dev] [ACL Meters 7/7] ovn: Add rate-limiting for ACL logs.

[Han Zhou]

On Mon, Jul 30, 2018 at 8:12 PM, Justin Pettit  wrote:
>
>
> > On Jul 30, 2018, at 5:58 PM, Justin Pettit  wrote:
> >
> > Thanks for the review!  I've pushed this series to master.
>
> I also just pushed this to branch-2.10.
>
> The rate-limiting is implemented using meters.  Unfortunately, there's a
bug in the current kernels that prevents meters from working properly.
I've sent a patch to upstream Linux, which should solve the problem:
>
> https://marc.info/?l=linux-netdev=153281677212826=2
>
> Once that is merged, I'll commit it to OVS, and it should work with both
Linux and userspace datapaths.
>
> --Justin
>
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Justin for the great work!!
Sorry that I didn't get time to review the series, just some quick
questions regarding the kernel bug you mentioned.

- What's the impact of having meter ID = 0? Does it mean the meters and ACL
rate limit feature can't be used at all, or is it just some limitations in
certain scenarios?

- For the bug fix, can we get it reviewed (and probably merged) in
datapath/compact first here in the OVS community without having to wait for
kernel upstream? What's the usual practice for kernel module patches?

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

Re: [ovs-dev] [PATCH v5 6/9] Documentation: IPsec tunnel tutorial and documentation.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 11:04:36AM -0700, Qiuyu Xiao wrote:
> tutorials/index.rst gives a step-by-setp guide to set up OVS IPsec
> tunnel.
> 
> tutorials/ipsec.rst gives detailed explanation on the IPsec tunnel
> configuration methods and forwarding modes.
> 
> Signed-off-by: Qiuyu Xiao 
> Signed-off-by: Ansis Atteka 
> Co-authored-by: Ansis Atteka 

Following our in-person discussion today, does the following correctly
reflect the real situation?

Thanks,

Ben.

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 3caef4f79539..e9a8d20feece 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -823,11 +823,15 @@
 
   
 
-  After an IPsec tunnel is configured, it takes a few round trips to
-  negotiate details of the encryption with the remote host.  In the
-  meantime, packets sent by the local host over the tunnel can be
-  transmitted in plaintext.  This setting controls the behavior in this
-  situation.
+  When an IPsec tunnel is configured in this database, multiple
+  independent components take responsibility for implementing it.
+  ovs-vswitchd and its datapath handle packet forwarding
+  to the tunnel and a separate daemon pushes the tunnel's IPsec policy
+  configuration to the kernel or other entity that implements it.
+  There is a race: if the former configuration completes before the
+  latter, then packets sent by the local host over the tunnel can be
+  transmitted in plaintext.  Using this setting, OVS users can avoid
+  this undesirable situation.
 
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v4] dpctl: Make opt_dpif_open() more general.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 03:55:55PM -0700, Darrell Ball wrote:
> By making opt_dpif_open() more general, it can be used effectively
> by all potential callers. Also, the error handling is improved by
> having more specific errors.
> 
> Signed-off-by: Darrell Ball 

I'm not sure I understand the improvement yet.  It looks to me like this
only provides value if the user specifies a dp name that contains @.
I'm not sure that's something that is commonly done; I only do it in
extremis.

It looks like the algorithm enumerates all the datapaths and opens them
in turn and then checks whether the name is what the user asked for.  Is
this better than just trying to open what the user asked for and seeing
if it worked?

Thanks,

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

Re: [ovs-dev] [PATCH v2] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[Darrell Ball]

On Mon, Aug 6, 2018 at 3:53 PM, Ben Pfaff  wrote:

> On Mon, Aug 06, 2018 at 12:47:39PM -1000, Han Zhou wrote:
> > On Mon, Aug 6, 2018 at 12:39 PM, Ben Pfaff  wrote:
> > >
> > > 10 of the travis builds are failing such as
> > > TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
> > >
> > > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
> > default")
> > > CC: Timothy Redaelli 
> > > Signed-off-by: Darrell Ball 
> > > Signed-off-by: Ben Pfaff 
> > > ---
> > > v1->v2: Add SSL_OP_NO_SSLv2 (thanks Han!).
> > >
> > >  lib/stream-ssl.c | 6 ++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > > index f3d623c035f8..fed71801b823 100644
> > > --- a/lib/stream-ssl.c
> > > +++ b/lib/stream-ssl.c
> > > @@ -1188,6 +1188,12 @@ stream_ssl_set_protocols(const char *arg)
> > >  }
> > >
> > >  /* Start with all the flags off and turn them on as requested. */
> > > +#ifndef SSL_OP_NO_SSL_MASK
> > > +/* For old OpenSSL without this macro, this is the correct
> value.  */
> > > +#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | \
> > > +SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1 | \
> > > +SSL_OP_NO_TLSv1_2)
> > > +#endif
> > >  long protocol_flags = SSL_OP_NO_SSL_MASK;
> > >
> > >  char *s = xstrdup(arg);
> > > --
> > > 2.16.1
> > >
> > > ___
> > > dev mailing list
> > > d...@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> > Acked-by: Han Zhou 
>
> Thanks.
>
> Darrell, does this make sense to you?
>


It looks fine and works locally; assuming you checked Travis (which I
expect should be fixed)
Acked-by: Darrell Ball 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [patch v4] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

By making opt_dpif_open() more general, it can be used effectively
by all potential callers. Also, the error handling is improved by
having more specific errors.

Signed-off-by: Darrell Ball 
---
 lib/dpctl.c | 71 +
 tests/system-traffic.at |  8 +++---
 2 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index c600eeb..51af4ff 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -187,18 +187,79 @@ parsed_dpif_open(const char *arg_, bool create, struct 
dpif **dpifp)
 return result;
 }
 
+static bool
+dp_exists(const char *queried_dp)
+{
+struct sset dpif_names = SSET_INITIALIZER(_names),
+dpif_types = SSET_INITIALIZER(_types);
+int error = 0;
+bool found = false;
+struct ds ds = DS_EMPTY_INITIALIZER;;
+
+dp_enumerate_types(_types);
+
+const char *type;
+SSET_FOR_EACH (type, _types) {
+error = dp_enumerate_names(type, _names);
+if (!error) {
+const char *name;
+SSET_FOR_EACH (name, _names) {
+struct dpif *dpif;
+error = dpif_open(name, type, );
+if (!error) {
+dpif_close(dpif);
+ds_destroy();
+ds_init();
+ds_put_cstr(, type);
+ds_put_cstr(, "@");
+ds_put_cstr(, name);
+if (!strcmp(queried_dp, ds_cstr())) {
+found = true;
+goto out;
+}
+}
+}
+}
+}
+
+out:
+ds_destroy();
+sset_destroy(_names);
+sset_destroy(_types);
+return found;
+}
+
+static bool
+dp_arg_exists(int argc, const char *argv[])
+{
+if (argc > 1 && dp_exists(argv[1])) {
+return true;
+}
+return false;
+}
+
 /* Open a dpif with an optional name argument.
  *
- * The datapath name is not a mandatory parameter for this command.  If
- * it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
- * the current setup, assuming only one exists.  On success stores the
- * opened dpif in '*dpifp'. */
+ * The datapath name is not a mandatory parameter for this command.  If it is
+ * not specified, we retrieve it from the current setup, assuming only one
+ * exists.  On success stores the opened dpif in '*dpifp'. */
 static int
 opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
   uint8_t max_args, struct dpif **dpifp)
 {
+char *dpname;
+if (dp_arg_exists(argc, argv)) {
+dpname = xstrdup(argv[1]);
+} else if (argc != max_args) {
+dpname = get_one_dp(dpctl_p);
+} else {
+/* If the arguments are the maximum possible number and there is no
+ * valid datapath argument, then we fall into the case of dpname is
+ * NULL, since this is an error. */
+dpname = NULL;
+}
+
 int error = 0;
-char *dpname = argc >= max_args ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
 if (!dpname) {
 error = EINVAL;
 dpctl_error(dpctl_p, error, "datapath not found");
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cbd9542..d43d2ee 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1124,17 +1124,17 @@ ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-set-maxconns one-bad-dp 10], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-maxconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-nconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
@@ -1164,7 +1164,7 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
 10
 ])
 
-OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath one-bad-dp of unknown 
type system/d"])
+OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - IPv6 ping])
-- 
1.9.1

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

[ovs-dev] [PATCH v3] tests: Test for ovs-ofctl snoop command

[Ashish Varma]

Added test for snoop command to check for the initial handshake messages
when a bridge connects to a controller via 'unix' connection method.

Signed-off-by: Ashish Varma 
---
v2-v3:

Moved 'on_exit kill" command before the start of the command to avoid the race 
condition between start of the process and registering to have it killed on
interrupt.
Removed deletion of snoopbr0.txt to debug any failure.
Cleaner shell re-direct.

---
 tests/ovs-ofctl.at | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 06597d7..21a576a 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -3184,3 +3184,30 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: zone 123" 
ovs-vswitchd.log])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+
+AT_SETUP([ovs-ofctl snoop-unix-connection])
+OVS_VSWITCHD_START
+
+dnl setup controller for br0 before starting the controller
+AT_CHECK([ovs-vsctl -vsyslog:off set-controller br0 unix:testcontroller])
+
+dnl then start listening on the '.snoop' connection
+on_exit 'kill `cat ovs-ofctl-snoop.pid`'
+AT_CHECK([ovs-ofctl -vsyslog:off --detach --pidfile=ovs-ofctl-snoop.pid snoop 
br0 > snoopbr0.txt 2>&1])
+
+dnl finally start the controller
+on_exit 'kill `cat ovs-testcontroller.pid`'
+AT_CHECK([ovs-testcontroller -vsyslog:off --detach --pidfile 
punix:testcontroller], [0], [ignore])
+OVS_WAIT_UNTIL([test -e testcontroller])
+
+dnl check for some of the initial handshake messages
+OVS_WAIT_UNTIL([egrep "OFPT_FEATURES_REQUEST" snoopbr0.txt 1> /dev/null 2>&1])
+OVS_WAIT_UNTIL([egrep "OFPT_FEATURES_REPLY" snoopbr0.txt 1> /dev/null 2>&1])
+OVS_WAIT_UNTIL([egrep "OFPT_SET_CONFIG" snoopbr0.txt 1> /dev/null 2>&1])
+
+dnl need to suppress the 'connection failed' WARN message in ovs-vswitchd
+dnl because we need ovs-vswitchd to have the controller config before starting
+dnl the controller to 'snoop' the OpenFlow messages from beginning
+OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
+AT_CLEANUP
-- 
2.7.4

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

Re: [ovs-dev] [PATCH v2] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 12:47:39PM -1000, Han Zhou wrote:
> On Mon, Aug 6, 2018 at 12:39 PM, Ben Pfaff  wrote:
> >
> > 10 of the travis builds are failing such as
> > TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
> >
> > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
> default")
> > CC: Timothy Redaelli 
> > Signed-off-by: Darrell Ball 
> > Signed-off-by: Ben Pfaff 
> > ---
> > v1->v2: Add SSL_OP_NO_SSLv2 (thanks Han!).
> >
> >  lib/stream-ssl.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > index f3d623c035f8..fed71801b823 100644
> > --- a/lib/stream-ssl.c
> > +++ b/lib/stream-ssl.c
> > @@ -1188,6 +1188,12 @@ stream_ssl_set_protocols(const char *arg)
> >  }
> >
> >  /* Start with all the flags off and turn them on as requested. */
> > +#ifndef SSL_OP_NO_SSL_MASK
> > +/* For old OpenSSL without this macro, this is the correct value.  */
> > +#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | \
> > +SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1 | \
> > +SSL_OP_NO_TLSv1_2)
> > +#endif
> >  long protocol_flags = SSL_OP_NO_SSL_MASK;
> >
> >  char *s = xstrdup(arg);
> > --
> > 2.16.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> Acked-by: Han Zhou 

Thanks.

Darrell, does this make sense to you?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] project idea for someone: hash function sensitivity

[Ben Pfaff]

In theory, all the OVS tests should pass if we change the hash
function.  In practice, some of them fail because changing the hash
function changes ordering of some of the output of the tests.  Currently
I see the following failures:

 805: tunnel.at:512  tunnel - ERSPAN v1/v2 metadata
 1038: dpif-netdev.at:193 dpif-netdev - meters
 1174: ofproto-dpif.at:6653 ofproto-dpif - sFlow packet sampling - tunnel push
 1783: ovsdb-execution.at:1140 index uniqueness checking
  ovsdb execute execution positive
 1813: ovsdb-tool.at:29   index uniqueness checking
  ovsdb file positive
 1850: ovsdb-server.at:41 index uniqueness checking
  ovsdb server positive unix
 1891: ovsdb-server.at:1248 index uniqueness checking
  ovsdb server positive ssl
 1917: ovsdb-server.at:1322 index uniqueness checking
  ovsdb server positive tcp
 1943: ovsdb-server.at:1395 index uniqueness checking
  ovsdb server positive transient
 2286: ovsdb-cluster.at:45 index uniqueness checking - cluster of 1
  ovsdb server positive unix cluster cluster1
 2311: ovsdb-cluster.at:54 index uniqueness checking - cluster of 3
  ovsdb server positive unix cluster cluster3
 2336: ovsdb-cluster.at:63 index uniqueness checking - cluster of 5
  ovsdb server positive unix cluster cluster5
 2342: ovsdb-cluster.at:217 OVSDB 3-server torture test - kill/restart leader
  ovsdb server positive unix cluster cluster3
 2343: ovsdb-cluster.at:221 OVSDB 3-server torture test - kill/restart follower 
1
  ovsdb server positive unix cluster cluster3
 2344: ovsdb-cluster.at:225 OVSDB 3-server torture test - kill/restart follower 
2
  ovsdb server positive unix cluster cluster3
 2345: ovsdb-cluster.at:229 OVSDB 5-server torture test - kill/restart leader
  ovsdb server positive unix cluster cluster5
 2346: ovsdb-cluster.at:233 OVSDB 5-server torture test - kill/restart follower 
1
  ovsdb server positive unix cluster cluster5
 2347: ovsdb-cluster.at:237 OVSDB 5-server torture test - kill/restart follower 
2
  ovsdb server positive unix cluster cluster5
 2348: ovsdb-cluster.at:241 OVSDB 5-server torture test - kill/restart follower 
3
  ovsdb server positive unix cluster cluster5
 2349: ovsdb-cluster.at:245 OVSDB 5-server torture test - kill/restart follower 
4
  ovsdb server positive unix cluster cluster5
 2350: ovsdb-cluster.at:250 OVSDB 3-server torture test - remove/re-add leader
  ovsdb server positive unix cluster cluster3
 2351: ovsdb-cluster.at:254 OVSDB 3-server torture test - remove/re-add 
follower 1
  ovsdb server positive unix cluster cluster3
 2352: ovsdb-cluster.at:258 OVSDB 3-server torture test - remove/re-add 
follower 2
  ovsdb server positive unix cluster cluster3
 2353: ovsdb-cluster.at:262 OVSDB 5-server torture test - remove/re-add leader
  ovsdb server positive unix cluster cluster5
 2354: ovsdb-cluster.at:266 OVSDB 5-server torture test - remove/re-add 
follower 1
  ovsdb server positive unix cluster cluster5
 2355: ovsdb-cluster.at:270 OVSDB 5-server torture test - remove/re-add 
follower 2
  ovsdb server positive unix cluster cluster5
 2356: ovsdb-cluster.at:274 OVSDB 5-server torture test - remove/re-add 
follower 3
  ovsdb server positive unix cluster cluster5
 2357: ovsdb-cluster.at:278 OVSDB 5-server torture test - remove/re-add 
follower 4
  ovsdb server positive unix cluster cluster5

Your mission, if you choose to accept it, is to fix some of these.
Sometimes this amounts to, for example, adding a "sort" filter to the
testsuite itself, or by sorting output in ofp-print.c or ovs-ofctl.c.
There might be other bugs exposed other ways, too.

You can probably reproduce these test failures by applying the following
patch:

diff --git a/lib/hash.h b/lib/hash.h
index a642a1e97954..30aeb6de5d38 100644
--- a/lib/hash.h
+++ b/lib/hash.h
@@ -87,7 +87,7 @@ static inline uint32_t mhash_finish(uint32_t hash)
 hash ^= hash >> 13;
 hash *= 0xc2b2ae35;
 hash ^= hash >> 16;
-return hash;
+return ~hash;
 }
 
 static inline uint32_t hash_add(uint32_t hash, uint32_t data);
@@ -350,7 +350,7 @@ static inline uint32_t hash_boolean(bool x, uint32_t basis)
 {
 const uint32_t P0 = 0xc2b73583;   /* This is hash_int(1, 0). */
 const uint32_t P1 = 0xe90f1258;   /* This is hash_int(2, 0). */
-return (x ? P0 : P1) ^ hash_rot(basis, 1);
+return ~(x ? P0 : P1) ^ hash_rot(basis, 1);
 }
 
 /* Helper functions for calling hash_add() for several 32- or 64-bit words in a
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[Han Zhou]

On Mon, Aug 6, 2018 at 12:39 PM, Ben Pfaff  wrote:
>
> 10 of the travis builds are failing such as
> TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
>
> Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
default")
> CC: Timothy Redaelli 
> Signed-off-by: Darrell Ball 
> Signed-off-by: Ben Pfaff 
> ---
> v1->v2: Add SSL_OP_NO_SSLv2 (thanks Han!).
>
>  lib/stream-ssl.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index f3d623c035f8..fed71801b823 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -1188,6 +1188,12 @@ stream_ssl_set_protocols(const char *arg)
>  }
>
>  /* Start with all the flags off and turn them on as requested. */
> +#ifndef SSL_OP_NO_SSL_MASK
> +/* For old OpenSSL without this macro, this is the correct value.  */
> +#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | \
> +SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1 | \
> +SSL_OP_NO_TLSv1_2)
> +#endif
>  long protocol_flags = SSL_OP_NO_SSL_MASK;
>
>  char *s = xstrdup(arg);
> --
> 2.16.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] ofctl: Fixup compare_flows function

[0-day Robot]

Bleep bloop.  Greetings Alin Gabriel Serdean, 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: Too many signoffs; are you missing Co-authored-by lines?
ERROR: Co-authored-by/Signed-off-by corruption
Lines checked: 46, Warnings: 0, Errors: 2


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 v2] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[Ben Pfaff]

10 of the travis builds are failing such as
TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.

Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by default")
CC: Timothy Redaelli 
Signed-off-by: Darrell Ball 
Signed-off-by: Ben Pfaff 
---
v1->v2: Add SSL_OP_NO_SSLv2 (thanks Han!).

 lib/stream-ssl.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index f3d623c035f8..fed71801b823 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -1188,6 +1188,12 @@ stream_ssl_set_protocols(const char *arg)
 }
 
 /* Start with all the flags off and turn them on as requested. */
+#ifndef SSL_OP_NO_SSL_MASK
+/* For old OpenSSL without this macro, this is the correct value.  */
+#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | \
+SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1 | \
+SSL_OP_NO_TLSv1_2)
+#endif
 long protocol_flags = SSL_OP_NO_SSL_MASK;
 
 char *s = xstrdup(arg);
-- 
2.16.1

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

Re: [ovs-dev] [PATCH] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 12:23:27PM -1000, Han Zhou wrote:
> On Mon, Aug 6, 2018 at 12:07 PM, Ben Pfaff  wrote:
> >
> > 10 of the travis builds are failing such as
> > TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
> >
> > I don't know why SSL_OP_NO_SSL_MASK doesn't include SSL_OP_NO_SSLv2,
> > but it doesn't in my copy of the appropriate header file.
> >
> > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
> default")
> > CC: Timothy Redaelli 
> > Signed-off-by: Darrell Ball 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/stream-ssl.c | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > index f3d623c035f8..bd4f2ff50dbc 100644
> > --- a/lib/stream-ssl.c
> > +++ b/lib/stream-ssl.c
> > @@ -1188,6 +1188,11 @@ stream_ssl_set_protocols(const char *arg)
> >  }
> >
> >  /* Start with all the flags off and turn them on as requested. */
> > +#ifndef SSL_OP_NO_SSL_MASK
> > +/* For old OpenSSL without this macro, this is the correct value.  */
> > +#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | \
> > +SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2)
> > +#endif
> >  long protocol_flags = SSL_OP_NO_SSL_MASK;
> >
> >  char *s = xstrdup(arg);
> > --
> > 2.16.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> Hi Ben,
> 
> In my environment I see it as:
> 
> #define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2|SSL_OP_NO_SSLv3|\
> SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2)
> 
> My openssl version is:
> Version : 1.0.2m

I've got headers for 1.1.0e here.

I guess that adding NO_SSLv2 should be harmless.  v2 coming up...
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v1] stream-ssl: Revert recent chamge to fix travis builds.

[Darrell Ball]

On Mon, Aug 6, 2018 at 3:18 PM, Han Zhou  wrote:

>
>
> On Mon, Aug 6, 2018 at 12:03 PM, Darrell Ball  wrote:
>
>>
>>
>> On Mon, Aug 6, 2018 at 1:37 PM, Han Zhou  wrote:
>>
>>>
>>>
>>> On Mon, Aug 6, 2018 at 9:59 AM, Darrell Ball  wrote:
>>> >
>>> > Sure, and probably s/chamge/change/ would be good
>>> >
>>> > On 8/6/18, 12:32 PM, "ovs-dev-boun...@openvswitch.org on behalf of
>>> Ben Pfaff" 
>>> wrote:
>>> >
>>> > On Mon, Aug 06, 2018 at 12:02:42PM -0700, Darrell Ball wrote:
>>> > > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions
>>> by default")
>>> > > CC: Timothy Redaelli 
>>> > > Signed-off-by: Darrell Ball 
>>> >
>>> > It'd be nice to cite one of the failing builds in the commit
>>> message.
>>> >
>>> > Timothy, do you want to comment on this?
>>> >
>>> > Thanks,
>>> >
>>> > Ben.
>>> > ___
>>> > dev mailing list
>>> > d...@openvswitch.org
>>> > https://na01.safelinks.protection.outlook.com/?url=https%3A%
>>> 2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&
>>> amp;data=02%7C01%7Cdball%40vmware.com%7C39a899ad6f0d47c80f61
>>> 08d5fbd3668c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%
>>> 7C636691807749274687sdata=n1WruYqqMBdAB4maQ3XbYR3ZN%
>>> 2B7MG%2FABnCHXtHqD0eM%3Dreserved=0
>>> >
>>> >
>>> > ___
>>> > dev mailing list
>>> > d...@openvswitch.org
>>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>> Is this something really should be reverted? I hit the same problem but
>>> I figured out it was because of my openssl version is old.
>>>
>>
>>
>> well, in this case, Travis is failing for Openvswitch repo
>>
>> One example:
>>
>> https://travis-ci.org/openvswitch/ovs/jobs/412223381
>>
>>
>> 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 -Wno-null-pointer-arithmetic -Werror -Wsparse-error -MT 
>> lib/dhparams.lo -MD -MP -MF lib/.deps/dhparams.Tpo -c lib/dhparams.c -o 
>> lib/dhparams.o
>> lib/stream-ssl.c:1191:27: error: undefined identifier 'SSL_OP_NO_SSL_MASK'
>> make[2]: *** [lib/stream-ssl.lo] Error 1
>> make[2]: *** Waiting for unfinished jobs
>> make[2]: Leaving directory `/home/travis/build/openvswitch/ovs'
>> make[1]: *** [all-recursive] Error 1
>> make[1]: Leaving directory `/home/travis/build/openvswitch/ovs'
>> make: *** [all] Error 2
>>
>>
>>
>> Hi Darrell, yes this is the same error I encountered. Using a newer
> version of openssl (e.g. 1.0.2m) fixes the problem. The macro
> SSL_OP_NO_SSL_MASK is defined in openssl/ssl.h:
>
> #define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2|SSL_OP_NO_SSLv3|\
> SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2)
>
>

That is fine, but this is nothing specific to do with my environment. It is
the Travis build environment that is being used for Openvswitch.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v2] stream-ssl: Tweak recent change to fix travis builds.

[Darrell Ball]

On Mon, Aug 6, 2018 at 3:07 PM, Ben Pfaff  wrote:

> On Mon, Aug 06, 2018 at 02:54:12PM -0700, Darrell Ball wrote:
> > 10 of the travis builds are failing such as
> > TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
> >
> > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
> default")
> > CC: Timothy Redaelli 
> > Signed-off-by: Darrell Ball 
> > ---
> >  lib/stream-ssl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > index f3d623c..0d5e0de 100644
> > --- a/lib/stream-ssl.c
> > +++ b/lib/stream-ssl.c
> > @@ -1188,7 +1188,7 @@ stream_ssl_set_protocols(const char *arg)
> >  }
> >
> >  /* Start with all the flags off and turn them on as requested. */
> > -long protocol_flags = SSL_OP_NO_SSL_MASK;
> > +long protocol_flags = 0;
> >
> >  char *s = xstrdup(arg);
> >  char *save_ptr = NULL;
>
> I believe that this defeats the purpose of the original patch.
>

True, it was really meant as a revert.



> How about this:
> https://patchwork.ozlabs.org/patch/954255/



I'll take a look
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[Han Zhou]

On Mon, Aug 6, 2018 at 12:07 PM, Ben Pfaff  wrote:
>
> 10 of the travis builds are failing such as
> TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
>
> I don't know why SSL_OP_NO_SSL_MASK doesn't include SSL_OP_NO_SSLv2,
> but it doesn't in my copy of the appropriate header file.
>
> Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
default")
> CC: Timothy Redaelli 
> Signed-off-by: Darrell Ball 
> Signed-off-by: Ben Pfaff 
> ---
>  lib/stream-ssl.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index f3d623c035f8..bd4f2ff50dbc 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -1188,6 +1188,11 @@ stream_ssl_set_protocols(const char *arg)
>  }
>
>  /* Start with all the flags off and turn them on as requested. */
> +#ifndef SSL_OP_NO_SSL_MASK
> +/* For old OpenSSL without this macro, this is the correct value.  */
> +#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | \
> +SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2)
> +#endif
>  long protocol_flags = SSL_OP_NO_SSL_MASK;
>
>  char *s = xstrdup(arg);
> --
> 2.16.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Hi Ben,

In my environment I see it as:

#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2|SSL_OP_NO_SSLv3|\
SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2)

My openssl version is:
Version : 1.0.2m

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

Re: [ovs-dev] [PATCH] utilities: Launch ovsdb-tool without using PAM

[Ben Pfaff]

On Mon, Aug 06, 2018 at 08:33:46AM -0400, Aaron Conole wrote:
> Timothy Redaelli  writes:
> 
> > When ovsdb-server is starting, it performs some DB steps such as
> > creating and upgrading the OvS DB. When we are running as
> > 'non-root' user, the 'runuser' tool is used to manage the privileges.
> > However, when this happens during systemd boot, we observe the following
> > errors in journald:
> >
> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to
> > scope's control group: No such process
> > Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
> > openvswitch.
> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed 
> > state.
> >
> > According to the analysis performed on openSUSE bugzilla[1], it seems
> > that ovsdb-server.service creates (via the call to runuser) a user
> > session and therefore call pam_systemd which in its turn tries to start
> > a systemd user instance: "user@474.service". However "user@474.service"
> > is supposed to be started after systemd-user-sessions.service which is
> > supposed to be started after network.target. Additionally,
> > ovsdb-server.service uses Before=network.target hence the deadlock.
> >
> > This commit uses "setpriv" instead of "runuser" to launch "ovsdb-tool" that
> > doesn't use PAM and so it permits to launch "ovsdb-tool" as a user without
> > having the deadlock. Since some old versions for "setpriv" (such as the
> > one used by RHEL7) doesn't support the username / groupname, but only the
> > user ids / group ids, "id" is used to get the user ID and the group IDs.
> > To replicate the same behaviour of "runuser", the effective group ID of
> > the user is used as GID (usually "openvswitch") and the remaining group
> > IDs are used as supplementary groups (usually "hugetlbfs", if OVS is
> > built with DPDK support).
> >
> > [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
> > Reported-by: Markos Chandras 
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349716.html
> > Co-authored-by: Aaron Conole 
> > Signed-off-by: Timothy Redaelli 
> > ---
> 
> Thanks all.
> 
> Signed-off-by: Aaron Conole 

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

Re: [ovs-dev] [patch v1] stream-ssl: Revert recent chamge to fix travis builds.

[Han Zhou]

On Mon, Aug 6, 2018 at 12:03 PM, Darrell Ball  wrote:

>
>
> On Mon, Aug 6, 2018 at 1:37 PM, Han Zhou  wrote:
>
>>
>>
>> On Mon, Aug 6, 2018 at 9:59 AM, Darrell Ball  wrote:
>> >
>> > Sure, and probably s/chamge/change/ would be good
>> >
>> > On 8/6/18, 12:32 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben
>> Pfaff"  wrote:
>> >
>> > On Mon, Aug 06, 2018 at 12:02:42PM -0700, Darrell Ball wrote:
>> > > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions
>> by default")
>> > > CC: Timothy Redaelli 
>> > > Signed-off-by: Darrell Ball 
>> >
>> > It'd be nice to cite one of the failing builds in the commit
>> message.
>> >
>> > Timothy, do you want to comment on this?
>> >
>> > Thanks,
>> >
>> > Ben.
>> > ___
>> > dev mailing list
>> > d...@openvswitch.org
>> > https://na01.safelinks.protection.outlook.com/?url=https%3A%
>> 2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-
>> devdata=02%7C01%7Cdball%40vmware.com%7C39a899ad6f0d47c
>> 80f6108d5fbd3668c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%
>> 7C0%7C636691807749274687sdata=n1WruYqqMBdAB4maQ3XbYR3ZN
>> %2B7MG%2FABnCHXtHqD0eM%3Dreserved=0
>> >
>> >
>> > ___
>> > dev mailing list
>> > d...@openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>> Is this something really should be reverted? I hit the same problem but I
>> figured out it was because of my openssl version is old.
>>
>
>
> well, in this case, Travis is failing for Openvswitch repo
>
> One example:
>
> https://travis-ci.org/openvswitch/ovs/jobs/412223381
>
>
> 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 -Wno-null-pointer-arithmetic -Werror -Wsparse-error -MT 
> lib/dhparams.lo -MD -MP -MF lib/.deps/dhparams.Tpo -c lib/dhparams.c -o 
> lib/dhparams.o
> lib/stream-ssl.c:1191:27: error: undefined identifier 'SSL_OP_NO_SSL_MASK'
> make[2]: *** [lib/stream-ssl.lo] Error 1
> make[2]: *** Waiting for unfinished jobs
> make[2]: Leaving directory `/home/travis/build/openvswitch/ovs'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory `/home/travis/build/openvswitch/ovs'
> make: *** [all] Error 2
>
>
>
> Hi Darrell, yes this is the same error I encountered. Using a newer
version of openssl (e.g. 1.0.2m) fixes the problem. The macro
SSL_OP_NO_SSL_MASK is defined in openssl/ssl.h:

#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2|SSL_OP_NO_SSLv3|\
SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs-ofctl: Better validate OpenFlow message length in "ofp-parse-pcap".

[Ben Pfaff]

On Mon, Aug 06, 2018 at 08:52:12AM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > Reported-by: Oscar Wilde 
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047070.html
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> Acked-by: Aaron Conole 

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

Re: [ovs-dev] [PATCH v2] tests: Test for ovs-ofctl snoop command

[Ben Pfaff]

On Mon, Aug 06, 2018 at 02:06:09PM -0700, Ashish Varma wrote:
> Added test for snoop command to check for the initial handshake messages
> when a bridge connects to a controller via 'unix' connection method.
> 
> Signed-off-by: Ashish Varma 
> ---
> v1-v2:
> 
> Removed the sleep and added OVS_WAIT_UNTIL.
> Added comment on why we are adding an exception for the 'connection failed'
> WARN log message.
> Added '-vsyslog:off'.
> Minor change for pid file name change.

Thanks for the patch!  It's great to improve test coverage.

It's a little unusual to write "1>file" in shell.  Usually one just
writes ">file".

It's a little better to move the on_exit "kill" command before the
command that starts the process.  That way, there is no race between
starting the process and registering to have it killed on interrupt.

I don't know of a reason to delete snoopbr0.txt when the test exits.
If there is no reason, it is better to leave it, because on failure it
allows the developer to examine it to figure out the reason for the
failure.

Even if it should be deleted, I don't know why the "unlink" program
would be used (I had to look it up to make sure it really exists).  I do
not think that it is portable.  "rm" is available everywhere.

Thanks!

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

[ovs-dev] [PATCH] stream-ssl: Define SSL_OP_NO_SSL_MASK for OpenSSL versions that lack it.

[Ben Pfaff]

10 of the travis builds are failing such as
TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.

I don't know why SSL_OP_NO_SSL_MASK doesn't include SSL_OP_NO_SSLv2,
but it doesn't in my copy of the appropriate header file.

Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by default")
CC: Timothy Redaelli 
Signed-off-by: Darrell Ball 
Signed-off-by: Ben Pfaff 
---
 lib/stream-ssl.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index f3d623c035f8..bd4f2ff50dbc 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -1188,6 +1188,11 @@ stream_ssl_set_protocols(const char *arg)
 }
 
 /* Start with all the flags off and turn them on as requested. */
+#ifndef SSL_OP_NO_SSL_MASK
+/* For old OpenSSL without this macro, this is the correct value.  */
+#define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | \
+SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2)
+#endif
 long protocol_flags = SSL_OP_NO_SSL_MASK;
 
 char *s = xstrdup(arg);
-- 
2.16.1

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

Re: [ovs-dev] [patch v2] stream-ssl: Tweak recent change to fix travis builds.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 02:54:12PM -0700, Darrell Ball wrote:
> 10 of the travis builds are failing such as
> TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
> 
> Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by default")
> CC: Timothy Redaelli 
> Signed-off-by: Darrell Ball 
> ---
>  lib/stream-ssl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index f3d623c..0d5e0de 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -1188,7 +1188,7 @@ stream_ssl_set_protocols(const char *arg)
>  }
>  
>  /* Start with all the flags off and turn them on as requested. */
> -long protocol_flags = SSL_OP_NO_SSL_MASK;
> +long protocol_flags = 0;
>  
>  char *s = xstrdup(arg);
>  char *save_ptr = NULL;

I believe that this defeats the purpose of the original patch.
How about this:
https://patchwork.ozlabs.org/patch/954255/
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v1] stream-ssl: Revert recent chamge to fix travis builds.

[Darrell Ball]

On Mon, Aug 6, 2018 at 1:37 PM, Han Zhou  wrote:

>
>
> On Mon, Aug 6, 2018 at 9:59 AM, Darrell Ball  wrote:
> >
> > Sure, and probably s/chamge/change/ would be good
> >
> > On 8/6/18, 12:32 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben
> Pfaff"  wrote:
> >
> > On Mon, Aug 06, 2018 at 12:02:42PM -0700, Darrell Ball wrote:
> > > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
> default")
> > > CC: Timothy Redaelli 
> > > Signed-off-by: Darrell Ball 
> >
> > It'd be nice to cite one of the failing builds in the commit message.
> >
> > Timothy, do you want to comment on this?
> >
> > Thanks,
> >
> > Ben.
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://na01.safelinks.protection.outlook.com/?url=
> https%3A%2F%2Fmail.openvswitch.org%2Fmailman%
> 2Flistinfo%2Fovs-devdata=02%7C01%7Cdball%40vmware.com%
> 7C39a899ad6f0d47c80f6108d5fbd3668c%7Cb39138ca3cee4b4aa4d6cd83d9dd
> 62f0%7C1%7C0%7C636691807749274687sdata=n1WruYqqMBdAB4maQ3XbYR3ZN%
> 2B7MG%2FABnCHXtHqD0eM%3Dreserved=0
> >
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Is this something really should be reverted? I hit the same problem but I
> figured out it was because of my openssl version is old.
>


well, in this case, Travis is failing for Openvswitch repo

One example:

https://travis-ci.org/openvswitch/ovs/jobs/412223381


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
-Wno-null-pointer-arithmetic -Werror -Wsparse-error -MT
lib/dhparams.lo -MD -MP -MF lib/.deps/dhparams.Tpo -c lib/dhparams.c
-o lib/dhparams.o
lib/stream-ssl.c:1191:27: error: undefined identifier 'SSL_OP_NO_SSL_MASK'
make[2]: *** [lib/stream-ssl.lo] Error 1
make[2]: *** Waiting for unfinished jobs
make[2]: Leaving directory `/home/travis/build/openvswitch/ovs'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/travis/build/openvswitch/ovs'
make: *** [all] Error 2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: Add gcc and gcc-c++ to BuildRequires

[Ben Pfaff]

On Mon, Aug 06, 2018 at 08:21:32PM +0200, Timothy Redaelli wrote:
> Starting from Fedora 29, gcc and gcc-c++ won't be installed by default in
> buildroot and so it's necessary to specify them explicitly in the spec file.
> 
> https://fedoraproject.org/wiki/Changes/Remove_GCC_from_BuildRoot
> 
> Signed-off-by: Timothy Redaelli 

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

Re: [ovs-dev] [patch v3 2/2] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

Doh, I’ll fix it up; thanks

On 8/6/18, 2:57 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

On Mon, Aug 06, 2018 at 10:55:27AM -0700, Darrell Ball wrote:
> By making opt_dpif_open() more general, it can be used effectively
> by all potential callers. Also, the error handling is improved by
> having more specific errors.
> 
> Signed-off-by: Darrell Ball 
> ---
>  lib/dpctl.c | 69 
+
>  tests/system-traffic.at |  8 +++---
>  2 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index c600eeb..ebb1603 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -187,18 +187,77 @@ parsed_dpif_open(const char *arg_, bool create, 
struct dpif **dpifp)
>  return result;
>  }
>  
> +static bool
> +dp_exists(const char *queried_dp)
> +{
> +struct sset dpif_names = SSET_INITIALIZER(_names),
> +dpif_types = SSET_INITIALIZER(_types);
> +int error = 0;
> +bool found = false;
> +struct ds ds = DS_EMPTY_INITIALIZER;;
> +
> +dp_enumerate_types(_types);
> +
> +const char *type;
> +SSET_FOR_EACH (type, _types) {
> +error = dp_enumerate_names(type, _names);
> +if (!error) {
> +const char *name;
> +SSET_FOR_EACH (name, _names) {
> +struct dpif *dpif;
> +error = dpif_open(name, type, );
> +if (!error) {
> +dpif_close(dpif);
> +ds_put_cstr(, type);
> +ds_put_cstr(, "@");
> +ds_put_cstr(, name);
> +if (!strcmp(queried_dp, ds_cstr())) {
> +found = true;
> +goto out;
> +}
> +}
> +}
> +}
> +}

It looks like this can only succeed on the first try, because after that
it keeps appending more datapath names to ds.

Thanks,

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

https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Cdball%40vmware.com%7Ca8378565e64b43e1d56e08d5fbe79ba4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636691894565531945sdata=zPwmoJfqASbNO53zVq4AW7L%2FggytO1cSDFR%2FzFtA6RM%3Dreserved=0


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

Re: [ovs-dev] [patch v3 2/2] dpctl: Make opt_dpif_open() more general.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 10:55:27AM -0700, Darrell Ball wrote:
> By making opt_dpif_open() more general, it can be used effectively
> by all potential callers. Also, the error handling is improved by
> having more specific errors.
> 
> Signed-off-by: Darrell Ball 
> ---
>  lib/dpctl.c | 69 
> +
>  tests/system-traffic.at |  8 +++---
>  2 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index c600eeb..ebb1603 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -187,18 +187,77 @@ parsed_dpif_open(const char *arg_, bool create, struct 
> dpif **dpifp)
>  return result;
>  }
>  
> +static bool
> +dp_exists(const char *queried_dp)
> +{
> +struct sset dpif_names = SSET_INITIALIZER(_names),
> +dpif_types = SSET_INITIALIZER(_types);
> +int error = 0;
> +bool found = false;
> +struct ds ds = DS_EMPTY_INITIALIZER;;
> +
> +dp_enumerate_types(_types);
> +
> +const char *type;
> +SSET_FOR_EACH (type, _types) {
> +error = dp_enumerate_names(type, _names);
> +if (!error) {
> +const char *name;
> +SSET_FOR_EACH (name, _names) {
> +struct dpif *dpif;
> +error = dpif_open(name, type, );
> +if (!error) {
> +dpif_close(dpif);
> +ds_put_cstr(, type);
> +ds_put_cstr(, "@");
> +ds_put_cstr(, name);
> +if (!strcmp(queried_dp, ds_cstr())) {
> +found = true;
> +goto out;
> +}
> +}
> +}
> +}
> +}

It looks like this can only succeed on the first try, because after that
it keeps appending more datapath names to ds.

Thanks,

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

Re: [ovs-dev] [patch v3 1/2] dpctl: Simplify dpctl_flush_conntrack.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 10:55:26AM -0700, Darrell Ball wrote:
> The function dpctl_flush_conntrack() and other such new functions with
> multiple optional arguments can be simplified by reodering the checks
> for optional parameters, where the datapath argument is checked for
> last.
> 
> Signed-off-by: Darrell Ball 

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

[ovs-dev] [patch v2] stream-ssl: Tweak recent change to fix travis builds.

[Darrell Ball]

10 of the travis builds are failing such as
TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.

Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by default")
CC: Timothy Redaelli 
Signed-off-by: Darrell Ball 
---
 lib/stream-ssl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index f3d623c..0d5e0de 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -1188,7 +1188,7 @@ stream_ssl_set_protocols(const char *arg)
 }
 
 /* Start with all the flags off and turn them on as requested. */
-long protocol_flags = SSL_OP_NO_SSL_MASK;
+long protocol_flags = 0;
 
 char *s = xstrdup(arg);
 char *save_ptr = NULL;
-- 
1.9.1

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

[ovs-dev] [PATCH v3 3/3] ovn-nbctl: Make daemon mode more transparent.

[Ben Pfaff]

This makes ovn-nbctl transparently use daemon mode if an appropriate
environment variable is set.

It also transforms ovn-nbctl.at so that it runs each ovn-nbctl test in
"direct" mode and in daemon mode.  It uses a combination of m4 macros and
shell functions to keep from expanding the generated testsuite more than
necessary.

Signed-off-by: Ben Pfaff 
Acked-by: Mark Michelson 
---
 NEWS  |   6 +-
 lib/daemon.h  |  19 +++
 lib/stream-ssl.h  |   5 +-
 ovn/utilities/ovn-nbctl.8.xml |  61 +++--
 ovn/utilities/ovn-nbctl.c | 292 +++---
 tests/ovn-nbctl.at| 246 +--
 6 files changed, 412 insertions(+), 217 deletions(-)

diff --git a/NEWS b/NEWS
index 27ef12d599d9..7875f6673e34 100644
--- a/NEWS
+++ b/NEWS
@@ -35,10 +35,8 @@ v2.10.0 - xx xxx 
  * ACL match conditions can now match on Port_Groups as well as address
sets that are automatically generated by Port_Groups.  ACLs can be
applied directly to Port_Groups as well.
- * ovn-nbctl can now run as a daemon (long-lived, background process)
-   running commands in response to JSON-RPC requests received over a UNIX
-   socket. Requests to run commands can be sent using ovs-appctl tool, same
-   as for any other OVS/OVN daemon. See ovn-nbctl(8) for details.
+ * ovn-nbctl can now run as a daemon (long-lived, background process).
+   See ovn-nbctl(8) for details.
- DPDK:
  * New 'check-dpdk' Makefile target to run a new system testsuite.
See Testing topic for the details.
diff --git a/lib/daemon.h b/lib/daemon.h
index bfa06408c390..f33e9df8df7a 100644
--- a/lib/daemon.h
+++ b/lib/daemon.h
@@ -84,6 +84,15 @@
 daemon_set_new_user(optarg);\
 break;
 
+#define DAEMON_OPTION_CASES \
+case OPT_DETACH:\
+case OPT_NO_SELF_CONFINEMENT:   \
+case OPT_NO_CHDIR:  \
+case OPT_PIDFILE:   \
+case OPT_OVERWRITE_PIDFILE: \
+case OPT_MONITOR:   \
+case OPT_USER_GROUP:
+
 void set_detach(void);
 void daemon_set_monitor(void);
 void set_no_chdir(void);
@@ -138,6 +147,16 @@ pid_t read_pidfile(const char *name);
 case OPT_USER_GROUP:\
 daemon_set_new_user(optarg);
 
+#define DAEMON_OPTION_CASES \
+case OPT_DETACH:\
+case OPT_NO_SELF_CONFINEMENT:   \
+case OPT_NO_CHDIR:  \
+case OPT_PIDFILE:   \
+case OPT_PIPE_HANDLE:   \
+case OPT_SERVICE:   \
+case OPT_SERVICE_MONITOR:   \
+case OPT_USER_GROUP:
+
 void control_handler(DWORD request);
 void set_pipe_handle(const char *pipe_handle);
 #endif /* _WIN32 */
diff --git a/lib/stream-ssl.h b/lib/stream-ssl.h
index 4bfe09b002c9..937f7c653ba9 100644
--- a/lib/stream-ssl.h
+++ b/lib/stream-ssl.h
@@ -58,6 +58,9 @@ void stream_ssl_set_ciphers(const char *arg);
 \
 case OPT_SSL_CIPHERS:   \
 stream_ssl_set_ciphers(optarg); \
-break;  
+break;
+
+#define STREAM_SSL_CASES \
+case 'p': case 'c': case 'C': case OPT_SSL_PROTOCOLS: case OPT_SSL_CIPHERS:
 
 #endif /* stream-ssl.h */
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index f09ef8767d2f..5e18fa7c06ee 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -971,15 +971,62 @@
 Daemon Mode
 
 
-  If ovn-nbctl is invoked with the --detach
-  option (see Daemon Options, below), it runs in the
-  background as a daemon and accepts commands from ovs-appctl
-  (or another JSON-RPC client) indefinitely.  The currently supported
-  commands are described below.
+  When it is invoked in the most ordinary way, ovn-nbctl
+  connects to an OVSDB server that hosts the northbound database, retrieves
+  a partial copy of the database that is complete enough to do its work,
+  sends a transaction request to the server, and receives and processes the
+  server's reply.  In common interactive use, this is fine, but if the
+  database is large, the step in which ovn-nbctl retrieves a
+  partial copy of the database can take a long time, which yields poor
+  performance overall.
 
 
 
+  To improve performance in such a case, ovn-nbctl offers a
+  "daemon mode," in which the user first starts ovn-nbctl
+  running in the background and afterward uses the daemon to execute
+  operations.  Over several ovn-nbctl command invocations,
+  this performs better overall 

[ovs-dev] [PATCH v3 2/3] ovn-nbctl: Separate command-line options parsing and interpretation.

[Ben Pfaff]

This will allow selected options to be interpreted locally and others to
be passed to the daemon, when the daemon is in use.

Signed-off-by: Ben Pfaff 
---
 lib/command-line.c| 108 ++
 lib/command-line.h|  10 +
 ovn/utilities/ovn-nbctl.c |  39 +
 3 files changed, 138 insertions(+), 19 deletions(-)

diff --git a/lib/command-line.c b/lib/command-line.c
index 81283314d1f3..9e000bd28d1a 100644
--- a/lib/command-line.c
+++ b/lib/command-line.c
@@ -52,6 +52,114 @@ ovs_cmdl_long_options_to_short_options(const struct option 
options[])
 return xstrdup(short_options);
 }
 
+static char * OVS_WARN_UNUSED_RESULT
+build_short_options(const struct option *long_options)
+{
+char *tmp, *short_options;
+
+tmp = ovs_cmdl_long_options_to_short_options(long_options);
+short_options = xasprintf("+:%s", tmp);
+free(tmp);
+
+return short_options;
+}
+
+static const struct option *
+find_option_by_value(const struct option *options, int value)
+{
+const struct option *o;
+
+for (o = options; o->name; o++) {
+if (o->val == value) {
+return o;
+}
+}
+return NULL;
+}
+
+/* Parses the command-line options in 'argc' and 'argv'.  The caller specifies
+ * the supported options in 'options'.  On success, stores the parsed options
+ * in '*pop', the number of options in '*n_pop', and returns NULL.  On failure,
+ * returns an error message and zeros the output arguments. */
+char * OVS_WARN_UNUSED_RESULT
+ovs_cmdl_parse_all(int argc, char *argv[],
+   const struct option *options,
+   struct ovs_cmdl_parsed_option **pop, size_t *n_pop)
+{
+/* Count number of options so we can have better assertions later. */
+size_t n_options OVS_UNUSED = 0;
+while (options[n_options].name) {
+n_options++;
+}
+
+char *short_options = build_short_options(options);
+
+struct ovs_cmdl_parsed_option *po = NULL;
+size_t allocated_po = 0;
+size_t n_po = 0;
+
+char *error;
+
+optind = 0;
+opterr = 0;
+for (;;) {
+int idx = -1;
+int c = getopt_long(argc, argv, short_options, options, );
+switch (c) {
+case -1:
+*pop = po;
+*n_pop = n_po;
+free(short_options);
+return NULL;
+
+case 0:
+/* getopt_long() processed the option directly by setting a flag
+ * variable.  This is probably undesirable for use with this
+ * function. */
+OVS_NOT_REACHED();
+
+case '?':
+if (optopt && find_option_by_value(options, optopt)) {
+error = xasprintf("option '%s' doesn't allow an argument",
+  argv[optind - 1]);
+} else if (optopt) {
+error = xasprintf("unrecognized option '%c'", optopt);
+} else {
+error = xasprintf("unrecognized option '%s'",
+  argv[optind - 1]);
+}
+goto error;
+
+case ':':
+error = xasprintf("option '%s' requires an argument",
+  argv[optind - 1]);
+goto error;
+
+default:
+if (n_po >= allocated_po) {
+po = x2nrealloc(po, _po, sizeof *po);
+}
+if (idx == -1) {
+po[n_po].o = find_option_by_value(options, c);
+} else {
+ovs_assert(idx >= 0 && idx < n_options);
+po[n_po].o = [idx];
+}
+po[n_po].arg = optarg;
+n_po++;
+break;
+}
+}
+OVS_NOT_REACHED();
+
+error:
+free(po);
+*pop = NULL;
+*n_pop = 0;
+free(short_options);
+return error;
+}
+
 /* Given the 'struct ovs_cmdl_command' array, prints the usage of all 
commands. */
 void
 ovs_cmdl_print_commands(const struct ovs_cmdl_command commands[])
diff --git a/lib/command-line.h b/lib/command-line.h
index 00ace949bad6..9d62dc2501c5 100644
--- a/lib/command-line.h
+++ b/lib/command-line.h
@@ -45,8 +45,18 @@ struct ovs_cmdl_command {
 };
 
 char *ovs_cmdl_long_options_to_short_options(const struct option *options);
+
+struct ovs_cmdl_parsed_option {
+const struct option *o;
+char *arg;
+};
+char *ovs_cmdl_parse_all(int argc, char *argv[], const struct option *,
+ struct ovs_cmdl_parsed_option **, size_t *)
+OVS_WARN_UNUSED_RESULT;
+
 void ovs_cmdl_print_options(const struct option *options);
 void ovs_cmdl_print_commands(const struct ovs_cmdl_command *commands);
+
 void ovs_cmdl_run_command(struct ovs_cmdl_context *,
   const struct ovs_cmdl_command[]);
 void ovs_cmdl_run_command_read_only(struct ovs_cmdl_context *,
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index c625546bb8c0..0659ced378ef 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ 

[ovs-dev] [PATCH v3 1/3] unixctl: Make path to unixctl_server socket available to the client.

[Ben Pfaff]

Acked-by: Alin Gabriel Serdean 
Acked-by: Mark Michelson 
Signed-off-by: Ben Pfaff 
---
 lib/unixctl.c   | 52 
 lib/unixctl.h   |  2 ++
 tests/daemon.at |  4 ++--
 3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/lib/unixctl.c b/lib/unixctl.c
index bd9c1caeedef..9b3b0671f33c 100644
--- a/lib/unixctl.c
+++ b/lib/unixctl.c
@@ -56,6 +56,7 @@ struct unixctl_conn {
 struct unixctl_server {
 struct pstream *listener;
 struct ovs_list conns;
+char *path;
 };
 
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
@@ -216,48 +217,44 @@ unixctl_command_reply_error(struct unixctl_conn *conn, 
const char *error)
 int
 unixctl_server_create(const char *path, struct unixctl_server **serverp)
 {
-struct unixctl_server *server;
-struct pstream *listener;
-char *punix_path;
-int error;
-
 *serverp = NULL;
 if (path && !strcmp(path, "none")) {
 return 0;
 }
 
-if (path) {
-char *abs_path;
-abs_path = abs_file_name(ovs_rundir(), path);
-punix_path = xasprintf("punix:%s", abs_path);
-free(abs_path);
-} else {
-#ifndef _WIN32
-punix_path = xasprintf("punix:%s/%s.%ld.ctl", ovs_rundir(),
-   program_name, (long int) getpid());
+#ifdef _WIN32
+enum { WINDOWS = 1 };
 #else
-punix_path = xasprintf("punix:%s/%s.ctl", ovs_rundir(), program_name);
+enum { WINDOWS = 0 };
 #endif
-}
 
-error = pstream_open(punix_path, , 0);
+long int pid = getpid();
+char *abs_path
+= (path ? abs_file_name(ovs_rundir(), path)
+   : WINDOWS ? xasprintf("%s/%s.ctl", ovs_rundir(), program_name)
+   : xasprintf("%s/%s.%ld.ctl", ovs_rundir(), program_name, pid));
+
+struct pstream *listener;
+char *punix_path = xasprintf("punix:%s", abs_path);
+int error = pstream_open(punix_path, , 0);
+free(punix_path);
+
 if (error) {
-ovs_error(error, "could not initialize control socket %s", punix_path);
-goto exit;
+ovs_error(error, "%s: could not initialize control socket", abs_path);
+free(abs_path);
+return error;
 }
 
 unixctl_command_register("list-commands", "", 0, 0, unixctl_list_commands,
  NULL);
 unixctl_command_register("version", "", 0, 0, unixctl_version, NULL);
 
-server = xmalloc(sizeof *server);
+struct unixctl_server *server = xmalloc(sizeof *server);
 server->listener = listener;
+server->path = abs_path;
 ovs_list_init(>conns);
 *serverp = server;
-
-exit:
-free(punix_path);
-return error;
+return 0;
 }
 
 static void
@@ -429,10 +426,17 @@ unixctl_server_destroy(struct unixctl_server *server)
 kill_connection(conn);
 }
 
+free (server->path);
 pstream_close(server->listener);
 free(server);
 }
 }
+
+const char *
+unixctl_server_get_path(const struct unixctl_server *server)
+{
+return server ? server->path : NULL;
+}
 
 /* On POSIX based systems, connects to a unixctl server socket.  'path' should
  * be the name of a unixctl server socket.  If it does not start with '/', it
diff --git a/lib/unixctl.h b/lib/unixctl.h
index ce43893c6a7d..4562dbc49113 100644
--- a/lib/unixctl.h
+++ b/lib/unixctl.h
@@ -28,6 +28,8 @@ void unixctl_server_run(struct unixctl_server *);
 void unixctl_server_wait(struct unixctl_server *);
 void unixctl_server_destroy(struct unixctl_server *);
 
+const char *unixctl_server_get_path(const struct unixctl_server *);
+
 /* Client for Unix domain socket control connection. */
 struct jsonrpc;
 int unixctl_client_create(const char *path, struct jsonrpc **client);
diff --git a/tests/daemon.at b/tests/daemon.at
index 952d5a7c7bbe..b379fa83f9aa 100644
--- a/tests/daemon.at
+++ b/tests/daemon.at
@@ -149,7 +149,7 @@ AT_SETUP([daemon --detach startup errors])
 AT_CAPTURE_FILE([pid])
 OVSDB_INIT([db])
 AT_CHECK([ovsdb-server --detach --no-chdir --pidfile 
--unixctl=nonexistent/unixctl db], [1], [], [stderr])
-AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
+AT_CHECK([grep 'could not initialize control socket' stderr],
   [0], [ignore])
 AT_CHECK([test ! -s pid])
 AT_CLEANUP
@@ -159,7 +159,7 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
 AT_CAPTURE_FILE([pid])
 OVSDB_INIT([db])
 AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --monitor 
--unixctl=nonexistent/unixctl db], [1], [], [stderr])
-AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
+AT_CHECK([grep 'could not initialize control socket' stderr],
   [0], [ignore])
 AT_CHECK([test ! -s pid])
 AT_CLEANUP
-- 
2.16.1

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

[ovs-dev] [PATCH v3 0/3] Transparent use of daemon for ovn-nbctl

[Ben Pfaff]

v1->v2:
  - Applied patches 1 and 2; added ack for patch 3 (thanks Alin!)
  - Polished up the daemon mode so that it works actually quite well
and added tests that show that it behaves equivalently.
v2->v3:
  - Fix bug in patch 2 pointed out by Mark Michelson (thanks!) and add
his acks.

Ben Pfaff (3):
  unixctl: Make path to unixctl_server socket available to the client.
  ovn-nbctl: Separate command-line options parsing and interpretation.
  ovn-nbctl: Make daemon mode more transparent.

 NEWS  |   6 +-
 lib/command-line.c| 108 +++
 lib/command-line.h|  10 ++
 lib/daemon.h  |  19 +++
 lib/stream-ssl.h  |   5 +-
 lib/unixctl.c |  52 
 lib/unixctl.h |   2 +
 ovn/utilities/ovn-nbctl.8.xml |  61 +++--
 ovn/utilities/ovn-nbctl.c | 301 +++---
 tests/daemon.at   |   4 +-
 tests/ovn-nbctl.at| 246 --
 11 files changed, 567 insertions(+), 247 deletions(-)

-- 
2.16.1

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

Re: [ovs-dev] [PATCH v2 2/3] ovn-nbctl: Separate command-line options parsing and interpretation.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 04:39:13PM -0400, Mark Michelson wrote:
> On 08/03/2018 01:54 PM, Ben Pfaff wrote:
> >This will allow selected options to be interpreted locally and others to
> >be passed to the daemon, when the daemon is in use.
> >
> >Signed-off-by: Ben Pfaff 

> >+default:
> >+if (allocated_po >= n_po) {
> 
> This if is backwards. It should be:
> 
> if (n_po >= allocated_po) {

Good catch.  Thanks, I fixed this.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofctl: Fixup compare_flows function

[Ben Pfaff]

On Tue, Aug 07, 2018 at 12:34:45AM +0300, Alin Gabriel Serdean wrote:
> In the case there was no sorting criteria the flows on Windows were being
> rearranged because it was always returning zero.
> 
> Also check if there we need sorting to save a few cycles.
> 
> CC: Ben Pfaff 
> Co-authored-by: Ben Pfaff 
> Signed-off-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [ovs-discuss] ovsdb-server core dump and ovsdb corruption using raft cluster

[Ben Pfaff]

With Guru's help, I believe I have fixed it:
https://patchwork.ozlabs.org/patch/954247/

On Wed, Aug 01, 2018 at 11:46:38AM -0700, Guru Shetty wrote:
> I was able to reproduce it. I will work with Ben to get this fixed.
> 
> On 26 July 2018 at 23:14, Girish Moodalbail  wrote:
> 
> > Hello Ben,
> >
> > Sorry, got distracted with something else at work. I am still able to
> > reproduce the issue, and this is what I have and what I did
> > (if you need the core, let me know and I can share it with you)
> >
> > - 3-cluster RAFT setup in Ubuntu VM (2 VCPUs with 8GB RAM)
> >   $ uname -r
> >   Linux u1804-HVM-domU 4.15.0-23-generic #25-Ubuntu SMP Wed May 23
> > 18:02:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
> >
> > - On all of the VMs, I have installed openvswitch-switch=2.9.2,
> > openvswitch-dbg=2.9.2, and ovn-central=2.9.2
> >   (all of these packages are from http://packages.wand.net.nz/)
> >
> > - I bring up the node in the cluster one after the other -- leader 1st and
> > followed by two followers
> > - I check for cluster status and everything is healthy
> > - ovn-nbctl show and ovn-sbctl show is all empty
> >
> > - on the leader with OVN_NB_DB set to comma-separated-NB connection
> > strings I did
> >for i in `seq 1 50`; do ovn-nbclt ls-add ls$i; ovn-nbctl lsp-add ls$i
> > port0_$i; done
> >
> > - Check for the presence of 50 logical switches and 50 logical ports (one
> > on each switch). Compact the database on all the nodes.
> >
> > - Next I try to delete the ports and whilst the deletion is happening I
> > run compact on one of the followers
> >
> >   leader_node# for i in `seq  1 50`; do ovn-nbctl lsp-del port0_$i;done
> >   follower_node# ovs-appctl -t /var/run/openvswitch/ovnnb_db.ctl
> > ovsdb-server/compact OVN_Northbound
> >
> > - On the follower node I see the crash:
> >
> > ● ovn-central.service - LSB: OVN central components
> >Loaded: loaded (/etc/init.d/ovn-central; generated)
> >Active: active (running) since Thu 2018-07-26 22:48:53 PDT; 19min ago
> >  Docs: man:systemd-sysv-generator(8)
> >   Process: 21883 ExecStop=/etc/init.d/ovn-central stop (code=exited,
> > status=0/SUCCESS)
> >   Process: 21934 ExecStart=/etc/init.d/ovn-central start (code=exited,
> > status=0/SUCCESS)
> > Tasks: 10 (limit: 4915)
> >CGroup: /system.slice/ovn-central.service
> >├─22047 ovsdb-server: monitoring pid 22134 (*1 crashes: pid
> > 22048 died, killed (Aborted), core dumped*
> >├─22059 ovsdb-server: monitoring pid 22060 (healthy)
> >├─22060 ovsdb-server -vconsole:off -vfile:info
> > --log-file=/var/log/openvswitch/ovsdb-server-sb.log -
> >├─22072 ovn-northd: monitoring pid 22073 (healthy)
> >├─22073 ovn-northd -vconsole:emer -vsyslog:err -vfile:info
> > --ovnnb-db=tcp:10.0.7.33:6641,tcp:10.0.7.
> >└─22134 ovsdb-server -vconsole:off -vfile:info
> > --log-file=/var/log/openvswitch/ovsdb-server-nb.log
> >
> >
> > Same call trace and reason:
> >
> > #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
> > #1  0x7f79599a1801 in __GI_abort () at abort.c:79
> > #2  0x5596879c017c in json_serialize (json=,
> > s=) at ../lib/json.c:1554
> > #3  0x5596879c01eb in json_serialize_object_member (i=,
> > s=, node=, node=) at
> > ../lib/json.c:1583
> > #4  0x5596879c0132 in json_serialize_object (s=0x7ffc17013bf0,
> > object=0x55968993dcb0) at ../lib/json.c:1612
> > #5  json_serialize (json=, s=0x7ffc17013bf0) at
> > ../lib/json.c:1533
> > #6  0x5596879c249c in json_to_ds (json=json@entry=0x559689950670,
> > flags=flags@entry=0, ds=ds@entry=0x7ffc17013c80) at ../lib/json.c:1511
> > #7  0x5596879ae8df in ovsdb_log_compose_record 
> > (json=json@entry=0x559689950670,
> > magic=0x55968993dc60 "CLUSTER", header=header@entry=0x7ffc17013c60,
> > data=data@entry=0x7ffc17013c80) at ../ovsdb/log.c:570
> > #8  0x5596879aebbf in ovsdb_log_write (file=0x5596899b5df0,
> > json=0x559689950670) at ../ovsdb/log.c:618
> > #9  0x5596879aed3e in ovsdb_log_write_and_free 
> > (log=log@entry=0x5596899b5df0,
> > json=0x559689950670) at ../ovsdb/log.c:651
> > #10 0x5596879b0954 in raft_write_snapshot 
> > (raft=raft@entry=0x5596899151a0,
> > log=0x5596899b5df0, new_log_start=new_log_start@entry=166,
> > new_snapshot=new_snapshot@entry=0x7ffc17013e30) at
> > ../ovsdb/raft.c:3588
> > #11 0x5596879b0ec3 in raft_save_snapshot 
> > (raft=raft@entry=0x5596899151a0,
> > new_start=new_start@entry=166, new_snapshot=new_snapshot@
> > entry=0x7ffc17013e30)
> > at ../ovsdb/raft.c:3647
> > #12 0x5596879b8aed in raft_store_snapshot (raft=0x5596899151a0,
> > new_snapshot_data=new_snapshot_data@entry=0x5596899505f0) at
> > ../ovsdb/raft.c:3849
> > #13 0x5596879a579e in ovsdb_storage_store_snapshot__
> > (storage=0x5596899137a0, schema=0x559689938ca0, data=0x559689946ea0) at
> > ../ovsdb/storage.c:541
> > #14 0x5596879a625e in ovsdb_storage_store_snapshot
> > 

[ovs-dev] [PATCH] ovsdb-client: Make "wait" command logging more sensible.

[Ben Pfaff]

The "wait" command in ovsdb-client (which was introduced as part of the
clustering support) fairly often logs things that are normal for it but
in other circumstances might be cause for concern, for example messages
about being unable to connect to a remote.  Until now, it has tried to
suppress some of those itself by raising log levels.  Unfortunately, in
some cases this had the opposite effect because it overrode any settings on
the command line, such as an attempt in ovsdb-cluster.at to suppress all
logging related to the timeval module.  This commit drops the special
log levels from the "wait" command and puts equivalents into the tests
themselves.

Signed-off-by: Ben Pfaff 
---
 ovsdb/ovsdb-client.c   | 4 
 tests/ovsdb-cluster.at | 6 +++---
 tests/ovsdb-server.at  | 2 +-
 tests/ovsdb-tool.at| 2 +-
 tests/ovsdb.at | 6 ++
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index cdb021f391ac..288732bd85f1 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -2328,10 +2328,6 @@ do_wait(struct jsonrpc *rpc_unused OVS_UNUSED,
 const char *database_unused OVS_UNUSED,
 int argc, char *argv[])
 {
-vlog_set_levels(NULL, VLF_CONSOLE, VLL_WARN);
-vlog_set_levels_from_string_assert("reconnect:err");
-vlog_set_levels_from_string_assert("jsonrpc:err");
-
 const char *database = argv[argc - 2];
 const char *state = argv[argc - 1];
 
diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
index 01650df4c531..acc5cdde8b09 100644
--- a/tests/ovsdb-cluster.at
+++ b/tests/ovsdb-cluster.at
@@ -18,7 +18,7 @@ ovsdb_check_cluster () {
 AT_CHECK([ovsdb-server -vraft -vconsole:off -vsyslog:off --detach 
--no-chdir --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i 
--remote=punix:s$i.ovsdb s$i.db])
 done
 for i in `seq $n`; do
-AT_CHECK([ovsdb-client --timeout=30 wait unix:s$i.ovsdb $schema 
connected])
+AT_CHECK([ovsdb_client_wait unix:s$i.ovsdb $schema connected])
 done
 
 for txn
@@ -100,14 +100,14 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' 
database from ephemeral
 connect_server() {
 local i=$1
 printf "\ns$i: waiting to connect to storage\n"
-AT_CHECK([ovsdb-client --timeout=30 -vtimeval:off -vfile -vsyslog:off 
-vvlog:off --log-file=connect$i.log wait unix:s$i.ovsdb $schema connected])
+AT_CHECK([ovsdb_client_wait --log-file=connect$i.log unix:s$i.ovsdb 
$schema connected])
 }
 remove_server() {
 local i=$1
 printf "\ns$i: removing from cluster\n"
 AT_CHECK([ovs-appctl --timeout=30 -t "`pwd`"/s$i cluster/leave 
OVN_Southbound])
 printf "\ns$i: waiting for removal to complete\n"
-AT_CHECK([ovsdb-client --timeout=30 -vtimeval:off -vfile -vsyslog:off 
-vvlog:off --log-file=remove$i.log wait unix:s$i.ovsdb $schema removed])
+AT_CHECK([ovsdb_client_wait --log-file=remove$i.log unix:s$i.ovsdb 
$schema removed])
 stop_server $i
 }
 add_server() {
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index edffae1bc075..add815c71f1f 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -668,7 +668,7 @@ ovsdb_check_online_compaction() {
   fi])
 dnl Start ovsdb-server.
 AT_CHECK([ovsdb-server -vvlog:off -vconsole:off --detach --no-chdir 
--pidfile --remote=punix:socket --log-file db], [0])
-AT_CHECK([ovsdb-client wait unix:socket ordinals connected])
+AT_CHECK([ovsdb_client_wait unix:socket ordinals connected])
 AT_CAPTURE_FILE([ovsdb-server.log])
 dnl Do a bunch of random transactions that put crap in the database log.
 AT_CHECK(
diff --git a/tests/ovsdb-tool.at b/tests/ovsdb-tool.at
index 359d1f063ee1..69c5d6afa801 100644
--- a/tests/ovsdb-tool.at
+++ b/tests/ovsdb-tool.at
@@ -452,7 +452,7 @@ ovsdb-tool create-cluster db2 db1 unix:s1.raft
 
 # Dump the data.
 AT_CHECK([ovsdb-server -vconsole:off -vfile -vvlog:off --detach --no-chdir 
--pidfile --log-file --remote=punix:db.sock db2])
-AT_CHECK([ovsdb-client wait ordinals connected])
+AT_CHECK([ovsdb_client_wait ordinals connected])
 AT_CHECK([ovsdb-client dump > dump2])
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 
diff --git a/tests/ovsdb.at b/tests/ovsdb.at
index afe0a54a4380..0c9856d016cc 100644
--- a/tests/ovsdb.at
+++ b/tests/ovsdb.at
@@ -129,6 +129,12 @@ m4_define([OVSDB_CHECK_NEGATIVE_CPY],
OVSDB_CHECK_NEGATIVE_PY([$1 - Python2], [$2], [$3], [$4], [$5])
OVSDB_CHECK_NEGATIVE_PY3([$1 - Python3], [$2], [$3], [$4], [$5])])
 
+OVS_START_SHELL_HELPERS
+ovsdb_client_wait() {
+ovsdb-client -vconsole:warn -vreconnect:err -vjsonrpc:err -vtimeval:off 
-vfile -vsyslog:off -vvlog:off --timeout=30 wait "$@"
+}
+OVS_END_SHELL_HELPERS
+
 m4_include([tests/ovsdb-log.at])
 m4_include([tests/ovsdb-types.at])
 m4_include([tests/ovsdb-data.at])
-- 
2.16.1

___
dev mailing list

[ovs-dev] [PATCH] raft: Fix use-after-free error in raft_store_snapshot().

[Ben Pfaff]

raft_store_snapshot() constructs a new snapshot in a local variable then
destroys the current snapshot and replaces it by the new one.  Until now,
it has not cloned the data in the new snapshot until it did the
replacement.  This led to the unexpected consequence that, if 'servers' in
the old and new snapshots was the same, then it would first be freed and
later cloned, which could cause a segfault.

Multiple people reported the crash.  Gurucharan Shetty provided a
reproduction case.

Signed-off-by: Ben Pfaff 
---
 ovsdb/raft.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ovsdb/raft.c b/ovsdb/raft.c
index c0c1e98977b9..02ba763e5fc4 100644
--- a/ovsdb/raft.c
+++ b/ovsdb/raft.c
@@ -3838,22 +3838,22 @@ raft_store_snapshot(struct raft *raft, const struct 
json *new_snapshot_data)
 }
 
 uint64_t new_log_start = raft->last_applied + 1;
-const struct raft_entry new_snapshot = {
+struct raft_entry new_snapshot = {
 .term = raft_get_term(raft, new_log_start - 1),
-.data = CONST_CAST(struct json *, new_snapshot_data),
+.data = json_clone(new_snapshot_data),
 .eid = *raft_get_eid(raft, new_log_start - 1),
-.servers = CONST_CAST(struct json *,
-  raft_servers_for_index(raft, new_log_start - 1)),
+.servers = json_clone(raft_servers_for_index(raft, new_log_start - 1)),
 };
 struct ovsdb_error *error = raft_save_snapshot(raft, new_log_start,
_snapshot);
 if (error) {
+raft_entry_uninit(_snapshot);
 return error;
 }
 
 raft->log_synced = raft->log_end - 1;
 raft_entry_uninit(>snap);
-raft_entry_clone(>snap, _snapshot);
+raft->snap = new_snapshot;
 for (size_t i = 0; i < new_log_start - raft->log_start; i++) {
 raft_entry_uninit(>entries[i]);
 }
-- 
2.16.1

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

[ovs-dev] [PATCH] ofctl: Fixup compare_flows function

[Alin Gabriel Serdean]

In the case there was no sorting criteria the flows on Windows were being
rearranged because it was always returning zero.

Also check if there we need sorting to save a few cycles.

CC: Ben Pfaff 
Co-authored-by: Ben Pfaff 
Signed-off-by: Alin Gabriel Serdean 
---
 utilities/ovs-ofctl.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 6acbbf140..d06c827c4 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1543,7 +1543,7 @@ compare_flows(const void *afs_, const void *bfs_)
 }
 }
 
-return 0;
+return a < b ? -1 : 1;
 }
 
 static void
@@ -1565,7 +1565,9 @@ ofctl_dump_flows(struct ovs_cmdl_context *ctx)
 run(vconn_dump_flows(vconn, , protocol, , _fses),
 "dump flows");
 
-qsort(fses, n_fses, sizeof *fses, compare_flows);
+if (n_criteria) {
+qsort(fses, n_fses, sizeof *fses, compare_flows);
+}
 
 struct ds s = DS_EMPTY_INITIALIZER;
 for (size_t i = 0; i < n_fses; i++) {
-- 
2.16.1.windows.1

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

Re: [ovs-dev] [PATCH v2 0/9] tests: Clean up syslog.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 06:18:19PM +0300, Ilya Maximets wrote:
> On 04.08.2018 03:17, Ben Pfaff wrote:
> > On Wed, Aug 01, 2018 at 05:00:09PM +0300, Ilya Maximets wrote:
> >> Each run of the testsuite produces millions lines in a system
> >> log. This is completely unnecessary and makes it difficult to
> >> use system logs on test / build servers.
> >>
> >> This series is aimed to disable most of the syslog messages.
> >> There are still few logs that requires significant changes in
> >> tests or code to disable. They will be removed separately if
> >> needed.
> > 
> > This series seems technically high-quality.  Thank you.
> > 
> > I find myself wondering whether we should just introduce an environment
> > variable to allowing configuring logging defaults.  Then we would cover
> > literally everything with a single line in atlocal.in.
> > 
> > Any opinions?
> 
> I thought about this. And implementation will have few issues:
> 
> 1. Many daemons already has logging related options in cmdlines.
>This means that it should not be "OVS_DEFAULT_LOGGING_OPTIONS",
>but "OVS_FORCE_LOGGING_OPTIONS". i.e. the value in the environment
>should have highest priority.
> 2. Leads from the 1st. I'm not sure that we should expose variables
>like this for the end users, because there will be no way to change
>the logging level for the targets forced by the environment.
>So, it should be test only variable like
>"TEST_OVS_FORCE_DEFAULT_OPTIONS". But we still can't be sure that
>no one will use it in production.
> 3. Some tests (like some from vlog.at) requires ability to change the
>log level for syslog, for example. We'll need to unset the variable
>for these tests or change the tests themselves.
> 4. Modifications for all the utils/daemons and/or the vlog subsystem
>are required to implement this environment variable which is
>intended for test purposes only
> 
> I'm not sure which solution is better.
> What do you think?

Most of these points seem to follow from #1, but I don't understand #1.
Why shouldn't the environment variable be just a default that can be
overridden by command-line arguments?  That is usually what environment
settings do.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] tests: Test for ovs-ofctl snoop command

[Ashish Varma]

Added test for snoop command to check for the initial handshake messages
when a bridge connects to a controller via 'unix' connection method.

Signed-off-by: Ashish Varma 
---
v1-v2:

Removed the sleep and added OVS_WAIT_UNTIL.
Added comment on why we are adding an exception for the 'connection failed'
WARN log message.
Added '-vsyslog:off'.
Minor change for pid file name change.
---
 tests/ovs-ofctl.at | 28 
 1 file changed, 28 insertions(+)

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 06597d7..a1eecad 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -3184,3 +3184,31 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: zone 123" 
ovs-vswitchd.log])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+
+AT_SETUP([ovs-ofctl snoop-unix-connection])
+OVS_VSWITCHD_START
+
+dnl setup controller for br0 before starting the controller
+AT_CHECK([ovs-vsctl -vsyslog:off set-controller br0 unix:testcontroller])
+
+dnl then start listening on the '.snoop' connection
+AT_CHECK([ovs-ofctl -vsyslog:off --detach --pidfile=ovs-ofctl-snoop.pid snoop 
br0 1> snoopbr0.txt 2>&1])
+on_exit 'kill `cat ovs-ofctl-snoop.pid`'
+on_exit 'unlink snoopbr0.txt'
+
+dnl finally start the controller
+AT_CHECK([ovs-testcontroller -vsyslog:off --detach --pidfile 
punix:testcontroller], [0], [ignore])
+on_exit 'kill `cat ovs-testcontroller.pid`'
+OVS_WAIT_UNTIL([test -e testcontroller])
+
+dnl check for some of the initial handshake messages
+OVS_WAIT_UNTIL([egrep "OFPT_FEATURES_REQUEST" snoopbr0.txt 1> /dev/null 2>&1])
+OVS_WAIT_UNTIL([egrep "OFPT_FEATURES_REPLY" snoopbr0.txt 1> /dev/null 2>&1])
+OVS_WAIT_UNTIL([egrep "OFPT_SET_CONFIG" snoopbr0.txt 1> /dev/null 2>&1])
+
+dnl need to suppress the 'connection failed' WARN message in ovs-vswitchd
+dnl because we need ovs-vswitchd to have the controller config before starting
+dnl the controller to 'snoop' the OpenFlow messages from beginning
+OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
+AT_CLEANUP
-- 
2.7.4

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

Re: [ovs-dev] [PATCH v2 1/3] unixctl: Make path to unixctl_server socket available to the client.

[Mark Michelson]

Acked-by: Mark Michelson 

On 08/03/2018 01:54 PM, Ben Pfaff wrote:

Acked-by: Alin Gabriel Serdean 
Signed-off-by: Ben Pfaff 
---
  lib/unixctl.c   | 52 
  lib/unixctl.h   |  2 ++
  tests/daemon.at |  4 ++--
  3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/lib/unixctl.c b/lib/unixctl.c
index bd9c1caeedef..9b3b0671f33c 100644
--- a/lib/unixctl.c
+++ b/lib/unixctl.c
@@ -56,6 +56,7 @@ struct unixctl_conn {
  struct unixctl_server {
  struct pstream *listener;
  struct ovs_list conns;
+char *path;
  };
  
  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);

@@ -216,48 +217,44 @@ unixctl_command_reply_error(struct unixctl_conn *conn, 
const char *error)
  int
  unixctl_server_create(const char *path, struct unixctl_server **serverp)
  {
-struct unixctl_server *server;
-struct pstream *listener;
-char *punix_path;
-int error;
-
  *serverp = NULL;
  if (path && !strcmp(path, "none")) {
  return 0;
  }
  
-if (path) {

-char *abs_path;
-abs_path = abs_file_name(ovs_rundir(), path);
-punix_path = xasprintf("punix:%s", abs_path);
-free(abs_path);
-} else {
-#ifndef _WIN32
-punix_path = xasprintf("punix:%s/%s.%ld.ctl", ovs_rundir(),
-   program_name, (long int) getpid());
+#ifdef _WIN32
+enum { WINDOWS = 1 };
  #else
-punix_path = xasprintf("punix:%s/%s.ctl", ovs_rundir(), program_name);
+enum { WINDOWS = 0 };
  #endif
-}
  
-error = pstream_open(punix_path, , 0);

+long int pid = getpid();
+char *abs_path
+= (path ? abs_file_name(ovs_rundir(), path)
+   : WINDOWS ? xasprintf("%s/%s.ctl", ovs_rundir(), program_name)
+   : xasprintf("%s/%s.%ld.ctl", ovs_rundir(), program_name, pid));
+
+struct pstream *listener;
+char *punix_path = xasprintf("punix:%s", abs_path);
+int error = pstream_open(punix_path, , 0);
+free(punix_path);
+
  if (error) {
-ovs_error(error, "could not initialize control socket %s", punix_path);
-goto exit;
+ovs_error(error, "%s: could not initialize control socket", abs_path);
+free(abs_path);
+return error;
  }
  
  unixctl_command_register("list-commands", "", 0, 0, unixctl_list_commands,

   NULL);
  unixctl_command_register("version", "", 0, 0, unixctl_version, NULL);
  
-server = xmalloc(sizeof *server);

+struct unixctl_server *server = xmalloc(sizeof *server);
  server->listener = listener;
+server->path = abs_path;
  ovs_list_init(>conns);
  *serverp = server;
-
-exit:
-free(punix_path);
-return error;
+return 0;
  }
  
  static void

@@ -429,10 +426,17 @@ unixctl_server_destroy(struct unixctl_server *server)
  kill_connection(conn);
  }
  
+free (server->path);

  pstream_close(server->listener);
  free(server);
  }
  }
+
+const char *
+unixctl_server_get_path(const struct unixctl_server *server)
+{
+return server ? server->path : NULL;
+}
  
  /* On POSIX based systems, connects to a unixctl server socket.  'path' should
   * be the name of a unixctl server socket.  If it does not start with '/', it
diff --git a/lib/unixctl.h b/lib/unixctl.h
index ce43893c6a7d..4562dbc49113 100644
--- a/lib/unixctl.h
+++ b/lib/unixctl.h
@@ -28,6 +28,8 @@ void unixctl_server_run(struct unixctl_server *);
  void unixctl_server_wait(struct unixctl_server *);
  void unixctl_server_destroy(struct unixctl_server *);
  
+const char *unixctl_server_get_path(const struct unixctl_server *);

+
  /* Client for Unix domain socket control connection. */
  struct jsonrpc;
  int unixctl_client_create(const char *path, struct jsonrpc **client);
diff --git a/tests/daemon.at b/tests/daemon.at
index 952d5a7c7bbe..b379fa83f9aa 100644
--- a/tests/daemon.at
+++ b/tests/daemon.at
@@ -149,7 +149,7 @@ AT_SETUP([daemon --detach startup errors])
  AT_CAPTURE_FILE([pid])
  OVSDB_INIT([db])
  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile 
--unixctl=nonexistent/unixctl db], [1], [], [stderr])
-AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
+AT_CHECK([grep 'could not initialize control socket' stderr],
[0], [ignore])
  AT_CHECK([test ! -s pid])
  AT_CLEANUP
@@ -159,7 +159,7 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
  AT_CAPTURE_FILE([pid])
  OVSDB_INIT([db])
  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --monitor 
--unixctl=nonexistent/unixctl db], [1], [], [stderr])
-AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
+AT_CHECK([grep 'could not initialize control socket' stderr],
[0], [ignore])
  AT_CHECK([test ! -s pid])
  AT_CLEANUP



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

Re: [ovs-dev] [PATCH v2 3/3] ovn-nbctl: Make daemon mode more transparent.

[Mark Michelson]

Acked-by: Mark Michelson 

On 08/03/2018 01:54 PM, Ben Pfaff wrote:

This makes ovn-nbctl transparently use daemon mode if an appropriate
environment variable is set.

It also transforms ovn-nbctl.at so that it runs each ovn-nbctl test in
"direct" mode and in daemon mode.  It uses a combination of m4 macros and
shell functions to keep from expanding the generated testsuite more than
necessary.

Signed-off-by: Ben Pfaff 
---
  NEWS  |   6 +-
  lib/daemon.h  |  19 +++
  lib/stream-ssl.h  |   5 +-
  ovn/utilities/ovn-nbctl.8.xml |  61 +++--
  ovn/utilities/ovn-nbctl.c | 292 +++---
  tests/ovn-nbctl.at| 246 +--
  6 files changed, 412 insertions(+), 217 deletions(-)

diff --git a/NEWS b/NEWS
index 27ef12d599d9..7875f6673e34 100644
--- a/NEWS
+++ b/NEWS
@@ -35,10 +35,8 @@ v2.10.0 - xx xxx 
   * ACL match conditions can now match on Port_Groups as well as address
 sets that are automatically generated by Port_Groups.  ACLs can be
 applied directly to Port_Groups as well.
- * ovn-nbctl can now run as a daemon (long-lived, background process)
-   running commands in response to JSON-RPC requests received over a UNIX
-   socket. Requests to run commands can be sent using ovs-appctl tool, same
-   as for any other OVS/OVN daemon. See ovn-nbctl(8) for details.
+ * ovn-nbctl can now run as a daemon (long-lived, background process).
+   See ovn-nbctl(8) for details.
 - DPDK:
   * New 'check-dpdk' Makefile target to run a new system testsuite.
 See Testing topic for the details.
diff --git a/lib/daemon.h b/lib/daemon.h
index bfa06408c390..f33e9df8df7a 100644
--- a/lib/daemon.h
+++ b/lib/daemon.h
@@ -84,6 +84,15 @@
  daemon_set_new_user(optarg);\
  break;
  
+#define DAEMON_OPTION_CASES \

+case OPT_DETACH:\
+case OPT_NO_SELF_CONFINEMENT:   \
+case OPT_NO_CHDIR:  \
+case OPT_PIDFILE:   \
+case OPT_OVERWRITE_PIDFILE: \
+case OPT_MONITOR:   \
+case OPT_USER_GROUP:
+
  void set_detach(void);
  void daemon_set_monitor(void);
  void set_no_chdir(void);
@@ -138,6 +147,16 @@ pid_t read_pidfile(const char *name);
  case OPT_USER_GROUP:\
  daemon_set_new_user(optarg);
  
+#define DAEMON_OPTION_CASES \

+case OPT_DETACH:\
+case OPT_NO_SELF_CONFINEMENT:   \
+case OPT_NO_CHDIR:  \
+case OPT_PIDFILE:   \
+case OPT_PIPE_HANDLE:   \
+case OPT_SERVICE:   \
+case OPT_SERVICE_MONITOR:   \
+case OPT_USER_GROUP:
+
  void control_handler(DWORD request);
  void set_pipe_handle(const char *pipe_handle);
  #endif /* _WIN32 */
diff --git a/lib/stream-ssl.h b/lib/stream-ssl.h
index 4bfe09b002c9..937f7c653ba9 100644
--- a/lib/stream-ssl.h
+++ b/lib/stream-ssl.h
@@ -58,6 +58,9 @@ void stream_ssl_set_ciphers(const char *arg);
  \
  case OPT_SSL_CIPHERS:   \
  stream_ssl_set_ciphers(optarg); \
-break;
+break;
+
+#define STREAM_SSL_CASES \
+case 'p': case 'c': case 'C': case OPT_SSL_PROTOCOLS: case OPT_SSL_CIPHERS:
  
  #endif /* stream-ssl.h */

diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index f09ef8767d2f..5e18fa7c06ee 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -971,15 +971,62 @@
  Daemon Mode
  
  

-  If ovn-nbctl is invoked with the --detach
-  option (see Daemon Options, below), it runs in the
-  background as a daemon and accepts commands from ovs-appctl
-  (or another JSON-RPC client) indefinitely.  The currently supported
-  commands are described below.
+  When it is invoked in the most ordinary way, ovn-nbctl
+  connects to an OVSDB server that hosts the northbound database, retrieves
+  a partial copy of the database that is complete enough to do its work,
+  sends a transaction request to the server, and receives and processes the
+  server's reply.  In common interactive use, this is fine, but if the
+  database is large, the step in which ovn-nbctl retrieves a
+  partial copy of the database can take a long time, which yields poor
+  performance overall.
  
  
  

+  To improve performance in such a case, ovn-nbctl offers a
+  "daemon mode," in which the user first starts ovn-nbctl
+  running in the background and afterward uses the daemon to execute
+  operations.  Over several ovn-nbctl command 

Re: [ovs-dev] [PATCH v2 2/3] ovn-nbctl: Separate command-line options parsing and interpretation.

[Mark Michelson]

On 08/03/2018 01:54 PM, Ben Pfaff wrote:

This will allow selected options to be interpreted locally and others to
be passed to the daemon, when the daemon is in use.

Signed-off-by: Ben Pfaff 
---
  lib/command-line.c| 108 ++
  lib/command-line.h|  10 +
  ovn/utilities/ovn-nbctl.c |  39 +
  3 files changed, 138 insertions(+), 19 deletions(-)

diff --git a/lib/command-line.c b/lib/command-line.c
index 81283314d1f3..235e59b25716 100644
--- a/lib/command-line.c
+++ b/lib/command-line.c
@@ -52,6 +52,114 @@ ovs_cmdl_long_options_to_short_options(const struct option 
options[])
  return xstrdup(short_options);
  }
  
+static char * OVS_WARN_UNUSED_RESULT

+build_short_options(const struct option *long_options)
+{
+char *tmp, *short_options;
+
+tmp = ovs_cmdl_long_options_to_short_options(long_options);
+short_options = xasprintf("+:%s", tmp);
+free(tmp);
+
+return short_options;
+}
+
+static const struct option *
+find_option_by_value(const struct option *options, int value)
+{
+const struct option *o;
+
+for (o = options; o->name; o++) {
+if (o->val == value) {
+return o;
+}
+}
+return NULL;
+}
+
+/* Parses the command-line options in 'argc' and 'argv'.  The caller specifies
+ * the supported options in 'options'.  On success, stores the parsed options
+ * in '*pop', the number of options in '*n_pop', and returns NULL.  On failure,
+ * returns an error message and zeros the output arguments. */
+char * OVS_WARN_UNUSED_RESULT
+ovs_cmdl_parse_all(int argc, char *argv[],
+   const struct option *options,
+   struct ovs_cmdl_parsed_option **pop, size_t *n_pop)
+{
+/* Count number of options so we can have better assertions later. */
+size_t n_options OVS_UNUSED = 0;
+while (options[n_options].name) {
+n_options++;
+}
+
+char *short_options = build_short_options(options);
+
+struct ovs_cmdl_parsed_option *po = NULL;
+size_t allocated_po = 0;
+size_t n_po = 0;
+
+char *error;
+
+optind = 0;
+opterr = 0;
+for (;;) {
+int idx = -1;
+int c = getopt_long(argc, argv, short_options, options, );
+switch (c) {
+case -1:
+*pop = po;
+*n_pop = n_po;
+free(short_options);
+return NULL;
+
+case 0:
+/* getopt_long() processed the option directly by setting a flag
+ * variable.  This is probably undesirable for use with this
+ * function. */
+OVS_NOT_REACHED();
+
+case '?':
+if (optopt && find_option_by_value(options, optopt)) {
+error = xasprintf("option '%s' doesn't allow an argument",
+  argv[optind - 1]);
+} else if (optopt) {
+error = xasprintf("unrecognized option '%c'", optopt);
+} else {
+error = xasprintf("unrecognized option '%s'",
+  argv[optind - 1]);
+}
+goto error;
+
+case ':':
+error = xasprintf("option '%s' requires an argument",
+  argv[optind - 1]);
+goto error;
+
+default:
+if (allocated_po >= n_po) {


This if is backwards. It should be:

if (n_po >= allocated_po) {


+po = x2nrealloc(po, _po, sizeof *po);
+}
+if (idx == -1) {
+po[n_po].o = find_option_by_value(options, c);
+} else {
+ovs_assert(idx >= 0 && idx < n_options);
+po[n_po].o = [idx];
+}
+po[n_po].arg = optarg;
+n_po++;
+break;
+}
+}
+OVS_NOT_REACHED();
+
+error:
+free(po);
+*pop = NULL;
+*n_pop = 0;
+free(short_options);
+return error;
+}
+
  /* Given the 'struct ovs_cmdl_command' array, prints the usage of all 
commands. */
  void
  ovs_cmdl_print_commands(const struct ovs_cmdl_command commands[])
diff --git a/lib/command-line.h b/lib/command-line.h
index 00ace949bad6..9d62dc2501c5 100644
--- a/lib/command-line.h
+++ b/lib/command-line.h
@@ -45,8 +45,18 @@ struct ovs_cmdl_command {
  };
  
  char *ovs_cmdl_long_options_to_short_options(const struct option *options);

+
+struct ovs_cmdl_parsed_option {
+const struct option *o;
+char *arg;
+};
+char *ovs_cmdl_parse_all(int argc, char *argv[], const struct option *,
+ struct ovs_cmdl_parsed_option **, size_t *)
+OVS_WARN_UNUSED_RESULT;
+
  void ovs_cmdl_print_options(const struct option *options);
  void ovs_cmdl_print_commands(const struct ovs_cmdl_command *commands);
+
  void ovs_cmdl_run_command(struct ovs_cmdl_context *,
const struct ovs_cmdl_command[]);
  void ovs_cmdl_run_command_read_only(struct ovs_cmdl_context *,
diff --git 

Re: [ovs-dev] [patch v1] stream-ssl: Revert recent chamge to fix travis builds.

[Han Zhou]

On Mon, Aug 6, 2018 at 9:59 AM, Darrell Ball  wrote:
>
> Sure, and probably s/chamge/change/ would be good
>
> On 8/6/18, 12:32 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben
Pfaff"  wrote:
>
> On Mon, Aug 06, 2018 at 12:02:42PM -0700, Darrell Ball wrote:
> > Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
default")
> > CC: Timothy Redaelli 
> > Signed-off-by: Darrell Ball 
>
> It'd be nice to cite one of the failing builds in the commit message.
>
> Timothy, do you want to comment on this?
>
> Thanks,
>
> Ben.
> ___
> dev mailing list
> d...@openvswitch.org
>
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Cdball%40vmware.com%7C39a899ad6f0d47c80f6108d5fbd3668c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636691807749274687sdata=n1WruYqqMBdAB4maQ3XbYR3ZN%2B7MG%2FABnCHXtHqD0eM%3Dreserved=0
>
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Is this something really should be reverted? I hit the same problem but I
figured out it was because of my openssl version is old.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] infiniband (IPoIB) support

[Vasiliy Tolstov]

пн, 6 авг. 2018 г. в 17:34, Vasiliy Tolstov :
>
> And if IPoIB device cant't be added to openvswitch bridge, how can i
> connect virtual network with physical in such setup:
>
> i have logical switch extnet with vm ports, each vm via ovn dhcp have
> ip address from external network.
> I want to route all traffic from this extnet via last ip address from
> /24 subnet (254).
> Does i need to create logical router for such thing?
> Can you explain me how can i solve this?
> пн, 6 авг. 2018 г. в 17:11, Vasiliy Tolstov :
> >
> > Hi. I know about dpdk, but i have mellanox connectx-2 card with IPoIB in 
> > linux.
> > And i want to add it to openvswitch bridge.
> > I found topics (7 years ago) that says about no plans to add support
> > for IPoIB devices.
> > How can i add to ovs bridge IPoIB device?
> >
> > --
> > Vasiliy Tolstov,
> > e-mail: v.tols...@selfip.ru
>
>
>
> --
> Vasiliy Tolstov,
> e-mail: v.tols...@selfip.ru

I'm partially solve problem by adding to extnet gw port with type localnet.
so connection from vm to external world works fine, but from external
world to vm not:
ovs-vsctl show
f39cbc63-2dd2-486e-a2c8-1b7faee48535
Bridge br-ext
Port patch-gw-to-br-int
Interface patch-gw-to-br-int
type: patch
options: {peer=patch-br-int-to-gw}
Port br-ext
Interface br-ext
type: internal
Bridge br-int
fail_mode: secure
Port patch-br-int-to-gw
Interface patch-br-int-to-gw
type: patch
options: {peer=patch-gw-to-br-int}
Port "vnet0"
Interface "vnet0"
Port br-int
Interface br-int
type: internal
ovs_version: "2.8.1"

port gw
type: localnet
addresses: ["unknown"]
port d2b171b6-c501-4e8c-b29d-adc79e573d4c
addresses: ["52:54:00:08:43:25 xx.xx.xx.xx"]
-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v1] stream-ssl: Revert recent chamge to fix travis builds.

[Darrell Ball]

Sure, and probably s/chamge/change/ would be good

On 8/6/18, 12:32 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

On Mon, Aug 06, 2018 at 12:02:42PM -0700, Darrell Ball wrote:
> Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by 
default")
> CC: Timothy Redaelli 
> Signed-off-by: Darrell Ball 

It'd be nice to cite one of the failing builds in the commit message.

Timothy, do you want to comment on this?

Thanks,

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

https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Cdball%40vmware.com%7C39a899ad6f0d47c80f6108d5fbd3668c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636691807749274687sdata=n1WruYqqMBdAB4maQ3XbYR3ZN%2B7MG%2FABnCHXtHqD0eM%3Dreserved=0


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

Re: [ovs-dev] [ovs-dev, v5, 9 of 9] Documentation: OVN RBAC and IPsec tutorial

[0-day Robot]

Bleep bloop.  Greetings Qiuyu Xiao, 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 81 characters long (recommended limit is 79)
#159 FILE: Documentation/tutorials/ovn-ipsec.rst:95:
   Interface name: ovn-host_2-0 v1 (CONFIGURED) <--- Should be set to 
CONFIGURED.

Lines checked: 354, 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] [patch v1] stream-ssl: Revert recent chamge to fix travis builds.

[Ben Pfaff]

On Mon, Aug 06, 2018 at 12:02:42PM -0700, Darrell Ball wrote:
> Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by default")
> CC: Timothy Redaelli 
> Signed-off-by: Darrell Ball 

It'd be nice to cite one of the failing builds in the commit message.

Timothy, do you want to comment on this?

Thanks,

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

Re: [ovs-dev] [patch v1] stream-ssl: Revert recent chamge to fix travis builds.

[Darrell Ball]

if this applied, pls remove the extra newline

On Mon, Aug 6, 2018 at 12:02 PM, Darrell Ball  wrote:

> Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by
> default")
> CC: Timothy Redaelli 
> Signed-off-by: Darrell Ball 
> ---
>  lib/stream-ssl.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index f3d623c..03aa972 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -1188,7 +1188,9 @@ stream_ssl_set_protocols(const char *arg)
>  }
>
>  /* Start with all the flags off and turn them on as requested. */
> -long protocol_flags = SSL_OP_NO_SSL_MASK;
> +long protocol_flags = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 |
> SSL_OP_NO_TLSv1;
> +protocol_flags |= SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2;
> +
>
>  char *s = xstrdup(arg);
>  char *save_ptr = NULL;
> --
> 1.9.1
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 4/4] Replace router internal MAC with gateway MAC for reply packets

[Mark Michelson]

On 08/06/2018 01:58 PM, Anil Venkata wrote:

Thanks Mark. Kindly look at my comment inline.

On Fri, Aug 3, 2018 at 2:17 AM, Mark Michelson > wrote:


On 08/01/2018 08:16 AM, vkomm...@redhat.com
 wrote:

From: venkata anil mailto:vkomm...@redhat.com>>

Previous patches in the series doesn't address issue 1 explained
in [1]
i.e
1) removal of router gateway port MAC address on external switches
     after expiring of aging time.
2) then external switches unable to learn the gateway MAC as
     reply packets carry router internal port MAC address as source

To fix this, router on gateway node will use router gateway MAC
address
instead of router internal port MAC address as source for reply
packets,
so that external switches can learn gateway MAC address.
This is done only for reply packets from router gateway to
tenant VLAN
switch ports.
Later before delivering the packet to the port, ovn-controller will
replace the gateway MAC with router internal port MAC in table 33.

[1]
//mail.openvswitch.org/pipermail/ovs-dev/2018-July/349803.html


Reported-by: Miguel Angel Ajo mailto:majop...@redhat.com>>
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349803.html

Signed-off-by: Venkata Anil mailto:vkomm...@redhat.com>>
---

v6->v7:
* Added this patch


   ovn/controller/physical.c   | 60
++---
   ovn/northd/ovn-northd.8.xml | 10 
   ovn/northd/ovn-northd.c     | 29 ++
   ovn/ovn-architecture.7.xml  |  4 ++-
   4 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index f269a1d..1f41f59 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -190,7 +190,9 @@ get_zone_ids(const struct sbrec_port_binding
*binding,
   static void
   put_local_common_flows(uint32_t dp_key, uint32_t port_key,
                          bool nested_container, const struct
zone_ids *zone_ids,
-                       struct ofpbuf *ofpacts_p, struct hmap
*flow_table)
+                       struct ofpbuf *ofpacts_p, struct hmap
*flow_table,
+                       struct local_datapath *ld,
+                       const struct hmap *local_datapaths)
   {
       struct match match;
   @@ -221,11 +223,63 @@ put_local_common_flows(uint32_t dp_key,
uint32_t port_key,
           }
       }
   +    struct ofpbuf *clone = NULL;
+    clone = ofpbuf_clone(ofpacts_p);
+


I don't understand the use of the cloned ofpbuf here. You could just
ofpbuf_clear(ofpacts_p) in each iteration of the for loop below and
reuse ofpacts_p where you've used clone below.




I need to add additional flow with priority 150 along with default flow 
which exists with prioirty 100.


1) table=33,  priority=100,reg15=0x5,metadata=0x2 
actions=load:0x2->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_REG12[],resubmit(,34)
2) table=33, 
  priority=150,reg15=0x5,metadata=0x2,dl_src=fa:16:3e:48:66:e7 
actions=load:0x2->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_REG12[],mod_dl_src:fa:16:3e:65:f2:ae,resubmit(,34)


The first flow with priority 100 is the default flow. This flow is added 
for each output port residing on the current chassis, sets some 
registers and resubmits to table 34.
But when the packet has router gateway MAC as source MAC address, I want 
to replace the MAC before resubmitting to table 34. Second flow has same 
match conditions as first flow and also same actions, additionally it 
checks for the source MAC and then modifies the source MAC address.


Before I construct the second flow, I have the registers and resubmit as 
part of actions i.e

actions=load:0x2->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_REG12[],resubmit(,34)

If I simply add MAC to actions with put_load(mac64, MFF_ETH_SRC, 0, 48, 
ofpacts_p); then actions in 2nd flow(with 150 priority) will look like

actions=load:0x2->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_REG12[],resubmit(,34),mod_dl_src:fa:16:3e:65:f2:ae
i.e modifying the MAC will happen after resubmit, which is not the 
expected behaviour.


So I am cloning the ofpacts_p before resubmit is added to it and using 
the cloned one in constructing flow with priority 150.

With this changes, actions will look like

[ovs-dev] Waiting to hear from you

[Jerry Nyemego via dev]

Attention Please,

We are surprised as you have not responded to our previous notification
concerning your \$650,000, United Nations has decided to compensate you as part
of the empowerment program to reinstate those who have lost money to scammers
in the past. To commence facilitating your transfer, kindly re-confirm the
following 
1. Your Names
2. Your Address
3. Your Phone
4. Your Age
5. Your Occupation
6. Copy of your ID
7. Your Country

Waiting for your confirmation, remember further delay will result in
cancellation of your file as unclaimed and your fund will be diverted to
government treasury.

Best regards,
Jerry Nyemego
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [patch v1] stream-ssl: Revert recent chamge to fix travis builds.

[Darrell Ball]

Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by default")
CC: Timothy Redaelli 
Signed-off-by: Darrell Ball 
---
 lib/stream-ssl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index f3d623c..03aa972 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -1188,7 +1188,9 @@ stream_ssl_set_protocols(const char *arg)
 }
 
 /* Start with all the flags off and turn them on as requested. */
-long protocol_flags = SSL_OP_NO_SSL_MASK;
+long protocol_flags = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1;
+protocol_flags |= SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2;
+
 
 char *s = xstrdup(arg);
 char *save_ptr = NULL;
-- 
1.9.1

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

[ovs-dev] [PATCH] rhel: Add gcc and gcc-c++ to BuildRequires

[Timothy Redaelli]

Starting from Fedora 29, gcc and gcc-c++ won't be installed by default in
buildroot and so it's necessary to specify them explicitly in the spec file.

https://fedoraproject.org/wiki/Changes/Remove_GCC_from_BuildRoot

Signed-off-by: Timothy Redaelli 
---
 rhel/openvswitch-fedora.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 8c18d39c2..9f8664e95 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -66,6 +66,7 @@ License: ASL 2.0 and LGPLv2+ and SISSL
 Release: 1%{?dist}
 Source: http://openvswitch.org/releases/%{name}-%{version}.tar.gz
 
+BuildRequires: gcc gcc-c++
 BuildRequires: autoconf automake libtool
 BuildRequires: systemd-units openssl openssl-devel
 BuildRequires: %{_py2}-devel
-- 
2.17.1

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

[ovs-dev] [PATCH v5 8/9] OVN: native support for tunnel encryption

[Qiuyu Xiao]

This patch adds IPsec support for OVN tunnel. Basically, OVN offers a
binary option to its user for encryption configuration. If the IPsec
option is turned on, all tunnels will be encrypted. Otherwise, no tunnel
will be encrypted.

The changes are summarized as below:
1) Added a ipsec column on the NB_Global table and SB_Global table. The
value of ipsec column is propagated by ovn-northd from NB_Global to
SB_Global.

2) ovn-controller monitors the ipsec column in SB_Global. If the ipsec
value is true, ovn-controller sets options of the tunnel interface by
specifying "options:remote_name=". If the ipsec
value is false, ovn-controller removes these options.

3) ovs-monitor-ipsec daemon
(https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348701.html)
monitors the tunnel interface options and configures IKE daemon
accordingly for IPsec encryption.

Signed-off-by: Qiuyu Xiao 
---
 ovn/controller/encaps.c | 31 ++
 ovn/controller/encaps.h |  7 +-
 ovn/controller/ovn-controller.c |  4 +++-
 ovn/northd/ovn-northd.c |  8 +--
 ovn/ovn-architecture.7.xml  | 39 +
 ovn/ovn-nb.ovsschema|  7 +++---
 ovn/ovn-nb.xml  |  6 +
 ovn/ovn-sb.ovsschema|  7 +++---
 ovn/ovn-sb.xml  |  6 +
 9 files changed, 101 insertions(+), 14 deletions(-)

diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index fde017586..2169920ba 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -79,8 +79,9 @@ tunnel_create_name(struct tunnel_ctx *tc, const char 
*chassis_id)
 }
 
 static void
-tunnel_add(struct tunnel_ctx *tc, const char *new_chassis_id,
-   const struct sbrec_encap *encap)
+tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
+   const char *new_chassis_id, const struct sbrec_encap *encap,
+   const char *local_ip)
 {
 struct smap options = SMAP_INITIALIZER();
 smap_add(, "remote_ip", encap->ip);
@@ -90,6 +91,16 @@ tunnel_add(struct tunnel_ctx *tc, const char *new_chassis_id,
 smap_add(, "csum", csum);
 }
 
+/* Add auth info if ipsec is enabled. */
+if (sbg->ipsec) {
+smap_add(, "remote_name", new_chassis_id);
+if (local_ip) {
+smap_add(, "local_ip", local_ip);
+} else {
+VLOG_INFO("Need to specify encap ip for IPsec tunnels.");
+}
+}
+
 /* If there's an existing chassis record that does not need any change,
  * keep it.  Otherwise, create a new record (if there was an existing
  * record, the new record will supplant it and encaps_run() will delete
@@ -157,7 +168,9 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
const struct ovsrec_bridge_table *bridge_table,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis_table *chassis_table,
-   const char *chassis_id)
+   const char *chassis_id,
+   const struct sbrec_sb_global *sbg,
+   const struct ovsrec_open_vswitch_table *ovs_table)
 {
 if (!ovs_idl_txn || !br_int) {
 return;
@@ -201,6 +214,16 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
 }
 }
 
+/* Get IP address of local chassis. */
+const char *chassis_ip;
+const struct ovsrec_open_vswitch *cfg;
+cfg = ovsrec_open_vswitch_table_first(ovs_table);
+if (cfg) {
+chassis_ip = smap_get(>external_ids, "ovn-encap-ip");
+} else {
+chassis_ip = NULL;
+}
+
 SBREC_CHASSIS_TABLE_FOR_EACH (chassis_rec, chassis_table) {
 if (strcmp(chassis_rec->name, chassis_id)) {
 /* Create tunnels to the other chassis. */
@@ -209,7 +232,7 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
 VLOG_INFO("No supported encaps for '%s'", chassis_rec->name);
 continue;
 }
-tunnel_add(, chassis_rec->name, encap);
+tunnel_add(, sbg, chassis_rec->name, encap, chassis_ip);
 }
 }
 
diff --git a/ovn/controller/encaps.h b/ovn/controller/encaps.h
index 054bdfa78..680b62df7 100644
--- a/ovn/controller/encaps.h
+++ b/ovn/controller/encaps.h
@@ -23,13 +23,18 @@ struct ovsdb_idl_txn;
 struct ovsrec_bridge;
 struct ovsrec_bridge_table;
 struct sbrec_chassis_table;
+struct sbrec_sb_global;
+struct ovsrec_open_vswitch_table;
 
 void encaps_register_ovs_idl(struct ovsdb_idl *);
 void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
 const struct ovsrec_bridge_table *,
 const struct ovsrec_bridge *br_int,
 const struct sbrec_chassis_table *,
-const char *chassis_id);
+const char *chassis_id,
+const struct sbrec_sb_global *,
+const struct ovsrec_open_vswitch_table *);
+
 bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn,
 const struct ovsrec_bridge *br_int);
 
diff --git 

[ovs-dev] [PATCH v5 9/9] Documentation: OVN RBAC and IPsec tutorial

[Qiuyu Xiao]

This patch adds step-by-step guide for configuring OVN Role-Based Access
Control and IPsec.

Signed-off-by: Qiuyu Xiao 
---
 Documentation/automake.mk |   2 +
 Documentation/index.rst   |   4 +-
 Documentation/tutorials/index.rst |   2 +
 Documentation/tutorials/ovn-ipsec.rst | 146 ++
 Documentation/tutorials/ovn-rbac.rst  | 134 +++
 5 files changed, 287 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/tutorials/ovn-ipsec.rst
 create mode 100644 Documentation/tutorials/ovn-rbac.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 5401b9bad..082438e09 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -29,6 +29,8 @@ DOC_SOURCE = \
Documentation/tutorials/ovn-sandbox.rst \
Documentation/tutorials/ovs-conntrack.rst \
Documentation/tutorials/ipsec.rst \
+   Documentation/tutorials/ovn-ipsec.rst \
+   Documentation/tutorials/ovn-rbac.rst \
Documentation/topics/index.rst \
Documentation/topics/bonding.rst \
Documentation/topics/idl-compound-indexes.rst \
diff --git a/Documentation/index.rst b/Documentation/index.rst
index bab5ba1f1..46261235c 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -66,7 +66,9 @@ vSwitch? Start here.
   :doc:`tutorials/ovn-sandbox` |
   :doc:`tutorials/ovn-openstack` |
   :doc:`tutorials/ovs-conntrack` |
-  :doc:`tutorials/ipsec`
+  :doc:`tutorials/ipsec` |
+  :doc:`tutorials/ovn-ipsec` |
+  :doc:`tutorials/ovn-rbac`
 
 Deeper Dive
 ---
diff --git a/Documentation/tutorials/index.rst 
b/Documentation/tutorials/index.rst
index b481090a0..35340ee56 100644
--- a/Documentation/tutorials/index.rst
+++ b/Documentation/tutorials/index.rst
@@ -44,4 +44,6 @@ vSwitch.
ovs-advanced
ovn-sandbox
ovn-openstack
+   ovn-rbac
+   ovn-ipsec
ovs-conntrack
diff --git a/Documentation/tutorials/ovn-ipsec.rst 
b/Documentation/tutorials/ovn-ipsec.rst
new file mode 100644
index 0..5a8701905
--- /dev/null
+++ b/Documentation/tutorials/ovn-ipsec.rst
@@ -0,0 +1,146 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+==
+OVN IPsec Tutorial
+==
+
+This document provides a step-by-step guide for encrypting tunnel traffic with
+IPsec in Open Virtual Network (OVN). OVN tunnel traffic is transported by
+physical routers and switches. These physical devices could be untrusted
+(devices in public network) or might be compromised.  Enabling IPsec encryption
+for the tunnel traffic can prevent the traffic data from being monitored and
+manipulated. More details about the OVN IPsec design can be found in
+``ovn-architecture``\(7) manpage.
+
+This document assumes OVN is installed in your system and runs normally. Also,
+you need to install OVS IPsec packages in each chassis (refer to
+:ref:`install-ovs-ipsec`).
+
+Generating Certificates and Keys
+
+
+OVN chassis uses CA-signed certificate to authenticate peer chassis for
+building IPsec tunnel. If you have enabled Role-Based Access Control (RBAC) in
+OVN, you can use the RBAC SSL certificates and keys to set up OVN IPsec. Or you
+can generate separate certificates and keys with ``ovs-pki`` (refer to
+:ref:`gen-certs-keys`).
+
+.. note::
+
+   OVN IPsec requires x.509 version 3 certificate with the subjectAltName DNS
+   field setting the same string as the common name (CN) field. CN should be
+   set as the chassis name.  ``ovs-pki`` in Open vSwitch 2.10.90 and later
+   generates such certificates.  Please generate compatible certificates if you
+   use another PKI tool, or an older version of ``ovs-pki``, to manage
+   certificates.
+
+Configuring OVN IPsec
+-
+
+You need to install the CA certificate, chassis certificate and private key in
+each chassis. Use the following command::
+
+$ ovs-vsctl set Open_vSwitch . \
+other_config:certificate=/path/to/chassis-cert.pem \
+other_config:private_key=/path/to/chassis-privkey.pem \
+other_config:ca_cert=/path/to/cacert.pem
+

[ovs-dev] [PATCH v5 6/9] Documentation: IPsec tunnel tutorial and documentation.

[Qiuyu Xiao]

tutorials/index.rst gives a step-by-setp guide to set up OVS IPsec
tunnel.

tutorials/ipsec.rst gives detailed explanation on the IPsec tunnel
configuration methods and forwarding modes.

Signed-off-by: Qiuyu Xiao 
Signed-off-by: Ansis Atteka 
Co-authored-by: Ansis Atteka 
---
 Documentation/automake.mk |   2 +
 Documentation/howto/index.rst |   1 +
 Documentation/howto/ipsec.rst | 200 +
 Documentation/index.rst   |   3 +-
 Documentation/tutorials/index.rst |   1 +
 Documentation/tutorials/ipsec.rst | 353 ++
 vswitchd/vswitch.xml  | 153 -
 7 files changed, 703 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/howto/ipsec.rst
 create mode 100644 Documentation/tutorials/ipsec.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 244479490..5401b9bad 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -28,6 +28,7 @@ DOC_SOURCE = \
Documentation/tutorials/ovn-openstack.rst \
Documentation/tutorials/ovn-sandbox.rst \
Documentation/tutorials/ovs-conntrack.rst \
+   Documentation/tutorials/ipsec.rst \
Documentation/topics/index.rst \
Documentation/topics/bonding.rst \
Documentation/topics/idl-compound-indexes.rst \
@@ -60,6 +61,7 @@ DOC_SOURCE = \
Documentation/howto/docker.rst \
Documentation/howto/dpdk.rst \
Documentation/howto/firewalld.rst \
+   Documentation/howto/ipsec.rst \
Documentation/howto/kvm.rst \
Documentation/howto/libvirt.rst \
Documentation/howto/selinux.rst \
diff --git a/Documentation/howto/index.rst b/Documentation/howto/index.rst
index 201d6936b..9a3487be3 100644
--- a/Documentation/howto/index.rst
+++ b/Documentation/howto/index.rst
@@ -37,6 +37,7 @@ OVS
:maxdepth: 2
 
kvm
+   ipsec
selinux
libvirt
ssl
diff --git a/Documentation/howto/ipsec.rst b/Documentation/howto/ipsec.rst
new file mode 100644
index 0..32e55b5ac
--- /dev/null
+++ b/Documentation/howto/ipsec.rst
@@ -0,0 +1,200 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+===
+Encrypt Open vSwitch Tunnels with IPsec
+===
+
+This document gives detailed description on the OVS IPsec tunnel and its
+configuration modes.  If you want to follow a step-by-step guide to run and
+test IPsec tunnel, please refer to :doc:`/tutorials/ipsec`.
+
+Overview
+
+
+Why do encryption?
+~~
+
+OVS tunnel packets are transported from one machine to another. Along the path,
+the packets are processed by physical routers and physical switches.  There are
+risks that these physical devices might read or write the contents of the
+tunnel packets. IPsec encrypts IP payload and prevents the malicious party
+sniffing or manipulating the tunnel traffic.
+
+OVS IPsec
+~
+
+OVS IPsec aims to provide a simple interface for user to add encryption on OVS
+tunnels. It supports GRE, GENEVE, VXLAN, and STT tunnel. The IPsec
+configuration is done by setting options of the tunnel interface and
+other_config of Open_vSwitch. You can choose different authentication methods
+and forwarding modes based on your requirements.
+
+OVS does not currently provide any support for IPsec encryption for traffic not
+encapsulated in a tunnel.
+
+Configuration
+-
+
+Authentication Methods
+~~
+
+Hosts of the IPsec tunnel need to authenticate each other to build a secure
+channel. There are three authentication methods:
+
+1) You can use a pre-shared key (PSK) to do authentication. In both hosts, set
+   the same PSK value. This PSK is like your password. You should never reveal
+   it to untrusted parties. This method is easier to use but less secure than
+   the certificate-based methods::
+
+  $ ovs-vsctl add-port br0 ipsec_gre0 -- \
+  set interface ipsec_gre0 type=gre \
+ options:local_ip=1.1.1.1 \
+ options:remote_ip=2.2.2.2 \
+  

[ovs-dev] [PATCH v5 7/9] ovs-pki: generate x.509 v3 certificate

[Qiuyu Xiao]

This patch modifies ovs-pki to generate x.509 version 3 certificate.
Compared with the x.509 v1 certificate generated by ovs-pki, version 3
certificate adds subjectAltName field and sets its value the same as
common name (CN). The main reason for this change is to enable
strongSwan IKE daemon to extract certificate identity string from the
subjectAltName field, which makes OVN IPsec implementation easier.

Signed-off-by: Qiuyu Xiao 
---
 NEWS |  3 +++
 utilities/ovs-pki.in | 25 +
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index f05a6e976..27ef12d59 100644
--- a/NEWS
+++ b/NEWS
@@ -59,6 +59,9 @@ v2.10.0 - xx xxx 
both kernel datapath and userspace datapath.
  * Added port-based and flow-based ERSPAN tunnel port support, added
OpenFlow rules matching ERSPAN fields. See ovs-fields(7).
+   - ovs-pki
+ * ovs-pki now generates x.509 version 3 certificate. The new format adds
+   subjectAltName field and sets its value the same as common name (CN).
 
 v2.9.0 - 19 Feb 2018
 
diff --git a/utilities/ovs-pki.in b/utilities/ovs-pki.in
index 4f6941865..e0ba910f9 100755
--- a/utilities/ovs-pki.in
+++ b/utilities/ovs-pki.in
@@ -284,7 +284,7 @@ policy = policy# default policy
 email_in_dn= no# Don't add the email into cert DN
 name_opt   = ca_default# Subject name display option
 cert_opt   = ca_default# Certificate display option
-copy_extensions = none # Don't copy extensions from request
+copy_extensions = copy # Copy extensions from request
 unique_subject = no# Allow certs with duplicate subjects
 
 # For the CA policy
@@ -295,6 +295,13 @@ organizationName= match
 organizationalUnitName  = optional
 commonName  = supplied
 emailAddress= optional
+
+# For the x509v3 extension
+[ ca_cert ]
+basicConstraints=CA:true
+
+[ usr_cert ]
+basicConstraints=CA:false
 EOF
 fi
 
@@ -307,7 +314,8 @@ EOF
 openssl req -config ca.cnf -nodes \
 -newkey $newkey -keyout private/cakey.pem -out careq.pem \
 1>&3 2>&3
-openssl ca -config ca.cnf -create_serial -out cacert.pem \
+openssl ca -config ca.cnf -create_serial \
+-extensions ca_cert -out cacert.pem \
 -days 3650 -batch -keyfile private/cakey.pem -selfsign \
 -infiles careq.pem 1>&3 2>&3
 chmod 0700 private/cakey.pem
@@ -445,6 +453,7 @@ make_request() {
 [ req ]
 prompt = no
 distinguished_name = req_distinguished_name
+req_extensions = v3_req
 
 [ req_distinguished_name ]
 C = US
@@ -453,6 +462,9 @@ L = Palo Alto
 O = Open vSwitch
 OU = Open vSwitch certifier
 CN = $cn
+
+[ v3_req ]
+subjectAltName = DNS:$cn
 EOF
 if test $keytype = rsa; then
 (umask 077 && openssl genrsa -out "$1-privkey.pem" $bits) 1>&3 2>&3 \
@@ -481,7 +493,7 @@ sign_request() {
 esac
 
 (cd "$pkidir/${type}ca" && 
- openssl ca -config ca.cnf -batch -in "$request_file") \
+ openssl ca -config ca.cnf -extensions usr_cert -batch -in 
"$request_file") \
 > "$2.tmp$$" 2>&3
 mv "$2.tmp$$" "$2"
 }
@@ -529,11 +541,16 @@ elif test "$command" = self-sign; then
 must_exist "$arg1-req.pem"
 must_exist "$arg1-privkey.pem"
 must_not_exist "$arg1-cert.pem"
+make_tmpdir
+cat > "$TMP/v3.ext" <&3 || exit $?
+  -signkey "$arg1-privkey.pem" -req -days 3650 -text \
+  -extfile $TMP/v3.ext) 2>&3 || exit $?
 
 # Reset the permissions on the certificate to the user's default.
 cat "$arg1-cert.pem.tmp" > "$arg1-cert.pem"
-- 
2.18.0

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

[ovs-dev] [PATCH v5 5/9] debian and rhel: Create IPsec package.

[Qiuyu Xiao]

Added rules and files to create debian and rpm ovs-ipsec packages.

Signed-off-by: Qiuyu Xiao 
Signed-off-by: Ansis Atteka 
Co-authored-by: Ansis Atteka 
---
 debian/automake.mk|   3 +
 debian/control|  21 ++
 debian/openvswitch-ipsec.dirs |   1 +
 debian/openvswitch-ipsec.init | 181 ++
 debian/openvswitch-ipsec.install  |   1 +
 rhel/automake.mk  |   1 +
 rhel/openvswitch-fedora.spec.in   |  19 +-
 ...b_systemd_system_openvswitch-ipsec.service |  12 ++
 utilities/ovs-ctl.in  |  18 ++
 9 files changed, 256 insertions(+), 1 deletion(-)
 create mode 100644 debian/openvswitch-ipsec.dirs
 create mode 100644 debian/openvswitch-ipsec.init
 create mode 100644 debian/openvswitch-ipsec.install
 create mode 100644 rhel/usr_lib_systemd_system_openvswitch-ipsec.service

diff --git a/debian/automake.mk b/debian/automake.mk
index 4d8e204bb..8a8d43c9f 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -20,6 +20,9 @@ EXTRA_DIST += \
debian/openvswitch-datapath-source.copyright \
debian/openvswitch-datapath-source.dirs \
debian/openvswitch-datapath-source.install \
+   debian/openvswitch-ipsec.dirs \
+   debian/openvswitch-ipsec.init \
+   debian/openvswitch-ipsec.install \
debian/openvswitch-pki.dirs \
debian/openvswitch-pki.postinst \
debian/openvswitch-pki.postrm \
diff --git a/debian/control b/debian/control
index 9ae248f27..cde93f20e 100644
--- a/debian/control
+++ b/debian/control
@@ -322,3 +322,24 @@ Description: Open vSwitch development package
  1000V.
  .
  This package provides openvswitch headers and libopenvswitch for developers.
+
+Package: openvswitch-ipsec
+Architecture: linux-any
+Depends: iproute2,
+ openvswitch-common (= ${binary:Version}),
+ openvswitch-switch (= ${binary:Version}),
+ python,
+ python-openvswitch (= ${source:Version}),
+ strongswan,
+ ${misc:Depends},
+ ${shlibs:Depends}
+Description: Open vSwitch IPsec tunneling support
+ Open vSwitch is a production quality, multilayer, software-based,
+ Ethernet virtual switch. It is designed to enable massive network
+ automation through programmatic extension, while still supporting
+ standard management interfaces and protocols (e.g. NetFlow, IPFIX,
+ sFlow, SPAN, RSPAN, CLI, LACP, 802.1ag). In addition, it is designed
+ to support distribution across multiple physical servers similar to
+ VMware's vNetwork distributed vswitch or Cisco's Nexus 1000V.
+ .
+ This package provides IPsec tunneling support for OVS tunnels.
diff --git a/debian/openvswitch-ipsec.dirs b/debian/openvswitch-ipsec.dirs
new file mode 100644
index 0..fca44aa7b
--- /dev/null
+++ b/debian/openvswitch-ipsec.dirs
@@ -0,0 +1 @@
+usr/share/openvswitch/scripts
\ No newline at end of file
diff --git a/debian/openvswitch-ipsec.init b/debian/openvswitch-ipsec.init
new file mode 100644
index 0..8488beccf
--- /dev/null
+++ b/debian/openvswitch-ipsec.init
@@ -0,0 +1,181 @@
+#!/bin/sh
+#
+# Copyright (c) 2007, 2009 Javier Fernandez-Sanguino 
+#
+# This is free software; you may redistribute it and/or modify
+# it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2,
+# or (at your option) any later version.
+#
+# This is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License with
+# the Debian operating system, in /usr/share/common-licenses/GPL;  if
+# not, write to the Free Software Foundation, Inc., 59 Temple Place,
+# Suite 330, Boston, MA 02111-1307 USA
+#
+### BEGIN INIT INFO
+# Provides:  openvswitch-ipsec
+# Required-Start:$network $local_fs $remote_fs openvswitch-switch
+# Required-Stop: $remote_fs
+# Default-Start: 2 3 4 5
+# Default-Stop:  0 1 6
+# Short-Description: Open vSwitch GRE-over-IPsec daemon
+# Description:   The ovs-monitor-ipsec script provides support for
+#encrypting GRE tunnels with IPsec.
+### END INIT INFO
+
+PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
+
+DAEMON=/usr/share/openvswitch/scripts/ovs-monitor-ipsec # Daemon's location
+NAME=ovs-monitor-ipsec  # Introduce the short server's name here
+LOGDIR=/var/log/openvswitch # Log directory to use
+DATADIR=/usr/share/openvswitch
+
+PIDFILE=/var/run/openvswitch/$NAME.pid
+
+test -x $DAEMON || exit 0
+
+. /lib/lsb/init-functions
+
+DODTIME=10  # Time to wait for the server to die, in seconds
+# If this value is set too low you might not
+# let some servers 

[ovs-dev] [PATCH v5 4/9] ipsec: reintroduce IPsec support for tunneling

[Qiuyu Xiao]

This patch reintroduces ovs-monitor-ipsec daemon that
was previously removed by commit 2b02d770 ("openvswitch:
Allow external IPsec tunnel management.")

After this patch, there are no IPsec flavored tunnels anymore.
IPsec is enabled by setting up the right values in:
1. OVSDB:Interface:options column;
2. OVSDB:Open_vSwitch:other_config column;
3. OpenFlow pipeline.

GRE, VXLAN, GENEVE, and STT IPsec tunnels are supported. LibreSwan and
StrongSwan IKE daemons are supported. User can choose pre-shared key,
self-signed peer certificate, or CA-signed certificate as authentication
method.

Signed-off-by: Qiuyu Xiao 
Signed-off-by: Ansis Atteka 
Co-authored-by: Ansis Atteka 
---
 Makefile.am |1 +
 ipsec/automake.mk   |   10 +
 ipsec/ovs-monitor-ipsec | 1173 +++
 3 files changed, 1184 insertions(+)
 create mode 100644 ipsec/automake.mk
 create mode 100755 ipsec/ovs-monitor-ipsec

diff --git a/Makefile.am b/Makefile.am
index 788972804..aeb2d108f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -481,6 +481,7 @@ include tests/automake.mk
 include include/automake.mk
 include third-party/automake.mk
 include debian/automake.mk
+include ipsec/automake.mk
 include vswitchd/automake.mk
 include ovsdb/automake.mk
 include rhel/automake.mk
diff --git a/ipsec/automake.mk b/ipsec/automake.mk
new file mode 100644
index 0..1e530cb42
--- /dev/null
+++ b/ipsec/automake.mk
@@ -0,0 +1,10 @@
+# Copyright (C) 2017 Nicira, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved.  This file is offered as-is,
+# without warranty of any kind.
+
+EXTRA_DIST += \
+ipsec/ovs-monitor-ipsec
+FLAKE8_PYFILES += ipsec/ovs-monitor-ipsec
diff --git a/ipsec/ovs-monitor-ipsec b/ipsec/ovs-monitor-ipsec
new file mode 100755
index 0..163b04004
--- /dev/null
+++ b/ipsec/ovs-monitor-ipsec
@@ -0,0 +1,1173 @@
+#!/usr/bin/env python
+# Copyright (c) 2017 Nicira, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import argparse
+import re
+import subprocess
+import sys
+import copy
+from string import Template
+
+import ovs.daemon
+import ovs.db.idl
+import ovs.dirs
+import ovs.unixctl
+import ovs.unixctl.server
+import ovs.util
+import ovs.vlog
+
+
+FILE_HEADER = "# Generated by ovs-monitor-ipsec...do not modify by hand!\n\n"
+SHUNT_POLICY = """conn prevent_unencrypted_gre
+type=drop
+leftprotoport=gre
+mark={0}
+
+conn prevent_unencrypted_geneve
+type=drop
+leftprotoport=udp/6081
+mark={0}
+
+conn prevent_unencrypted_stt
+type=drop
+leftprotoport=tcp/7471
+mark={0}
+
+conn prevent_unencrypted_vxlan
+type=drop
+leftprotoport=udp/4789
+mark={0}
+
+"""
+transp_tmpl = {"gre": Template("""\
+conn $ifname-$version
+$auth_section
+leftprotoport=gre
+rightprotoport=gre
+
+"""), "gre64": Template("""\
+conn $ifname-$version
+$auth_section
+leftprotoport=gre
+rightprotoport=gre
+
+"""), "geneve": Template("""\
+conn $ifname-in-$version
+$auth_section
+leftprotoport=udp/6081
+rightprotoport=udp
+
+conn $ifname-out-$version
+$auth_section
+leftprotoport=udp
+rightprotoport=udp/6081
+
+"""), "stt": Template("""\
+conn $ifname-in-$version
+$auth_section
+leftprotoport=tcp/7471
+rightprotoport=tcp
+
+conn $ifname-out-$version
+$auth_section
+leftprotoport=tcp
+rightprotoport=tcp/7471
+
+"""), "vxlan": Template("""\
+conn $ifname-in-$version
+$auth_section
+leftprotoport=udp/4789
+rightprotoport=udp
+
+conn $ifname-out-$version
+$auth_section
+leftprotoport=udp
+rightprotoport=udp/4789
+
+""")}
+vlog = ovs.vlog.Vlog("ovs-monitor-ipsec")
+exiting = False
+monitor = None
+xfrm = None
+
+
+class XFRM(object):
+"""This class is a simple wrapper around ip-xfrm (8) command line
+utility.  We are using this class only for informational purposes
+so that ovs-monitor-ipsec could verify that IKE keying daemon has
+installed IPsec policies and security associations into kernel as
+expected."""
+
+def __init__(self, ip_root_prefix):
+self.IP = ip_root_prefix + "/sbin/ip"
+
+def get_policies(self):
+"""This function returns IPsec policies (from kernel) in a dictionary
+where  is destination IPv4 address and  is SELECTOR of
+the IPsec policy."""
+policies = {}
+proc = 

[ovs-dev] [PATCH v5 3/9] datapath: add transport ports in route lookup for stt

[Qiuyu Xiao]

This patch adds transport ports information for route lookup so that
IPsec can select stt tunnel traffic to do encryption.

Signed-off-by: Qiuyu Xiao 
Reviewed-by: Greg Rose 
Tested-by: Greg Rose 
---
 datapath/linux/compat/stt.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
index fc7e74f6d..8d16c1f2e 100644
--- a/datapath/linux/compat/stt.c
+++ b/datapath/linux/compat/stt.c
@@ -972,7 +972,8 @@ err_free_rt:
 static struct rtable *stt_get_rt(struct sk_buff *skb,
 struct net_device *dev,
 struct flowi4 *fl,
-const struct ip_tunnel_key *key)
+const struct ip_tunnel_key *key,
+__be16 dport, __be16 sport)
 {
struct net *net = dev_net(dev);
 
@@ -983,6 +984,8 @@ static struct rtable *stt_get_rt(struct sk_buff *skb,
fl->flowi4_tos = RT_TOS(key->tos);
fl->flowi4_mark = skb->mark;
fl->flowi4_proto = IPPROTO_TCP;
+   fl->fl4_dport = dport;
+   fl->fl4_sport = sport;
 
return ip_route_output_key(net, fl);
 }
@@ -1009,14 +1012,14 @@ netdev_tx_t ovs_stt_xmit(struct sk_buff *skb)
 
tun_key = _info->key;
 
-   rt = stt_get_rt(skb, dev, , tun_key);
+   sport = udp_flow_src_port(net, skb, 1, USHRT_MAX, true);
+   rt = stt_get_rt(skb, dev, , tun_key, dport, sport);
if (IS_ERR(rt)) {
err = PTR_ERR(rt);
goto error;
}
 
df = tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0;
-   sport = udp_flow_src_port(net, skb, 1, USHRT_MAX, true);
skb->ignore_df = 1;
 
stt_xmit_skb(skb, rt, fl.saddr, tun_key->u.ipv4.dst,
@@ -1818,20 +1821,22 @@ int ovs_stt_fill_metadata_dst(struct net_device *dev, 
struct sk_buff *skb)
struct stt_dev *stt_dev = netdev_priv(dev);
struct net *net = stt_dev->net;
__be16 dport = stt_dev->dst_port;
+   __be16 sport;
struct flowi4 fl4;
struct rtable *rt;
 
if (ip_tunnel_info_af(info) != AF_INET)
return -EINVAL;
 
-   rt = stt_get_rt(skb, dev, , >key);
+   sport = udp_flow_src_port(net, skb, 1, USHRT_MAX, true);
+   rt = stt_get_rt(skb, dev, , >key, dport, sport);
if (IS_ERR(rt))
return PTR_ERR(rt);
 
ip_rt_put(rt);
 
info->key.u.ipv4.src = fl4.saddr;
-   info->key.tp_src = udp_flow_src_port(net, skb, 1, USHRT_MAX, true);
+   info->key.tp_src = sport;
info->key.tp_dst = dport;
return 0;
 }
-- 
2.18.0

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

[ovs-dev] [PATCH v5 2/9] datapath: add transport ports in route lookup for vxlan

[Qiuyu Xiao]

This patch adds transport ports information for route lookup so that
IPsec can select vxlan tunnel traffic to do encryption.

Signed-off-by: Qiuyu Xiao 
Reviewed-by: Greg Rose 
Tested-by: Greg Rose 
---
 datapath/linux/compat/vxlan.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 7f5d5ce64..b850fdd44 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -896,6 +896,7 @@ out_free:
 static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
  struct sk_buff *skb, int oif, u8 tos,
  __be32 daddr, __be32 *saddr,
+ __be16 dport, __be16 sport,
  struct dst_cache *dst_cache,
  const struct ip_tunnel_info *info)
 {
@@ -918,6 +919,8 @@ static struct rtable *vxlan_get_route(struct vxlan_dev 
*vxlan,
fl4.flowi4_proto = IPPROTO_UDP;
fl4.daddr = daddr;
fl4.saddr = *saddr;
+   fl4.fl4_dport = dport;
+   fl4.fl4_sport = sport;
 
rt = ip_route_output_key(vxlan->net, );
if (!IS_ERR(rt)) {
@@ -934,6 +937,7 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev 
*vxlan,
  __be32 label,
  const struct in6_addr *daddr,
  struct in6_addr *saddr,
+ __be16 dport, __be16 sport,
  struct dst_cache *dst_cache,
  const struct ip_tunnel_info *info)
 {
@@ -961,6 +965,8 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev 
*vxlan,
fl6.flowlabel = ip6_make_flowinfo(RT_TOS(tos), label);
fl6.flowi6_mark = skb->mark;
fl6.flowi6_proto = IPPROTO_UDP;
+   fl6.fl6_dport = dport;
+   fl6.fl6_sport = sport;
 
 #ifdef HAVE_IPV6_DST_LOOKUP_NET
err = ipv6_stub->ipv6_dst_lookup(vxlan->net,
@@ -1090,6 +1096,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct 
net_device *dev,
 rdst ? rdst->remote_ifindex : 0, tos,
 dst->sin.sin_addr.s_addr,
 >sin.sin_addr.s_addr,
+dst_port, src_port,
 dst_cache, info);
if (IS_ERR(rt)) {
netdev_dbg(dev, "no route to %pI4\n",
@@ -1149,6 +1156,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct 
net_device *dev,
rdst ? rdst->remote_ifindex : 0, tos,
label, >sin6.sin6_addr,
>sin6.sin6_addr,
+   dst_port, src_port,
dst_cache, info);
if (IS_ERR(ndst)) {
netdev_dbg(dev, "no route to %pI6\n",
@@ -1439,7 +1447,8 @@ int ovs_vxlan_fill_metadata_dst(struct net_device *dev, 
struct sk_buff *skb)
return -EINVAL;
rt = vxlan_get_route(vxlan, skb, 0, info->key.tos,
 info->key.u.ipv4.dst,
->key.u.ipv4.src, NULL, info);
+>key.u.ipv4.src,
+dport, sport, NULL, info);
if (IS_ERR(rt))
return PTR_ERR(rt);
ip_rt_put(rt);
@@ -1449,7 +1458,8 @@ int ovs_vxlan_fill_metadata_dst(struct net_device *dev, 
struct sk_buff *skb)
 
ndst = vxlan6_get_route(vxlan, skb, 0, info->key.tos,
info->key.label, >key.u.ipv6.dst,
-   >key.u.ipv6.src, NULL, info);
+   >key.u.ipv6.src,
+   dport, sport, NULL, info);
if (IS_ERR(ndst))
return PTR_ERR(ndst);
dst_release(ndst);
-- 
2.18.0

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

[ovs-dev] [PATCH v5 1/9] datapath: add transport ports in route lookup for geneve

[Qiuyu Xiao]

This patch adds transport ports information for route lookup so that
IPsec can select geneve tunnel traffic to do encryption.

Signed-off-by: Qiuyu Xiao 
Reviewed-by: Greg Rose 
Tested-by: Greg Rose 
---
 datapath/linux/compat/geneve.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
index 435a23fb7..95a665ddd 100644
--- a/datapath/linux/compat/geneve.c
+++ b/datapath/linux/compat/geneve.c
@@ -836,7 +836,8 @@ free_dst:
 static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
   struct net_device *dev,
   struct flowi4 *fl4,
-  struct ip_tunnel_info *info)
+  struct ip_tunnel_info *info,
+  __be16 dport, __be16 sport)
 {
bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
struct geneve_dev *geneve = netdev_priv(dev);
@@ -850,6 +851,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
memset(fl4, 0, sizeof(*fl4));
fl4->flowi4_mark = skb->mark;
fl4->flowi4_proto = IPPROTO_UDP;
+   fl4->fl4_dport = dport;
+   fl4->fl4_sport = sport;
 
if (info) {
fl4->daddr = info->key.u.ipv4.dst;
@@ -895,7 +898,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
 static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
   struct net_device *dev,
   struct flowi6 *fl6,
-  struct ip_tunnel_info *info)
+  struct ip_tunnel_info *info,
+ __be16 dport, __be16 sport)
 {
bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
struct geneve_dev *geneve = netdev_priv(dev);
@@ -911,6 +915,8 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff 
*skb,
memset(fl6, 0, sizeof(*fl6));
fl6->flowi6_mark = skb->mark;
fl6->flowi6_proto = IPPROTO_UDP;
+   fl6->fl6_dport = dport;
+   fl6->fl6_sport = sport;
 
if (info) {
fl6->daddr = info->key.u.ipv6.dst;
@@ -1005,13 +1011,13 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, 
struct net_device *dev,
goto tx_error;
}
 
-   rt = geneve_get_v4_rt(skb, dev, , info);
+   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
+   rt = geneve_get_v4_rt(skb, dev, , info, geneve->dst_port, sport);
if (IS_ERR(rt)) {
err = PTR_ERR(rt);
goto tx_error;
}
 
-   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
skb_reset_mac_header(skb);
 
iip = ip_hdr(skb);
@@ -1097,13 +1103,13 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff 
*skb, struct net_device *dev,
}
}
 
-   dst = geneve_get_v6_dst(skb, dev, , info);
+   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
+   dst = geneve_get_v6_dst(skb, dev, , info, geneve->dst_port, sport);
if (IS_ERR(dst)) {
err = PTR_ERR(dst);
goto tx_error;
}
 
-   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
skb_reset_mac_header(skb);
 
iip = ip_hdr(skb);
@@ -1232,13 +1238,17 @@ int ovs_geneve_fill_metadata_dst(struct net_device 
*dev, struct sk_buff *skb)
struct geneve_dev *geneve = netdev_priv(dev);
struct rtable *rt;
struct flowi4 fl4;
+   __be16 sport;
 #if IS_ENABLED(CONFIG_IPV6)
struct dst_entry *dst;
struct flowi6 fl6;
 #endif
 
+   sport = udp_flow_src_port(geneve->net, skb,
+1, USHRT_MAX, true);
+
if (ip_tunnel_info_af(info) == AF_INET) {
-   rt = geneve_get_v4_rt(skb, dev, , info);
+   rt = geneve_get_v4_rt(skb, dev, , info, geneve->dst_port, 
sport);
if (IS_ERR(rt))
return PTR_ERR(rt);
 
@@ -1246,7 +1256,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device *dev, 
struct sk_buff *skb)
info->key.u.ipv4.src = fl4.saddr;
 #if IS_ENABLED(CONFIG_IPV6)
} else if (ip_tunnel_info_af(info) == AF_INET6) {
-   dst = geneve_get_v6_dst(skb, dev, , info);
+   dst = geneve_get_v6_dst(skb, dev, , info, geneve->dst_port, 
sport);
if (IS_ERR(dst))
return PTR_ERR(dst);
 
@@ -1257,8 +1267,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device *dev, 
struct sk_buff *skb)
return -EINVAL;
}
 
-   info->key.tp_src = udp_flow_src_port(geneve->net, skb,
-1, USHRT_MAX, true);
+   info->key.tp_src = sport;

[ovs-dev] [PATCH v5 0/9] IPsec support for tunneling

[Qiuyu Xiao]

This patch series reintroduce IPsec support for OVS tunneling and enable OVN to
use IPsec tunnels. GRE, VXLAN, GENEVE, and STT IPsec tunnels are supported.
StrongSwan and LibreSwan IKE daemons are supported.

Changes from v1 to v2
-
1. Merge the ovs-monitor-ipsec code to a single patch. Add LibreSwan IKE
daemon support.
2. Add ovs-monitor-ipsec to flake8 check.
3. Use openssl to extract CN from certificate so that users don't need to
specify the CN information in the configuration interface.
4. Improve documentations as suggested.

Changes from v2 to v3
-
1. Add scripts and rules to create ovs-ipsec RPM package.
2. Add Documentation/tutorials/ipsec.rst which gives a step-by-step OVS IPsec
tutorial. Modify Documentation/howto/ipsec.rst which gives a detailed
description on OVS IPsec configuration modes.
3. Modify ovs-pki to generate x.509 version 3 certificate when do self-sign.
4. IPsec tunnel interface needs 'local_ip' information. Modify ovn-controller
to add 'local_ip' when IPsec is enabled.
5. Add a section on ovn/ovn-architecture.7.xml to introduce ovn IPsec.

Changes from v3 to v4
-
1. Split the datapath patch to three patches (geneve, vxlan, stt).
2. Add tutorial for OVN RBAC and OVN IPsec.

Changes from v4 to v5
-
1. Fix coding style issues in ovs-monitor-ipsec.
2. Improve IPsec and OVN-IPsec tutorials as suggested.

Qiuyu Xiao (9):
  datapath: add transport ports in route lookup for geneve
  datapath: add transport ports in route lookup for vxlan
  datapath: add transport ports in route lookup for stt
  ipsec: reintroduce IPsec support for tunneling
  debian and rhel: Create IPsec package.
  Documentation: IPsec tunnel tutorial and documentation.
  ovs-pki: generate x.509 v3 certificate
  OVN: native support for tunnel encryption
  Documentation: OVN RBAC and IPsec tutorial

 Documentation/automake.mk |4 +
 Documentation/howto/index.rst |1 +
 Documentation/howto/ipsec.rst |  200 +++
 Documentation/index.rst   |5 +-
 Documentation/tutorials/index.rst |3 +
 Documentation/tutorials/ipsec.rst |  353 +
 Documentation/tutorials/ovn-ipsec.rst |  146 ++
 Documentation/tutorials/ovn-rbac.rst  |  134 ++
 Makefile.am   |1 +
 NEWS  |3 +
 datapath/linux/compat/geneve.c|   29 +-
 datapath/linux/compat/stt.c   |   15 +-
 datapath/linux/compat/vxlan.c |   14 +-
 debian/automake.mk|3 +
 debian/control|   21 +
 debian/openvswitch-ipsec.dirs |1 +
 debian/openvswitch-ipsec.init |  181 +++
 debian/openvswitch-ipsec.install  |1 +
 ipsec/automake.mk |   10 +
 ipsec/ovs-monitor-ipsec   | 1173 +
 ovn/controller/encaps.c   |   31 +-
 ovn/controller/encaps.h   |7 +-
 ovn/controller/ovn-controller.c   |4 +-
 ovn/northd/ovn-northd.c   |8 +-
 ovn/ovn-architecture.7.xml|   39 +
 ovn/ovn-nb.ovsschema  |7 +-
 ovn/ovn-nb.xml|6 +
 ovn/ovn-sb.ovsschema  |7 +-
 ovn/ovn-sb.xml|6 +
 rhel/automake.mk  |1 +
 rhel/openvswitch-fedora.spec.in   |   19 +-
 ...b_systemd_system_openvswitch-ipsec.service |   12 +
 utilities/ovs-ctl.in  |   18 +
 utilities/ovs-pki.in  |   25 +-
 vswitchd/vswitch.xml  |  153 ++-
 35 files changed, 2595 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/howto/ipsec.rst
 create mode 100644 Documentation/tutorials/ipsec.rst
 create mode 100644 Documentation/tutorials/ovn-ipsec.rst
 create mode 100644 Documentation/tutorials/ovn-rbac.rst
 create mode 100644 debian/openvswitch-ipsec.dirs
 create mode 100644 debian/openvswitch-ipsec.init
 create mode 100644 debian/openvswitch-ipsec.install
 create mode 100644 ipsec/automake.mk
 create mode 100755 ipsec/ovs-monitor-ipsec
 create mode 100644 rhel/usr_lib_systemd_system_openvswitch-ipsec.service

-- 
2.18.0

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

Re: [ovs-dev] [patch v2] dpctl: Simplify dpctl_flush_conntrack.

[Darrell Ball]

thanks for checking that; I thought the system tests has full coverage for 
these aspects.
The proposed change would cause inappropriate log entries.
Anyways, I rethought the approach to make it more general and sent a V3.


On 8/3/18, 5:36 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

On Thu, Aug 02, 2018 at 08:20:58PM -0700, Darrell Ball wrote:
> The function dpctl_flush_conntrack() and other such functions with
> multiple optional arguments can be simplified by introducing a new
> function to check whether a valid datapath name is supplied as an
> argument to the functions.
> 
> opt_dpif_open() can also make use of this new function to allow it
> to handle callers with multiple optional arguments.
> 
> Signed-off-by: Darrell Ball 

Thanks for working on making the OVS code easier to understand.

When I applied this patch, I get two test failures:

1195: ofproto-dpif - ovs-appctl dpif/get-flow FAILED 
(ofproto-dpif.at:7838)
1196: ofproto-dpif - ovs-appctl dpif/get-flow - pmd   FAILED 
(ofproto-dpif.at:7839)

Can you take a look?

Thanks,

Ben


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

https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Cdball%40vmware.com%7Ca777b208e07c4fa14fb308d5f9a25c24%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636689398104481064sdata=L8oeG8pqYM6UpPwOPCz%2Fd1%2BnTcTyr%2Ff6T6b2qTkkzSA%3Dreserved=0








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

Re: [ovs-dev] [PATCH v7 4/4] Replace router internal MAC with gateway MAC for reply packets

[Anil Venkata]

Thanks Mark. Kindly look at my comment inline.

On Fri, Aug 3, 2018 at 2:17 AM, Mark Michelson  wrote:

> On 08/01/2018 08:16 AM, vkomm...@redhat.com wrote:
>
>> From: venkata anil 
>>
>> Previous patches in the series doesn't address issue 1 explained in [1]
>> i.e
>> 1) removal of router gateway port MAC address on external switches
>> after expiring of aging time.
>> 2) then external switches unable to learn the gateway MAC as
>> reply packets carry router internal port MAC address as source
>>
>> To fix this, router on gateway node will use router gateway MAC address
>> instead of router internal port MAC address as source for reply packets,
>> so that external switches can learn gateway MAC address.
>> This is done only for reply packets from router gateway to tenant VLAN
>> switch ports.
>> Later before delivering the packet to the port, ovn-controller will
>> replace the gateway MAC with router internal port MAC in table 33.
>>
>> [1] //mail.openvswitch.org/pipermail/ovs-dev/2018-July/349803.html
>>
>> Reported-by: Miguel Angel Ajo 
>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349
>> 803.html
>> Signed-off-by: Venkata Anil 
>> ---
>>
>> v6->v7:
>> * Added this patch
>>
>>
>>   ovn/controller/physical.c   | 60 ++
>> ---
>>   ovn/northd/ovn-northd.8.xml | 10 
>>   ovn/northd/ovn-northd.c | 29 ++
>>   ovn/ovn-architecture.7.xml  |  4 ++-
>>   4 files changed, 99 insertions(+), 4 deletions(-)
>>
>> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
>> index f269a1d..1f41f59 100644
>> --- a/ovn/controller/physical.c
>> +++ b/ovn/controller/physical.c
>> @@ -190,7 +190,9 @@ get_zone_ids(const struct sbrec_port_binding *binding,
>>   static void
>>   put_local_common_flows(uint32_t dp_key, uint32_t port_key,
>>  bool nested_container, const struct zone_ids
>> *zone_ids,
>> -   struct ofpbuf *ofpacts_p, struct hmap *flow_table)
>> +   struct ofpbuf *ofpacts_p, struct hmap *flow_table,
>> +   struct local_datapath *ld,
>> +   const struct hmap *local_datapaths)
>>   {
>>   struct match match;
>>   @@ -221,11 +223,63 @@ put_local_common_flows(uint32_t dp_key, uint32_t
>> port_key,
>>   }
>>   }
>>   +struct ofpbuf *clone = NULL;
>> +clone = ofpbuf_clone(ofpacts_p);
>> +
>>
>
> I don't understand the use of the cloned ofpbuf here. You could just
> ofpbuf_clear(ofpacts_p) in each iteration of the for loop below and reuse
> ofpacts_p where you've used clone below.
>
>
>

I need to add additional flow with priority 150 along with default flow
which exists with prioirty 100.

1) table=33,  priority=100,reg15=0x5,metadata=0x2 actions=load:0x2->NXM_NX_
REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_REG12[],resubmit(,34)
2) table=33,  priority=150,reg15=0x5,metadata=0x2,dl_src=fa:16:3e:48:66:e7
actions=load:0x2->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_
REG12[],mod_dl_src:fa:16:3e:65:f2:ae,resubmit(,34)

The first flow with priority 100 is the default flow. This flow is added
for each output port residing on the current chassis, sets some registers
and resubmits to table 34.
But when the packet has router gateway MAC as source MAC address, I want to
replace the MAC before resubmitting to table 34. Second flow has same match
conditions as first flow and also same actions, additionally it checks for
the source MAC and then modifies the source MAC address.

Before I construct the second flow, I have the registers and resubmit as
part of actions i.e
actions=load:0x2->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_
REG12[],resubmit(,34)

If I simply add MAC to actions with put_load(mac64, MFF_ETH_SRC, 0, 48,
ofpacts_p); then actions in 2nd flow(with 150 priority) will look like
actions=load:0x2->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_
REG12[],resubmit(,34),mod_dl_src:fa:16:3e:65:f2:ae
i.e modifying the MAC will happen after resubmit, which is not the expected
behaviour.

So I am cloning the ofpacts_p before resubmit is added to it and using the
cloned one in constructing flow with priority 150.
With this changes, actions will look like
actions=load:0x2->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_
REG12[],mod_dl_src:fa:16:3e:65:f2:ae,resubmit(,34)



>   /* Resubmit to table 34. */
>>   put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
>>   ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
>>   , ofpacts_p);
>>   +/* For a reply packet from gateway with VLAN switch port as
>> destination
>> + * (excluding localnet_port and external VLAN networks), gateway
>> router
>> + * will use gateway MAC address as source MAC instead of router
>> internal
>> + * port MAC, so that external switches can learn gateway MAC address.
>> + * Here (before packet is given to 

[ovs-dev] [patch v3 1/2] dpctl: Simplify dpctl_flush_conntrack.

[Darrell Ball]

The function dpctl_flush_conntrack() and other such new functions with
multiple optional arguments can be simplified by reodering the checks
for optional parameters, where the datapath argument is checked for
last.

Signed-off-by: Darrell Ball 
---
 lib/dpctl.c | 58 --
 1 file changed, 16 insertions(+), 42 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 4f1e443..c600eeb 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1309,61 +1309,35 @@ static int
 dpctl_flush_conntrack(int argc, const char *argv[],
   struct dpctl_params *dpctl_p)
 {
-struct dpif *dpif;
+struct dpif *dpif = NULL;
 struct ct_dpif_tuple tuple, *ptuple = NULL;
 struct ds ds = DS_EMPTY_INITIALIZER;
 uint16_t zone, *pzone = NULL;
-char *name;
-int error, i = 1;
-bool got_dpif = false;
-
-/* Parse datapath name. It is not a mandatory parameter for this command.
- * If it is not specified, we retrieve it from the current setup,
- * assuming only one exists. */
-if (argc >= 2) {
-error = parsed_dpif_open(argv[i], false, );
-if (!error) {
-got_dpif = true;
-i++;
-} else if (argc == 4) {
-dpctl_error(dpctl_p, error, "invalid datapath");
-return error;
-}
-}
-if (!got_dpif) {
-name = get_one_dp(dpctl_p);
-if (!name) {
-return EINVAL;
-}
-error = parsed_dpif_open(name, false, );
-free(name);
-if (error) {
-dpctl_error(dpctl_p, error, "opening datapath");
-return error;
-}
+int error;
+int args = argc - 1;
+
+/* Parse ct tuple */
+if (args && ct_dpif_parse_tuple(, argv[args], )) {
+ptuple = 
+args--;
 }
 
 /* Parse zone */
-if (argc > i && ovs_scan(argv[i], "zone=%"SCNu16, )) {
+if (args && ovs_scan(argv[args], "zone=%"SCNu16, )) {
 pzone = 
-i++;
+args--;
 }
+
 /* Report error if there are more than one unparsed argument. */
-if (argc - i > 1) {
-ds_put_cstr(, "invalid zone");
+if (args > 1) {
+ds_put_cstr(, "invalid arguments");
 error = EINVAL;
 goto error;
 }
 
-/* Parse ct tuple */
-if (argc > i && ct_dpif_parse_tuple(, argv[i], )) {
-ptuple = 
-i++;
-}
-/* Report error if there is an unparsed argument. */
-if (argc - i) {
-error = EINVAL;
-goto error;
+error = opt_dpif_open(argc, argv, dpctl_p, 4, );
+if (error) {
+return error;
 }
 
 error = ct_dpif_flush(dpif, pzone, ptuple);
-- 
1.9.1

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

[ovs-dev] (no subject)

[mcr]

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

Re: [ovs-dev] [PATCH v2 0/9] tests: Clean up syslog.

[Ilya Maximets]

On 04.08.2018 03:17, Ben Pfaff wrote:
> On Wed, Aug 01, 2018 at 05:00:09PM +0300, Ilya Maximets wrote:
>> Each run of the testsuite produces millions lines in a system
>> log. This is completely unnecessary and makes it difficult to
>> use system logs on test / build servers.
>>
>> This series is aimed to disable most of the syslog messages.
>> There are still few logs that requires significant changes in
>> tests or code to disable. They will be removed separately if
>> needed.
> 
> This series seems technically high-quality.  Thank you.
> 
> I find myself wondering whether we should just introduce an environment
> variable to allowing configuring logging defaults.  Then we would cover
> literally everything with a single line in atlocal.in.
> 
> Any opinions?

I thought about this. And implementation will have few issues:

1. Many daemons already has logging related options in cmdlines.
   This means that it should not be "OVS_DEFAULT_LOGGING_OPTIONS",
   but "OVS_FORCE_LOGGING_OPTIONS". i.e. the value in the environment
   should have highest priority.
2. Leads from the 1st. I'm not sure that we should expose variables
   like this for the end users, because there will be no way to change
   the logging level for the targets forced by the environment.
   So, it should be test only variable like
   "TEST_OVS_FORCE_DEFAULT_OPTIONS". But we still can't be sure that
   no one will use it in production.
3. Some tests (like some from vlog.at) requires ability to change the
   log level for syslog, for example. We'll need to unset the variable
   for these tests or change the tests themselves.
4. Modifications for all the utils/daemons and/or the vlog subsystem
   are required to implement this environment variable which is
   intended for test purposes only

I'm not sure which solution is better.
What do you think?

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/9] tests: Set default timeout for utils in subshell.

[Ilya Maximets]

On 06.08.2018 16:47, Timothy Redaelli wrote:
> On Wed, 01 Aug 2018 17:00:11 +0300
> Ilya Maximets  wrote:
> 
>> Aliases are not inheritable. To add a default options for utils
>> executed in subshell we may try to catch them here and append
>> options explicitly.
>>
>> There are still few cases with utils invocation in subshell inside
>> the functions that we can not track this way, but they are not
>> very frequent.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  tests/ofproto-macros.at | 12 +++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
>> index 2a56ae6..96219cd 100644
>> --- a/tests/ofproto-macros.at
>> +++ b/tests/ofproto-macros.at
>> @@ -107,7 +107,17 @@ sim_add () {
>>  # there.
>>  as() {
>>  if test "X$2" != X; then
>> -(ovs_setenv $1; shift; "$@")
>> +(
>> + ovs_setenv $1; shift;
>> + cmd=$1; shift;
>> + for util in $OVS_UTILS_LIST; do
>> + if test "X$util" == "X$cmd"; then
>  ^^
> Tests are started with "/bin/sh" (that is not always bash), so please
> use "=" instead of "==" since the latter is not portable and it doesn't
> work on some shells (for example it doesn't work on dash that
> Debian/Ubuntu uses by default).

Hmm. Thanks for pointing that. I know about sh and, actually, my main
machine has ubuntu. It's kind of weird, but this code works perfectly
on ubuntu with dash as a default shell. From the other side I see that
'==' is a bash extension and it fails to work if I'm running it in
dash by hands. That is strange.

[root]$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 Feb 17  2016 /bin/sh -> dash
[root]$ /bin/sh
$ test "abc" == "cde"
/bin/sh: 1: test: abc: unexpected operator

But, I rechecked twice and above code works while executing in a
testsuite.

---
Looking closely to the generated testsuite file I found that
it, actually, tries to execute in as Bourne and/or POSIX as possible.
It looks through all the available shells in a system and calls 'exec'
to continue execution inside it. Look for "AS_BOURNE_COMPATIBLE" for
details.
So, reality is that most of linux distros has bash installed and it
will be chosen regardless of configured default shell.
---

Anyway, I'll change '==' to '=' because it looks sane. This will help
for systems that doesn't have bash installed, or for the cases where
shell is hard-chosen by user.

Best regards, Ilya Maximets.

> 
>> + $cmd --timeout=$OVS_TIMEOUT "$@"
>> + exit "$?"
>> + fi
>> + done
>> + $cmd "$@"
>> +)
>>  else
>>  ovs_setenv $1
>>  fi
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] infiniband (IPoIB) support

[Vasiliy Tolstov]

And if IPoIB device cant't be added to openvswitch bridge, how can i
connect virtual network with physical in such setup:

i have logical switch extnet with vm ports, each vm via ovn dhcp have
ip address from external network.
I want to route all traffic from this extnet via last ip address from
/24 subnet (254).
Does i need to create logical router for such thing?
Can you explain me how can i solve this?
пн, 6 авг. 2018 г. в 17:11, Vasiliy Tolstov :
>
> Hi. I know about dpdk, but i have mellanox connectx-2 card with IPoIB in 
> linux.
> And i want to add it to openvswitch bridge.
> I found topics (7 years ago) that says about no plans to add support
> for IPoIB devices.
> How can i add to ovs bridge IPoIB device?
>
> --
> Vasiliy Tolstov,
> e-mail: v.tols...@selfip.ru



-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] infiniband (IPoIB) support

[Vasiliy Tolstov]

Hi. I know about dpdk, but i have mellanox connectx-2 card with IPoIB in linux.
And i want to add it to openvswitch bridge.
I found topics (7 years ago) that says about no plans to add support
for IPoIB devices.
How can i add to ovs bridge IPoIB device?

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/9] tests: Set default timeout for utils in subshell.

[Timothy Redaelli]

On Wed, 01 Aug 2018 17:00:11 +0300
Ilya Maximets  wrote:

> Aliases are not inheritable. To add a default options for utils
> executed in subshell we may try to catch them here and append
> options explicitly.
> 
> There are still few cases with utils invocation in subshell inside
> the functions that we can not track this way, but they are not
> very frequent.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  tests/ofproto-macros.at | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> index 2a56ae6..96219cd 100644
> --- a/tests/ofproto-macros.at
> +++ b/tests/ofproto-macros.at
> @@ -107,7 +107,17 @@ sim_add () {
>  # there.
>  as() {
>  if test "X$2" != X; then
> -(ovs_setenv $1; shift; "$@")
> +(
> + ovs_setenv $1; shift;
> + cmd=$1; shift;
> + for util in $OVS_UTILS_LIST; do
> + if test "X$util" == "X$cmd"; then
 ^^
Tests are started with "/bin/sh" (that is not always bash), so please
use "=" instead of "==" since the latter is not portable and it doesn't
work on some shells (for example it doesn't work on dash that
Debian/Ubuntu uses by default).

> + $cmd --timeout=$OVS_TIMEOUT "$@"
> + exit "$?"
> + fi
> + done
> + $cmd "$@"
> +)
>  else
>  ovs_setenv $1
>  fi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 00/14] ovn-controller incremental processing.

[Mark Michelson]

Hi Han,

I thought about this more over the weekend, and I was hoping I'd get to 
respond to my own e-mail before you saw it, because I realized I had a 
fundamental misunderstanding of the scope and nature of change handlers. 
I'll reply to your comments in-line below.


On 08/05/2018 03:11 PM, Han Zhou wrote:

Hi Mark,

Thanks for the review and very valuable comments! (I was on vacation 
last week so sorry for slow response).


On Tue, Jul 31, 2018 at 3:47 AM, Mark Michelson > wrote:

 >
 > Hi Han,
 >
 > I've given this patchset a look, and I was following along pretty 
well until I got to about patch 11. From that point on, I had to re-read 
the code more times than I care to admit before I finally understood 
what was going on :)

 >
 > What you have is a structure (lflow_ref_list_node) that is 
simultaneously in two lists. These two lists are each the data in 
separate hmap nodes. The hmap nodes are in two separate hmaps. One hmap 
uses the reference type and name as a key, and the other uses the lflow 
UUID as a key. This way given an address set name, you can find the 
associated logical flow UUIDs in the ref_lflow_table. Or given a logical 
flow UUID, you can find address sets.

 >
 > I'm wondering if this can be simplified somehow.
 >
 > Right now, if logical flows change, the change handler has to update 
the ref_lflow_table so that address sets no longer reference that 
logical flow. If address sets change, then the lflow_ref_table is 
updated. In both cases consider_logical_flow() gets called and realigns 
the tables as appropriate.

 >
 > The problem with this is that it reeks of cross-cutting concerns, and 
it seems like it wouldn't scale well (consider a 3- or 4-chain of 
dependencies). Ideally, the dependency chain would make sure that the 
change handler for logical flows only deals with logical flows, and the 
change handler for address sets only deals with address sets.


I agree that maintaining the cross reference has some overhead, but I 
don't see a scaling issue in this case. Adding entries to the cross 
reference table is a by-product of the consider_logical_flow() when 
parsing the lflow, and deleting the entries is also efficient with O(N), 
N = number of address sets used by a lflow, which in most cases should 
be a very small number (correct me if I am wrong). As to memory 
consumption, it maintains only the mapping between resource names and 
lflow uuids, so I don't expect it to be a significant cost either. Could 
you explain a little more about the "3- or 4-chain of dependencies" example?


My thought was along the lines that table A references table B, which 
references table C. A change in table A might result in a change to 
table B, which then results in a change to table C. In my head, I 
thought this would mean having to maintain a large series of hashmaps 
that cross-referenced each other. I realize this isn't correct, though, 
and that as long as the dependency chain is linear, this isn't any 
different from what you already have proposed here.


In reality, I can't think of any example changes in the southbound 
database that would cause such a series of events, but I may not be 
thinking hard enough :)




For the "cross-cutting concern", I don't see it that way. I see it as a 
pattern of change handler implementation. In general, output of an 
engine node is the result of a "join" operation of its inputs. When 
there are multiple inputs and one of them changes, for a change handler 
to compute the output incrementally, we will need to use the changed row 
to probe all the other inputs to update the final output. For the 
address-set and port-group handlers, it is join between two tables only, 
and the cross reference table is built to make the probing efficient in 
change handler. The cross reference table is also generalized so that 
any resources referenced by logical flows can reuse the same data 
structure and interfaces, and now it is reused by both address-sets and 
port-groups. We can make it more generalized to be used for other 
mappings if needed.


Yes, and this sort of thinking is what I had over the weekend that made 
me have a "Eureka!" moment and realize what I had been missing here.


I had been looking at the address set change handler and thinking of the 
change handler as being an "owner" of address set data. The reason is 
that the engine node is tied directly to updates to the address set 
table. It felt like it was overstepping its boundaries by then stepping 
through data that I thought was owned by the logical flow change 
handler. The fact that both change handlers acted on the same data just 
struck me as wrong.


However, I realized I need to stop thinking about data ownership in that 
sort of way when it comes to change handlers. Engine nodes do have 
ownership like I was imagining in their run() method, but change 
handlers are very different. They are responsible for analyzing more 
than just the data 

Re: [ovs-dev] [PATCH] ovs-ofctl: Better validate OpenFlow message length in "ofp-parse-pcap".

[Aaron Conole]

Ben Pfaff  writes:

> Reported-by: Oscar Wilde 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047070.html
> Signed-off-by: Ben Pfaff 
> ---

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

Re: [ovs-dev] [PATCH] utilities: Launch ovsdb-tool without using PAM

[Aaron Conole]

Timothy Redaelli  writes:

> When ovsdb-server is starting, it performs some DB steps such as
> creating and upgrading the OvS DB. When we are running as
> 'non-root' user, the 'runuser' tool is used to manage the privileges.
> However, when this happens during systemd boot, we observe the following
> errors in journald:
>
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to
> scope's control group: No such process
> Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
> openvswitch.
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state.
>
> According to the analysis performed on openSUSE bugzilla[1], it seems
> that ovsdb-server.service creates (via the call to runuser) a user
> session and therefore call pam_systemd which in its turn tries to start
> a systemd user instance: "user@474.service". However "user@474.service"
> is supposed to be started after systemd-user-sessions.service which is
> supposed to be started after network.target. Additionally,
> ovsdb-server.service uses Before=network.target hence the deadlock.
>
> This commit uses "setpriv" instead of "runuser" to launch "ovsdb-tool" that
> doesn't use PAM and so it permits to launch "ovsdb-tool" as a user without
> having the deadlock. Since some old versions for "setpriv" (such as the
> one used by RHEL7) doesn't support the username / groupname, but only the
> user ids / group ids, "id" is used to get the user ID and the group IDs.
> To replicate the same behaviour of "runuser", the effective group ID of
> the user is used as GID (usually "openvswitch") and the remaining group
> IDs are used as supplementary groups (usually "hugetlbfs", if OVS is
> built with DPDK support).
>
> [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
> Reported-by: Markos Chandras 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349716.html
> Co-authored-by: Aaron Conole 
> Signed-off-by: Timothy Redaelli 
> ---

Thanks all.

Signed-off-by: Aaron Conole 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] utilities: Launch ovsdb-tool without using PAM

[0-day Robot]

Bleep bloop.  Greetings Timothy Redaelli, 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: Too many signoffs; are you missing Co-authored-by lines?
ERROR: Co-authored-by/Signed-off-by corruption
Lines checked: 64, Warnings: 0, Errors: 2


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] utilities: Launch ovsdb-tool without using PAM

[Markos Chandras]

Hello Timothy,

On 08/06/2018 01:03 PM, Timothy Redaelli wrote:
> When ovsdb-server is starting, it performs some DB steps such as
> creating and upgrading the OvS DB. When we are running as
> 'non-root' user, the 'runuser' tool is used to manage the privileges.
> However, when this happens during systemd boot, we observe the following
> errors in journald:
> 
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to
> scope's control group: No such process
> Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
> openvswitch.
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state.
> 
> According to the analysis performed on openSUSE bugzilla[1], it seems
> that ovsdb-server.service creates (via the call to runuser) a user
> session and therefore call pam_systemd which in its turn tries to start
> a systemd user instance: "user@474.service". However "user@474.service"
> is supposed to be started after systemd-user-sessions.service which is
> supposed to be started after network.target. Additionally,
> ovsdb-server.service uses Before=network.target hence the deadlock.
> 
> This commit uses "setpriv" instead of "runuser" to launch "ovsdb-tool" that
> doesn't use PAM and so it permits to launch "ovsdb-tool" as a user without
> having the deadlock. Since some old versions for "setpriv" (such as the
> one used by RHEL7) doesn't support the username / groupname, but only the
> user ids / group ids, "id" is used to get the user ID and the group IDs.
> To replicate the same behaviour of "runuser", the effective group ID of
> the user is used as GID (usually "openvswitch") and the remaining group
> IDs are used as supplementary groups (usually "hugetlbfs", if OVS is
> built with DPDK support).
> 
> [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
> Reported-by: Markos Chandras 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349716.html
> Co-authored-by: Aaron Conole 
> Signed-off-by: Timothy Redaelli 
> ---

Thank you for the fix

Reviewed-by: Markos Chandras 

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] utilities: Launch ovsdb-tool without using PAM

[Timothy Redaelli]

When ovsdb-server is starting, it performs some DB steps such as
creating and upgrading the OvS DB. When we are running as
'non-root' user, the 'runuser' tool is used to manage the privileges.
However, when this happens during systemd boot, we observe the following
errors in journald:

Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to
scope's control group: No such process
Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user openvswitch.
Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state.

According to the analysis performed on openSUSE bugzilla[1], it seems
that ovsdb-server.service creates (via the call to runuser) a user
session and therefore call pam_systemd which in its turn tries to start
a systemd user instance: "user@474.service". However "user@474.service"
is supposed to be started after systemd-user-sessions.service which is
supposed to be started after network.target. Additionally,
ovsdb-server.service uses Before=network.target hence the deadlock.

This commit uses "setpriv" instead of "runuser" to launch "ovsdb-tool" that
doesn't use PAM and so it permits to launch "ovsdb-tool" as a user without
having the deadlock. Since some old versions for "setpriv" (such as the
one used by RHEL7) doesn't support the username / groupname, but only the
user ids / group ids, "id" is used to get the user ID and the group IDs.
To replicate the same behaviour of "runuser", the effective group ID of
the user is used as GID (usually "openvswitch") and the remaining group
IDs are used as supplementary groups (usually "hugetlbfs", if OVS is
built with DPDK support).

[1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
Reported-by: Markos Chandras 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349716.html
Co-authored-by: Aaron Conole 
Signed-off-by: Timothy Redaelli 
---
 utilities/ovs-lib.in | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 10c709792..090a14434 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -389,7 +389,10 @@ move_ip_routes () {
 
 ovsdb_tool () {
 if [ "$OVS_USER" != "" ]; then
-runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
+local uid=$(id -u "${OVS_USER%:*}")
+local gid=$(id -g "${OVS_USER%:*}")
+local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
+setpriv --reuid "$uid" --regid "$gid" --groups "$groups" ovsdb-tool 
-vconsole:off "$@"
 else
 ovsdb-tool -vconsole:off "$@"
 fi
-- 
2.17.1

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

[ovs-dev] Contact my secretary.

[Barr. Collins]

Hello,

I am pleased to inform you about my success in getting those funds
transferred with the cooperation of a new partner from Vietnam.
Currently, i am with him for investment projects with my share of the
total sum. However, i did not forget your past efforts and attempts to
assist me in transferring the funds, despite that it failed us
somehow.

I raised $750,000.00 USD, which was credited in an ATM Debit card for
you to compensate you for your initial efforts in the past to help me
in a transaction that I thought it was divine for both of us to
succeed together. This is my last effort in reaching you, after
several attempts to contact you had failed. Now contact my Secretary,
his name is Mr. Adetola, contact him now to send you the ATM card. His
email address is (jonadet...@gmail.com).

Make sure you confirm your information to him to avoid giving the ATM
card to the wrong person, send him your full name, address,
occupation, age, direct phone # and your country.

Best regards,
Barr. Collins.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev