Re: [ovs-dev] [PATCH] encaps: Fix potential null pointer dereferences.

[Ben Pfaff]

On Wed, Jul 20, 2016 at 02:55:19PM -0700, Ben Pfaff wrote:
> Found by inspection.
> 
> Signed-off-by: Ben Pfaff 

I found another one, in patch.c, so I sent a v2:
http://openvswitch.org/pipermail/dev/2016-July/075785.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] ovn-controller: Fix potential null pointer dereferences.

[Ben Pfaff]

Found by inspection.

Signed-off-by: Ben Pfaff 
---
v1->v2: Found another one to fix.

 ovn/controller/encaps.c | 11 ---
 ovn/controller/patch.c  |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 03ec732..977cc3a 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -103,6 +103,10 @@ port_hash_rec(const struct ovsrec_port *port)
 
 iface = port->interfaces[0];
 ip = smap_get(>options, "remote_ip");
+if (!ip) {
+/* This should not happen for an OVN-created port. */
+return 0;
+}
 
 return port_hash(chassis_id, iface->type, ip);
 }
@@ -314,7 +318,8 @@ check_and_update_tunnel(const struct sbrec_chassis 
*chassis_rec)
 if (strcmp(encap->type, iface->type)) {
 ovsrec_interface_set_type(iface, encap->type);
 }
-if (strcmp(encap->ip, smap_get(>options, "remote_ip"))) {
+const char *ip = smap_get(>options, "remote_ip");
+if (!ip || strcmp(encap->ip, ip)) {
 struct smap options = SMAP_INITIALIZER();
 smap_add(, "remote_ip", encap->ip);
 smap_add(, "key", "flow");
@@ -322,8 +327,8 @@ check_and_update_tunnel(const struct sbrec_chassis 
*chassis_rec)
 smap_destroy();
 }
 
-if (strcmp(chassis_rec->name, smap_get(>external_ids,
-   "ovn-chassis-id"))) {
+const char *chassis = smap_get(>external_ids, "ovn-chassis-id");
+if (!chassis || strcmp(chassis_rec->name, chassis)) {
 const struct smap id = SMAP_CONST1(, "ovn-chassis-id",
chassis_rec->name);
 ovsrec_port_set_external_ids(port, );
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 823389d..fa38f78 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -349,7 +349,7 @@ add_logical_patch_ports(struct controller_ctx *ctx,
 if (!strcmp(binding->type, "gateway")) {
 const char *chassis = smap_get(>options,
"gateway-chassis");
-if (!strcmp(local_chassis_id, chassis)) {
+if (chassis && !strcmp(local_chassis_id, chassis)) {
 local_port = true;
 }
 }
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v6] ovn-northd: Add logical flows to support native DHCPv4

[Numan Siddique]

OVN implements a native DHCPv4 support which caters to the common
use case of providing an IP address to a booting instance by
providing stateless replies to DHCPv4 requests based on statically
configured address mappings. To do this it allows a short list of
DHCPv4 options to be configured and applied at each compute host
running ovn-controller.

A new table 'DHCP_Options' is added in OVN NB DB to store the DHCP
options. Logical ports refer to this table to configure the DHCPv4
options.

For each logical port configured with DHCPv4 Options following flows
are added
 - A logical flow which copies the DHCPv4 options to the DHCPv4
   request packets using the 'put_dhcp_opts' action and advances the
   packet to the next stage.

 - A logical flow which implements the DHCP reponder by sending
   the DHCPv4 reply back to the inport once the 'put_dhcp_opts' action
   is applied.

Signed-off-by: Numan Siddique 
Co-authored-by: Ben Pfaff 
Signed-off-by: Ben Pfaff 
---
 ovn/northd/ovn-northd.8.xml   |  91 +-
 ovn/northd/ovn-northd.c   | 256 +-
 ovn/ovn-nb.ovsschema  |  20 ++-
 ovn/ovn-nb.xml| 270 
 ovn/utilities/ovn-nbctl.8.xml |  30 +
 ovn/utilities/ovn-nbctl.c | 197 +
 tests/ovn.at  | 281 ++
 7 files changed, 1135 insertions(+), 10 deletions(-)

v5 -> v6
  - Rebased and fixed the compilation errors because of renaming of struct 
ovn_port's nbs
to nbsp

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index ced2839..b95caef 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -457,7 +457,90 @@ output;
   
 
 
-Ingress Table 10: Destination Lookup
+Ingress Table 10: DHCP option processing
+
+
+  This table adds the DHCPv4 options to a DHCPv4 packet from the
+  logical ports configured with IPv4 address(es) and DHCPv4 options.
+
+
+
+  
+
+  A priority-100 logical flow is added for these logical ports
+  which matches the IPv4 packet with udp.src = 68 and
+  udp.dst = 67 and applies the action
+  put_dhcp_opts and advances the packet to the next table.
+
+
+
+reg0[3] = put_dhcp_opts(offer_ip = O, options...);
+next;
+
+
+
+  For DHCPDISCOVER and DHCPREQUEST, this transforms the packet into a
+  DHCP reply, adds the DHCP offer IP O and options to the
+  packet, and stores 1 into reg0[3].  For other kinds of packets, it
+  just stores 0 into reg0[3].  Either way, it continues to the next
+  table.
+
+
+  
+
+  
+A priority-0 flow that matches all packets to advances to table 11.
+  
+
+
+Ingress Table 11: DHCP responses
+
+
+  This table implements DHCP responder for the DHCP replies generated by
+  the previous table.
+
+
+
+  
+
+  A priority 100 logical flow is added for the logical ports configured
+  with DHCPv4 options which matches IPv4 packets with udp.src == 
68
+   udp.dst == 67  reg0[3] == 1 and
+  responds back to the inport after applying these
+  actions.  If reg0[3] is set to 1, it means that the
+  action put_dhcp_opts was successful.
+
+
+
+eth.dst = eth.src;
+eth.src = E;
+ip4.dst = O;
+ip4.src = S;
+udp.src = 67;
+udp.dst = 68;
+outport = P;
+inport = ""; /* Allow sending out inport. */
+output;
+
+
+
+  where E is the server MAC address and S is the
+  server IPv4 address defined in the DHCPv4 options and O is
+  the IPv4 address defined in the logical port's addresses column.
+
+
+
+  (This terminates ingress packet processing; the packet does not go
+   to the next ingress table.)
+
+  
+
+  
+A priority-0 flow that matches all packets to advances to table 12.
+  
+
+
+Ingress Table 12: Destination Lookup
 
 
   This table implements switching behavior.  It contains these logical
@@ -531,6 +614,12 @@ output;
   there are no rules added for load balancing new connections.
 
 
+
+  Also a priority 34000 logical flow is added for each logical port which
+  has DHCPv4 options defined to allow the DHCPv4 reply packet from the
+  Ingress Table 11: DHCP responses.
+
+
 Egress Table 6: Egress Port Security - IP
 
 
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 53a8b7f..730fa47 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -26,6 +26,7 @@
 #include "hash.h"
 #include "hmap.h"
 #include "json.h"
+#include "ovn/lib/ovn-dhcp.h"
 #include "ovn/lib/lex.h"
 #include "ovn/lib/ovn-nb-idl.h"
 #include "ovn/lib/ovn-sb-idl.h"
@@ -99,7 +100,9 @@ 

[ovs-dev] Payment Confirmation

[Will]

 - This mail is in HTML. Some elements may be ommited in plain text. -

Good Day ,
Attached is the payment acknowledgement from our valued customer to you.
Will Burton
Accountant
Orient Exchange Co. (L.L.C.)
Mohd.  Obaid Al Mulla Building, Shop No. 4,
Street No 18, Community 119
Next To Twin Towers, Deira,
Dubai, U.A.E.
Tel: [009714] 2522599
Fax: [009714] 2522527
williefar...@aol.com
acco...@orientexch.ae

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth flow action to data plane

[Yang, Yi Y]

It's clear enough, thanks a lot, Jan

But http://openvswitch.org/pipermail/dev/2016-May/071657.html also mentioned 
recirculate will result in 35% performance degradation, for SFC use case, this 
is really a serious issue, how do you think we can alleviate this very 
efficiently?

I think the performance is top one for SFC use case.

-Original Message-
From: Jan Scheurich [mailto:jan.scheur...@ericsson.com] 
Sent: Wednesday, July 20, 2016 11:07 PM
To: Yang, Yi Y ; Li, Johnson ; 
dev@openvswitch.org
Cc: Simon Horman 
Subject: RE: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth flow action 
to data plane

Hi Yi,

Recirculation is implicitly triggered by calling  ctx_trigger_freeze(ctx) at 
the end of the composition phase of an action. 

The example for this is pop_mpls, but unfortunately it is a bit hidden there 
because the invocation of  ctx_trigger_freeze(ctx) is deferred in order to 
avoid unnecessary recirculation of the decapsulated packet, for example when it 
is directly sent out to a port. (For the background see 
http://openvswitch.org/pipermail/dev/2016-May/071657.html)

Instead compose_mpls_pop_action() sets ctx->was_mpls = true and subsequent 
actions that do require recirculation to happen check the was_mpls flag and 
call ctx_trigger_freeze(ctx) when needed. An example is xlate_select_group().

For pop_nsh and pop_eth you should not worry about such complications and call 
ctx_trigger_freeze(ctx) directly after having modified the flow according to 
the pop action (have a look at flow_pop_mpls() function in flow.c for an idea 
how to do this).

BR, Jan


> -Original Message-
> From: Yang, Yi Y [mailto:yi.y.y...@intel.com]
> Sent: Wednesday, 20 July, 2016 04:00
> To: Jan Scheurich; Li, Johnson; dev@openvswitch.org
> Cc: Simon Horman
> Subject: RE: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth 
> flow action to data plane
> 
> Hi, Jan
> 
> How do you know we trigger recirculate after pop_eth? pop_nsh also 
> needs to do so, is there any existing example for reference?
> 
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Jan 
> Scheurich
> Sent: Tuesday, July 19, 2016 3:46 PM
> To: Li, Johnson ; dev@openvswitch.org
> Cc: Simon Horman 
> Subject: Re: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth 
> flow action to data plane
> 
> Hi Johnson,
> 
> After basing these patches on Simon's L3 tunneling support the 
> push_eth action should take the flow-> base_layer into account:
> 
> If it is LAYER_2, pushing an Ethernet header encapsulates the inner L2 
> frame and therefore requires a call to xlate_commit_actions(ctx) 
> before composing the push_eth action. We need to pick a specific 
> "eth_type" for the resulting
> L2 frame so that OVS can identify a raw Eth over Eth encapsulation and 
> handle it correctly at pop_eth (see below). A natural choice could be 
> 0x6558 used e.g. as protocol type for Transparent Ethernet Bridging 
> (Ethernet over GRE).
> 
> If it is LAYER_3, push_eth just adds the missing MAC header to the "L3"
> packet without encapsulating it. The ethertype remains unchanged. No 
> need to call xlate_commit_actions(ctx) in that case.
> 
> For pop_eth there is a reverse problem (It is obviously only allowed 
> if flow-
> >base_layer is LAYER_2):
> 
> If the eth_type of the L2 frame indicates a known "L3" payload, you 
> only remove the MAC header from the flow and change the base_layer to 
> LAYER_3. Supported "L3" eth_types would include MPLS, ARP, IPv4, IPv6, 
> NSH (anything else?). I am not sure how much sense it would make to 
> continue processing unknown eth_types as L3 packets in the pipeline. 
> But perhaps it won't do any harm either.
> 
> If the eth_type is the chosen value for Eth over Eth (e.g. 0x6658), 
> pop_eth should trigger recirculation to reparse the inner L2 packet. 
> In this case the base_layer should remain LAYER_2.
> 
> BR, Jan
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Johnson 
> > Li
> > Sent: Tuesday, 12 July, 2016 19:30
> > To: dev@openvswitch.org
> > Subject: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth flow 
> > action to data plane
> >
> > Signed-off-by: Johnson Li 
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c 
> > b/ofproto/ofproto-dpif-xlate.c index f5c1888..fb3cd2e 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -5129,8 +5129,17 @@ do_xlate_actions(const struct ofpact 
> > *ofpacts, size_t ofpacts_len,
> >  memset(>nsh, 0x0, sizeof flow->nsh);
> >  nl_msg_put_flag(ctx->odp_actions, OVS_ACTION_ATTR_POP_NSH);
> >  break;
> > -case OFPACT_PUSH_ETH:
> > +case OFPACT_PUSH_ETH: {
> > +struct ovs_action_push_eth eth;
> > +
> > +

[ovs-dev] [PATCH] test: change replication test to use unix domain socket

[Andy Zhou]

The ovsdb replication feature is not specific to the ovsdb socket types.
Switching the tests to use Unix domain socket simplifies the tests.

Signed-off-by: Andy Zhou 

Although they don't make any difference in my local testing, travis
tests are less likely to report intermittent replication test failures.
---
 tests/ovsdb-server.at | 70 ++-
 1 file changed, 30 insertions(+), 40 deletions(-)

diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 2f2ef99..102f9f3 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -978,7 +978,7 @@ m4_define([OVSDB_CHECK_EXECUTION],
 
 EXECUTION_EXAMPLES
 
-AT_BANNER([OVSDB -- ovsdb-server replication (TCP IPv4 sockets)])
+AT_BANNER([OVSDB -- ovsdb-server replication])
 
 # OVSDB_CHECK_EXECUTION(TITLE, SCHEMA, TRANSACTIONS, OUTPUT, [KEYWORDS])
 #
@@ -998,21 +998,19 @@ m4_define([OVSDB_CHECK_EXECUTION],
AT_CHECK([ovsdb-tool create db1 schema], [0], [stdout], [ignore])
AT_CHECK([ovsdb-tool create db2 schema], [0], [stdout], [ignore])
 
-   AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server1.log 
--pidfile="`pwd`"/pid --remote=ptcp:0:127.0.0.1 --unixctl="`pwd`"/unixctl db1], 
[0], [ignore], [ignore])
-   PARSE_LISTENING_PORT([ovsdb-server1.log], [TCP_PORT1])
+   AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server1.log 
--pidfile="`pwd`"/pid --remote=punix:db.sock --unixctl="`pwd`"/unixctl db1], 
[0], [ignore], [ignore])
 
-   AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server2.log 
--pidfile="`pwd`"/pid2 --remote=ptcp:0:127.0.0.1 --unixctl="`pwd`"/unixctl2 
--sync-from=tcp:127.0.0.1:$TCP_PORT1 db2], [0], [ignore], [ignore])
-   PARSE_LISTENING_PORT([ovsdb-server2.log], [TCP_PORT2])
+   AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server2.log 
--pidfile="`pwd`"/pid2 --remote=punix:db2.sock --unixctl="`pwd`"/unixctl2 
--sync-from=unix:db.sock db2], [0], [ignore], [ignore])
 
m4_foreach([txn], [$3],
- [AT_CHECK([ovsdb-client transact tcp:127.0.0.1:$TCP_PORT1 'txn'; sleep 
2], [0], [stdout], [ignore],
+ [AT_CHECK([ovsdb-client transact 'txn'; sleep 2], [0], [stdout], [ignore],
  [test ! -e pid || kill `cat pid`; test ! -e pid2 || kill `cat pid2`])
])
 
-   AT_CHECK([ovsdb-client dump tcp:127.0.0.1:$TCP_PORT1], [0], [stdout], 
[ignore],
+   AT_CHECK([ovsdb-client dump], [0], [stdout], [ignore],
  [test ! -e pid || kill `cat pid`; test ! -e pid2 || kill `cat pid2`])
cat stdout >> dump1
-   AT_CHECK([ovsdb-client dump tcp:127.0.0.1:$TCP_PORT2], [0], [stdout], 
[ignore],
+   AT_CHECK([ovsdb-client dump unix:db2.sock], [0], [stdout], [ignore],
  [test ! -e pid || kill `cat pid`; test ! -e pid2 || kill `cat pid2`])
cat stdout >> dump2
 
@@ -1024,7 +1022,7 @@ m4_define([OVSDB_CHECK_EXECUTION],
 
 EXECUTION_EXAMPLES
 
-AT_BANNER([OVSDB -- ovsdb-server replication table-exclusion (TCP IPv4 
sockets)])
+AT_BANNER([OVSDB -- ovsdb-server replication table-exclusion])
 
 # OVSDB_CHECK_REPLICATION(TITLE, SCHEMA, TRANSACTIONS, OUTPUT, [KEYWORDS])
 #
@@ -1049,21 +1047,19 @@ m4_define([OVSDB_CHECK_REPLICATION],
AT_CHECK([ovsdb-tool create db1 schema], [0], [stdout], [ignore])
AT_CHECK([ovsdb-tool create db2 schema], [0], [stdout], [ignore])
 
-   AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server1.log 
--pidfile="`pwd`"/pid --remote=ptcp:0:127.0.0.1 --unixctl="`pwd`"/unixctl db1], 
[0], [ignore], [ignore])
-   PARSE_LISTENING_PORT([ovsdb-server1.log], [TCP_PORT1])
+   AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server1.log 
--pidfile="`pwd`"/pid --remote=punix:db.sock --unixctl="`pwd`"/unixctl db1], 
[0], [ignore], [ignore])
 
-   AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server2.log 
--pidfile="`pwd`"/pid2 --remote=ptcp:0:127.0.0.1 --unixctl="`pwd`"/unixctl2 
--sync-from=tcp:127.0.0.1:$TCP_PORT1 --sync-exclude-tables=mydb:b db2], [0], 
[ignore], [ignore])
-   PARSE_LISTENING_PORT([ovsdb-server2.log], [TCP_PORT2])
+   AT_CHECK([ovsdb-server --detach --no-chdir --log-file=ovsdb-server2.log 
--pidfile="`pwd`"/pid2 --remote=punix:db2.sock --unixctl="`pwd`"/unixctl2 
--sync-from=unix:db.sock --sync-exclude-tables=mydb:b db2], [0], [ignore], 
[ignore])
 
m4_foreach([txn], [$3],
- [AT_CHECK([ovsdb-client transact tcp:127.0.0.1:$TCP_PORT1 'txn'; sleep 
2], [0], [stdout], [ignore],
+ [AT_CHECK([ovsdb-client transact 'txn'; sleep 2], [0], [stdout], [ignore],
  [test ! -e pid || kill `cat pid`; test ! -e pid2 || kill `cat pid2`])
])
 
-   AT_CHECK([ovsdb-client dump tcp:127.0.0.1:$TCP_PORT1], [0], [stdout], 
[ignore],
+   AT_CHECK([ovsdb-client dump], [0], [stdout], [ignore],
  [test ! -e pid || kill `cat pid`; test ! -e pid2 || kill `cat pid2`])
cat stdout >> dump1
-   AT_CHECK([ovsdb-client dump tcp:127.0.0.1:$TCP_PORT2], [0], [stdout], 
[ignore],
+   AT_CHECK([ovsdb-client dump unix:db2.sock], [0], [stdout], [ignore],
  

Re: [ovs-dev] [PATCH v6 1/1] ovn: Apply ACL changes to existing connections.

[Justin Pettit]

> On Jul 20, 2016, at 2:26 PM, Russell Bryant  wrote:
> 
> After some help rebasing and re-testing from Babu Shanmugam, I went through 
> this again, addressed comments, and applied this to master.

Great!  Glad to see it finally merged.  Thanks for being patient with me.

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3] [OVN-northd] Rename nbs/nbr port names tonbsp/nbrp

[Justin Pettit]

> On Jul 20, 2016, at 7:25 AM, Hui Kang  wrote:
> 
> "dev"  wrote on 07/19/2016 04:15:56 PM:
> 
> > From: Justin Pettit 
> > To: Hui Kang 
> > Cc: dev@openvswitch.org
> > Date: 07/19/2016 04:16 PM
> > Subject: Re: [ovs-dev] [PATCH v3] [OVN-northd] Rename nbs/nbr port 
> > names to nbsp/nbrp
> > Sent by: "dev" 
> > 
> > 
> > > On Jul 19, 2016, at 11:36 AM, Hui Kang  wrote:
> > > 
> > > These variables indicate ports in nb switches or routers.
> > > 
> > > Signed-off-by: Hui Kang 
> > > 
> > > --
> > > v2->v3:
> > > - rebase agains master branch
> > > v1->v2:
> > > - modify commit message
> > > ---
> > 
> > Thanks for the patch.  Just a few things first, though:
> > 
> >- A couple of lines you changed went over 80 columns.  I'll wrap those.
> >- The way you wrote your revision history, it shows up in the 
> > commit message.  I'll fix it up.
> >- Your author email address and Signed-off-by don't match.  Which
> > do you want to use?
> 
> I'd like to use the corporate email account, which is ka...@us.ibm.com
> I will figure out how to setup the mail account in my dev box and keep them
> consistent next time. Thanks for your help.

Great.  I've updated the author, and I'll push it in a minute.

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Answer another question.

[Justin Pettit]

I thought you were just surprised to see one of your patches reviewed quickly.

--Justin


> On Jul 20, 2016, at 4:09 PM, Ben Pfaff  wrote:
> 
> This isn't the kind of review I'm used to.  Are you impersonating Mork
> with a speech impediment, or is there more to it?
> 
> On Wed, Jul 20, 2016 at 04:05:18PM -0700, Justin Pettit wrote:
>> NACK! NACK! NACK!
>> 
>>> On Jul 20, 2016, at 4:04 PM, Ben Pfaff  wrote:
>>> 
>>> Signed-off-by: Ben Pfaff 
>>> ---
>>> FAQ.md | 7 +++
>>> 1 file changed, 7 insertions(+)
>>> 
>>> diff --git a/FAQ.md b/FAQ.md
>>> index 063bd70..7f72b5c 100644
>>> --- a/FAQ.md
>>> +++ b/FAQ.md
>>> @@ -118,6 +118,13 @@ A: Starting in OVS 2.4, we switched the default ports 
>>> to the
>>>   cannot, all the programs allow overriding the default port.  See the
>>>   appropriate man page.
>>> 
>>> +### Q: I have a question.
>>> +
>>> +A: Have you checked the FAQ?  It answers many questions.
>>> +
>>> +   If you can't find the answer in the FAQ, then you might want to ask
>>> +   Justin Pettit .  He knows a lot.
>>> +
>>> 
>>> Releases
>>> 
>>> -- 
>>> 2.1.3
>>> 
>>> ___
>>> dev mailing list
>>> dev@openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
>> 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Answer another question.

[Ben Pfaff]

This isn't the kind of review I'm used to.  Are you impersonating Mork
with a speech impediment, or is there more to it?

On Wed, Jul 20, 2016 at 04:05:18PM -0700, Justin Pettit wrote:
> NACK! NACK! NACK!
> 
> > On Jul 20, 2016, at 4:04 PM, Ben Pfaff  wrote:
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> > FAQ.md | 7 +++
> > 1 file changed, 7 insertions(+)
> > 
> > diff --git a/FAQ.md b/FAQ.md
> > index 063bd70..7f72b5c 100644
> > --- a/FAQ.md
> > +++ b/FAQ.md
> > @@ -118,6 +118,13 @@ A: Starting in OVS 2.4, we switched the default ports 
> > to the
> >cannot, all the programs allow overriding the default port.  See the
> >appropriate man page.
> > 
> > +### Q: I have a question.
> > +
> > +A: Have you checked the FAQ?  It answers many questions.
> > +
> > +   If you can't find the answer in the FAQ, then you might want to ask
> > +   Justin Pettit .  He knows a lot.
> > +
> > 
> > Releases
> > 
> > -- 
> > 2.1.3
> > 
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Answer another question.

[Justin Pettit]

NACK! NACK! NACK!


> On Jul 20, 2016, at 4:04 PM, Ben Pfaff  wrote:
> 
> Signed-off-by: Ben Pfaff 
> ---
> FAQ.md | 7 +++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/FAQ.md b/FAQ.md
> index 063bd70..7f72b5c 100644
> --- a/FAQ.md
> +++ b/FAQ.md
> @@ -118,6 +118,13 @@ A: Starting in OVS 2.4, we switched the default ports to 
> the
>cannot, all the programs allow overriding the default port.  See the
>appropriate man page.
> 
> +### Q: I have a question.
> +
> +A: Have you checked the FAQ?  It answers many questions.
> +
> +   If you can't find the answer in the FAQ, then you might want to ask
> +   Justin Pettit .  He knows a lot.
> +
> 
> Releases
> 
> -- 
> 2.1.3
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] FAQ: Answer another question.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 FAQ.md | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/FAQ.md b/FAQ.md
index 063bd70..7f72b5c 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -118,6 +118,13 @@ A: Starting in OVS 2.4, we switched the default ports to 
the
cannot, all the programs allow overriding the default port.  See the
appropriate man page.
 
+### Q: I have a question.
+
+A: Have you checked the FAQ?  It answers many questions.
+
+   If you can't find the answer in the FAQ, then you might want to ask
+   Justin Pettit .  He knows a lot.
+
 
 Releases
 
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] encaps: Fix potential null pointer dereferences.

[Ben Pfaff]

Found by inspection.

Signed-off-by: Ben Pfaff 
---
 ovn/controller/encaps.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 03ec732..977cc3a 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -103,6 +103,10 @@ port_hash_rec(const struct ovsrec_port *port)
 
 iface = port->interfaces[0];
 ip = smap_get(>options, "remote_ip");
+if (!ip) {
+/* This should not happen for an OVN-created port. */
+return 0;
+}
 
 return port_hash(chassis_id, iface->type, ip);
 }
@@ -314,7 +318,8 @@ check_and_update_tunnel(const struct sbrec_chassis 
*chassis_rec)
 if (strcmp(encap->type, iface->type)) {
 ovsrec_interface_set_type(iface, encap->type);
 }
-if (strcmp(encap->ip, smap_get(>options, "remote_ip"))) {
+const char *ip = smap_get(>options, "remote_ip");
+if (!ip || strcmp(encap->ip, ip)) {
 struct smap options = SMAP_INITIALIZER();
 smap_add(, "remote_ip", encap->ip);
 smap_add(, "key", "flow");
@@ -322,8 +327,8 @@ check_and_update_tunnel(const struct sbrec_chassis 
*chassis_rec)
 smap_destroy();
 }
 
-if (strcmp(chassis_rec->name, smap_get(>external_ids,
-   "ovn-chassis-id"))) {
+const char *chassis = smap_get(>external_ids, "ovn-chassis-id");
+if (!chassis || strcmp(chassis_rec->name, chassis)) {
 const struct smap id = SMAP_CONST1(, "ovn-chassis-id",
chassis_rec->name);
 ovsrec_port_set_external_ids(port, );
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v6 1/1] ovn: Apply ACL changes to existing connections.

[Russell Bryant]

On Thu, Jul 7, 2016 at 1:13 PM, Justin Pettit  wrote:

>
> > On Jun 30, 2016, at 10:14 PM, Russell Bryant  wrote:
> >
> > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> > index 260cc14..d2bddcb 100644
> > --- a/ovn/northd/ovn-northd.8.xml
> > +++ b/ovn/northd/ovn-northd.8.xml
> >
>
> >
> > @@ -282,20 +299,37 @@
> >   
> >
> >   
> > -A priority-65535 flow that allows any traffic that has been
> > -committed to the connection tracker (i.e., established flows).
> > +A priority-65535 flow that allows any traffic in the reply
> > +direction for a connection that has been committed to the
> > +connection tracker (i.e., established flows), as long as
> > +the committed flow does not have ct_label[0]=1 set.
>
> There are a couple of places where "ct_label[0]=1 set", but I
> think it would be clearer to just drop "=1".  Especially if you take my
> later suggestion to use a symbolic name for the field.
>

I dropped the "=1" part.


>
> > @@ -1444,38 +1472,106 @@ build_acls(struct ovn_datapath *od, struct hmap
> *lflows, struct hmap *ports)
> > ...
> > +ds_put_format(, "((ct.new && !ct.est)"
> > +  " || (!ct.new && ct.est &&
> !ct.rpl "
> > +   "&& ct_label[0] == 1)) "
>
> It might be nice to use a symbolic name, similar to "eth.mcast".  That way
> it's meaning is clearer, and if we ever need to change the label, it only
> needs to be done in one place.


Yes, that's a very nice suggestion.  I decided to address this in a
follow-up patch, though.  I hope you don't mind.  I'm including it in
ovn/TODO in the meantime.


>
> > +if (has_stateful) {
> > +/* The implementation of "drop" differs if stateful
> ACLs are in
> > + * use for this datapath.  In that case, the actions
> differ
> > + * depending on whether the connection was previously
> committed
> > + * to the connection tracker with ct_commit.
> > + *
> > + * If the packet is not part of an established
> connection, then
> > + * we can simply drop it. */
>
> Minor nit: I think it might be clearer to break the two paragraphs into
> two comments because the first paragraph applies to the entire code block.


Done.


>
> > +ds_put_format(,
> > +  "(!ct.est || (ct.est && ct_label[0] ==
> 1)) "
> > +  "&& (%s)",
> > +  acl->match);
> > +ovn_lflow_add(lflows, od, stage, acl->priority +
> > +OVN_ACL_PRI_OFFSET, ds_cstr(), "drop;");
> > +
> > +/* For an existing connection without ct_label set,
> we've
> > + * encountered a policy change. ACLs previously allowed
> > + * this connection and we committed the connection
> tracking
> > + * entry.  Current policy says that we should drop this
> > + * connection.  First, we set bit 0 of ct_label to
> indicate
> > + * that this connection is set for deletion.  By not
> > + * specifying "next;", we implicitly drop the packet
> after
> > + * updating conntrack state. */
> > +
> > +ds_clear();
>
> I think you can drop the blank line after the comment.
>

Done.


> Thanks for implementing this.  I apologize for taking so long to review
> it.  As penance, I'd be happy to rebase the code if you'd like.
>
> Acked-by: Justin Pettit 
>

After some help rebasing and re-testing from Babu Shanmugam, I went through
this again, addressed comments, and applied this to master.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 3/3] Red Hat Systemd Integration

[Aaron Conole]

This commit builds upon some of the recent ovs-ctl changes to build a
more integrated systemd setup.  A new service (ovs-vswitchd) is
added to track the ovs-vswitchd, and ovsdb-server service is reserved
for the ovsdb-server daemon.  The systemd scripts still use ovs-ctl to
actually initialize the daemons.

Signed-off-by: Aaron Conole 
---
 rhel/automake.mk |  1 +
 rhel/etc_sysconfig_network-scripts_ifup-ovs  |  6 +++---
 rhel/openvswitch-fedora.spec.in  |  3 ++-
 rhel/usr_lib_systemd_system_openvswitch.service  |  6 --
 rhel/usr_lib_systemd_system_ovs-vswitchd.service | 18 ++
 rhel/usr_lib_systemd_system_ovsdb-server.service | 17 ++---
 6 files changed, 38 insertions(+), 13 deletions(-)
 create mode 100644 rhel/usr_lib_systemd_system_ovs-vswitchd.service

diff --git a/rhel/automake.mk b/rhel/automake.mk
index 7907a87..a3c180c 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -27,6 +27,7 @@ EXTRA_DIST += \
rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
rhel/usr_lib_systemd_system_openvswitch.service \
rhel/usr_lib_systemd_system_ovsdb-server.service \
+   rhel/usr_lib_systemd_system_ovs-vswitchd.service \
rhel/usr_lib_systemd_system_ovn-controller.service \
rhel/usr_lib_systemd_system_ovn-controller-vtep.service \
rhel/usr_lib_systemd_system_ovn-northd.service
diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs 
b/rhel/etc_sysconfig_network-scripts_ifup-ovs
index eb58c3a..6f1c891 100755
--- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
@@ -60,10 +60,10 @@ fi
fi
 done
 
-SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
+SERVICE_UNIT=/usr/lib/systemd/system/openvswitch.service
 if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
-   if ! systemctl --quiet is-active ovsdb-server.service; then
-   systemctl start ovsdb-server.service
+   if ! systemctl --quiet is-active openvswitch.service; then
+   systemctl start openvswitch.service
fi
 else
if [ ! -f /var/lock/subsys/openvswitch ]; then
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 253d5bc..b1f07e7 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -189,7 +189,7 @@ install -d -m 0755 $RPM_BUILD_ROOT%{_sysconfdir}/openvswitch
 install -p -D -m 0644 \
 rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
 $RPM_BUILD_ROOT/%{_sysconfdir}/sysconfig/openvswitch
-for service in openvswitch ovsdb-server \
+for service in openvswitch ovsdb-server ovs-vswitchd \
ovn-controller ovn-controller-vtep ovn-northd; do
install -p -D -m 0644 \
rhel/usr_lib_systemd_system_${service}.service \
@@ -417,6 +417,7 @@ fi
 %config(noreplace) %{_sysconfdir}/logrotate.d/openvswitch
 %{_unitdir}/openvswitch.service
 %{_unitdir}/ovsdb-server.service
+%{_unitdir}/ovs-vswitchd.service
 %{_datadir}/openvswitch/scripts/openvswitch.init
 %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
 %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
diff --git a/rhel/usr_lib_systemd_system_openvswitch.service 
b/rhel/usr_lib_systemd_system_openvswitch.service
index 96c697b..a44574b 100644
--- a/rhel/usr_lib_systemd_system_openvswitch.service
+++ b/rhel/usr_lib_systemd_system_openvswitch.service
@@ -1,11 +1,13 @@
 [Unit]
 Description=Open vSwitch
-After=syslog.target network.target ovsdb-server.service
-Requires=ovsdb-server.service
+PartOf=network.target
+BindsTo=ovsdb-server.service
+BindsTo=ovs-vswitchd.service
 
 [Service]
 Type=oneshot
 ExecStart=/bin/true
+ExecReload=/bin/true
 ExecStop=/bin/true
 RemainAfterExit=yes
 
diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service 
b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
new file mode 100644
index 000..d3d020a
--- /dev/null
+++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
@@ -0,0 +1,18 @@
+[Unit]
+Description=Open vSwitch Forwarding Unit
+After=ovsdb-server.service
+Requires=ovsdb-server.service
+ReloadPropagatedFrom=ovsdb-server.service
+AssertPathIsReadWrite=/var/run/openvswitch/db.sock
+PartOf=openvswitch.service
+
+[Service]
+Type=forking
+EnvironmentFile=-/etc/sysconfig/openvswitch
+ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
+  --no-ovsdb-server --no-monitor --system-id=random \
+  start $OPTIONS
+ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
+ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
+   --no-monitor --system-id=random \
+   restart $OPTIONS
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index e4c2a66..847948e 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ 

[ovs-dev] [PATCH 0/3] redhat: Improve the systemd integration

[Aaron Conole]

These patches make adjustments to the way systemd is done so that we have:

* one daemon started per service
* services which correspond to actual daemon names
* a single 'dummy' service to start all at once
* some convenient information to poll from systemd.

These have been tested on Fedora 23 and RHEL-7.

Aaron Conole (3):
  utilities/ovs-ctl.in: Allow non-monitoring daemons
  rhel/ovsdb-server.service: Rename the nonetwork service
  Red Hat Systemd Integration

 rhel/automake.mk   |  3 ++-
 rhel/etc_sysconfig_network-scripts_ifdown-ovs  |  6 +++---
 rhel/etc_sysconfig_network-scripts_ifup-ovs|  6 +++---
 rhel/openvswitch-fedora.spec.in|  5 +++--
 ...sr_lib_systemd_system_openvswitch-nonetwork.service | 15 ---
 rhel/usr_lib_systemd_system_openvswitch.service|  6 --
 rhel/usr_lib_systemd_system_ovs-vswitchd.service   | 18 ++
 rhel/usr_lib_systemd_system_ovsdb-server.service   | 18 ++
 utilities/ovs-ctl.8|  5 +
 utilities/ovs-ctl.in   |  1 +
 utilities/ovs-lib.in   |  3 ++-
 11 files changed, 59 insertions(+), 27 deletions(-)
 delete mode 100644 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
 create mode 100644 rhel/usr_lib_systemd_system_ovs-vswitchd.service
 create mode 100644 rhel/usr_lib_systemd_system_ovsdb-server.service

-- 
2.5.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/3] utilities/ovs-ctl.in: Allow non-monitoring daemons

[Aaron Conole]

This commit allows the ovs-ctl command to spawn daemons without the
internal process monitor.  This is useful when integrating with,
ex. systemd, which provides its own monitoring facilities.

Signed-off-by: Aaron Conole 
Acked-by: Ben Pfaff 
Acked-by: Flavio Leitner 
---
 utilities/ovs-ctl.8  | 5 +
 utilities/ovs-ctl.in | 1 +
 utilities/ovs-lib.in | 3 ++-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/utilities/ovs-ctl.8 b/utilities/ovs-ctl.8
index 662b83e..6b8fba7 100644
--- a/utilities/ovs-ctl.8
+++ b/utilities/ovs-ctl.8
@@ -162,6 +162,11 @@ after reboot, but other ports need to be persisted in the 
database.
 .PP
 The following options are less important:
 .
+.IP "\fB\-\-no\-monitor\fR"
+By default \fBovs\-ctl\fR passes \fB\-\-monitor\fR to \fBovs\-vswitchd\fR and
+\fBovsdb\-server\fR, requesting that it spawn a process monitor which will
+restart the daemon if it crashes.  This option suppresses that behavior.
+.
 .IP "\fB\-\-daemon-cwd=\fIdirectory\fR"
 Specifies the current working directory that the OVS daemons should
 run from.  The default is \fB/\fR (the root directory) if this option
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 8ec825b..d92cf3c 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -499,6 +499,7 @@ set_defaults () {
 FORCE_COREFILES=yes
 MLOCKALL=yes
 SELF_CONFINEMENT=yes
+MONITOR=yes
 OVSDB_SERVER=yes
 OVS_VSWITCHD=yes
 OVSDB_SERVER_PRIORITY=-10
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 773efb3..5f23269 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -151,7 +151,8 @@ start_daemon () {
 # pidfile and monitoring
 test -d "$rundir" || install -d -m 755 -o root -g root "$rundir"
 set "$@" --pidfile="$rundir/$daemon.pid"
-set "$@" --detach --monitor
+set "$@" --detach
+test X"$MONITOR" = "Xno" || set "$@" --monitor
 
 # wrapper
 case $wrapper in
-- 
2.5.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 2/3] rhel/ovsdb-server.service: Rename the nonetwork service

[Aaron Conole]

Currently, openvswitch.service calls out to start
openvswitch-nonetwork.service.  However, openvswitch-nonetwork.service
will be called ovsdb-server, so that it is a bit more reflective of
the dependencies.  This commit does make the file a bit of a misnomer as
currently the ovsdb-server SERVICE will start the ovs-vswitchd service
as well.  A future commit will clean this up, and change the ifup
configuration in the process.

Signed-off-by: Aaron Conole 
Acked-by: Flavio Leitner 
---
 rhel/automake.mk  |  2 +-
 rhel/etc_sysconfig_network-scripts_ifdown-ovs |  6 +++---
 rhel/etc_sysconfig_network-scripts_ifup-ovs   |  6 +++---
 rhel/openvswitch-fedora.spec.in   |  4 ++--
 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service | 15 ---
 rhel/usr_lib_systemd_system_openvswitch.service   |  4 ++--
 rhel/usr_lib_systemd_system_ovsdb-server.service  | 15 +++
 7 files changed, 26 insertions(+), 26 deletions(-)
 delete mode 100644 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
 create mode 100644 rhel/usr_lib_systemd_system_ovsdb-server.service

diff --git a/rhel/automake.mk b/rhel/automake.mk
index dc30715..7907a87 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -26,7 +26,7 @@ EXTRA_DIST += \
rhel/usr_share_openvswitch_scripts_sysconfig.template \
rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
rhel/usr_lib_systemd_system_openvswitch.service \
-   rhel/usr_lib_systemd_system_openvswitch-nonetwork.service \
+   rhel/usr_lib_systemd_system_ovsdb-server.service \
rhel/usr_lib_systemd_system_ovn-controller.service \
rhel/usr_lib_systemd_system_ovn-controller-vtep.service \
rhel/usr_lib_systemd_system_ovn-northd.service
diff --git a/rhel/etc_sysconfig_network-scripts_ifdown-ovs 
b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
index 46b6ca5..dd98d23 100755
--- a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
@@ -34,10 +34,10 @@ if [ ! -x ${OTHERSCRIPT} ]; then
 OTHERSCRIPT="/etc/sysconfig/network-scripts/ifdown-eth"
 fi
 
-SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
+SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
 if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
-   if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
-   systemctl start openvswitch-nonetwork.service
+   if ! systemctl --quiet is-active ovsdb-server.service; then
+   systemctl start ovsdb-server.service
fi
 else
if [ ! -f /var/lock/subsys/openvswitch ]; then
diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs 
b/rhel/etc_sysconfig_network-scripts_ifup-ovs
index f3fc05e..eb58c3a 100755
--- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
@@ -60,10 +60,10 @@ fi
fi
 done
 
-SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
+SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
 if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
-   if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
-   systemctl start openvswitch-nonetwork.service
+   if ! systemctl --quiet is-active ovsdb-server.service; then
+   systemctl start ovsdb-server.service
fi
 else
if [ ! -f /var/lock/subsys/openvswitch ]; then
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 088afcb..253d5bc 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -189,7 +189,7 @@ install -d -m 0755 $RPM_BUILD_ROOT%{_sysconfdir}/openvswitch
 install -p -D -m 0644 \
 rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
 $RPM_BUILD_ROOT/%{_sysconfdir}/sysconfig/openvswitch
-for service in openvswitch openvswitch-nonetwork \
+for service in openvswitch ovsdb-server \
ovn-controller ovn-controller-vtep ovn-northd; do
install -p -D -m 0644 \
rhel/usr_lib_systemd_system_${service}.service \
@@ -416,7 +416,7 @@ fi
 %config(noreplace) %{_sysconfdir}/sysconfig/openvswitch
 %config(noreplace) %{_sysconfdir}/logrotate.d/openvswitch
 %{_unitdir}/openvswitch.service
-%{_unitdir}/openvswitch-nonetwork.service
+%{_unitdir}/ovsdb-server.service
 %{_datadir}/openvswitch/scripts/openvswitch.init
 %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
 %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
diff --git a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service 
b/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
deleted file mode 100644
index e4c2a66..000
--- a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
+++ /dev/null
@@ -1,15 +0,0 @@
-[Unit]
-Description=Open vSwitch Internal Unit

[ovs-dev] [PATCH] datapath: Fix vxlan local traffic.

[Pravin B Shelar]

vxlan driver has bypass for local vxlan traffic, but that
depends on information about all VNIs on local system in
vxlan driver. This is not available in case of LWT.
Therefore following patch disable encap bypass for LWT
vxlan traffic.

Reported-by: Jakub Libosvar 
Signed-off-by: Pravin B Shelar 
---
This patch is for testing only. I am planing on sending fix
for upstream vxlan soon.
---
 datapath/linux/compat/vxlan.c | 71 ++-
 1 file changed, 37 insertions(+), 34 deletions(-)

diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 6d77527..d17a30d 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -1109,23 +1109,23 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct 
net_device *dev,
}
 
/* Bypass encapsulation if the destination is local */
-   if (rt->rt_flags & RTCF_LOCAL &&
-   !(rt->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
-   struct vxlan_dev *dst_vxlan;
-
-   ip_rt_put(rt);
-   dst_vxlan = vxlan_find_vni(vxlan->net, vni,
-  dst->sa.sa_family, dst_port,
-  vxlan->flags);
-   if (!dst_vxlan)
-   goto tx_error;
-   vxlan_encap_bypass(skb, vxlan, dst_vxlan);
-   return;
-   }
-
-   if (!info)
+   if (!info) {
+   if (rt->rt_flags & RTCF_LOCAL &&
+   !(rt->rt_flags & (RTCF_BROADCAST | 
RTCF_MULTICAST))) {
+   struct vxlan_dev *dst_vxlan;
+
+   ip_rt_put(rt);
+   dst_vxlan = vxlan_find_vni(vxlan->net, vni,
+  dst->sa.sa_family,
+  dst_port,
+  vxlan->flags);
+   if (!dst_vxlan)
+   goto tx_error;
+   vxlan_encap_bypass(skb, vxlan, dst_vxlan);
+   return;
+   }
udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM_TX);
-   else if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT)
+   } else if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT)
df = htons(IP_DF);
 
tos = ip_tunnel_ecn_encap(tos, old_iph, skb);
@@ -1142,7 +1142,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct 
net_device *dev,
} else {
struct dst_entry *ndst;
struct in6_addr saddr;
-   u32 rt6i_flags;
 
if (!vxlan->vn6_sock)
goto drop;
@@ -1167,24 +1166,28 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct 
net_device *dev,
goto tx_error;
}
 
-   /* Bypass encapsulation if the destination is local */
-   rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
-   if (rt6i_flags & RTF_LOCAL &&
-   !(rt6i_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
-   struct vxlan_dev *dst_vxlan;
-
-   dst_release(ndst);
-   dst_vxlan = vxlan_find_vni(vxlan->net, vni,
-  dst->sa.sa_family, dst_port,
-  vxlan->flags);
-   if (!dst_vxlan)
-   goto tx_error;
-   vxlan_encap_bypass(skb, vxlan, dst_vxlan);
-   return;
-   }
-
-   if (!info)
+   if (!info) {
+   u32 rt6i_flags;
+
+   rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
+
+   /* Bypass encapsulation if the destination is local */
+   if (rt6i_flags & RTF_LOCAL &&
+   !(rt6i_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
+   struct vxlan_dev *dst_vxlan;
+
+   dst_release(ndst);
+   dst_vxlan = vxlan_find_vni(vxlan->net, vni,
+  dst->sa.sa_family,
+  dst_port,
+  vxlan->flags);
+   if (!dst_vxlan)
+   goto tx_error;
+   vxlan_encap_bypass(skb, vxlan, dst_vxlan);
+   return;
+  

Re: [ovs-dev] [PATCH] ovn: Add datapth of gateway port tolocal_datapaths

[Russell Bryant]

On Mon, Jul 18, 2016 at 11:22 AM, Ryan Moats  wrote:

> "dev"  wrote on 07/18/2016 07:50:06 AM:
>
> > From: Chandra Sekhar Vejendla/San Jose/IBM@IBMUS
> > To: dev@openvswitch.org
> > Date: 07/18/2016 07:50 AM
> > Subject: [ovs-dev] [PATCH] ovn: Add datapth of gateway port to
> local_datapaths
> > Sent by: "dev" 
> >
> > When a l3 gateway port is created on a chassis, the corresponding
> > datapath is not added to local datapths. This results in patch
> > ports not getting created between local network and br-int
> >
> > Signed-off-by: Chandra Sekhar Vejendla 
>
> Can you please add a simple test case that verifies that the patch
> ports get created?  That way we can see them not being created
> without this code and being created with it and avoid later
> regressions.
>

You might want to consider an end-to-end test that sets up this scenario
and makes sure a packet makes it through.  That covers more than just
making sure a port binding is created.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] release-process.md: Document OVS release process and propose a schedule.

[Ben Pfaff]

On Wed, Jul 20, 2016 at 03:48:08PM -0400, Russell Bryant wrote:
> On Wed, Jul 20, 2016 at 12:30 PM, Ben Pfaff  wrote:
> 
> > On Wed, Jul 20, 2016 at 10:00:22AM -0400, Russell Bryant wrote:
> > > On Tue, Jul 19, 2016 at 1:58 PM, Ben Pfaff  wrote:
> > >
> > > > This document has two different kinds of text:
> > > >
> > > >- The first sections of the document, "Release Strategy" and
> > "Release
> > > >  Numbering", describe what we've already been doing for most of the
> > > >  history of Open vSwitch.  If there is anything surprising in them,
> > > >  then it's because our process has not been transparent enough,
> > and not
> > > >  because we're making a change.
> > > >
> > > >- The final section of the document, "Release Scheduling", is a
> > proposal
> > > >  for current and future releases.  We have not had a regular
> > release
> > > >  schedule in the past, but it seems important to have one in the
> > > >  future, so this section requires review and feedback from
> > everyone in
> > > >  the community.
> > > I think this is a great step forward.  Thank you!
> >
> > Thanks for the review.
> >
> > > One topic that could be added to this document is discussion of how long
> > > each release branch is maintained.  LTS is defined in FAQ.md, but it
> > could
> > > be defined in this document now.  How an LTS branch is chosen, and the
> > > maintenance difference between LTS and non-LTS would also be good topics
> > to
> > > cover.
> >
> > I forgot that the FAQ talked about releases.  I'm appending an
> > incremental that I will fold into this patch.
> >
> > It is a good idea to describe LTS releases, but I don't have answers for
> > the questions you ask.  Here are some thoughts about principles we've
> > considered before:
> >
> > * We try to avoid making releases that include disruptive internal
> >   changes LTS, because they are harder to support.
> >
> > * It is good to make LTS releases at least every 2 years or so,
> >   because it is useful to distributions and other downstreams, but
> >   not much more often than that, because it is more work to maintain
> >   multiple upstreams.
> >
> > * In the past we have maintained a given LTS until we release the
> >   next LTS.  This is probably too vague and may not be long enough
> >   in any case.
> >
> > Anyone want to suggest what we should do?
> >
> 
> I don't think needs to be very formal.  It may not be all the useful to try
> to lay out a 5-year release schedule based on LTS planning, as it seems
> incredibly likely that things will change.  Being specific about the
> 6-month cadence is enough commitment for me.  :-)
> 
> Here's some proposed text ...
> 
> At most two release branches are maintained at any given time: the latest
> release and the latest release designed as LTS.  An LTS release is one that
> the OVS project has designated as being maintained for a longer period of
> time.  Currently, an LTS release is maintained until the next LTS is
> chosen.  There is not currently a strict guideline on how often a new LTS
> release is chosen, but so far it has been about every 2 years.  That could
> change based on the current state of OVS development.  For example, we do
> not want to designate a new release as LTS that includes disruptive
> internal changes, as that may make it harder to support for a longer period
> of time.  Discussion about choosing the next LTS release occurs on the OVS
> development mailing list.

That's good, I folded that in.  Here's the current version.

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

From: Ben Pfaff 
Date: Wed, 20 Jul 2016 12:59:26 -0700
Subject: [PATCH] release-process.md: Document OVS release process and propose
 a schedule.

This document has two different kinds of text:

   - The first sections of the document, "Release Strategy" and "Release
 Numbering", describe what we've already been doing for most of the
 history of Open vSwitch.  If there is anything surprising in them,
 then it's because our process has not been transparent enough, and not
 because we're making a change.

   - The final section of the document, "Release Scheduling", is a proposal
 for current and future releases.  We have not had a regular release
 schedule in the past, but it seems important to have one in the
 future, so this section requires review and feedback from everyone in
 the community.

Signed-off-by: Ben Pfaff 
Acked-by: Russell Bryant 
---
 Documentation/automake.mk|  3 +-
 Documentation/release-process.md | 98 
 FAQ.md   |  8 +++-
 3 files changed, 106 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/release-process.md

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index aae41d2..d709e77 100644
--- 

Re: [ovs-dev] [PATCH] release-process.md: Document OVS release process and propose a schedule.

[Russell Bryant]

On Wed, Jul 20, 2016 at 12:30 PM, Ben Pfaff  wrote:

> On Wed, Jul 20, 2016 at 10:00:22AM -0400, Russell Bryant wrote:
> > On Tue, Jul 19, 2016 at 1:58 PM, Ben Pfaff  wrote:
> >
> > > This document has two different kinds of text:
> > >
> > >- The first sections of the document, "Release Strategy" and
> "Release
> > >  Numbering", describe what we've already been doing for most of the
> > >  history of Open vSwitch.  If there is anything surprising in them,
> > >  then it's because our process has not been transparent enough,
> and not
> > >  because we're making a change.
> > >
> > >- The final section of the document, "Release Scheduling", is a
> proposal
> > >  for current and future releases.  We have not had a regular
> release
> > >  schedule in the past, but it seems important to have one in the
> > >  future, so this section requires review and feedback from
> everyone in
> > >  the community.
> > I think this is a great step forward.  Thank you!
>
> Thanks for the review.
>
> > One topic that could be added to this document is discussion of how long
> > each release branch is maintained.  LTS is defined in FAQ.md, but it
> could
> > be defined in this document now.  How an LTS branch is chosen, and the
> > maintenance difference between LTS and non-LTS would also be good topics
> to
> > cover.
>
> I forgot that the FAQ talked about releases.  I'm appending an
> incremental that I will fold into this patch.
>
> It is a good idea to describe LTS releases, but I don't have answers for
> the questions you ask.  Here are some thoughts about principles we've
> considered before:
>
> * We try to avoid making releases that include disruptive internal
>   changes LTS, because they are harder to support.
>
> * It is good to make LTS releases at least every 2 years or so,
>   because it is useful to distributions and other downstreams, but
>   not much more often than that, because it is more work to maintain
>   multiple upstreams.
>
> * In the past we have maintained a given LTS until we release the
>   next LTS.  This is probably too vague and may not be long enough
>   in any case.
>
> Anyone want to suggest what we should do?
>

I don't think needs to be very formal.  It may not be all the useful to try
to lay out a 5-year release schedule based on LTS planning, as it seems
incredibly likely that things will change.  Being specific about the
6-month cadence is enough commitment for me.  :-)

Here's some proposed text ...

At most two release branches are maintained at any given time: the latest
release and the latest release designed as LTS.  An LTS release is one that
the OVS project has designated as being maintained for a longer period of
time.  Currently, an LTS release is maintained until the next LTS is
chosen.  There is not currently a strict guideline on how often a new LTS
release is chosen, but so far it has been about every 2 years.  That could
change based on the current state of OVS development.  For example, we do
not want to designate a new release as LTS that includes disruptive
internal changes, as that may make it harder to support for a longer period
of time.  Discussion about choosing the next LTS release occurs on the OVS
development mailing list.


> > Acked-by: Russell Bryant 
>
> Thanks.  I'm going to let the discussion develop for a while before I
> push anything, because I want to see a semblance of consensus on the
> schedule.
>

Of course.  I was just registering my formal +1 in the meantime.


> --8<--cut here-->8--
>
> diff --git a/FAQ.md b/FAQ.md
> index 063bd70..290e66c 100644
> --- a/FAQ.md
> +++ b/FAQ.md
> @@ -125,13 +125,16 @@ Releases
>  ### Q: What does it mean for an Open vSwitch release to be LTS (long-term
> support)?
>
>  A: All official releases have been through a comprehensive testing
> -   process and are suitable for production use.  Planned releases will
> -   occur several times a year.  If a significant bug is identified in an
> +   process and are suitable for production use.  Planned releases
> +   occur twice a year.  If a significant bug is identified in an
> LTS release, we will provide an updated release that includes the
> fix.  Releases that are not LTS may not be fixed and may just be
> supplanted by the next major release.  The current LTS release is
> 2.3.x.
>
> +   For more information on the Open vSwitch release process, please
> +   see [release-process.md].
> +
>  ### Q: What Linux kernel versions does each Open vSwitch release work
> with?
>
>  A: The following table lists the Linux kernel versions against which the
> @@ -2140,3 +2143,4 @@ http://openvswitch.org/
>  [OPENFLOW-1.1+.md]:OPENFLOW-1.1+.md
>  [INSTALL.DPDK.md]:INSTALL.DPDK.md
>  [Tutorial.md]:tutorial/Tutorial.md
> +[release-process.md]:Documentation/release-process.md
>



-- 
Russell Bryant

[ovs-dev] Bug#831924: openvswitch: FTBFS with dpkg-buildpackage -A: cp: cannot create regular file 'debian/openvswitch-switch/usr/share/openvswitch/switch/default.template': No such file or directory

[Lucas Nussbaum]

Source: openvswitch
Version: 2.5.1~pre+git20160626-2
Severity: important
Tags: stretch sid
User: debian...@lists.debian.org
Usertags: qa-ftbfs-20160720 qa-ftbfs qa-indep
Justification: FTBFS on amd64

Hi,

During a rebuild of all packages in sid, your package failed to build on
amd64.  This rebuild was done by building only the architecture-independent
packages.  At the same time, a normal build succeeded, which points the
problem specifically to build-indep/binary-indep targets.

If all the arch-independent packages are dummy transitional packages
released with jessie, the easy fix is to drop them now. If not, debian/rules
should be modified so that the build-indep and binary-indep target generates
the architecture independent packages (and only those).

After checking that both "dpkg-buildpackage -A" and "dpkg-buildpackage -B" work
properly, this package will be suitable to be uploaded in source-only form if
you wish.

I file this bug as severity: important, but Santiago Vila, who led this
effort (kudos to him), got approval from the release team to consider those
bugs RC for stretch. The severity will be increased to 'serious' shortly.
See #830997 for details.

Relevant part (hopefully):
> make[1]: Entering directory '/<>/openvswitch-2.5.1~pre+git20160626'
> dh_install
> # openvswitch-switch
> cp debian/openvswitch-switch.template 
> debian/openvswitch-switch/usr/share/openvswitch/switch/default.template
> cp: cannot create regular file 
> 'debian/openvswitch-switch/usr/share/openvswitch/switch/default.template': No 
> such file or directory
> debian/rules:51: recipe for target 'override_dh_install' failed
> make[1]: *** [override_dh_install] Error 1

The full build log is available from:
   
http://people.debian.org/~lucas/logs/2016/07/20/openvswitch_2.5.1~pre+git20160626-2_unstable_archallonly.log

A list of current common problems and possible solutions is available at
http://wiki.debian.org/qa.debian.org/FTBFS . You're welcome to contribute!

About the archive rebuild: The rebuild was done on EC2 VM instances from
Amazon Web Services, using a clean, minimal and up-to-date chroot. Every
failed build was retried once to eliminate random failures.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Scanned image from cop...@openvswitch.com

[copier@]

Reply to: cop...@openvswitch.com 
Device Name: cop...@openvswitch.com
Device Model: MX-2310U

File Format: Microsoft Office Word
Resolution: 200dpi x 200dpi

Attached file is scanned image in Microsoft Office Word format.
Use Microsoft Office Word to view the document.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Add support for 802.1ad (QinQ tunneling)

[Eric Garver]

Sorry for the late reply. I missed this sub-thread somehow.

On Thu, Jul 14, 2016 at 09:38:09AM +0800, Xiao Liang wrote:
> As far as I know, Eric and Tom are working on the kernel patch set and
> would submit a new version to net-next. Kernel patches usually go to
> netdev first and then backported to OVS tree. Meanwhile, this

Yes. I'm currently resuming Tom's work. I'm hoping to post a new set of
kernel patches to netdev in the coming weeks. Stay tuned!

> userspace patch works with current kernel module, with a limitation
> that inner VLAN header can not be matched, which should only impact
> the egress cvlan check. It would be nice if you could help try and
> review these patches.
> Regarding neutron part, Bo is working on it, and we are looking for a
> way to get OVS capabilities. Do you think it's better submit for
> discuss now or wait for the OVS part to be ready?
> 
> Thanks,
> Xiao
> 
> On Thu, Jul 14, 2016 at 2:35 AM, Carl Baldwin  wrote:
> > On Sat, Jun 25, 2016 at 4:13 AM, Xiao Liang  wrote:
> >>
> >> Hi,
> >>
> >> I'm looking for QinQ support in OVS for some time. And found patches[1-3]
> >> by Thomas, and Gayathri. What I want is to use QinQ tunneling in neutron
> >> networking, which needs vlan_mode configuration of ports (as in Gayathri's
> >> patch) as well as openflow VLAN manipulating action support (as in Thomas's
> >> patch). Unfortunately, these patches are not compatible, so I made this
> >> patch.
> >> It's based on Thomas's kernel patch and reference the idea from Gayathri,
> >> and is (in my tests) backward compatible. Although the kernel patch not
> >> merged yet, I believe the nlmsg will not change.
> >
> >
> > I haven't seen much discussion on the kernel patches that you reference
> > since they were proposed in November.  I'm unfamiliar with the process for
> > landing changes like this which span the linux and ovs repositories.  Is
> > there effort being made elsewhere to get these to land?  Where might I look
> > to follow that effort?  Is there anything I can do to help with the process?
> >
> > Carl
> >
> >> References:
> >> [1] openvswitch: 802.1ad uapi changes, Thomas F Herbert,
> >> https://patchwork.ozlabs.org/patch/540673/
> >> Check for vlan ethernet types for 8021.q or 802.1ad,
> >> https://patchwork.ozlabs.org/patch/540668/
> >> openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink
> >> attributes, https://patchwork.ozlabs.org/patch/540671/
> >> [2] Add 802.1ad (qinq) support, Thomas F Herbert,
> >> http://openvswitch.org/pipermail/dev/2015-October/060874.html
> >> [3] 802.1ad support in OVS & OVS-DPDK, Gayathri Manepalli,
> >> http://openvswitch.org/pipermail/dev/2016-February/066265.html
> >>
> >
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Considering the possibility of integrating DPDK generic classifier APIs in OVS.

[Gray, Mark D]

> 
> On Wed, Jul 20, 2016 at 6:43 PM, Gray, Mark D 
> wrote:
> >  [Gray, Mark D] I think we should focus on one or two use cases rather
> > than a general offload like you discuss below. A general offload
> > involves a huge amount of code churn and there are a lot of difficulties,
> some that you have highlighted below.
> > A more focused implementation will flush out any issues with the API.
> > In particular, the VxLAN use case that you mentioned above and perhaps
> > the offload of the hash calculation (but the generic filtering api
> > would also need to support generation of hashes) could be two targets for
> this DPDK api.
> 
> I agree that targeting a specific use case is a good idea (as well as your 
> other
> comments). It's probably worthwhile talking to John Fastabend about this
> (also from Intel) since he has tried to something similar for the past several
> years in Linux. Many of the general problems listed in the original email turn
> out to be very difficult.
> (Examples include capabilities; describing flows in a hardware independent
> manner is something that OpenFlow tried to tackle for a long time; which
> flows to offload in the face of table size limits while maintaining correct
> forwarding behavior; etc.)
 [Gray, Mark D] 
Yes, John and I have discussed a lot of this in depth and we have done 
whiteboarding
of possible hw offload designs in OVS which is why I am quite familiar with the 
issues.

> 
> I think the VXLAN acceleration was a good use case since the vswitch is the
> owner of the tunnel endpoint and therefore is better equipped to make
> policy decisions. The main concern that I had with the previous
> implementation was that it was making assumptions about the contents of
> the inner flow based on the UDP source port, which is not really safe since
> that is just a hash.
[Gray, Mark D] 
I read your comments on this I had a look through Sugesh's code to try and see 
where
this was happening. I couldn’t see it but I agree that the source port is 
basically
random and it's only a hash of the inner flow by convention. Sugesh, is Jesse's 
concern valid in your implementation? I thought it was actually extracting the
inner header and you weren't making an assumption about the source port?
> 
> Providing hashes or other flow lookup hints that software can use to
> accelerate lookups while still actually doing the forwarding itself are also 
> good
> examples of relatively simpler offloads because there is no danger of
> violating rules. If a flow can't be offloaded to a hardware flow table then 
> the
> worst that happens is performance suffers vs. possibly picking a (wrong)
> lower priority flow.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net-next v11 5/6] openvswitch: add layer 3 flow/port support

[pravin shelar]

On Tue, Jul 19, 2016 at 5:02 PM, Simon Horman
 wrote:
> On Mon, Jul 18, 2016 at 03:34:52PM -0700, pravin shelar wrote:
>> On Sun, Jul 17, 2016 at 9:50 PM, Simon Horman
>>  wrote:
>> > [CC Jiri Benc for portion regarding GRE]
>> >
>> > Hi Pravin,
>> >
>> > On Fri, Jul 15, 2016 at 02:07:37PM -0700, pravin shelar wrote:
>> >> On Wed, Jul 13, 2016 at 12:31 AM, Simon Horman
>> >>  wrote:
>> >> > Hi Pravin,
>> >> >
>> >> > On Thu, Jul 07, 2016 at 01:54:15PM -0700, pravin shelar wrote:
>> >> >> On Wed, Jul 6, 2016 at 10:59 AM, Simon Horman
>> >> >>  wrote:
>> >> >
>> >> > ...
>> >>
>> >> >
>> >> >> > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
>> >> >> > index 0ea128eeeab2..86f2cfb19de3 100644
>> >> >> > --- a/net/openvswitch/flow.c
>> >> >> > +++ b/net/openvswitch/flow.c
>> >> >> ...
>> >> >>
>> >> >> > @@ -723,9 +729,17 @@ int ovs_flow_key_extract(const struct 
>> >> >> > ip_tunnel_info *tun_info,
>> >> >> > key->phy.skb_mark = skb->mark;
>> >> >> > ovs_ct_fill_key(skb, key);
>> >> >> > key->ovs_flow_hash = 0;
>> >> >> > +   key->phy.is_layer3 = skb->mac_len == 0;
>> >> >>
>> >> >> I do not think mac_len can be used. mac_header needs to be checked.
>> >> >> ...
>> >> >
>> >> > Yes, indeed. The update to use skb_mac_header_was_set() here accidently
>> >> > slipped into the following patch, sorry about that.
>> >> >
>> >> > With that change in place I believe that this patch is internally
>> >> > consistent because mac_header and mac_len are set correctly by the
>> >> > call to key_extract() which is called by ovs_flow_key_extract() just
>> >> > after where the excerpt above ends.
>> >> >
>> >> > That said, I do think that it is possible to rely on 
>> >> > skb_mac_header_was_set
>> >> > throughout the datapath, including action processing etc... I have 
>> >> > provided
>> >> > an incremental patch - which I created on top of this entire series - at
>> >> > the end of this email. If you prefer that approach I am happy to take 
>> >> > it,
>> >> > though I do feel that using mac_len leads to slightly cleaner code. Let 
>> >> > me
>> >> > know what you think.
>> >> >
>> >>
>> >>
>> >> I am not sure if you can use only mac_len to detect L3 packet. This
>> >> does not work with MPLS packets, mac_len is used to account MPLS
>> >> headers pushed on skb. Therefore in case of a MPLS header on L3
>> >> packet, mac_len would be non zero and we have to look at either
>> >> mac_header or some other metadata like is_layer3 flag from key to
>> >> check for L3 packet.
>> >
>> > At least within OvS mac_len does not include the length of the MPLS label
>> > stack. Rather, the MPLS label stack length is the difference between the
>> > end of (mac_header + mac_len) and network_header.
>> >
>> > So I think that the scheme does work as mac_len is 0 if there is no L2
>> > header regardless of if an MPLS label stack is present or not.
>> >
>>
>> I was thinking in overall networking stack rather than just ovs
>> datapath. I think we should have consistent method of detecting L3
>> packet. As commented in previous mail it could be achieved using
>> skb-protocol and device type.
>
> This is somewhat of a surprise to me. As far as I recall when MPLS support
> was added to OvS it and the accompanying support for MPLS GSO was the only
> MPLS support present in the kernel. And at the time the scheme developed by
> Jesse Gross, myself and others was as I describe above.
>
> Internally OvS relies on this scheme and in particular it is used
> by skb_mpls_header() to calculate the beginning of the MPLS label stack
> accurately in the presence of VLAN tags.
>
> Is it mpls_gso_segment() that you are concerned about?
> If so, perhaps the problem could be addressed there.

Yes.
Can you read the comment I made in previous main in context of
function skb_mpls_header(). I have given rational for requested
change.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Considering the possibility of integrating DPDK generic classifier APIs in OVS.

[Jesse Gross]

On Wed, Jul 20, 2016 at 6:43 PM, Gray, Mark D  wrote:
>  [Gray, Mark D] I think we should focus on one or two use cases rather than
> a general offload like you discuss below. A general offload involves a huge 
> amount of code
> churn and there are a lot of difficulties, some that you have highlighted 
> below.
> A more focused implementation will flush out any issues with the API. In 
> particular,
> the VxLAN use case that you mentioned above and perhaps the offload of the 
> hash
> calculation (but the generic filtering api would also need to support 
> generation of hashes)
> could be two targets for this DPDK api.

I agree that targeting a specific use case is a good idea (as well as
your other comments). It's probably worthwhile talking to John
Fastabend about this (also from Intel) since he has tried to something
similar for the past several years in Linux. Many of the general
problems listed in the original email turn out to be very difficult.
(Examples include capabilities; describing flows in a hardware
independent manner is something that OpenFlow tried to tackle for a
long time; which flows to offload in the face of table size limits
while maintaining correct forwarding behavior; etc.)

I think the VXLAN acceleration was a good use case since the vswitch
is the owner of the tunnel endpoint and therefore is better equipped
to make policy decisions. The main concern that I had with the
previous implementation was that it was making assumptions about the
contents of the inner flow based on the UDP source port, which is not
really safe since that is just a hash.

Providing hashes or other flow lookup hints that software can use to
accelerate lookups while still actually doing the forwarding itself
are also good examples of relatively simpler offloads because there is
no danger of violating rules. If a flow can't be offloaded to a
hardware flow table then the worst that happens is performance suffers
vs. possibly picking a (wrong) lower priority flow.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH v2 00/13] Add Network Service Header Support

[Jesse Gross]

On Wed, Jul 20, 2016 at 5:06 PM, Paul Quinn (paulq)  wrote:
>
>> On Jul 14, 2016, at 11:39 AM, Jesse Gross  wrote:
>>
>> On Wed, Jul 13, 2016 at 10:44 PM, Elzur, Uri  wrote:
>>> +1 on starting w MD Type = 1
>>>
>>> Not sure I understand the concern expressed with " implementations that 
>>> don't implement TLVs will become deployed and  then when there is a use for 
>>> them it's no longer possible." - why will it not be possible to add MD 
>>> Type=2 later?
>>
>> As I said, it's a classic problem with IP options. Classic enough that
>> people frequently content that TLVs are not usable at all because they
>> don't get implemented which then becomes a self fulfilling prophesy.
>>
>
> Jesse, the issue with IPv4 options has nothing do the actual option(s) but 
> rather the "cost" associated with the handling, particularly in hardware, of 
> variable length packets.

This is a cost vs. benefit tradeoff. I'm fairly certain that had
options been an integral part of handling IP from the start, router
vendors would have decided to handle them rather than not processing
IP. However, since there was little benefit at that time, it was
generally decided that it wasn't worth it and certainly nobody is
going to bother doing it today because everybody knows that options
are unusable since no one implements them.

I think in general it is a pretty well known property that
extensibility is a use it or lose it type of thing. TCP has options as
well that directly resemble IP and they are available and implemented
today because they are necessary for common functionality. I am aware
that TCP is primarily implemented at the software layer and not
hardware but as I think we can see from the discussion in this thread,
trying to limit implementations to the minimum functionality that
seems necessary at the time is not only applicable to silicon.

>> I think I've been extremely clear on this matter. I also think that
>> I've been extremely consistent - I think I've said the same thing on
>> every review of this patch series, so it should not exactly be a
>> surprise. However, the bottom line is I want to see a complete
>> implementation of the protocol and not a half measure that will catch
>> people by surprise or limit future usage. That seems 100% reasonable
>> to me.
>
> The adopted NSH draft clearly states that type-1 support in mandatory, and 
> that type-2 support is optional.  As such the OVS patches are compliant.  
> Having said that, the current code also supports skipping the type-2 based on 
> the length of NSH conveyed in the first word of the header.  This, I believe, 
> constitutes support: type-2 NSH packets, if used, are supported: the type-2 
> info is skipped and OVS functions as expected.
>
> It appears that your definition of support differs from that, can you expand 
> on it please?

As I have said, I would like to see real support for MD type 2 so that
it is useable in the future. I don't think that this patch set
fulfills that criteria.

At this point, I have not heard a technical argument as to why MD type
2 support shouldn't be implemented now. I have given my reason why it
is important to do it and the only objection seems to be that the
authors don't wish to take the time. Considering that the discussion
of how to do it has continued in another thread (thank you Jan for
helping to move things forward), it seems more productive to focus on
that rather that continue this debate.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Considering the possibility of integrating DPDK generic classifier APIs in OVS.

[Gray, Mark D]

> Hi Dev Team,
> 
> I submitted a RFC patch few months ago to optimize the tunneling
> performance in OVS-DPDK (specifically VxLAN performance) using Intel
> Fortville Flow director feature. More details about the patch can be found
> below.
> 
> http://openvswitch.org/pipermail/dev/2016-March/067988.html
> 
> The proposal is not accepted by the community, though the patch gives very
> significant tunneling performance improvement. One of the major concern
> was the implementation is too hardware specific.
> Now DPDK is working on a generic classifier API sets to expose these
> hardware packet classification/flow in more generic way. More details on this
> proposal can be found in the following link.
> 
> http://dpdk.org/ml/archives/dev/2016-July/043365.html
> 
[Gray, Mark D] 
The key thing for me is that the DPDK community will provide a performant 
software
solution for all functionality in case the hardware doesn't support it.

> I feel that OVS should definitely leverage these feature because this will be
> big advantage for the customers who are using OVS +smart NICs.
> The early engagement on this proposal can influence the design to co-exists
> with OVS architecture natively. I had already provided few comments based
> on the feedback that I received from OVS community on tunneling
> optimization patch.
>  Please feel free to provide more comments if I have missed out any.
> 
> This time I am thinking of a more generic design to accommodate these APIs
> in OVS. At very high level, this design involves changes in OVS control plane
> as well as the data plane, something like below,

 [Gray, Mark D] I think we should focus on one or two use cases rather than
a general offload like you discuss below. A general offload involves a huge 
amount of code
churn and there are a lot of difficulties, some that you have highlighted below.
A more focused implementation will flush out any issues with the API. In 
particular,
the VxLAN use case that you mentioned above and perhaps the offload of the hash
calculation (but the generic filtering api would also need to support 
generation of hashes)
could be two targets for this DPDK api.

> 
> 1) Control plane modification for ,
>   *)  All the rules cannot offload to the hardware. So its important to
> identify which one can offload and which cannot. The possible options can
> be,
>   1) Let the user choose a rule can be hardware/only software
>   2) Let the control plane decide a hardware rule based on the
> flow parameters.
>   Once the rule is decided as hardware rule, next is choose
> right type of hardware rule(for eg: ID or Queue or RSS or chained) .
>   This is going to be little bit complicated because there are
> multiple parameters need to consider before decide a type. Or we could use
> the ID flow type for all the hardware flows. Any suggestions?

[Gray, Mark D] 
This is a general problem with hardware offload of vswitch functionality. You 
need
to consider the capabilities of the offload target but also the capacity of the 
hardware.

>   *) The control plane modification to program the rule only in
> hardware, only in software, or in both (A hardware rule need to handle
> further in software as well).
>   The hardware flow insertion is something like below,
>   + Insert the software fallback rule in OVS. (This is the normal
> software rule to handle the specific type packets)
>   + Insert the hardware rule, and corresponding post software
> rule to handle hardware processed packets.
>   + Remove the software fallback rule from OVS.
>   Similarly the hardware flow deletion can be
>   + Delete the software fallback path if exists any,
>   +  Remove the post-software rule for the corresponding
> hardware rule.
>   + Remove the hardware rule.
> 
> Please note, the hardware rules can be used for long lived rules.

[Gray, Mark D] 
Yes, if you can determine what is long lived. Also, you need to consider 
statistics,
revalidation, flow deletion, - all the corner cases.

I have always thought for a full offload like what you are discussing, a 
double-dpif
model would make more sense - i.e. implement a dpif for a particular hardware 
target and allow two dpifs to act as back ends for a bridge with hardware ports 
associated
with the hardware dpif and software ports with the software dpif. It may also
be useful for ovs-kernel and ovs-dpdk to interact in the same way.
> 
> 2) Dataplane changes are for
>   *) Miniflow extract change to handle hardware processed rules. I
> feel in most of the cases we could skip miniflow extract on packets hit by a
> hardware rule.
>   *) changes in emc_insert, dpcls_insert, emc_evict, dpcls_evict to
> handle the hardware rules.
> 3) Changes for manage hardware flow stats
> 
> 
> This is just an initial thought to enable these generic APIs in OVS.  
> Ofcourse I
> might have missed out many 

Re: [ovs-dev] [PATCH] release-process.md: Document OVS release process and propose a schedule.

[Ben Pfaff]

On Wed, Jul 20, 2016 at 10:00:22AM -0400, Russell Bryant wrote:
> On Tue, Jul 19, 2016 at 1:58 PM, Ben Pfaff  wrote:
> 
> > This document has two different kinds of text:
> >
> >- The first sections of the document, "Release Strategy" and "Release
> >  Numbering", describe what we've already been doing for most of the
> >  history of Open vSwitch.  If there is anything surprising in them,
> >  then it's because our process has not been transparent enough, and not
> >  because we're making a change.
> >
> >- The final section of the document, "Release Scheduling", is a proposal
> >  for current and future releases.  We have not had a regular release
> >  schedule in the past, but it seems important to have one in the
> >  future, so this section requires review and feedback from everyone in
> >  the community.
> I think this is a great step forward.  Thank you!

Thanks for the review.

> One topic that could be added to this document is discussion of how long
> each release branch is maintained.  LTS is defined in FAQ.md, but it could
> be defined in this document now.  How an LTS branch is chosen, and the
> maintenance difference between LTS and non-LTS would also be good topics to
> cover.

I forgot that the FAQ talked about releases.  I'm appending an
incremental that I will fold into this patch.

It is a good idea to describe LTS releases, but I don't have answers for
the questions you ask.  Here are some thoughts about principles we've
considered before:

* We try to avoid making releases that include disruptive internal
  changes LTS, because they are harder to support.

* It is good to make LTS releases at least every 2 years or so,
  because it is useful to distributions and other downstreams, but
  not much more often than that, because it is more work to maintain
  multiple upstreams.

* In the past we have maintained a given LTS until we release the
  next LTS.  This is probably too vague and may not be long enough
  in any case.

Anyone want to suggest what we should do?

> Acked-by: Russell Bryant 

Thanks.  I'm going to let the discussion develop for a while before I
push anything, because I want to see a semblance of consensus on the
schedule.

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

diff --git a/FAQ.md b/FAQ.md
index 063bd70..290e66c 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -125,13 +125,16 @@ Releases
 ### Q: What does it mean for an Open vSwitch release to be LTS (long-term 
support)?
 
 A: All official releases have been through a comprehensive testing
-   process and are suitable for production use.  Planned releases will
-   occur several times a year.  If a significant bug is identified in an
+   process and are suitable for production use.  Planned releases
+   occur twice a year.  If a significant bug is identified in an
LTS release, we will provide an updated release that includes the
fix.  Releases that are not LTS may not be fixed and may just be
supplanted by the next major release.  The current LTS release is
2.3.x.
 
+   For more information on the Open vSwitch release process, please
+   see [release-process.md].
+
 ### Q: What Linux kernel versions does each Open vSwitch release work with?
 
 A: The following table lists the Linux kernel versions against which the
@@ -2140,3 +2143,4 @@ http://openvswitch.org/
 [OPENFLOW-1.1+.md]:OPENFLOW-1.1+.md
 [INSTALL.DPDK.md]:INSTALL.DPDK.md
 [Tutorial.md]:tutorial/Tutorial.md
+[release-process.md]:Documentation/release-process.md
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH v2 00/13] Add Network Service Header Support

[Jan Scheurich]

At the end of my initial long response 
(http://openvswitch.org/pipermail/dev/2016-July/075115.html) I had included an 
example for how the OF pipeline could be programmed if the NSH fields are 
treated as packet header match fields and not metadata fields. As you say, the 
interesting case is how to match NSH header fields after having popped the NSH 
header.

The clean OVS way (which is already supported today) is to save the interesting 
fields into NXM_REGISTER fields before popping the NSH header. Here's the 
example again:

# SFF flow entry for terminating an RSP and forwarding the decapsulated packet
# saving the NSH C2 metadata in register to be used as e.g. VRF tag in IP 
forwarding decision
table=4, priority=100,eth_type=0x894f,nsi=252,nsp=4 
actions=move:nsh_c2[]->NXM_NX_REG0[],pop_nsh,goto_table:10

# Exit IP forwarding table (e.g. in Netvirt complex)
table=10, priority=100, ip,nw_dst=10.0.36.4,NXM_NX_REG0=0x64, 
actions=push_eth,set_field:22:33:44:55:66:77->eth_src,set_field:11:22:33:44:55:66->eth_dst,output:4
 

This may appear a bit clumsy from controller perspective, but it has the 
advantage that the datapath does not have to save all NSH headers in metadata 
fields at pop_mpls just in case the anybody needs to match on them later, which 
is not the normal case for most of the fields.

I understand that your current patches do not distinguish packet header and 
metadata usage of the NSH fields. That looks very convenient from controller 
perspective but it is conceptually broken. Packet header match fields are 
typically bound to a pre-requisite in OF. In the NSH case the pre-requisite 
should be eth_type=0x894f. It must not be allowed to match on NSH fields unless 
you also match on eth_type=0x894f.

Metadata fields, on the other hand, would always be present with the packet. If 
they have not been set by a pop_nsh action or a set_field() action, they will 
simply be zero. There is no way a flow entry can distinguish between a 
non-initialized metadata field and a true value of zero. The eth_type of the 
packet is no longer 0x894f after pop_nsh but typically 0x800 (IPv4). The 
controller must rely on implicit knowledge of the history of the packet in the 
pipeline.

As we definitely must have NSH packet header fields and we can solve access to 
selected headers after pop_nsh as shown above, my preference would be to go for 
packet header match fields. Unless the OVS community has an acceptable solution 
for the problem using the same fields in both ways.

BR, Jan


N.B.:
There is one potential future use case I can see which may complicate things. 
I'm thinking about adding or removing TLV fields to an NSH header in MD2 
format. This cannot be done in-place. One way to do this would be to pop the 
NSH header into metadata fields, add or delete some MD2 TLV metadata (how?), 
and finally push the NSH header again with the changed set of MD2 TLVs. This 
might become very cumbersome without implicit storage of NSH headers into 
metadata fields at pop_mpls and implicit loading from metadata at push_nsh. But 
perhaps that use case will never be needed


> -Original Message-
> From: Yang, Yi Y [mailto:yi.y.y...@intel.com]
> Sent: Wednesday, 20 July, 2016 04:30
> To: Jan Scheurich; Li, Johnson; dev@openvswitch.org
> Cc: Manuel Buil; László Sürü
> Subject: RE: [ovs-dev] [RFC PATCH v2 00/13] Add Network Service Header
> Support
> 
> Jan, although NSH isn't tunnel, but we still need to do some matches after
> pop_nsh, this is similar to vxlan tunnel, we still can match vxlan tunnel
> metadata after the patch is de-capsulated, such as TUN_ID and TUN_DST. So
> my point is maybe we need both header fields and metadata fields.
> 
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Jan
> Scheurich
> Sent: Friday, July 15, 2016 6:27 PM
> To: Jan Scheurich ; Li, Johnson
> ; dev@openvswitch.org
> Cc: Manuel Buil ; László Sürü
> 
> Subject: Re: [ovs-dev] [RFC PATCH v2 00/13] Add Network Service Header
> Support
> 
> It would be great if we could get guidance by the OVS committers on below
> question whether to model NSH fields as packet header match fields or as
> metada fields or both.
> 
> Right now patch v2 mixes those and uses the same set of NXM fields both
> when matching packet headers of an NSH packet and as metadata fields
> after pop_nsh or before push_nsh.
> 
> Is that acceptable from an OpenFlow and OVS perspective?
> 
> > Access to NSH header fields in the OF pipeline
> > ---
> >
> > OpenFlow has two ways for this: packet header and metadata match fields.
> > Packet header match fields are populated when parsing a packet with
> > NSH ethertype and are only valid and accessible while the NSH header
> > is in place (i.e. before pop_nsh or after push_nsh). Metadata fields,
> > in contrast, would be populated 

[ovs-dev] [PATCH v3 2/3] lib/chutil: Add chmod and chown for opened files

[Aaron Conole]

This commit extends the chutil library by providing a set of helpers
which operate on files already opened by the process.

The implementation provided only works with linux systems, but any
system which provides a system call mechanism to do this (such as
FreeBSD), could be used.  The linux implementation also resolves some
unix domain sockets to their filenames.

Signed-off-by: Aaron Conole 
---
 lib/chutil-unix.c   | 108 
 lib/chutil.h|   4 ++
 tests/test-chutil.c |   6 +++
 3 files changed, 118 insertions(+)

diff --git a/lib/chutil-unix.c b/lib/chutil-unix.c
index e2ae8e9..3e9eb88 100644
--- a/lib/chutil-unix.c
+++ b/lib/chutil-unix.c
@@ -18,6 +18,7 @@
 
 #include "chutil.h"
 
+#include 
 #include 
 #include 
 #include 
@@ -31,6 +32,12 @@
 #include "util.h"
 #include "openvswitch/vlog.h"
 
+#ifdef __linux__
+#define LINUX 1
+#else
+#define LINUX 0
+#endif
+
 VLOG_DEFINE_THIS_MODULE(chutil_unix);
 
 #ifndef S_ISLNK
@@ -345,3 +352,104 @@ ovs_fchown(int fd, const char *owner)
 
 return 0;
 }
+
+struct unix_socket_map {
+char name[PATH_MAX];
+char path[PATH_MAX];
+};
+
+static int
+opened_unix_sockets(struct unix_socket_map *map_data, int map_size) {
+char data[PATH_MAX * 2] = {0};
+FILE *netfile = NULL;
+int total = 0;
+
+netfile = fopen("/proc/self/net/unix", "r");
+if (!netfile) {
+return -1;
+}
+
+for (char *line = fgets(data, sizeof(data), netfile); line;
+ line = fgets(data, sizeof(data), netfile)) {
+char *path = strrchr(data, ' ');
+if (path && *(path + 1) == '/') {
+*path++ = 0;
+path[strcspn(path, "\n")] = 0;
+if (total < map_size) {
+snprintf(map_data[total].name, PATH_MAX, "socket:[%s]",
+ strrchr(data, ' ')+1);
+snprintf(map_data[total].path, PATH_MAX, "%s", path);
+}
+total++;
+}
+}
+return total;
+}
+
+static int
+name_to_fd(const char *name)
+{
+int result = -1;
+if (LINUX) {
+DIR *d = opendir("/proc/self/fd");
+struct unix_socket_map mp[256];
+
+opened_unix_sockets(mp, ARRAY_SIZE(mp));
+
+if (!d) {
+return -1;
+}
+
+for (struct dirent *ent = readdir(d); ent; ent = readdir(d)) {
+char check_path[PATH_MAX];
+char resolved_path[PATH_MAX] = {0};
+snprintf(check_path, PATH_MAX, "/proc/self/fd/%s", ent->d_name);
+if (readlink(check_path, resolved_path, PATH_MAX) < 0) {
+continue;
+}
+
+if (!strcmp(resolved_path, name)) {
+result = strtol(ent->d_name, NULL, 10);
+break;
+} else if (!strncmp(resolved_path, "socket:", 7)) {
+for (size_t i = 0; i < ARRAY_SIZE(mp) && mp[i].name[0]; ++i) {
+if (!strcmp(resolved_path, mp[i].name)) {
+result = strtol(ent->d_name, NULL, 10);
+break;
+}
+}
+}
+}
+closedir(d);
+if (result < 0) {
+errno = ENOENT;
+}
+} else {
+errno = ENOTSUP;
+}
+return result;
+}
+
+int
+ovs_chown_open_file(const char *name, const char *owner)
+{
+int fd = name_to_fd(name);
+if (fd < 0) {
+VLOG_ERR("ovs_chown_open_file: name_to_fd: %s\n", ovs_strerror(errno));
+return errno;
+}
+
+return ovs_fchown(fd, owner);
+}
+
+int
+ovs_chmod_open_file(const char *name, const char *mode)
+{
+int fd = name_to_fd(name);
+if (fd < 0) {
+VLOG_ERR("ovs_chmod_open_file: name_to_fd: %s\n", ovs_strerror(errno));
+return errno;
+}
+
+return ovs_fchmod(fd, mode);
+}
diff --git a/lib/chutil.h b/lib/chutil.h
index cdd4d52..ea6bfc9 100644
--- a/lib/chutil.h
+++ b/lib/chutil.h
@@ -27,6 +27,10 @@ int ovs_fchmod(int fd, const char *mode) 
OVS_WARN_UNUSED_RESULT;
 int ovs_fchown(int fd, const char *usrstr) OVS_WARN_UNUSED_RESULT;
 int ovs_strtousr(const char *user_spec, uid_t *uid, char **user,
  gid_t *gid, bool validate_user_group) OVS_WARN_UNUSED_RESULT;
+int ovs_chown_open_file(const char *name, const char *owner)
+OVS_WARN_UNUSED_RESULT;
+int ovs_chmod_open_file(const char *name, const char *mode)
+OVS_WARN_UNUSED_RESULT;
 #endif
 
 #endif
diff --git a/tests/test-chutil.c b/tests/test-chutil.c
index dce07b4..878e33e 100644
--- a/tests/test-chutil.c
+++ b/tests/test-chutil.c
@@ -154,6 +154,12 @@ run_chmod_str_successes(const char *pathname, int fd)
 printf("run_chmod_successes:assignF\n");
 return -1;
 }
+
+if (ovs_chmod_open_file(pathname, "ugo-rwx") ||
+get_mode(pathname, ) || pmode != 0) {
+printf("run_chmod_successes:byOpenNameF\n");
+return -1;
+}
 return 0;
 }
 
-- 
2.5.5

[ovs-dev] [PATCH v3 1/3] chutil: introduce a new change-utils lib

[Aaron Conole]

It will be useful in the future to be able to set ownership and permissions
on files which Open vSwitch creates. Allowing the specification of such
ownership and permissions using the standard user:group, uog+-rwxs, and
numerical forms commonly associated with those actions.

This patch introduces a new chutil library, currently with a posix command
implementation. WIN32 support does not exist at this time, but could be added
in the future.

As part of this, the daemon-unix.c was refactored to move the ownership
parsing code to the chutil library. A new set of tests was added, and the
fchmod and fchown calls are implemented.

Signed-off-by: Aaron Conole 
Acked-by: Ben Pfaff 
---
 lib/automake.mk |   2 +
 lib/chutil-unix.c   | 347 
 lib/chutil.h|  32 +
 lib/daemon-unix.c   | 149 +-
 tests/automake.mk   |   2 +
 tests/library.at|   5 +
 tests/test-chutil.c | 243 
 7 files changed, 636 insertions(+), 144 deletions(-)
 create mode 100644 lib/chutil-unix.c
 create mode 100644 lib/chutil.h
 create mode 100644 tests/test-chutil.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 1a44cc0..858f2e1 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -35,6 +35,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/byteq.h \
lib/cfm.c \
lib/cfm.h \
+   lib/chutil.h \
lib/classifier.c \
lib/classifier.h \
lib/classifier-private.h \
@@ -299,6 +300,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/strsep.c
 else
 lib_libopenvswitch_la_SOURCES += \
+   lib/chutil-unix.c \
lib/daemon-unix.c \
lib/latch-unix.c \
lib/signals.c \
diff --git a/lib/chutil-unix.c b/lib/chutil-unix.c
new file mode 100644
index 000..e2ae8e9
--- /dev/null
+++ b/lib/chutil-unix.c
@@ -0,0 +1,347 @@
+/*
+ * Copyright (C) 2016 Red Hat, 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.
+ */
+
+#include 
+
+#include "chutil.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "daemon.h"
+#include "util.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(chutil_unix);
+
+#ifndef S_ISLNK
+#define S_ISLNK(mode) (0)
+#endif
+
+#define USR_MODES (S_ISUID | S_IRWXU)
+#define GRP_MODES (S_ISGID | S_IRWXG)
+#define OTH_MODES (S_IRWXO)
+#define ALL_MODES (USR_MODES | GRP_MODES | OTH_MODES)
+
+#define READ_MODES  (S_IRUSR | S_IRGRP | S_IROTH)
+#define WRITE_MODES (S_IWUSR | S_IWGRP | S_IWOTH)
+#define EXEC_MODES  (S_IXUSR | S_IXGRP | S_IXOTH)
+
+#define SUID_MODES  (S_ISUID | S_ISGID)
+
+/* Convert a chown-style string to uid/gid; supports numeric arguments
+ * as well as usernames. */
+int
+ovs_strtousr(const char *user_spec, uid_t *uid, char **user, gid_t *gid,
+ bool validate_user_group)
+{
+char *pos = strchr(user_spec, ':');
+size_t bufsize = 0;
+user_spec += strspn(user_spec, " \t\r\n");
+
+size_t len = pos ? pos - user_spec : strlen(user_spec);
+char *buf = NULL;
+struct passwd pwd, *res = NULL;
+int e;
+
+buf = x2nrealloc(NULL, , sizeof pwd);
+char *user_search = NULL;
+uid_t uid_search = getuid();
+if (len) {
+user_search = xmemdup0(user_spec, len);
+if (!strcspn(user_search, "0123456789")) {
+uid_search = strtoul(user_search, NULL, 10);
+free(user_search);
+user_search = NULL;
+}
+}
+
+if (user_search) {
+while ((e = getpwnam_r(user_search, , buf,
+   bufsize * sizeof pwd, )) == ERANGE) {
+buf = x2nrealloc(buf, , sizeof pwd);
+}
+} else {
+while ((e = getpwuid_r(uid_search, , buf, bufsize * sizeof pwd,
+   )) == ERANGE) {
+buf = x2nrealloc(buf, , sizeof pwd);
+}
+}
+
+if (!res && !e) {
+e = ENOENT;
+}
+
+if (e) {
+VLOG_ERR("Failed to retrieve user pwentry (%s), aborting.",
+ ovs_strerror(e));
+goto release;
+}
+
+if (!user_search) {
+user_search = xstrdup(pwd.pw_name);
+}
+
+if (user) {
+*user = user_search;
+}
+
+if (uid) {
+*uid = pwd.pw_uid;
+}
+
+if (gid) {
+*gid = pwd.pw_gid;
+}
+
+if (pos) {
+gid_t tmpgid = pwd.pw_gid;
+char *grpstr = pos + 1;
+

[ovs-dev] [PATCH v3 0/3] vhost-user: Add the ability to control ownership/permissions

[Aaron Conole]

Currently, when using Open vSwitch with DPDK and qemu guests, the recommended
method for joining the guests is via the dpdkvhostuser interface. This
interface uses Unix Domain sockets to communicate. When these sockets are
created, they inherit the permissions and ownership from the vswitchd process.
This can lead to an undesirable state where the QEMU process cannot use the
socket file until manual intervention is performed (via `chown` and/or `chmod`
calls).

This patchset gives the ability to set the permissions and ownership of all
dpdkvhostuser sockets from the database, avoiding the manual intervention
required to connect QEMU and OVS via DPDK.

The first patch adds chmod and chown calls to lib, with unit tests. The
second patch hooks those calls into the netdev_dpdk_vhost_user_construct
function, after the socket is created.

Changes from v2:
* Added a new 2nd patch to series for chmod/chown on already opened files.
  There exist known implementations for other systems, including FreeBSD, but
  only linux is implemented.  ENOTSUP is set when these calls fail on non-linux
  systems.

Aaron Conole (3):
  chutil: introduce a new change-utils lib
  lib/chutil: Add chmod and chown for opened files
  netdev-dpdk: Support user-defined socket attribs

 INSTALL.DPDK.md  |   7 +
 lib/automake.mk  |   2 +
 lib/chutil-unix.c| 455 +++
 lib/chutil.h |  36 
 lib/daemon-unix.c| 149 +
 lib/netdev-dpdk.c|  37 -
 tests/automake.mk|   2 +
 tests/library.at |   5 +
 tests/test-chutil.c  | 249 
 vswitchd/vswitch.xml |  23 +++
 10 files changed, 818 insertions(+), 147 deletions(-)
 create mode 100644 lib/chutil-unix.c
 create mode 100644 lib/chutil.h
 create mode 100644 tests/test-chutil.c

-- 
2.5.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 0/2] vhost-user: Add the ability to control ownership/permissions

[Aaron Conole]

Aaron Conole  writes:

> Currently, when using Open vSwitch with DPDK and qemu guests, the recommended
> method for joining the guests is via the dpdkvhostuser interface. This
> interface uses Unix Domain sockets to communicate. When these sockets are
> created, they inherit the permissions and ownership from the vswitchd process.
> This can lead to an undesirable state where the QEMU process cannot use the
> socket file until manual intervention is performed (via `chown` and/or `chmod`
> calls).
>
> This patchset gives the ability to set the permissions and ownership of all
> dpdkvhostuser sockets from the database, avoiding the manual intervention
> required to connect QEMU and OVS via DPDK.
>
> The first patch adds chmod and chown calls to lib, with unit tests. The
> second patch hooks those calls into the netdev_dpdk_vhost_user_construct
> function, after the socket is created.
>

I've posted a followup series to this, which I believe integrates all of
the feedback over the last 8 weeks.  It introduces an additional call,
currently only implemented for linux but which could be extended to
other operating systems, to set an already opened file by filename.
This is then used by the vhostuser server code to set discretionary
access controls.

The series can be found here:
http://openvswitch.org/pipermail/dev/2016-July/075749.html

Thanks,
-Aaron
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Mail System Error - Returned Mail

[Bounced mail]

The message could not be delivered

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v3 3/3] netdev-dpdk: Support user-defined socket attribs

[Aaron Conole]

Currently, when dpdkvhostuser devices are created, they inherit whatever the
running umask and uid/gid of the vswitchd process. This leads to difficulties
when using vhost_user consumers (such as qemu).

This patch introduces two new database entries, 'vhost-sock-owner' to set the
ownership, and 'vhost-sock-perms' to set the permissions bits for the
vhost_user sockets.  These settings apply to all vhost-user sockets.

Signed-off-by: Aaron Conole 
---
 INSTALL.DPDK.md  |  7 +++
 lib/netdev-dpdk.c| 37 ++---
 vswitchd/vswitch.xml | 23 +++
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 5407794..0cd4bfe 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -223,6 +223,13 @@ advanced install guide [INSTALL.DPDK-ADVANCED.md]
  * vhost-sock-dir
  Option to set the path to the vhost_user unix socket files.
 
+ * vhost-sock-owner
+ Option to set the owner of the vhost_user unix socket files.
+
+ * vhost-sock-perms
+ Option to set the file-system permissions of the vhost_user unix socket
+ files.
+
  NOTE: Changing any of these options requires restarting the ovs-vswitchd
  application.
 
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 85b18fd..ffa62c9 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 
+#include "chutil.h"
 #include "dirs.h"
 #include "dp-packet.h"
 #include "dpif-netdev.h"
@@ -141,6 +142,10 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
 static char *cuse_dev_name = NULL;/* Character device cuse_dev_name. */
 #endif
 static char *vhost_sock_dir = NULL;   /* Location of vhost-user sockets */
+static char *vhost_sock_def_owner = NULL; /* Default owner of vhost-user
+ sockets*/
+static char *vhost_sock_def_perms = NULL; /* Default permissions of
+ vhost-user sockets */
 
 #define VHOST_ENQ_RETRY_NUM 8
 
@@ -824,6 +829,23 @@ vhost_construct_helper(struct netdev *netdev) 
OVS_REQUIRES(dpdk_mutex)
 }
 
 static int
+vhost_set_permissions(struct netdev_dpdk *dev) OVS_REQUIRES(dpdk_mutex)
+{
+int err = 0;
+if (vhost_sock_def_owner &&
+(err = ovs_chown_open_file(dev->vhost_id, vhost_sock_def_owner))) {
+VLOG_ERR("vhost-user socket device ownership change failed.");
+}
+
+if (!err && vhost_sock_def_perms &&
+(err = ovs_chmod_open_file(dev->vhost_id, vhost_sock_def_perms))) {
+VLOG_ERR("vhost-user socket device permission change failed.");
+}
+
+return err;
+}
+
+static int
 netdev_dpdk_vhost_cuse_construct(struct netdev *netdev)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
@@ -879,6 +901,10 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev)
 err = vhost_construct_helper(netdev);
 }
 
+if (!err) {
+err = vhost_set_permissions(dev);
+}
+
 ovs_mutex_unlock(_mutex);
 return err;
 }
@@ -3221,8 +3247,8 @@ dpdk_init__(const struct smap *ovs_other_config)
 VLOG_INFO("DPDK Enabled, initializing");
 
 #ifdef VHOST_CUSE
-if (process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
-PATH_MAX, ovs_other_config, _dev_name)) {
+process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
+PATH_MAX, ovs_other_config, _dev_name);
 #else
 if (process_vhost_flags("vhost-sock-dir", xstrdup(ovs_rundir()),
 NAME_MAX, ovs_other_config,
@@ -3246,9 +3272,14 @@ dpdk_init__(const struct smap *ovs_other_config)
 free(sock_dir_subcomponent);
 } else {
 vhost_sock_dir = sock_dir_subcomponent;
-#endif
 }
 
+process_vhost_flags("vhost-sock-owner", NULL, NAME_MAX, ovs_other_config,
+_sock_def_owner);
+process_vhost_flags("vhost-sock-perms", NULL, NAME_MAX, ovs_other_config,
+_sock_def_perms);
+#endif
+
 argv = grow_argv(, 0, 1);
 argc = 1;
 argv[0] = xstrdup(ovs_get_program_name());
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index fed6f56..05d2a14 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -311,6 +311,29 @@
 
   
 
+  
+
+  Specifies the owner of the vhost-user unix domain socket files.
+
+
+  The default is to inherit from the running user and group id's. The
+  argument is specified in the same form as the 'chown' unix utility.
+
+  
+
+  
+
+  Specifies the permissions for the vhost-user unix domain socket
+  files.
+
+
+  The default is derived from the running mask. The argument is
+  specified in the same form as the 'chmod' unix utility.
+
+  
+
   
 
-- 
2.5.5

Re: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth flow action to data plane

[Jan Scheurich]

Hi Yi,

Recirculation is implicitly triggered by calling  ctx_trigger_freeze(ctx) at 
the end of the composition phase of an action. 

The example for this is pop_mpls, but unfortunately it is a bit hidden there 
because the invocation of  ctx_trigger_freeze(ctx) is deferred in order to 
avoid unnecessary recirculation of the decapsulated packet, for example when it 
is directly sent out to a port. (For the background see 
http://openvswitch.org/pipermail/dev/2016-May/071657.html)

Instead compose_mpls_pop_action() sets ctx->was_mpls = true and subsequent 
actions that do require recirculation to happen check the was_mpls flag and 
call ctx_trigger_freeze(ctx) when needed. An example is xlate_select_group().

For pop_nsh and pop_eth you should not worry about such complications and call 
ctx_trigger_freeze(ctx) directly after having modified the flow according to 
the pop action (have a look at flow_pop_mpls() function in flow.c for an idea 
how to do this).

BR, Jan


> -Original Message-
> From: Yang, Yi Y [mailto:yi.y.y...@intel.com]
> Sent: Wednesday, 20 July, 2016 04:00
> To: Jan Scheurich; Li, Johnson; dev@openvswitch.org
> Cc: Simon Horman
> Subject: RE: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth flow
> action to data plane
> 
> Hi, Jan
> 
> How do you know we trigger recirculate after pop_eth? pop_nsh also needs
> to do so, is there any existing example for reference?
> 
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Jan
> Scheurich
> Sent: Tuesday, July 19, 2016 3:46 PM
> To: Li, Johnson ; dev@openvswitch.org
> Cc: Simon Horman 
> Subject: Re: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth
> flow action to data plane
> 
> Hi Johnson,
> 
> After basing these patches on Simon's L3 tunneling support the push_eth
> action should take the flow-> base_layer into account:
> 
> If it is LAYER_2, pushing an Ethernet header encapsulates the inner L2 frame
> and therefore requires a call to xlate_commit_actions(ctx) before composing
> the push_eth action. We need to pick a specific "eth_type" for the resulting
> L2 frame so that OVS can identify a raw Eth over Eth encapsulation and
> handle it correctly at pop_eth (see below). A natural choice could be 0x6558
> used e.g. as protocol type for Transparent Ethernet Bridging (Ethernet over
> GRE).
> 
> If it is LAYER_3, push_eth just adds the missing MAC header to the "L3"
> packet without encapsulating it. The ethertype remains unchanged. No need
> to call xlate_commit_actions(ctx) in that case.
> 
> For pop_eth there is a reverse problem (It is obviously only allowed if flow-
> >base_layer is LAYER_2):
> 
> If the eth_type of the L2 frame indicates a known "L3" payload, you only
> remove the MAC header from the flow and change the base_layer to
> LAYER_3. Supported "L3" eth_types would include MPLS, ARP, IPv4, IPv6,
> NSH (anything else?). I am not sure how much sense it would make to
> continue processing unknown eth_types as L3 packets in the pipeline. But
> perhaps it won't do any harm either.
> 
> If the eth_type is the chosen value for Eth over Eth (e.g. 0x6658), pop_eth
> should trigger recirculation to reparse the inner L2 packet. In this case the
> base_layer should remain LAYER_2.
> 
> BR, Jan
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Johnson Li
> > Sent: Tuesday, 12 July, 2016 19:30
> > To: dev@openvswitch.org
> > Subject: [ovs-dev] [RFC PATCH v2 12/13] Commit push_eth/pop_eth flow
> > action to data plane
> >
> > Signed-off-by: Johnson Li 
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c
> > b/ofproto/ofproto-dpif-xlate.c index f5c1888..fb3cd2e 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -5129,8 +5129,17 @@ do_xlate_actions(const struct ofpact *ofpacts,
> > size_t ofpacts_len,
> >  memset(>nsh, 0x0, sizeof flow->nsh);
> >  nl_msg_put_flag(ctx->odp_actions, OVS_ACTION_ATTR_POP_NSH);
> >  break;
> > -case OFPACT_PUSH_ETH:
> > +case OFPACT_PUSH_ETH: {
> > +struct ovs_action_push_eth eth;
> > +
> > +eth.addresses.eth_dst = flow->dl_dst;
> > +eth.addresses.eth_src = flow->dl_src;
> > +nl_msg_put_unspec(ctx->odp_actions,
> > OVS_ACTION_ATTR_PUSH_ETH,
> > +  , sizeof eth);
> > +break;
> > +}
> >  case OFPACT_POP_ETH:
> > +nl_msg_put_flag(ctx->odp_actions,
> > + OVS_ACTION_ATTR_POP_ETH);
> >  break;
> >  }
> >
> > --
> > 1.8.4.2
> >
> > --
> > Intel Research and Development Ireland Limited Registered in Ireland
> > Registered Office: Collinstown Industrial Park, Leixlip, County
> > Kildare Registered Number: 308263
> >
> >
> > This e-mail and any attachments may 

Re: [ovs-dev] [RFC PATCH v2 00/13] Add Network Service Header Support

[Paul Quinn (paulq)]

> On Jul 14, 2016, at 11:39 AM, Jesse Gross  wrote:
> 
> On Wed, Jul 13, 2016 at 10:44 PM, Elzur, Uri  wrote:
>> +1 on starting w MD Type = 1
>> 
>> Not sure I understand the concern expressed with " implementations that 
>> don't implement TLVs will become deployed and  then when there is a use for 
>> them it's no longer possible." - why will it not be possible to add MD 
>> Type=2 later?
> 
> As I said, it's a classic problem with IP options. Classic enough that
> people frequently content that TLVs are not usable at all because they
> don't get implemented which then becomes a self fulfilling prophesy.
> 

Jesse, the issue with IPv4 options has nothing do the actual option(s) but 
rather the "cost" associated with the handling, particularly in hardware, of 
variable length packets.  

> I think I've been extremely clear on this matter. I also think that
> I've been extremely consistent - I think I've said the same thing on
> every review of this patch series, so it should not exactly be a
> surprise. However, the bottom line is I want to see a complete
> implementation of the protocol and not a half measure that will catch
> people by surprise or limit future usage. That seems 100% reasonable
> to me.

The adopted NSH draft clearly states that type-1 support in mandatory, and that 
type-2 support is optional.  As such the OVS patches are compliant.  Having 
said that, the current code also supports skipping the type-2 based on the 
length of NSH conveyed in the first word of the header.  This, I believe, 
constitutes support: type-2 NSH packets, if used, are supported: the type-2 
info is skipped and OVS functions as expected.  

It appears that your definition of support differs from that, can you expand on 
it please?

Thanks
Paul
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters

[bschanmu]

ovn-northd processes the list of Port_Bindings and hashes the list of
queues per chassis. When it finds a port with qos_parameters and without
a queue_id, it allocates a free queue for the chassis that this port belongs.
The queue_id information is stored in the options field of Port_binding table.
Adds an action set_queue to the ingress table 0 of the logical flows
which will be translated to openflow set_queue by ovn-controller

ovn-controller opens the netdev corresponding to the tunnel interface's
status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for
each SB port_binding that has queue_id set, it allocates a queue with the
qos_parameters of that port. It also frees up unused queues.

This patch replaces the older approach of policing

Signed-off-by: Babu Shanmugam 
---
 ovn/controller/binding.c | 233 ++-
 ovn/lib/actions.c|  29 ++
 ovn/lib/actions.h|   3 +
 ovn/northd/ovn-northd.c  | 131 --
 ovn/ovn-nb.xml   |   8 +-
 ovn/ovn-sb.xml   |  37 +++-
 6 files changed, 406 insertions(+), 35 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 0dea828..64bcc92 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -23,6 +23,7 @@
 #include "lib/poll-loop.h"
 #include "lib/sset.h"
 #include "lib/util.h"
+#include "lib/netdev.h"
 #include "lib/vswitch-idl.h"
 #include "openvswitch/vlog.h"
 #include "ovn/lib/ovn-sb-idl.h"
@@ -30,6 +31,8 @@
 
 VLOG_DEFINE_THIS_MODULE(binding);
 
+#define OVN_QOS_TYPE "linux-htb"
+
 /* A set of the iface-id values of local interfaces on this chassis. */
 static struct sset local_ids = SSET_INITIALIZER(_ids);
 
@@ -42,6 +45,13 @@ binding_reset_processing(void)
 process_full_binding = true;
 }
 
+struct qos_queue {
+struct hmap_node node;
+uint32_t queue_id;
+uint32_t max_rate;
+uint32_t burst;
+};
+
 void
 binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 {
@@ -55,18 +65,21 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 ovsdb_idl_add_table(ovs_idl, _table_port);
 ovsdb_idl_add_column(ovs_idl, _port_col_name);
 ovsdb_idl_add_column(ovs_idl, _port_col_interfaces);
+ovsdb_idl_add_column(ovs_idl, _port_col_qos);
 
 ovsdb_idl_add_table(ovs_idl, _table_interface);
 ovsdb_idl_add_column(ovs_idl, _interface_col_name);
 ovsdb_idl_add_column(ovs_idl, _interface_col_external_ids);
-ovsdb_idl_add_column(ovs_idl, _interface_col_ingress_policing_rate);
-ovsdb_idl_add_column(ovs_idl,
- _interface_col_ingress_policing_burst);
+ovsdb_idl_add_column(ovs_idl, _interface_col_status);
+
+ovsdb_idl_add_table(ovs_idl, _table_qos);
+ovsdb_idl_add_column(ovs_idl, _qos_col_type);
 }
 
 static bool
 get_local_iface_ids(const struct ovsrec_bridge *br_int,
-struct shash *lport_to_iface)
+struct shash *lport_to_iface,
+struct sset *egress_ifaces)
 {
 int i;
 bool changed = false;
@@ -88,13 +101,23 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
 
 iface_rec = port_rec->interfaces[j];
 iface_id = smap_get(_rec->external_ids, "iface-id");
-if (!iface_id) {
-continue;
+
+if (iface_id) {
+shash_add(lport_to_iface, iface_id, iface_rec);
+if (!sset_find_and_delete(_local_ids, iface_id)) {
+sset_add(_ids, iface_id);
+changed = true;
+}
 }
-shash_add(lport_to_iface, iface_id, iface_rec);
-if (!sset_find_and_delete(_local_ids, iface_id)) {
-sset_add(_ids, iface_id);
-changed = true;
+
+/* Check if this is a tunnel interface. */
+if (smap_get(_rec->options, "remote_ip")) {
+const char *tunnel_iface
+= smap_get(_rec->status, "tunnel_egress_iface");
+if (tunnel_iface) {
+sset_add(egress_ifaces,
+ tunnel_iface);
+}
 }
 }
 }
@@ -154,20 +177,166 @@ add_local_datapath(struct hmap *local_datapaths,
 }
 
 static void
-update_qos(const struct ovsrec_interface *iface_rec,
-   const struct sbrec_port_binding *pb)
+get_qos_params(const struct sbrec_port_binding *pb, struct hmap *queue_map)
+{
+uint32_t max_rate = smap_get_int(>options, "qos_max_rate", 0);
+uint32_t burst = smap_get_int(>options, "qos_burst", 0);
+uint32_t queue_id = smap_get_int(>options, "qdisc_queue_id", 0);
+
+if ((!max_rate && !burst) || !queue_id) {
+/* Qos is not configured for this port. */
+return;
+}
+
+struct qos_queue *node = xzalloc(sizeof *node);
+hmap_insert(queue_map, >node, hash_int(queue_id, 0));
+node->max_rate = max_rate;
+node->burst = 

[ovs-dev] [PATCH v6 2/2] DSCP marking on packets egressing VIF interface

[bschanmu]

ovn-northd sets 'ip.dscp' to the DSCP value

Signed-off-by: Babu Shanmugam 
Acked-by: Ben Pfaff 
---
 ovn/controller/lflow.c  | 2 +-
 ovn/northd/ovn-northd.c | 4 
 ovn/ovn-nb.xml  | 6 ++
 ovn/ovn-sb.xml  | 5 +
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 42c9055..140377f 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -118,7 +118,7 @@ lflow_init(void)
 expr_symtab_add_predicate(, "ip6", "eth.type == 0x86dd");
 expr_symtab_add_predicate(, "ip", "ip4 || ip6");
 expr_symtab_add_field(, "ip.proto", MFF_IP_PROTO, "ip", true);
-expr_symtab_add_field(, "ip.dscp", MFF_IP_DSCP, "ip", false);
+expr_symtab_add_field(, "ip.dscp", MFF_IP_DSCP_SHIFTED, "ip", 
false);
 expr_symtab_add_field(, "ip.ecn", MFF_IP_ECN, "ip", false);
 expr_symtab_add_field(, "ip.ttl", MFF_IP_TTL, "ip", false);
 
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 66e9c8a..a2903e4 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1929,6 +1929,10 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
);
 
 const char *queue_id = smap_get(>sb->options, "qdisc_queue_id");
+const char *dscp = smap_get(>sb->options, "qos_dscp");
+if (dscp) {
+ds_put_format(, "ip.dscp = %s;", dscp);
+}
 if (queue_id) {
 ds_put_format(, "set_queue(%s); ", queue_id);
 }
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index ebcd118..4c8ea90 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -235,6 +235,12 @@
   If set, indicates the maximum burst size for data sent from this
   interface, in bits.
 
+
+
+  If set, indicates the DSCP code to be marked on the packets egressing
+  the VIF interface. Value should be in the range of
+  0 to 63 (inclusive).
+
   
 
 
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index a454f5f..9549254 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1734,6 +1734,11 @@ tcp.flags = RST;
 interface, in bits.
   
 
+  
+If set, indicates the DSCP code to be marked on the packets egressing
+the VIF interface. Value should be in the range of 0 to 63 (inclusive).
+  
+
   
 Indicates the queue number on the physical device. This is same as the
 queue_id used in OpenFlow in struct ofp_action_enqueue. Value should
-- 
2.5.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v6 0/2] ovn: QOS updates with DSCP support

[bschanmu]

From: Babu Shanmugam 

v5->v6:
- Rebased to the latest master
- Avoid updating the qos column of port record when txn is NULL

Babu Shanmugam (2):
  Check and allocate free qdisc queue id for ports with qos parameters
  DSCP marking on packets egressing VIF interface

 ovn/controller/binding.c | 233 ++-
 ovn/controller/lflow.c   |   2 +-
 ovn/lib/actions.c|  29 ++
 ovn/lib/actions.h|   3 +
 ovn/northd/ovn-northd.c  | 135 +--
 ovn/ovn-nb.xml   |  14 ++-
 ovn/ovn-sb.xml   |  42 -
 7 files changed, 422 insertions(+), 36 deletions(-)

-- 
2.5.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3] [OVN-northd] Rename nbs/nbr port names tonbsp/nbrp

[Hui Kang]

"dev"  wrote on 07/19/2016 04:15:56 PM:

> From: Justin Pettit 
> To: Hui Kang 
> Cc: dev@openvswitch.org
> Date: 07/19/2016 04:16 PM
> Subject: Re: [ovs-dev] [PATCH v3] [OVN-northd] Rename nbs/nbr port
> names to nbsp/nbrp
> Sent by: "dev" 
>
>
> > On Jul 19, 2016, at 11:36 AM, Hui Kang  wrote:
> >
> > These variables indicate ports in nb switches or routers.
> >
> > Signed-off-by: Hui Kang 
> >
> > --
> > v2->v3:
> > - rebase agains master branch
> > v1->v2:
> > - modify commit message
> > ---
>
> Thanks for the patch.  Just a few things first, though:
>
>- A couple of lines you changed went over 80 columns.  I'll wrap
those.
>- The way you wrote your revision history, it shows up in the
> commit message.  I'll fix it up.
>- Your author email address and Signed-off-by don't match.  Which
> do you want to use?

I'd like to use the corporate email account, which is ka...@us.ibm.com
I will figure out how to setup the mail account in my dev box and keep them
consistent next time. Thanks for your help.

- Hui

>
> I'll merge it after I get clarification on that last question.
>
> --Justin
>
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] release-process.md: Document OVS release process and propose a schedule.

[Russell Bryant]

On Tue, Jul 19, 2016 at 1:58 PM, Ben Pfaff  wrote:

> This document has two different kinds of text:
>
>- The first sections of the document, "Release Strategy" and "Release
>  Numbering", describe what we've already been doing for most of the
>  history of Open vSwitch.  If there is anything surprising in them,
>  then it's because our process has not been transparent enough, and not
>  because we're making a change.
>
>- The final section of the document, "Release Scheduling", is a proposal
>  for current and future releases.  We have not had a regular release
>  schedule in the past, but it seems important to have one in the
>  future, so this section requires review and feedback from everyone in
>  the community.
>
> Signed-off-by: Ben Pfaff 
> ---
>  Documentation/automake.mk|  3 +-
>  Documentation/release-process.md | 85
> 
>  2 files changed, 87 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/release-process.md
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index aae41d2..d709e77 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -2,4 +2,5 @@ docs += \
> Documentation/committer-responsibilities.md \
> Documentation/committer-grant-revocation.md \
> Documentation/group-selection-method-property.txt \
> -   Documentation/OVSDB-replication.md
> +   Documentation/OVSDB-replication.md \
> +   Documentation/release-process.md
> diff --git a/Documentation/release-process.md b/Documentation/
> release-process.md
> new file mode 100644
> index 000..a6af363
> --- /dev/null
> +++ b/Documentation/release-process.md
> @@ -0,0 +1,85 @@
> +Open vSwitch Release Process
> +
> +
> +This document describes the process ordinarily used for Open vSwitch
> +development and release.  Exceptions are sometimes necessary, so all
> +of the statements here should be taken as subject to change through
> +rough consensus of Open vSwitch contributors.
> +
> +Release Strategy
> +
> +
> +Open vSwitch feature development takes place on the "master" branch.
> +Ordinarily, new features are rebased against master and applied
> +directly.  For features that take significant development, sometimes
> +it is more appropriate to merge a separate branch into master; please
> +discuss this on ovs-dev in advance.
> +
> +Periodically, the OVS developers fork a branch from master to become
> +an official release.  These release branches are named for expected
> +release number, e.g. "branch-2.3" for the branch that will yield Open
> +vSwitch 2.3.x.  Release branches should receive only bug fixes, not
> +new features.  Bug fixes applied to release branches should be
> +backports of corresponding bug fixes to the master branch, except for
> +bugs present only on release branches (which are rare in practice).
> +
> +Sometimes there can be exceptions to the rule that a release branch
> +receives only bug fixes.  In particular, after a release branch is
> +created, but before the first actual release from that branch, it can
> +be appropriate to add features.  Like bug fixes, new features on
> +release branches should be backports of the corresponding commits on
> +the master branch.  Features to be added to release branches should be
> +limited in scope and risk and discussed on ovs-dev before creating the
> +branch.
> +
> +After a period of testing and stabilization, and rough consensus by
> +contributors that the release is ready, the developers release the .0
> +release on its branch, e.g. 2.3.0 for branch-2.3.  To make the actual
> +release, a developer pushes a signed tag named, e.g. v2.3.0, to the
> +Open vSwitch repository, makes a release tarball available on
> +openvswitch.org, and posts a release announcement to ovs-announce.
> +
> +As a number of bug fixes accumulate, or after important bugs or
> +vulnerabilities are fixed, the OVS developers may make additional
> +releases from a branch: 2.3.1, 2.3.2, and so on.  The process is the
> +same for these additional release as for a .0 release.
> +
> +Release Numbering
> +-
> +
> +The version number on master should normally end in .90.  This
> +indicates that the Open vSwitch version is "almost" the next version
> +to branch.
> +
> +Forking master into branch-x.y requires two commits to master.  The
> +first is titled "Prepare for x.y.0" and increments the version number
> +to x.y.  This is the initial commit on branch-x.y.  The second is
> +titled "Prepare for post-x.y.0 (x.y.90)" and increments the version
> +number to x.y.90.
> +
> +The version number on a release branch is x.y.z, where z is initially
> +0.  Making a release requires two commits.  The first is titled "Set
> +release dates for x.y.z." and updates NEWS and debian/changelog to
> +specify the release date of the new release.  This commit is 

Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket directory in ovsdb

[Mooney, Sean K]

Hi sorry for the delay
Replies inline.

> -Original Message-
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Monday, July 11, 2016 6:44 PM
> To: Mooney, Sean K 
> Cc: Flavio Leitner ; dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket directory in
> ovsdb
> 
> "Mooney, Sean K"  writes:
> 
> > "Wojciechowicz, RobertX"  writes:
> >
> >> Hi Ben,
> >>
> >>
> >>> -Original Message-
> >>> From: Ben Pfaff [mailto:blp at ovn.org]
> >>> Sent: Tuesday, July 5, 2016 5:07 PM
> >>> To: Wojciechowicz, RobertX 
> >>> Cc: dev at openvswitch.org
> >>> Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user socket
> >>> directory in ovsdb
> >>>
> >>> On Mon, Jul 04, 2016 at 07:22:40AM +, Wojciechowicz, RobertX wrote:
> >>> > Hi,
> >>> >
> >>> > > -Original Message-
> >>> > > From: Ben Pfaff [mailto:blp at ovn.org]
> >>> > > Sent: Saturday, July 2, 2016 2:49 AM
> >>> > > To: Wojciechowicz, RobertX 
> >>> > > Cc: dev at openvswitch.org
> >>> > > Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user
> >>> > > socket
> >>> directory
> >>> > > in ovsdb
> >>> > >
> >>> > > On Mon, Jun 20, 2016 at 10:16:51AM +, Wojciechowicz, RobertX
> >>> wrote:
> >>> > > > Hi,
> >>> > > >
> >>> > > > > -Original Message-
> >>> > > > > From: Ben Pfaff [mailto:blp at ovn.org]
> >>> > > > > Sent: Wednesday, June 8, 2016 10:41 PM
> >>> > > > > To: Wojciechowicz, RobertX  >>> > > > > intel.com>
> >>> > > > > Cc: dev at openvswitch.org
> >>> > > > > Subject: Re: [ovs-dev] [PATCH v2] ovsdb: Expose vhost-user
> >>> > > > > socket
> >>> > > directory
> >>> > > > > in ovsdb
> >>> > > > >
> >>> > > > > On Thu, Jun 02, 2016 at 11:25:56AM +0100, Robert
> >>> > > > > Wojciechowicz
> >>> wrote:
> >>> > > > > > In order to correctly interoperate with Openstack and ODL,
> >>> > > > > > the vhost-user socket directory must be exposed from OVS
> >>> > > > > > via
> >>> OVSDB.
> >>> > > > > > Different distros may package OVS in different ways, so
> >>> > > > > > the locations of these sockets may vary depending on how
> >>> > > > > > ovs-vswitchd has been started. Some clients need
> >>> > > > > > information
> >>> where
> >>> > > > > > the sockets are located when instantiating Qemu virtual 
> >>> > > > > > machines.
> >>> > > > > > The full vhost-user socket directory is constructed from
> >>> > > > > > current OVS working directory and optionally from specified
> subdirectory.
> >>> > > > > > This patch exposes vhost-user socket directory in
> >>> > > > > > Open_vSwitch table in other_config column in two following keys:
> >>> > > > > > 1. ovs-run-dir- OVS working directory
> >>> > > > > > 2. vhost-sock-dir - subdirectory of ovs-run-dir (might be
> >>> > > > > > empty)
> >>> > > > > >
> >>> > > > > > Signed-off-by: Robert Wojciechowicz
> >>> > > 
> >>> > > > > >
> >>> > > > > > v1->v2
> >>> > > > > > - moving vswitch-idl.h dependency inside #ifdef block
> >>> > > > > > - sock_dir_subcomponent initialization with ""
> >>> > > > >
> >>> > > > > Same comment as v1: architecturally, ovs-vswitchd only reads
> >>> > > > > other-config columns, it never writes to them.  Please fix.
> >>> > > >
> >>> > > > If ovs-vswitchd cannot writes to other-config then the only
> >>> > > > place for writing default values to this column I can think of
> >>> > > > is vswitch startup script ovs-ctl.
> >>> > > > Basically I tested in my environment the below solution and it
> >>> > > > seems to solve our issue.
> >>> > > > Is it acceptable approach?
> >>> > >
> >>> > > It looks like you're trying to use other-config to report
> >>> > > something, instead of to configure something.  That's not what it's 
> >>> > > for.
> >>> >
> >>> > Actually I'm trying to add missing information to the OVSDB.
> >>> > By default ovs-vswitchd is already configured that vhost-user
> >>> > sockects are created in the rundir, but this information is not
> >>> > available in the OVSDB. Third-party scripts, which need this
> >>> > information are forced to take some guesses about this.
> >>> > Basically this approach is very similar to storing hostname in
> >>> > this patch:
> >>> > http://openvswitch.org/pipermail/dev/2016-March/068511.html
> >>>
> >>> There is a difference between external-ids and other-config.
> >>> other-config is to configure the switch.  That patch uses external-ids.
> >>
> >> [RW] Yes, of course, but my point is that the configuration currently
> >> looks as follows:
> >> 1. start ovsdb
> >> 2. vhost-sock-dir is not configured
> >> 3. start ovs-vswitchd
> >> 4. ovs-vswitchd in the function dpdk_init__ configures vhost-sock-dir
> >> from ovs_rundir() and sock_dir_subcomponent 5. vhost-sock-dir is now
> >> configured, but still there is no information in the ovsdb
> >
> > I don't understand this flow.  Can you tell me what you mean by
> > vhost-sock-dir is configured but not configured?
> >
> > [sean-k-mooney]
> > @aaron 

[ovs-dev] Scanned image from cop...@openvswitch.com

[copier@]

Reply to: cop...@openvswitch.com 
Device Name: cop...@openvswitch.com
Device Model: MX-2310U

File Format: Microsoft Office Word
Resolution: 200dpi x 200dpi

Attached file is scanned image in Microsoft Office Word format.
Use Microsoft Office Word to view the document.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Returned mail: see transcript for details

[Mail Delivery Subsystem]

This message was not delivered due to the following reason:

Your message was not delivered because the destination server was
not reachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message could not be delivered within 4 days:
Host 41.221.215.188 is not responding.

The following recipients could not receive this message:


Please reply to postmas...@openvswitch.org
if you feel this message to be in error.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Scanned image from cop...@openvswitch.com

[copier@]

Reply to: cop...@openvswitch.com 
Device Name: cop...@openvswitch.com
Device Model: MX-2310U

File Format: Microsoft Office Word
Resolution: 200dpi x 200dpi

Attached file is scanned image in Microsoft Office Word format.
Use Microsoft Office Word to view the document.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] ovn: the implementation of icmp4 reject actions.

[nickcooper-zhangtonghao]

Hi Ryan,
Thank you for your advice. The controller will drop the icmp4 packet whose 
eth.src or eth.dst is
broadcast/multicast address and the type of icmp4 is “destination unreachable”. 
That may reduce 
the broadcast storm attack(e.g. icmp flood, dhcp flood). I write the rate 
limiting of icmp4 into 
my to-do list. Thanks.

ovn: the implementation of icmp4 reject actions.

It support icmp4 reject (e.g. icmp-net-unreachable, icmp-host-prohibited, 
tcp-reset,
icmp-admin-prohibited, icmp-port-unreachable, icmp-net-prohibited, 
icmp-host-unreachable,
and icmp-proto-unreachable). The icmp-net-unreachable is default. The "TCP RST" 
function
will be completed soon. Reject action support only "from-lport" direction. In 
general,
considering performance requirements, it might make sense to support only 
"from-lport" direction.

Signed-off-by: nickcooper-zhangtonghao 
---
 lib/dp-packet.h   |  16 +++
 lib/packets.c |  21 +
 lib/packets.h |   1 +
 ovn/controller/pinctrl.c  | 113 ++
 ovn/lib/actions.c |   4 +-
 ovn/lib/actions.h |   6 +++
 ovn/northd/ovn-northd.c   |  37 +--
 ovn/ovn-nb.ovsschema  |   6 ++-
 ovn/ovn-nb.xml|  44 --
 ovn/ovn-sb.xml|   4 --
 ovn/utilities/ovn-nbctl.c |  41 +++--
 11 files changed, 277 insertions(+), 16 deletions(-)

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 7c1e637..a6df36f 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -318,6 +318,15 @@ dp_packet_set_l3(struct dp_packet *b, void *l3)
 b->l3_ofs = l3 ? (char *) l3 - (char *) dp_packet_data(b) : UINT16_MAX;
 }
 
+static inline size_t
+dp_packet_l3_size(const struct dp_packet *b)
+{
+return b->l3_ofs != UINT16_MAX
+? (const char *)dp_packet_tail(b) - (const char *)dp_packet_l3(b)
+- dp_packet_l2_pad_size(b)
+: 0;
+}
+
 static inline void *
 dp_packet_l4(const struct dp_packet *b)
 {
@@ -342,6 +351,13 @@ dp_packet_l4_size(const struct dp_packet *b)
 }
 
 static inline const void *
+dp_packet_get_ip4_payload(const struct dp_packet *b)
+{
+return OVS_LIKELY(dp_packet_l3_size(b) >= IP_HEADER_LEN)
+? (const char *)dp_packet_l3(b) + IP_HEADER_LEN : NULL;
+}
+
+static inline const void *
 dp_packet_get_tcp_payload(const struct dp_packet *b)
 {
 size_t l4_size = dp_packet_l4_size(b);
diff --git a/lib/packets.c b/lib/packets.c
index a27264c..de90c9b 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1251,6 +1251,9 @@ packet_format_tcp_flags(struct ds *s, uint16_t tcp_flags)
 #define ARP_PACKET_SIZE  (2 + ETH_HEADER_LEN + VLAN_HEADER_LEN + \
   ARP_ETH_HEADER_LEN)
 
+#define ICMP4_PACKET_SIZE  (2 + ETH_HEADER_LEN + VLAN_HEADER_LEN + \
+IP_HEADER_LEN + ICMP_HEADER_LEN)
+
 /* Clears 'b' and replaces its contents by an ARP frame with the specified
  * 'arp_op', 'arp_sha', 'arp_tha', 'arp_spa', and 'arp_tpa'.  The outer
  * Ethernet frame is initialized with Ethernet source 'arp_sha' and destination
@@ -1299,6 +1302,24 @@ compose_arp__(struct dp_packet *b)
 dp_packet_set_l3(b, arp);
 }
 
+void
+compose_icmp4__(struct dp_packet *b)
+{
+dp_packet_clear(b);
+dp_packet_prealloc_tailroom(b, ICMP4_PACKET_SIZE);
+dp_packet_reserve(b, 2 + VLAN_HEADER_LEN);
+
+struct eth_header *eth = dp_packet_put_zeros(b, sizeof *eth);
+eth->eth_type = htons(ETH_TYPE_IP);
+
+struct ip_header *ip4 = dp_packet_put_zeros(b, sizeof *ip4);
+struct icmp_header *icmp4 = dp_packet_put_zeros(b, sizeof *icmp4);
+
+dp_packet_reset_offsets(b);
+dp_packet_set_l4(b, icmp4);
+dp_packet_set_l3(b, ip4);
+}
+
 /* This function expect packet with ethernet header with correct
  * l3 pointer set. */
 static void *
diff --git a/lib/packets.h b/lib/packets.h
index 077ccfa..adfb02b 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1067,6 +1067,7 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
  const struct eth_addr arp_sha,
  const struct eth_addr arp_tha, bool broadcast,
  ovs_be32 arp_spa, ovs_be32 arp_tpa);
+void compose_icmp4__(struct dp_packet *);
 void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
 struct in6_addr *, struct in6_addr *);
 void compose_na(struct dp_packet *,
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 1370301..7893872 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -72,6 +72,10 @@ static void send_garp_run(const struct ovsrec_bridge *,
 static void pinctrl_handle_na(const struct flow *ip_flow,
   const struct match *md,
   struct ofpbuf *userdata);
+static void pinctrl_handle_icmp4(struct dp_packet *pkt_in,
+const struct flow *ip_flow,
+const struct 

Re: [ovs-dev] [ovn-ipv6 06/26] ovn: Renumber logical field registers to the newly extended registers.

[Zong Kai Li]

On Wed, Jul 20, 2016 at 4:50 AM, Justin Pettit  wrote:
>
>> On Jul 13, 2016, at 3:19 AM, Zong Kai Li  wrote:
>>
>> Hi, Justin. I tried your patches one by one. P1~P5 are OK.
>> But something wrong seems happened to P6.
>> In my OpenStack integrated environment, after using P6, I cannot ping
>> VM dhcp netns. And later when I dump ovs flows, I found there is no
>> table 0/OFTABLE_PHY_TO_LOG.
>>
>> I tried revert definition for MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPORT, and
>> this time, table OFTABLE_PHY_TO_LOG comes back.
>> I hope you can check this.
>
> I wasn't able to reproduce this.  I don't have a full OpenStack environment, 
> but I've done a fair amount of testing and don't know how it would pass any 
> traffic without those physical to logical flows.
>
> --Justin
>
>

Hi, Justin. Sorry to trouble you. I did reply in another mail to tell
it's caused by operation mistake, maybe I should add your mail address
next time, not only to ovs dev, when I try to report something.

Thanks,
Zong Kai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] packets: Fix in6_is_lla() on systems without s6_addr32 defined.

[Justin Pettit]

> On Jul 19, 2016, at 11:39 PM, Ben Pfaff  wrote:
> 
> On Tue, Jul 19, 2016 at 11:04:20PM -0700, Justin Pettit wrote:
>> Reported-by: Ben Pfaff 
>> Signed-off-by: Justin Pettit 
> 
> Might add a Fixes: header.

Done.

> Acked-by: Ben Pfaff 

Thanks for spotting the problem and the quick review.  Pushed to master.

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] proposed OVS release schedule (was: [PATCH] release-process.md: Document OVS release process and propose a schedule.)

[Ben Pfaff]

Perhaps the title buried the lead.  My main goal for this patch is to
propose a new regular release schedule for OVS.  Any comments?

Thanks,

Ben.

On Tue, Jul 19, 2016 at 10:58:08AM -0700, Ben Pfaff wrote:
> This document has two different kinds of text:
> 
>- The first sections of the document, "Release Strategy" and "Release
>  Numbering", describe what we've already been doing for most of the
>  history of Open vSwitch.  If there is anything surprising in them,
>  then it's because our process has not been transparent enough, and not
>  because we're making a change.
> 
>- The final section of the document, "Release Scheduling", is a proposal
>  for current and future releases.  We have not had a regular release
>  schedule in the past, but it seems important to have one in the
>  future, so this section requires review and feedback from everyone in
>  the community.
> 
> Signed-off-by: Ben Pfaff 
> ---
>  Documentation/automake.mk|  3 +-
>  Documentation/release-process.md | 85 
> 
>  2 files changed, 87 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/release-process.md
> 
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index aae41d2..d709e77 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -2,4 +2,5 @@ docs += \
>   Documentation/committer-responsibilities.md \
>   Documentation/committer-grant-revocation.md \
>   Documentation/group-selection-method-property.txt \
> - Documentation/OVSDB-replication.md
> + Documentation/OVSDB-replication.md \
> + Documentation/release-process.md
> diff --git a/Documentation/release-process.md 
> b/Documentation/release-process.md
> new file mode 100644
> index 000..a6af363
> --- /dev/null
> +++ b/Documentation/release-process.md
> @@ -0,0 +1,85 @@
> +Open vSwitch Release Process
> +
> +
> +This document describes the process ordinarily used for Open vSwitch
> +development and release.  Exceptions are sometimes necessary, so all
> +of the statements here should be taken as subject to change through
> +rough consensus of Open vSwitch contributors.
> +
> +Release Strategy
> +
> +
> +Open vSwitch feature development takes place on the "master" branch.
> +Ordinarily, new features are rebased against master and applied
> +directly.  For features that take significant development, sometimes
> +it is more appropriate to merge a separate branch into master; please
> +discuss this on ovs-dev in advance.
> +
> +Periodically, the OVS developers fork a branch from master to become
> +an official release.  These release branches are named for expected
> +release number, e.g. "branch-2.3" for the branch that will yield Open
> +vSwitch 2.3.x.  Release branches should receive only bug fixes, not
> +new features.  Bug fixes applied to release branches should be
> +backports of corresponding bug fixes to the master branch, except for
> +bugs present only on release branches (which are rare in practice).
> +
> +Sometimes there can be exceptions to the rule that a release branch
> +receives only bug fixes.  In particular, after a release branch is
> +created, but before the first actual release from that branch, it can
> +be appropriate to add features.  Like bug fixes, new features on
> +release branches should be backports of the corresponding commits on
> +the master branch.  Features to be added to release branches should be
> +limited in scope and risk and discussed on ovs-dev before creating the
> +branch.
> +
> +After a period of testing and stabilization, and rough consensus by
> +contributors that the release is ready, the developers release the .0
> +release on its branch, e.g. 2.3.0 for branch-2.3.  To make the actual
> +release, a developer pushes a signed tag named, e.g. v2.3.0, to the
> +Open vSwitch repository, makes a release tarball available on
> +openvswitch.org, and posts a release announcement to ovs-announce.
> +
> +As a number of bug fixes accumulate, or after important bugs or
> +vulnerabilities are fixed, the OVS developers may make additional
> +releases from a branch: 2.3.1, 2.3.2, and so on.  The process is the
> +same for these additional release as for a .0 release.
> +
> +Release Numbering
> +-
> +
> +The version number on master should normally end in .90.  This
> +indicates that the Open vSwitch version is "almost" the next version
> +to branch.
> +
> +Forking master into branch-x.y requires two commits to master.  The
> +first is titled "Prepare for x.y.0" and increments the version number
> +to x.y.  This is the initial commit on branch-x.y.  The second is
> +titled "Prepare for post-x.y.0 (x.y.90)" and increments the version
> +number to x.y.90.
> +
> +The version number on a release branch is x.y.z, where z is initially
> +0.  Making a release requires two commits.  The first is titled "Set
> 

Re: [ovs-dev] [PATCH] packets: Fix in6_is_lla() on systems without s6_addr32 defined.

[Ben Pfaff]

On Tue, Jul 19, 2016 at 11:04:20PM -0700, Justin Pettit wrote:
> Reported-by: Ben Pfaff 
> Signed-off-by: Justin Pettit 

Might add a Fixes: header.

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] packets: Fix in6_is_lla() on systems without s6_addr32 defined.

[Justin Pettit]

Reported-by: Ben Pfaff 
Signed-off-by: Justin Pettit 
---
 lib/packets.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/packets.h b/lib/packets.h
index 8f11e2c..5fd1e51 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -987,8 +987,9 @@ in6_is_lla(struct in6_addr *addr)
 #ifdef s6_addr32
 return addr->s6_addr32[0] == htonl(0xfe80) && !(addr->s6_addr32[1]);
 #else
-return addr->s6_addr[0] == htons(0xfe80) &&
- !(addr->s6_addr[1] | addr->s6_addr[2] | addr->s6_addr[3]);
+return addr->s6_addr[0] == 0xfe && addr->s6_addr[1] == 0x80 &&
+ !(addr->s6_addr[2] | addr->s6_addr[3] | addr->s6_addr[4] |
+   addr->s6_addr[5] | addr->s6_addr[6] | addr->s6_addr[7]);
 #endif
 }
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev