[ovs-dev] OVS DPDK build errors with commit 01961bb

[Aurojit Panda]

Hi
I found that OVS DPDK fails to build with DPDK 16.07 after commit
01961bb. I am not sure if this is because of some expectations about
DPDK configuration or problems in the code. I could fix this fairly
easily with 4 additions to lib/dpdk.c (patch below), but am unsure
about whether this is what was intended.

Panda



diff --git a/lib/dpdk.c b/lib/dpdk.c
index caea0f4..e998b65 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -22,12 +22,16 @@
 #include 

 #include 
+#include 
+#include 
+#include 

 #include "dirs.h"
 #include "netdev-dpdk.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
 #include "smap.h"
+#include "fatal-signal.h"

 VLOG_DEFINE_THIS_MODULE(dpdk);
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] dpdk: Fix DPDK pdump compilation

[Aaron Conole]

Ciara Loftus  writes:

> The rte_pdump header file was not included in the file that requires it.
> Fix this.
>
> Fixes: 01961bbdd34a ("dpdk: New module with some code from netdev-dpdk.")
> Signed-off-by: Ciara Loftus 
> ---

I'm not sure why I didn't see this compilation error when I built;  I'm
on dpdk commit 13a1317d3ba; did something major change in the last month
or so?

Oh, now I see - I didn't enable pdump.  So I wouldn't have seen this
compilation issue.  I should change my default configuration scripts.
D'oh!

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

[ovs-dev] [PATCH] dpdk: Fix DPDK pdump compilation

[Ciara Loftus]

The rte_pdump header file was not included in the file that requires it.
Fix this.

Fixes: 01961bbdd34a ("dpdk: New module with some code from netdev-dpdk.")
Signed-off-by: Ciara Loftus 
---
 lib/dpdk.c| 4 
 lib/netdev-dpdk.c | 3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/dpdk.c b/lib/dpdk.c
index caea0f4..c2182a7 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -22,8 +22,12 @@
 #include 
 
 #include 
+#ifdef DPDK_PDUMP
+#include 
+#endif
 
 #include "dirs.h"
+#include "fatal-signal.h"
 #include "netdev-dpdk.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ab8c34f..7c1523e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -31,9 +31,6 @@
 #include 
 #include 
 #include 
-#ifdef DPDK_PDUMP
-#include 
-#endif
 #include 
 
 #include "dirs.h"
-- 
2.4.3

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

Re: [ovs-dev] OVS DPDK build errors with commit 01961bb

[Loftus, Ciara]

> 
> Hi
> I found that OVS DPDK fails to build with DPDK 16.07 after commit
> 01961bb. I am not sure if this is because of some expectations about
> DPDK configuration or problems in the code. I could fix this fairly
> easily with 4 additions to lib/dpdk.c (patch below), but am unsure
> about whether this is what was intended.

Hi,

Apologies, I just submitted a very similar patch. I hadn't seen yours!
I think the pdump include should be conditional, depending on whether 
DPDK_PDUMP is detected. It can also be taken out of netdev-dpdk.c as it's no 
longer required there.
For my build I didn't require adding the ring and mempool includes to this 
file. Do you have additional DPDK config options set that require these headers 
to be included here?

Thanks,
Ciara

> 
> Panda
> 
> 
> 
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index caea0f4..e998b65 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -22,12 +22,16 @@
>  #include 
> 
>  #include 
> +#include 
> +#include 
> +#include 
> 
>  #include "dirs.h"
>  #include "netdev-dpdk.h"
>  #include "openvswitch/dynamic-string.h"
>  #include "openvswitch/vlog.h"
>  #include "smap.h"
> +#include "fatal-signal.h"
> 
>  VLOG_DEFINE_THIS_MODULE(dpdk);
> ___
> 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 0/9 md->rst] Convert most INSTALL guides

[Stephen Finucane]

On 2016-10-13 13:10, Russell Bryant wrote:

On Thu, Oct 13, 2016 at 6:00 AM, Stephen Finucane 
wrote:


On 2016-10-12 20:20, Russell Bryant wrote:
On Sat, Oct 8, 2016 at 12:30 PM, Stephen Finucane

wrote:

I'm going to dripfeed the conversion patches to avoid overloading
peoples (there are ~25 of them so far). This is the first batch.

Stephen Finucane (9):
dist-docs: Add support for rST
doc: Convert INSTALL to rST
doc: Convert INSTALL.DPDK to rST
doc: Convert INSTALL.Debian to rST
doc: Convert INSTALL.Docker to rST
doc: Convert INSTALL.Windows to rST
doc: Convert INSTALL.userspace to rST
doc: Convert INSTALL.XenServer to rST
doc: Convert INSTALL.KVM to rST

Do you have the end result published anywhere?  It would be nice to
see the before and after before diving into the detailed reviews.


 I visually inspected them myself but I haven't published them
anywhere. You can run 'make dist-docs' and visually compare the
resulting HTML files to what's on openvswitch.org [1] (remembering
that the reStructuredText versions should actually be more complete
due to latent bugs in the Markdown versions). I could find somewhere
to publish them if it would help but I don't know if it's really
necessary :)

To be honest though, I don't know how detailed we need to be right
now. IMO if the functional aspects of these patches work (i.e. the
'dist-docs' target generates sane HTML for all rST files) and we
haven't lost any of the raw information then any stylistic issues I
may have introduced can be fixed in follow up patches. Most of these
docs are going to get shuffled around or rewritten when we start
moving to sphinx anyway, so we'll have the chance to really review the
content itself then.

OK, sounds good.  I also see the previous conversation about this work
now.  Sorry about that.  I'm on board with all of this.

Ben, I'm happy to review all of the patches for this conversion if
you'd like.



..and sorry for not setting the scene: I forget how active this mailing 
list is :)


Let me know if you've any questions.

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

Re: [ovs-dev] [PATCH] dpdk: Fix DPDK pdump compilation

[Daniele Di Proietto]

2016-10-13 10:27 GMT-07:00 Ciara Loftus :

> The rte_pdump header file was not included in the file that requires it.
> Fix this.
>
> Fixes: 01961bbdd34a ("dpdk: New module with some code from netdev-dpdk.")
> Signed-off-by: Ciara Loftus 
>

Oops, thanks for the fix!

I was able to reproduce the build failure only by setting in DPDK
CONFIG_RTE_LIBRTE_PMD_PCAP=y.

I had to add  before  as Aurojit was suggesting.

With that addition I applied this to master.

Thanks,

Daniele


> ---
>  lib/dpdk.c| 4 
>  lib/netdev-dpdk.c | 3 ---
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index caea0f4..c2182a7 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -22,8 +22,12 @@
>  #include 
>
>  #include 
> +#ifdef DPDK_PDUMP
> +#include 
> +#endif
>
>  #include "dirs.h"
> +#include "fatal-signal.h"
>  #include "netdev-dpdk.h"
>  #include "openvswitch/dynamic-string.h"
>  #include "openvswitch/vlog.h"
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ab8c34f..7c1523e 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -31,9 +31,6 @@
>  #include 
>  #include 
>  #include 
> -#ifdef DPDK_PDUMP
> -#include 
> -#endif
>  #include 
>
>  #include "dirs.h"
> --
> 2.4.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] ovn: Improving southbound database security

[Lance Richardson]

> From: "Andy Zhou" 
> To: "Ben Pfaff" 
> Cc: "ovs dev" , "Numan Siddique" , 
> "Babu Shanmugam" ,
> "Lance Richardson" , "Justin Pettit" , 
> "Russell Bryant" 
> Sent: Thursday, October 13, 2016 3:05:40 PM
> Subject: Re: ovn: Improving southbound database security
> 
> On Thu, Oct 13, 2016 at 11:26 AM, Ben Pfaff  wrote:
> 
> > On Wed, Oct 12, 2016 at 01:51:39PM -0400, Russell Bryant wrote:
> > > 1) Add support to ovsdb-server for read-only remotes.  The port reachable
> > > by ovn-controller would only accept read-only connections.
> >
> > Andy, is this something that you can put on your to-do list?  I guess
> > that it is not a huge amount of work.
> >
> > Thanks,
> >
> > Ben.
> >
> 
> Sure, I think the read-only OVSDB server has already been implemented as
> part of the replication work.
> Currently, it is only tied to active/backup state. We probably just need to
> make this feature decouple from replication.
> 

Right, the prototype RFC I just posted builds on the work done for replication,
essentially this:

 ovsdb_jsonrpc_session_create(remote, jsonrpc_session_open(name, true),
-  svr->read_only);
+  svr->read_only ||
+  stream_or_pstream_is_read_only(name));
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [RFC] ovsdb: implement read-only remote connection type

[Lance Richardson]

This change set adds a new optional "access control" specifier to
remote connection descriptors used by ovsdb.

Examples:
--remote=ptcp:ro:0:192.168.0.10
--remote=punix:ro:asocket.sock
--remote=pssl:ro:0:192.168.0.10
--remote=tcp:ro:192.168.0.99:
--remote=unix:ro:asocket.sock
--remote=ssl:ro:192.168.0.10:

To-do:
   - documentation

Notes/questions:
- Other possibilities might be worth considering, e.g.
  a "--remote-ro" command-line option that would be 
  analogous to "--remote"). This approach was cooked
  up in the dark, might not be to everyone's taste, and
  could easily be scrapped in favor of another approach
  if needed.

Side-note about odd autotest behavior:

 Doing 'make check TESTSUITEFLAGS="1876-1881"' executes these test
 cases:
1876: ovsdb-server/read-only ptcp connection  ok
1877: ovsdb-server/read-only punix connection ok
1878: ovsdb-server/read-only pssl connection  ok
1879: ovsdb-server/read-only tcp connection   ok
1880: ovsdb-server/read-only unix connection  ok
1881: ovsdb-server/read-only ssl connection   ok

 But doing 'make check TESTSUITEFLAGS="-k read-only"' only executes
 these:
1876: ovsdb-server/read-only ptcp connection  ok
1877: ovsdb-server/read-only punix connection ok
1879: ovsdb-server/read-only tcp connection   ok
1880: ovsdb-server/read-only unix connection  ok

 Shouldn't they all match the "read-only" keyword??


Signed-off-by: Lance Richardson 
---

 lib/stream-ssl.c  |  11 +-
 lib/stream-tcp.c  |  21 +--
 lib/stream.c  |  98 +-
 lib/stream.h  |   4 +-
 ovn/controller-vtep/ovn-controller-vtep.c |   2 +-
 ovn/controller/ovn-controller.c   |   2 +-
 ovn/northd/ovn-northd.c   |   2 +-
 ovn/utilities/ovn-sbctl.c |   2 +-
 ovn/utilities/ovn-trace.c |   2 +-
 ovsdb/jsonrpc-server.c|   7 +-
 ovsdb/ovsdb-client.c  |   2 +-
 ovsdb/ovsdb-server.c  |   2 +-
 tests/ovsdb-server.at | 208 ++
 tests/test-jsonrpc.c  |   2 +-
 utilities/ovs-vsctl.c |   2 +-
 vswitchd/ovs-vswitchd.c   |   2 +-
 vtep/vtep-ctl.c   |   2 +-
 17 files changed, 337 insertions(+), 34 deletions(-)

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index a5c32a1..2443005 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -778,13 +778,14 @@ pssl_pstream_cast(struct pstream *pstream)
 }
 
 static int
-pssl_open(const char *name OVS_UNUSED, char *suffix, struct pstream **pstreamp,
+pssl_open(const char *name, char *suffix, struct pstream **pstreamp,
   uint8_t dscp)
 {
 char bound_name[SS_NTOP_BUFSIZE + 16];
 char addrbuf[SS_NTOP_BUFSIZE];
 struct sockaddr_storage ss;
 struct pssl_pstream *pssl;
+const char *access = "";
 uint16_t port;
 int retval;
 int fd;
@@ -799,9 +800,13 @@ pssl_open(const char *name OVS_UNUSED, char *suffix, 
struct pstream **pstreamp,
 return -fd;
 }
 
+if (!strncmp(name, "pssl:ro:", 8)) {
+access = "ro:";
+}
+
 port = ss_get_port();
-snprintf(bound_name, sizeof bound_name, "pssl:%"PRIu16":%s",
- port, ss_format_address(, addrbuf, sizeof addrbuf));
+snprintf(bound_name, sizeof bound_name, "pssl:%s%"PRIu16":%s",
+ access, port, ss_format_address(, addrbuf, sizeof addrbuf));
 
 pssl = xmalloc(sizeof *pssl);
 pstream_init(>pstream, _pstream_class, bound_name);
diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
index 1749fad..e0aaa68 100644
--- a/lib/stream-tcp.c
+++ b/lib/stream-tcp.c
@@ -84,13 +84,13 @@ static int
 new_pstream(char *suffix, const char *name, struct pstream **pstreamp,
 int dscp, char *unlink_path, bool kernel_print_port)
 {
-char bound_name[SS_NTOP_BUFSIZE + 16];
+char bound_name[SS_NTOP_BUFSIZE + 20];
 char addrbuf[SS_NTOP_BUFSIZE];
 struct sockaddr_storage ss;
+const char *access = "";
 int error;
 uint16_t port;
 int fd;
-char *conn_name = CONST_CAST(char *, name);
 
 fd = inet_open_passive(SOCK_STREAM, suffix, -1, , dscp,
kernel_print_port);
@@ -98,14 +98,15 @@ new_pstream(char *suffix, const char *name, struct pstream 
**pstreamp,
 return -fd;
 }
 
-port = ss_get_port();
-if (!conn_name) {
-snprintf(bound_name, sizeof bound_name, "ptcp:%"PRIu16":%s",
- port, ss_format_address(, addrbuf, sizeof addrbuf));
-conn_name = bound_name;
+if (!strncmp(name, "ptcp:ro:", 8)) {
+access = "ro:";
 }
 
-error = new_fd_pstream(conn_name, fd, ptcp_accept, unlink_path, 

[ovs-dev] [RFC v2] ovsdb: implement read-only remote connection type

[Lance Richardson]

This change set adds a new optional "access control" specifier to
remote connection descriptors used by ovsdb.

Examples:
--remote=ptcp:ro:0:192.168.0.10
--remote=punix:ro:asocket.sock
--remote=pssl:ro:0:192.168.0.10
--remote=tcp:ro:192.168.0.99:
--remote=unix:ro:asocket.sock
--remote=ssl:ro:192.168.0.10:

To-do:
   - documentation

Notes/questions:
- Other possibilities might be worth considering, e.g.
  a "--remote-ro" command-line option that would be 
  analogous to "--remote"). This approach was cooked
  up in the dark, might not be to everyone's taste, and
  could easily be scrapped in favor of another approach
  if needed.

Side-note about odd autotest behavior:

 Doing 'make check TESTSUITEFLAGS="1876-1881"' executes these test
 cases:
1876: ovsdb-server/read-only ptcp connection  ok
1877: ovsdb-server/read-only punix connection ok
1878: ovsdb-server/read-only pssl connection  ok
1879: ovsdb-server/read-only tcp connection   ok
1880: ovsdb-server/read-only unix connection  ok
1881: ovsdb-server/read-only ssl connection   ok

 But doing 'make check TESTSUITEFLAGS="-k read-only"' only executes
 these:
1876: ovsdb-server/read-only ptcp connection  ok
1877: ovsdb-server/read-only punix connection ok
1879: ovsdb-server/read-only tcp connection   ok
1880: ovsdb-server/read-only unix connection  ok

 Shouldn't they all match the "read-only" keyword??


Signed-off-by: Lance Richardson 
---
v2: resending with corrected cc: list

 lib/stream-ssl.c  |  11 +-
 lib/stream-tcp.c  |  21 +--
 lib/stream.c  |  98 +-
 lib/stream.h  |   4 +-
 ovn/controller-vtep/ovn-controller-vtep.c |   2 +-
 ovn/controller/ovn-controller.c   |   2 +-
 ovn/northd/ovn-northd.c   |   2 +-
 ovn/utilities/ovn-sbctl.c |   2 +-
 ovn/utilities/ovn-trace.c |   2 +-
 ovsdb/jsonrpc-server.c|   7 +-
 ovsdb/ovsdb-client.c  |   2 +-
 ovsdb/ovsdb-server.c  |   2 +-
 tests/ovsdb-server.at | 208 ++
 tests/test-jsonrpc.c  |   2 +-
 utilities/ovs-vsctl.c |   2 +-
 vswitchd/ovs-vswitchd.c   |   2 +-
 vtep/vtep-ctl.c   |   2 +-
 17 files changed, 337 insertions(+), 34 deletions(-)

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index a5c32a1..2443005 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -778,13 +778,14 @@ pssl_pstream_cast(struct pstream *pstream)
 }
 
 static int
-pssl_open(const char *name OVS_UNUSED, char *suffix, struct pstream **pstreamp,
+pssl_open(const char *name, char *suffix, struct pstream **pstreamp,
   uint8_t dscp)
 {
 char bound_name[SS_NTOP_BUFSIZE + 16];
 char addrbuf[SS_NTOP_BUFSIZE];
 struct sockaddr_storage ss;
 struct pssl_pstream *pssl;
+const char *access = "";
 uint16_t port;
 int retval;
 int fd;
@@ -799,9 +800,13 @@ pssl_open(const char *name OVS_UNUSED, char *suffix, 
struct pstream **pstreamp,
 return -fd;
 }
 
+if (!strncmp(name, "pssl:ro:", 8)) {
+access = "ro:";
+}
+
 port = ss_get_port();
-snprintf(bound_name, sizeof bound_name, "pssl:%"PRIu16":%s",
- port, ss_format_address(, addrbuf, sizeof addrbuf));
+snprintf(bound_name, sizeof bound_name, "pssl:%s%"PRIu16":%s",
+ access, port, ss_format_address(, addrbuf, sizeof addrbuf));
 
 pssl = xmalloc(sizeof *pssl);
 pstream_init(>pstream, _pstream_class, bound_name);
diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
index 1749fad..e0aaa68 100644
--- a/lib/stream-tcp.c
+++ b/lib/stream-tcp.c
@@ -84,13 +84,13 @@ static int
 new_pstream(char *suffix, const char *name, struct pstream **pstreamp,
 int dscp, char *unlink_path, bool kernel_print_port)
 {
-char bound_name[SS_NTOP_BUFSIZE + 16];
+char bound_name[SS_NTOP_BUFSIZE + 20];
 char addrbuf[SS_NTOP_BUFSIZE];
 struct sockaddr_storage ss;
+const char *access = "";
 int error;
 uint16_t port;
 int fd;
-char *conn_name = CONST_CAST(char *, name);
 
 fd = inet_open_passive(SOCK_STREAM, suffix, -1, , dscp,
kernel_print_port);
@@ -98,14 +98,15 @@ new_pstream(char *suffix, const char *name, struct pstream 
**pstreamp,
 return -fd;
 }
 
-port = ss_get_port();
-if (!conn_name) {
-snprintf(bound_name, sizeof bound_name, "ptcp:%"PRIu16":%s",
- port, ss_format_address(, addrbuf, sizeof addrbuf));
-conn_name = bound_name;
+if (!strncmp(name, "ptcp:ro:", 8)) {
+access = "ro:";
 }
 
-error = new_fd_pstream(conn_name, 

Re: [ovs-dev] Subject: [PATCH] ovs-monitor-ipsec: ipsec_vxlan support and IPsec tunnel mode options support.

[Ben Pfaff]

The commit message says:

openvswitch: Allow external IPsec tunnel management.

OVS GRE IPsec tunnel support has multiple issues, Therefore
it was deprecated in OVS 2.6.

Following patch removes support for GRE IPsec and allows external
IPsec tunnel management for any type of tunnel not just GRE.
e.g. user can encrypt Geneve or VxLan traffic.

It can be done by using openflow pipeline to set skb-mark
and using IPsec keying daemons to implement IPsec tunnels.
This packet can be matched for the skb-mark to encrypt
selective tunnel traffic.

VMware-BZ: 1710701
Signed-off-by: Pravin B Shelar 
Acked-by: Ansis Atteka 


On Thu, Oct 13, 2016 at 06:06:10AM +, Muthukrishnan Thangasamy wrote:
> Dear Ben ,
> 
> 
> Thank you for your response.
> 
> 
> Could you please throw some light on ,any specific reason why OVS stopped 
> support for ovs-monitor-ipsec and decided to remove support for ipsec 
> altogether in ovs.
> 
> 
> Regards
> 
> Muthukrishnan
> 
> 9952012433
> 
> 
> From: Ben Pfaff 
> Sent: Thursday, September 29, 2016 8:43:43 PM
> To: Muthukrishnan Thangasamy
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] Subject: [PATCH] ovs-monitor-ipsec: ipsec_vxlan 
> support and IPsec tunnel mode options support.
> 
> On Thu, Sep 29, 2016 at 07:13:55AM +, Muthukrishnan Thangasamy wrote:
> > From 76e504a38a3c556884782323459cc6862a38747c Mon Sep 17 00:00:00 2001
> > From: Muthukrishnan T 
> > Date: Thu, 29 Sep 2016 12:21:29 +0530
> > Subject: [PATCH] ovs-monitor-ipsec: ipsec_vxlan support and IPsec tunnel 
> > mode options support.
> 
> We just removed ovs-monitor-ipsec, so this can't be applied.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] ovn: Improving southbound database security

[Lance Richardson]

> From: "Ben Pfaff" 
> To: "Andy Zhou" 
> Cc: "ovs dev" , "Numan Siddique" , 
> "Babu Shanmugam" ,
> "Lance Richardson" , "Justin Pettit" , 
> "Russell Bryant" 
> Sent: Thursday, October 13, 2016 2:26:13 PM
> Subject: Re: ovn: Improving southbound database security
> 
> On Wed, Oct 12, 2016 at 01:51:39PM -0400, Russell Bryant wrote:
> > 1) Add support to ovsdb-server for read-only remotes.  The port reachable
> > by ovn-controller would only accept read-only connections.
> 
> Andy, is this something that you can put on your to-do list?  I guess
> that it is not a huge amount of work.
> 
> Thanks,
> 
> Ben.
> 

Hi Ben,

I have an initial prototype for this, I can post later today as an
RFC.

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

Re: [ovs-dev] ovn: Improving southbound database security

[Ben Pfaff]

On Wed, Oct 12, 2016 at 01:51:39PM -0400, Russell Bryant wrote:
> 1) Add support to ovsdb-server for read-only remotes.  The port reachable
> by ovn-controller would only accept read-only connections.

Andy, is this something that you can put on your to-do list?  I guess
that it is not a huge amount of work.

Thanks,

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

Re: [ovs-dev] ovn: Improving southbound database security

[Andy Zhou]

On Thu, Oct 13, 2016 at 11:26 AM, Ben Pfaff  wrote:

> On Wed, Oct 12, 2016 at 01:51:39PM -0400, Russell Bryant wrote:
> > 1) Add support to ovsdb-server for read-only remotes.  The port reachable
> > by ovn-controller would only accept read-only connections.
>
> Andy, is this something that you can put on your to-do list?  I guess
> that it is not a huge amount of work.
>
> Thanks,
>
> Ben.
>

Sure, I think the read-only OVSDB server has already been implemented as
part of the replication work.
Currently, it is only tied to active/backup state. We probably just need to
make this feature decouple from replication.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] ovsdb: implement read-only remote connection type

[Ben Pfaff]

On Thu, Oct 13, 2016 at 03:08:46PM -0400, Lance Richardson wrote:
>  Doing 'make check TESTSUITEFLAGS="1876-1881"' executes these test
>  cases:
> 1876: ovsdb-server/read-only ptcp connection  ok
> 1877: ovsdb-server/read-only punix connection ok
> 1878: ovsdb-server/read-only pssl connection  ok
> 1879: ovsdb-server/read-only tcp connection   ok
> 1880: ovsdb-server/read-only unix connection  ok
> 1881: ovsdb-server/read-only ssl connection   ok
> 
>  But doing 'make check TESTSUITEFLAGS="-k read-only"' only executes
>  these:
> 1876: ovsdb-server/read-only ptcp connection  ok
> 1877: ovsdb-server/read-only punix connection ok
> 1879: ovsdb-server/read-only tcp connection   ok
> 1880: ovsdb-server/read-only unix connection  ok
> 
>  Shouldn't they all match the "read-only" keyword??

Autotest is really dumb when it comes to defining a "word".  It thinks
that "ovsdb-server/read-only" is a single word.

(Some of them did match because you included AT_KEYWORDS([read-only]) in
them.)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] ovsdb: implement read-only remote connection type

[Lance Richardson]

> From: "Ben Pfaff" 
> To: "Lance Richardson" 
> Cc: dev@openvswitch.org, jpe...@ovn.org
> Sent: Thursday, October 13, 2016 5:08:42 PM
> Subject: Re: [ovs-dev] [RFC] ovsdb: implement read-only remote connection type
> 
> On Thu, Oct 13, 2016 at 03:08:46PM -0400, Lance Richardson wrote:
> >  Doing 'make check TESTSUITEFLAGS="1876-1881"' executes these test
> >  cases:
> > 1876: ovsdb-server/read-only ptcp connection  ok
> > 1877: ovsdb-server/read-only punix connection ok
> > 1878: ovsdb-server/read-only pssl connection  ok
> > 1879: ovsdb-server/read-only tcp connection   ok
> > 1880: ovsdb-server/read-only unix connection  ok
> > 1881: ovsdb-server/read-only ssl connection   ok
> > 
> >  But doing 'make check TESTSUITEFLAGS="-k read-only"' only executes
> >  these:
> > 1876: ovsdb-server/read-only ptcp connection  ok
> > 1877: ovsdb-server/read-only punix connection ok
> > 1879: ovsdb-server/read-only tcp connection   ok
> > 1880: ovsdb-server/read-only unix connection  ok
> > 
> >  Shouldn't they all match the "read-only" keyword??
> 
> Autotest is really dumb when it comes to defining a "word".  It thinks
> that "ovsdb-server/read-only" is a single word.
> 
> (Some of them did match because you included AT_KEYWORDS([read-only]) in
> them.)
> 

I thought I had checked that, but you're absolutely correct. It's not
just autotest that's challenged, apparently.

Thanks,

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

Re: [ovs-dev] [PATCH v3 1/4] ovn: ovn-ctl support for HA ovn DB servers

[Andy Zhou]

On Wed, Oct 12, 2016 at 10:57 PM, Babu Shanmugam 
wrote:

>
>
> On Thursday 13 October 2016 07:26 AM, Andy Zhou wrote:
>
>
>
> On Sun, Oct 9, 2016 at 12:02 AM, Babu Shanmugam 
> wrote:
>
>>
>>
>> On Friday 07 October 2016 05:33 AM, Andy Zhou wrote:
>>
>>> Babu,  Thank you for working on this.  At a high level, it is not clear
>>> to me the boundary between ocf scripts and the ovn-ctl script -- i.e. which
>>> aspect is managed by which entity.  For example, 1) which scripts are
>>> responsible for starting the ovsdb servers.
>>>
>> ovsdb servers are started by the pacemaker. It uses the OCF script and
>> the OCF script uses ovn-ctl.
>>
>> 2) Which script should manage the fail-over -- I tried to shut down a
>>> cluster node using the "pcs" command, and fail-over did not happen.
>>>
>> The OCF script for OVN DB servers is capable of understanding the promote
>> and demote calls. So, pacemaker will use this script to run ovsdb server in
>> all the nodes and promote one node as the master(active server). If the
>> node in which the master instance is running fails, pacemaker automatically
>> promotes another node as the master. OCF script is an agent for the
>> pacemaker for the OVN db resource.
>> The above behavior depends on the way you are configuring the resource
>> that uses this OCF script. I am attaching a simple set of commands to
>> configure the ovsdb server. You can create the resources after creating the
>> cluster with the following command
>>
>> crm configure < ovndb.pcmk
>>
>> Please note, you have to replace the macros VM1_NAME, VM2_NAME, VM3_NAME
>> and MASTER_IP with the respective values before using ovndb.pcmk. This
>> script works with a 3 node cluster. I am assuming the node ids as 101, 102,
>> and 103. Please replace them as well to work with your cluster.
>>
>>
>> --
>> Babu
>>
>
> Unfortunately, CRM is not distributed with pacemaker on centos anymore.
> It took me some time to get it installed.  I think other may ran into
> similar issues, so
> it may be worth while do document this, or change the script to use "pcs"
> which is part of the distribution.
>
>
> I agree. Is INSTALL*.md good enough? In openstack, we are managing the
> resource through puppet manifests.
>

O.K.

>
>
>
> I adapted the script with my setup.  I have two nodes, "h1"(10.33.74.77)
> and "h2"(10.33.75.158), For Master_IP, I used 10.33.75.220.
>
> This is the output of crm configure show:
>
> --
>
>  [root@h2 azhou]# crm configure show
>
> node 1: h1 \
>
> attributes
>
> node 2: h2
>
> primitive ClusterIP IPaddr2 \
>
> params ip=10.33.75.200 cidr_netmask=32 \
>
> op start interval=0s timeout=20s \
>
> op stop interval=0s timeout=20s \
>
> op monitor interval=30s
>
> primitive WebSite apache \
>
> params configfile="/etc/httpd/conf/httpd.conf" statusurl="
> http://127.0.0.1/server-status; \
>
> op start interval=0s timeout=40s \
>
> op stop interval=0s timeout=60s \
>
> op monitor interval=1min \
>
> meta
>
> primitive ovndb ocf:ovn:ovndb-servers \
>
> op start interval=0s timeout=30s \
>
> op stop interval=0s timeout=20s \
>
> op promote interval=0s timeout=50s \
>
> op demote interval=0s timeout=50s \
>
> op monitor interval=1min \
>
> meta
>
> colocation colocation-WebSite-ClusterIP-INFINITY inf: WebSite ClusterIP
>
> order order-ClusterIP-WebSite-mandatory ClusterIP:start WebSite:start
>
> property cib-bootstrap-options: \
>
> have-watchdog=false \
>
> dc-version=1.1.13-10.el7_2.4-44eb2dd \
>
> cluster-infrastructure=corosync \
>
> cluster-name=mycluster \
>
> stonith-enabled=false
>
>
>
> You seem to have configured ovndb just as a primitive resource and not as
> a master slave resource. And there is no colocation resource configured for
> the ovndb with ClusterIP. Only with the colocation resource, ovndb server
> will be co-located with the ClusterIP resource.  You will have to include
> the following lines for crm configure. You can configure the same with pcs
> as well.
>
> ms ovndb-master ovndb meta notify="true"
> colocation colocation-ovndb-master-ClusterIP-INFINITY inf:
> ovndb-master:Started ClusterIP:Master
> order order-ClusterIP-ovndb-master-mandatory inf: ClusterIP:start
> ovndb-master:start
>
> Done. Now it shows the following.

[root@h2 ovs]# crm configure show
>
> node 1: h1 \
>
> attributes
>
> node 2: h2
>
> primitive ClusterIP IPaddr2 \
>
> params ip=10.33.75.200 cidr_netmask=32 \
>
> op start interval=0s timeout=20s \
>
> op stop interval=0s timeout=20s \
>
> op monitor interval=30s
>
> primitive ovndb ocf:ovn:ovndb-servers \
>
> op start interval=0s timeout=30s \
>
> op stop interval=0s timeout=20s \
>
> op promote interval=0s timeout=50s \
>
> op demote interval=0s timeout=50s \
>
> op monitor interval=1min \
>
> meta
>
> ms ovndb-master ovndb \
>
> meta notify=true
>
> colocation colocation-ovndb-master-ClusterIP-INFINITY inf: 
> ovndb-master:Started
> ClusterIP:Master
>
> order order-ClusterIP-ovndb-master-mandatory inf: ClusterIP:start
> 

[ovs-dev] [PATCH v3, 5/7] ovn-controller: add 'put_nd_ra' and 'put_nd_opt_*' actions support

[Zongkai LI]

This patch adds action 'nd_ra' specific inner actions 'put_nd_ra',
'put_nd_opt_prefix', 'put_nd_opt_mtu', 'put_nd_opt_sll'.

Action put_nd_ra will set following fields in Router Advertisement (RA) packet:
 - cur_hop_limit(8-bit unsigned integer),
 - "Managed address configuration" and "Other configuration"
   flags(8-bit unsigned integer, with 6-bit 0 in low endian),
 - router lifetime(16-bit unsigned integer),
 - reachable time(32-bit unsigned integer),
 - retrans timer(32-bit unsigned integer).
e.g. put_nd_ra(64,0,10800,0,0);

Action put_nd_ra_opt_sll will append Source Link-layer Address Option for RA
packet with inner ethernet address parameter.
e.g. put_nd_ra_opt_sll(12:34:56:78:9a:bc);

Action put_nd_ra_opt_mtu will append MTU Option for RA packet with inner
integer value(32-bit unsigned integer).
e.g. put_nd_ra_opt_mtu(1450);

Action put_nd_ra_opt_prefix will append Prefix Information Option with
following inner parameters for RA packet:
 - prefix length(8-bit unsigned integer),
 - on-link flag and autonomous address-configuration flag
   (8-bit unsigned integer, with 6-bit 0 in low endian),
 - valid lifetime(32-bit unsigned integer),
 - preferred lifetime(32-bit unsigned integer),
 - prefix(128-bit IPv6 address prefix).
e.g. put_nd_ra_opt_prefix(64,192,10800,10800,fdad:a0f9:a012::);

Signed-off-by: Zongkai LI 
---
 include/ovn/actions.h |  79 +-
 ovn/controller/pinctrl.c  | 186 ++--
 ovn/lib/actions.c | 264 +-
 ovn/ovn-sb.xml|  39 +++
 ovn/utilities/ovn-trace.c |   4 +
 tests/ovn.at  |  16 ++-
 6 files changed, 575 insertions(+), 13 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index f5f1e62..91d968e 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -69,7 +69,11 @@ struct simap;
 OVNACT(PUT_ND,ovnact_put_mac_bind)  \
 OVNACT(PUT_DHCPV4_OPTS, ovnact_put_dhcp_opts)   \
 OVNACT(PUT_DHCPV6_OPTS, ovnact_put_dhcp_opts)   \
-OVNACT(SET_QUEUE,   ovnact_set_queue)
+OVNACT(SET_QUEUE,   ovnact_set_queue)   \
+OVNACT(PUT_ND_RA,   ovnact_put_nd_ra)   \
+OVNACT(PUT_ND_OPT_SLL,  ovnact_put_nd_opt_sll)  \
+OVNACT(PUT_ND_OPT_MTU,  ovnact_put_nd_opt_mtu)  \
+OVNACT(PUT_ND_OPT_PREFIX, ovnact_put_nd_opt_prefix)
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -235,6 +239,38 @@ struct ovnact_set_queue {
 uint16_t queue_id;
 };
 
+/* OVNACT_PUT_ND_RA. */
+struct ovnact_put_nd_ra {
+struct ovnact ovnact;
+uint8_t  cur_hop_limit;
+uint8_t  mo_flags;
+uint16_t router_lifetime;
+uint32_t reachable_time;
+uint32_t retrans_timer;
+};
+
+/* OVNACT_PUT_ND_OPT_SLL. */
+struct ovnact_put_nd_opt_sll {
+struct ovnact ovnact;
+struct eth_addr mac;
+};
+
+/* OVNACT_PUT_ND_OPT_MTU. */
+struct ovnact_put_nd_opt_mtu {
+struct ovnact ovnact;
+uint32_t mtu;
+};
+
+/* OVNACT_PUT_ND_OPT_PREFIX. */
+struct ovnact_put_nd_opt_prefix {
+struct ovnact ovnact;
+uint8_t prefix_len;
+uint8_t la_flags;
+uint32_t valid_lifetime;
+uint32_t preferred_lifetime;
+struct in6_addr prefix;
+};
+
 /* Internal use by the helpers below. */
 void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
 void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
@@ -368,6 +404,44 @@ enum action_opcode {
  * The actions, in OpenFlow 1.3 format, follow the action_header.
  */
 ACTION_OPCODE_ND_RA,
+
+/* "put_nd_ra(c, mo, router_lt, reach_t, retrans_t)".
+ *
+ * Specific inner action for ACTION_OPCODE_ND_RA. Arguments in this format:
+ *   - c: 8-bit unsigned integer for current hop limit.
+ *   - mo: 8-bit unsigned integer for Managed address configuration flag
+ * and Other configuration flag. (including 6-bit reserved 0)
+ *   - router_lt: 16-bit unsigned integer for router lifetime.
+ *   - reach_t: 32-bit unisgned integer for reachable time.
+ *   - retrans_t: 32-bit unsigned integer for retrans timer.
+ */
+ACTION_OPCODE_PUT_ND_RA,
+
+/* "put_nd_opt_sll(mac)".
+ *
+ * Specific inner action for ACTION_OPCODE_ND_RA. Arguments in this format:
+ *   - mac: 48-bit ethernet address.
+ */
+ACTION_OPCODE_PUT_ND_OPT_SLL,
+
+/* "put_nd_opt_mtu(mtu)".
+ *
+ * Specific inner action for ACTION_OPCODE_ND_RA. Arguments in this format:
+ *   - mtu: 32-bit unsigned integer MTU.
+ */
+ACTION_OPCODE_PUT_ND_OPT_MTU,
+
+/* "put_nd_opt_prefix(plen, la, valid_lt, preferred_lt, prefix)".
+ *
+ * Specific inner action for ACTION_OPCODE_ND_RA. Arguments in this format:
+ *   - plen: 8-bit unsigned integer for prefix length.
+ *   - la: 8-bit unsigned integer for on-link flag and autonomous
+ * address-configuration flag. (including 6-bit reserved 0)
+ *   - 

[ovs-dev] [PATCH v3, 7/7] ovn-northd: add IPv6 RS responder support

[Zongkai LI]

This patch add support for building Router Solicitation responder flows, which
will be used to reply Router Advertisement packet from ovn-controller for
VM/VIF port.

It adds table Logical_Router_RA_Prefix_Config to define a set of ND Prefix
informations configurations(such as valid_lifetime, preferred_lifetime) for
a IPv6 prefix.

It adds table Logical_Router_Port new columns:
 - ipv6_ra_prefix_configs: to refer records in Logical_Router_RA_Prefix_Config
   for lrouter port to use for its networks.
 - ipv6_ra_configs: to define a set of RA configurations, such as
   "managed_address" flag, router_lifetime.

Signed-off-by: Zongkai LI 
---
 ovn/northd/ovn-northd.c   | 140 --
 ovn/ovn-nb.ovsschema  |  21 ++-
 ovn/ovn-nb.xml| 103 ++
 ovn/utilities/ovn-nbctl.c |   5 ++
 tests/ovn.at  | 137 +
 5 files changed, 398 insertions(+), 8 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 093d5ff..0400dea 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -130,9 +130,10 @@ enum ovn_stage {
 PIPELINE_STAGE(ROUTER, IN,  DEFRAG,  2, "lr_in_defrag")   \
 PIPELINE_STAGE(ROUTER, IN,  UNSNAT,  3, "lr_in_unsnat")   \
 PIPELINE_STAGE(ROUTER, IN,  DNAT,4, "lr_in_dnat") \
-PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  5, "lr_in_ip_routing")   \
-PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 6, "lr_in_arp_resolve")  \
-PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 7, "lr_in_arp_request")  \
+PIPELINE_STAGE(ROUTER, IN,  RS_RSP,  5, "lr_in_rs_rsp")   \
+PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  6, "lr_in_ip_routing")   \
+PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 7, "lr_in_arp_resolve")  \
+PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 8, "lr_in_arp_request")  \
   \
 /* Logical router egress stages. */   \
 PIPELINE_STAGE(ROUTER, OUT, SNAT,  0, "lr_out_snat")  \
@@ -4040,7 +4041,134 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
   "ip", "flags.loopback = 1; ct_dnat;");
 }
 
-/* Logical router ingress table 5: IP Routing.
+/* Logical router ingress table 5: RS responder, reply for router port
+ * with IPv6 networks configured. (priority 50)*/
+HMAP_FOR_EACH (op, key_node, ports) {
+if (!op->nbrp || op->nbrp->peer || !op->peer) {
+continue;
+}
+
+if (!op->lrp_networks.n_ipv6_addrs) {
+continue;
+}
+
+ds_clear();
+ds_put_format(, "inport == %s && ip6.dst == ff02::2 && nd_rs",
+  op->json_key);
+ds_clear();
+
+const char *hop_lmt = smap_get(
+>nbrp->ipv6_ra_configs, "cur_hop_limit");
+const char *mgd_addr = smap_get(
+>nbrp->ipv6_ra_configs, "managed_address");
+const char *other_cfg = smap_get(
+>nbrp->ipv6_ra_configs, "other_config");
+const char *rtr_lt = smap_get(
+>nbrp->ipv6_ra_configs, "router_lifetime");
+const char *rch_t = smap_get(
+>nbrp->ipv6_ra_configs, "reachable_time");
+const char *rts_t = smap_get(
+>nbrp->ipv6_ra_configs, "retrans_timer");
+const char *mtu_s = smap_get(
+>nbrp->ipv6_ra_configs, "mtu");
+
+uint8_t managed_address_flag = ND_RA_MANAGED_ADDRESS;
+if (mgd_addr && !strcmp(mgd_addr, "0")) {
+managed_address_flag = 0;
+}
+uint8_t other_config_flag = ND_RA_OTHER_CONFIG;
+if (other_cfg && !strcmp(other_cfg, "0")) {
+other_config_flag = 0;
+}
+uint32_t mtu = (mtu_s && atoi(mtu_s) >= 1280) ? atoi(mtu_s) : 0;
+
+ds_put_format(, "nd_ra{put_nd_ra(%u, %u, %u, %u, %u); "
+"put_nd_opt_sll(%s); ",
+  (hop_lmt && atoi(hop_lmt) > 0) ? atoi(hop_lmt) : 64,
+  managed_address_flag | other_config_flag,
+  (rtr_lt && atoi(rtr_lt) > 0) ? atoi(rtr_lt) : 10800,
+  (rch_t && atoi(rch_t) > 0) ? atoi(rch_t) : 0,
+  (rts_t && atoi(rts_t) > 0) ? atoi(rts_t) : 0,
+  op->lrp_networks.ea_s);
+if (mtu > 0) {
+ds_put_format(, "put_nd_opt_mtu(%u); ",
+ mtu);
+}
+size_t prev_actions_len = actions.length;
+struct ds lrp_prefix = DS_EMPTY_INITIALIZER;
+for (size_t i = 0; i != op->lrp_networks.n_ipv6_addrs; i++) {
+if (in6_is_lla(>lrp_networks.ipv6_addrs[i].network)) {
+continue;
+}
+uint8_t onlink_flag = ND_PREFIX_ON_LINK;
+uint8_t autonomous_address_flag = 

[ovs-dev] [PATCH v3, 6/7] ovn-northd: add Router Advertisement packet forward flow

[Zongkai LI]

This patch add lflow support for Router Advertisement packet forward.
Forward RA packets to router port in lswitch l2_lkup table.

Signed-off-by: Zongkai LI 
---
 ovn/northd/ovn-northd.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 9d74ec6..093d5ff 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3051,6 +3051,32 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 ovn_lflow_add(lflows, od, S_SWITCH_IN_DHCP_RESPONSE, 0, "1", "next;");
 }
 
+/* Ingress table 13: Destination lookup, router solicitation handling
+ * (priority 110). */
+HMAP_FOR_EACH (od, key_node, datapaths) {
+if (!od->nbs) {
+continue;
+}
+
+if (!od->n_router_ports) {
+continue;
+}
+
+ds_clear();
+for (size_t i = 0; i != od->n_router_ports; i++) {
+op = od->router_ports[i];
+if (!op->lsp_addrs || !op->lsp_addrs->n_ipv6_addrs) {
+continue;
+}
+ds_put_format(, "outport = %s; output; ", op->json_key);
+}
+if (actions.length != 0) {
+ds_chomp(, ' ');
+ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110, "nd_rs",
+  ds_cstr());
+}
+}
+
 /* Ingress table 13: Destination lookup, broadcast and multicast handling
  * (priority 100). */
 HMAP_FOR_EACH (op, key_node, ports) {
-- 
2.7.4

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

[ovs-dev] [PATCH v3, 2/7] lib: rename ovs_nd_opt to ovs_nd_lla_opt

[Zongkai LI]

Since ovs_nd_mtu_opt and ovs_nd_prefix_opt is introducted, rename
ovs_nd_opt to ovs_nd_lla_opt to specify it's Source/Target Link-layer
Address Option.

Signed-off-by: Zongkai LI 
---
 lib/flow.c| 30 +++
 lib/odp-execute.c | 22 -
 lib/packets.c | 72 +++
 lib/packets.h | 19 +++
 4 files changed, 71 insertions(+), 72 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index f4ac8b3..bf1b2f1 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -400,8 +400,8 @@ parse_icmpv6(const void **datap, size_t *sizep, const 
struct icmp6_hdr *icmp,
 while (*sizep >= 8) {
 /* The minimum size of an option is 8 bytes, which also is
  * the size of Ethernet link-layer options. */
-const struct ovs_nd_opt *nd_opt = *datap;
-int opt_len = nd_opt->nd_opt_len * ND_OPT_LEN;
+const struct ovs_nd_lla_opt *lla_opt = *datap;
+int opt_len = lla_opt->len * ND_LLA_OPT_LEN;
 
 if (!opt_len || opt_len > *sizep) {
 return;
@@ -410,17 +410,17 @@ parse_icmpv6(const void **datap, size_t *sizep, const 
struct icmp6_hdr *icmp,
 /* Store the link layer address if the appropriate option is
  * provided.  It is considered an error if the same link
  * layer option is specified twice. */
-if (nd_opt->nd_opt_type == ND_OPT_SOURCE_LINKADDR
+if (lla_opt->type == ND_OPT_SOURCE_LINKADDR
 && opt_len == 8) {
 if (OVS_LIKELY(eth_addr_is_zero(arp_buf[0]))) {
-arp_buf[0] = nd_opt->nd_opt_mac;
+arp_buf[0] = lla_opt->mac;
 } else {
 goto invalid;
 }
-} else if (nd_opt->nd_opt_type == ND_OPT_TARGET_LINKADDR
+} else if (lla_opt->type == ND_OPT_TARGET_LINKADDR
&& opt_len == 8) {
 if (OVS_LIKELY(eth_addr_is_zero(arp_buf[1]))) {
-arp_buf[1] = nd_opt->nd_opt_mac;
+arp_buf[1] = lla_opt->mac;
 } else {
 goto invalid;
 }
@@ -2274,7 +2274,7 @@ flow_compose_l4(struct dp_packet *p, const struct flow 
*flow)
 (icmp->icmp6_type == ND_NEIGHBOR_SOLICIT ||
  icmp->icmp6_type == ND_NEIGHBOR_ADVERT)) {
 struct in6_addr *nd_target;
-struct ovs_nd_opt *nd_opt;
+struct ovs_nd_lla_opt *lla_opt;
 
 l4_len += sizeof *nd_target;
 nd_target = dp_packet_put_zeros(p, sizeof *nd_target);
@@ -2282,17 +2282,17 @@ flow_compose_l4(struct dp_packet *p, const struct flow 
*flow)
 
 if (!eth_addr_is_zero(flow->arp_sha)) {
 l4_len += 8;
-nd_opt = dp_packet_put_zeros(p, 8);
-nd_opt->nd_opt_len = 1;
-nd_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
-nd_opt->nd_opt_mac = flow->arp_sha;
+lla_opt = dp_packet_put_zeros(p, 8);
+lla_opt->len = 1;
+lla_opt->type = ND_OPT_SOURCE_LINKADDR;
+lla_opt->mac = flow->arp_sha;
 }
 if (!eth_addr_is_zero(flow->arp_tha)) {
 l4_len += 8;
-nd_opt = dp_packet_put_zeros(p, 8);
-nd_opt->nd_opt_len = 1;
-nd_opt->nd_opt_type = ND_OPT_TARGET_LINKADDR;
-nd_opt->nd_opt_mac = flow->arp_tha;
+lla_opt = dp_packet_put_zeros(p, 8);
+lla_opt->len = 1;
+lla_opt->type = ND_OPT_TARGET_LINKADDR;
+lla_opt->mac = flow->arp_tha;
 }
 }
 }
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 65a6fcd..9c66de7 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -185,33 +185,33 @@ odp_set_nd(struct dp_packet *packet, const struct 
ovs_key_nd *key,
const struct ovs_key_nd *mask)
 {
 const struct ovs_nd_msg *ns = dp_packet_l4(packet);
-const struct ovs_nd_opt *nd_opt = dp_packet_get_nd_payload(packet);
+const struct ovs_nd_lla_opt *lla_opt = dp_packet_get_nd_payload(packet);
 
-if (OVS_LIKELY(ns && nd_opt)) {
+if (OVS_LIKELY(ns && lla_opt)) {
 int bytes_remain = dp_packet_l4_size(packet) - sizeof(*ns);
 ovs_be32 tgt_buf[4];
 struct eth_addr sll_buf = eth_addr_zero;
 struct eth_addr tll_buf = eth_addr_zero;
 
-while (bytes_remain >= ND_OPT_LEN && nd_opt->nd_opt_len != 0) {
-if (nd_opt->nd_opt_type == ND_OPT_SOURCE_LINKADDR
-&& nd_opt->nd_opt_len == 1) {
-sll_buf = nd_opt->nd_opt_mac;
+while (bytes_remain >= ND_LLA_OPT_LEN && lla_opt->len != 0) {
+

[ovs-dev] [PATCH v3, 0/7] ovn: add Router Solicitation responder support

[Zongkai LI]

This serial patches try to support Router Solicitation (RS) message responder,
which will reply Router Advertisement (RA) message, for OVN.

There will be logical flows in table ls_in_rs_rsp to work as RS responder, with:
 - match: ip6.dst == ff02::2 && nd_rs
   (nd_rs: icmp6.type == 133 && icmp6.code == 0 && ttl == 255)
 - action: nd_ra{put_nd_ra(64,0,10800,0,0);
 put_nd_ra_opt_sll(12:34:56:78:9a:bc);
 put_nd_ra_opt_mtu(1450);
 put_nd_ra_opt_prefix(64,192,10800,10800,fdad:a0f9:a012::);
 eth.src = 12:34:56:78:9a:bc;
 ip6.src = fe80::f334:56ff:fe78:9abc;
 outport = inport; flags.loopback = 1; output;};
   (nd_ra is a new action which stands for RS responder;
put_nd_ra is a new specific inner action for nd_ra to set cur_hop_limit,
  "Managed address configuration" and "Other configuration" flags, router
  lifetime, reachable time and retrans timer in RA packet;
put_nd_ra_opt_sll is a new specific inner action for nd_ra to append Source
  Link-layer Address Option for RA message with given ethernet address,
  such as 12:34:56:78:9a:bc;
put_nd_ra_opt_mtu is a new specific inner action for nd_ra to append MTU
  Option for RA message with given integer value, such as 1450;
put_nd_ra_opt_prefix is a new specific inner action for nd_ra to append
  Prefix Information Option for RA message with inner parameters. The inner
  parameters will set prefix length, on-link flag and autonomous
  address-configuration flag, valid lifetime, preferred lifetime, prefix
  in Prefix Information Option.
After a RA packet is composed, the left nested actions will make RA packet
transmitted back to the inport, where Router Solicitation (RS) packet comes.

To configure RA packet fields, such as "Managed address configuration" flag,
router_litefime, user can set Logical_Router_Port.ipv6_ra_config to do that,
e.g.
  ovn-nbctl add Logical_Router_Port lrp0 ipv6_ra_config managed_address="0" \
  other_config="0"

To configure Prefix Infromation Option fields, such as valid_lifetime, user
can insert records into new table Logical_Router_RA_Prefix_Config, and let
Logical_Router_Port.ipv6_ra_prefix_configs to refer them, e.g.
  ovn-nbctl -- --id=@d1 create Logical_Router_RA_Prefix_Config \
  prefix=\"fdad:1234:5678::/64\" options="\"valid_lifetime\"="21600" \
  \"preferred_lifetime\"="21600" \"autonomous_address\"="0"" \
  -- add Logical_Router_Port lrp0 ipv6_ra_prefix_configs @d1

Most fields in a RA packet, such as "Managed address configuration" and "Other
configuration" flags, router_lifetime, valid lifetime will have default values.
But for mtu value in MTU Option, there is no valid default value. User should
configure this value for each lrouter port.

v1 -> v2:
rebase on upstream
separete ovs_nd_opt rename into a single patch
separete lflow changing for RS packet forward in ovn-northd into a single patch
update is_nd to include RS and RA type, add is_nd_neighbor to inherit previous
  is_nd behavior.


v2 -> v3:
rebase on upstream
address comments from Justin, including:
  revert changing on is_nd and is_nd_neighbor brought in version 2
  update annotations in lib/packets.h for ovs_ra_msg, ovs_nd_prefix_opt,
ovs_nd_lla_opt, ovs_nd_mtu_opt.
  combine methods compse_nd_ra, packet_put_ra_sll_opt, packet_put_ra_mtu_opt in
version 2 into method compse_nd_ra_with_sll_mtu_opts.
  update pinctrl_handle_nd to fit new changes on compse_nd_ra_with_sll_mtu_opts.


Zongkai LI (7):
  packets: add compose_nd_ra
  lib: rename ovs_nd_opt to ovs_nd_lla_opt
  ovn: extend expr symbols for ND
  ovn-controller: add 'nd_ra' action
  ovn-controller: add 'put_nd_ra' and 'put_nd_opt_*' actions support
  ovn-northd: add Router Advertisement packet forward flow
  ovn-northd: add IPv6 RS responder support

 include/ovn/actions.h |  86 +-
 lib/flow.c|  30 ++---
 lib/odp-execute.c |  22 ++--
 lib/packets.c | 146 ++-
 lib/packets.h |  76 ++--
 ovn/controller/pinctrl.c  | 235 +
 ovn/lib/actions.c | 290 +-
 ovn/lib/logical-fields.c  |  18 ++-
 ovn/northd/ovn-northd.c   | 166 +-
 ovn/ovn-nb.ovsschema  |  21 +++-
 ovn/ovn-nb.xml| 103 
 ovn/ovn-sb.xml|  84 +-
 ovn/utilities/ovn-nbctl.c |   5 +
 ovn/utilities/ovn-trace.c |  23 ++--
 tests/ovn.at  | 157 -
 15 files changed, 1354 insertions(+), 108 deletions(-)

-- 
2.7.4

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

[ovs-dev] [PATCH v3 1/7] packets: add compose_nd_ra

[Zongkai LI]

This patch introduces methods to compose a Router Advertisement (RA) packet,
introduces flags for RA. RA packet composed structures against specification
in RFC4861.

Caller can use compse_nd_ra_with_sll_mtu_opts to compose a RA packet with
Source Link-layer Address Option and MTU Option.

Caller can use packet_put_ra_prefix_opt to append a Prefix Information Option
to a RA packet.

Signed-off-by: Zongkai LI 
---
 lib/packets.c | 88 +++
 lib/packets.h | 59 +++
 2 files changed, 147 insertions(+)

diff --git a/lib/packets.c b/lib/packets.c
index 990c407..7557e76 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -35,6 +35,7 @@
 
 const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT;
 const struct in6_addr in6addr_all_hosts = IN6ADDR_ALL_HOSTS_INIT;
+const struct in6_addr in6addr_all_routers = IN6ADDR_ALL_ROUTERS_INIT;
 
 struct in6_addr
 flow_tnl_dst(const struct flow_tnl *tnl)
@@ -1439,6 +1440,93 @@ compose_nd_na(struct dp_packet *b,
   ND_MSG_LEN + 
ND_OPT_LEN));
 }
 
+/* Compose an IPv6 Neighbor Discovery Router Advertisement message with
+ * Source Link-layer Address Option and MTU Option.
+ * Caller can call packet_put_ra_prefix_opt to append Prefix Information
+ * Options to composed messags in 'b'. */
+void
+compose_nd_ra_with_sll_mtu_opts(struct dp_packet *b,
+const struct eth_addr eth_src,
+const struct eth_addr eth_dst,
+const struct in6_addr *ipv6_src,
+const struct in6_addr *ipv6_dst,
+uint8_t cur_hop_limit, uint8_t mo_flags,
+ovs_be16 router_lt, ovs_be32 reachable_time,
+ovs_be32 retrans_timer, ovs_be32 mtu)
+{
+struct ovs_ra_msg *ra;
+struct ovs_nd_mtu_opt *mtu_opt;
+struct ovs_nd_opt *lla_opt;
+uint32_t icmp_csum;
+
+/* Don't compose Router Advertisement packet with MTU Option if mtu
+ * value is 0. */
+bool with_mtu = mtu != 0;
+size_t mtu_opt_len = with_mtu ? ND_MTU_OPT_LEN : 0;
+
+eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+ra = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src, ipv6_dst, 0, 0, 255,
+  RA_MSG_LEN + ND_OPT_LEN + mtu_opt_len);
+
+ra->icmph.icmp6_type = ND_ROUTER_ADVERT;
+ra->icmph.icmp6_code = 0;
+ra->cur_hop_limit = cur_hop_limit;
+ra->mo_flags = mo_flags;
+ra->router_lifetime = router_lt;
+ra->reachable_time = reachable_time;
+ra->retrans_timer = retrans_timer;
+
+lla_opt = >options[0];
+lla_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
+lla_opt->nd_opt_len = 1;
+lla_opt->nd_opt_mac = eth_src;
+
+if (with_mtu) {
+/* ovs_nd_mtu_opt has the same size with ovs_nd_opt. */
+mtu_opt = (struct ovs_nd_mtu_opt *)(lla_opt + 1);
+mtu_opt->type = ND_OPT_MTU;
+mtu_opt->len = 1;
+mtu_opt->reserved = 0;
+mtu_opt->mtu = mtu;
+}
+
+ra->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+ra->icmph.icmp6_cksum = csum_finish(csum_continue(
+icmp_csum, ra, RA_MSG_LEN + ND_OPT_LEN + mtu_opt_len));
+}
+
+/* Append an IPv6 Neighbor Discovery Prefix Information option to a
+ * Router Advertisement message. */
+void
+packet_put_ra_prefix_opt(struct dp_packet *b,
+ uint8_t plen, uint8_t la_flags, ovs_be32 
valid_lifetime,
+ ovs_be32 preferred_lifetime, const ovs_be128 prefix)
+{
+size_t prev_l4_size = dp_packet_l4_size(b);
+struct ip6_hdr *nh = dp_packet_l3(b);
+nh->ip6_plen = htons(prev_l4_size + ND_PREFIX_OPT_LEN);
+
+struct ovs_ra_msg *ra = dp_packet_l4(b);
+struct ovs_nd_prefix_opt *prefix_opt;
+uint32_t icmp_csum;
+
+prefix_opt = dp_packet_put_uninit(b, sizeof(struct ovs_nd_prefix_opt));
+prefix_opt->type = ND_OPT_PREFIX_INFORMATION;
+prefix_opt->len = 4;
+prefix_opt->prefix_len = plen;
+prefix_opt->la_flags = la_flags;
+prefix_opt->valid_lifetime = valid_lifetime;
+prefix_opt->preferred_lifetime = preferred_lifetime;
+prefix_opt->reserved = 0;
+memcpy(prefix_opt->prefix.be32, prefix.be32, sizeof(ovs_be32[4]));
+
+ra->icmph.icmp6_cksum = 0;
+icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+ra->icmph.icmp6_cksum = csum_finish(csum_continue(
+icmp_csum, ra, prev_l4_size + ND_PREFIX_OPT_LEN));
+}
+
 uint32_t
 packet_csum_pseudoheader(const struct ip_header *ip)
 {
diff --git a/lib/packets.h b/lib/packets.h
index 21bd35c..337ddf6 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -851,6 +851,33 @@ struct ovs_nd_opt {
 };
 BUILD_ASSERT_DECL(ND_OPT_LEN == sizeof(struct ovs_nd_opt));
 
+/* Neighbor Discovery option: Prefix Information. */
+#define ND_PREFIX_OPT_LEN 32

Re: [ovs-dev] [PATCH 0/9 md->rst] Convert most INSTALL guides

[Russell Bryant]

On Thu, Oct 13, 2016 at 9:32 AM, Ben Pfaff  wrote:

> On Thu, Oct 13, 2016 at 09:10:58AM -0400, Russell Bryant wrote:
> > On Thu, Oct 13, 2016 at 6:00 AM, Stephen Finucane 
> wrote:
> >
> > > On 2016-10-12 20:20, Russell Bryant wrote:
> > >
> > >> On Sat, Oct 8, 2016 at 12:30 PM, Stephen Finucane 
> > >> wrote:
> > >>
> > >> I'm going to dripfeed the conversion patches to avoid overloading
> > >>> peoples (there are ~25 of them so far). This is the first batch.
> > >>>
> > >>> Stephen Finucane (9):
> > >>> dist-docs: Add support for rST
> > >>> doc: Convert INSTALL to rST
> > >>> doc: Convert INSTALL.DPDK to rST
> > >>> doc: Convert INSTALL.Debian to rST
> > >>> doc: Convert INSTALL.Docker to rST
> > >>> doc: Convert INSTALL.Windows to rST
> > >>> doc: Convert INSTALL.userspace to rST
> > >>> doc: Convert INSTALL.XenServer to rST
> > >>> doc: Convert INSTALL.KVM to rST
> > >>>
> > >>
> > >> Do you have the end result published anywhere?  It would be nice to
> > >> see the before and after before diving into the detailed reviews.
> > >>
> > >
> > > I visually inspected them myself but I haven't published them anywhere.
> > > You can run 'make dist-docs' and visually compare the resulting HTML
> files
> > > to what's on openvswitch.org (remembering that the reStructuredText
> > > versions should actually be more complete due to latent bugs in the
> > > Markdown versions). I could find somewhere to publish them if it would
> help
> > > but I don't know if it's really necessary :)
> > >
> > > To be honest though, I don't know how detailed we need to be right now.
> > > IMO if the functional aspects of these patches work (i.e. the
> 'dist-docs'
> > > target generates sane HTML for all rST files) and we haven't lost any
> of
> > > the raw information then any stylistic issues I may have introduced
> can be
> > > fixed in follow up patches. Most of these docs are going to get
> shuffled
> > > around or rewritten when we start moving to sphinx anyway, so we'll
> have
> > > the chance to really review the content itself then.
> > >
> >
> > OK, sounds good.  I also see the previous conversation about this work
> > now.  Sorry about that.  I'm on board with all of this.
> >
> > Ben, I'm happy to review all of the patches for this conversion if you'd
> > like.
>
> That would be really valuable, thanks Russell!
>

No problem, consider it delegated.

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

Re: [ovs-dev] ovn: Improving southbound database security

[Russell Bryant]

I should have also noted who was already looking at each of these items ...

On Thu, Oct 13, 2016 at 10:02 AM, Numan Siddique 
wrote:

> We may have to add one more item in the task breakdown list. Please see
> below
>
>
> On Wed, Oct 12, 2016 at 11:21 PM, Russell Bryant  wrote:
>
>> Hello, I'm back to looking at southbound database security concerns in
>> OVN.  A previous thread discussing approaches was here:
>>
>> http://openvswitch.org/pipermail/dev/2016-August/078106.html
>>
>> I'm now working with a few others on implementing a proposed solution.
>> The overview is that we'd like to make ovn-controller a read-only client of
>> the southbound database.
>>
>> The task breakdown is:
>>
>> 1) Add support to ovsdb-server for read-only remotes.  The port reachable
>> by ovn-controller would only accept read-only connections.
>>
>
Lance has an RFC patch posted for this.


>
>> 2) Remove support from ovn-controller for automatically creating chassis
>> and encap records.  Document this as an administrative step for adding a
>> new chassis to the system, instead.
>>
>
Numan has a patch for this, not yet posted as I wanted to post 2-4 together.


> 3) Remove support from ovn-controller for setting the chassis column of
>> Port_Binding records.  Instead, have this handled by ovn-northd based on
>> binding instructions given in the northbound database.
>>
>> As a nice side effect, this helps solve one of the difficulties with live
>> migration (two chassis fighting to own a port while the migration is in
>> progress).  Instead, we would update OVN when we know the migration is
>> complete.
>>
>> I was originally thinking we may need an upgrade utility to help existing
>> environments, but I think ovn-northd can handle this automatically.
>>
>> For the northbound database, I was thinking of adding a new option for
>> logical ports called "chassis" with a value of the hostname of the chassis
>> the port should be bound to.
>>
>
I'm working on this.


> 4) Remove use of MAC_bindings table from ovn-controller.  Replace it with
>> a local cache, instead.  I'm proposing just keeping it in memory in
>> ovn-controller, but we could also make use of the openvswitch db.
>>
>> One detail I haven't fully thought through: what should happen to the
>> MAC_Bindings table?  Dropping the table seems not backwards compatible and
>> would break a rolling upgrade scenario.  Should we leave it as unused for
>> one release, and then remove it in the next release?  More generally, I
>> think we need to document our upgrade strategy and related rules for
>> database schema changes.
>>
>
Babu has a patch for this.


>
>>
> ​5) Remove support from ovn-controller updating the 'Chassis.hv_cfg'
> column​ and handle the side effect in "--wait=hv" in ovn-nbctl.
>

Ah yes, I missed this.

My initial thought is that since this is primarily useful for testing, we
could accept this as a limitation of the feature and it just wouldn't work
if you didn't expose write access to ovn-controller.  A huge downside to
this is that if we're not enforcing read-only access in our test suite,
we're more likely to have regressions.

Any other ideas?

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

Re: [ovs-dev] [PATCH 0/9 md->rst] Convert most INSTALL guides

[Russell Bryant]

On Thu, Oct 13, 2016 at 1:46 PM, Stephen Finucane  wrote:

>
> ..and sorry for not setting the scene: I forget how active this mailing
> list is :)
>
> Let me know if you've any questions.
>

No problem.  I think it's more my fault.  I've been out a lot lately so
there's a bunch I haven't looked at.

Thanks a lot for working on this!!

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

Re: [ovs-dev] [PATCH v2 01/12] dpcls: Use 32 packet batches for lookups.

[Daniele Di Proietto]

I think this looks good, thanks.

I have a few comments inline to simplify the code a little bit more.

2016-10-13 2:58 GMT-07:00 Bhanuprakash Bodireddy <
bhanuprakash.bodire...@intel.com>:

> This patch increases the number of packets processed in a batch during a
> lookup from 16 to 32. Processing batches of 32 packets improves
> performance and also one of the internal loops can be avoided here.
>
> Signed-off-by: Antonio Fischetti 
> Co-authored-by: Bhanuprakash Bodireddy 
> ---
>  lib/dpif-netdev.c | 110 +++---
> 
>  1 file changed, 47 insertions(+), 63 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index eb9f764..3b3556a 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4985,23 +4985,21 @@ dpcls_lookup(struct dpcls *cls, const struct
> netdev_flow_key keys[],
>   int *num_lookups_p)
>  {
>  /* The received 'cnt' miniflows are the search-keys that will be
> processed
> - * in batches of 16 elements.  N_MAPS will contain the number of these
> - * 16-elements batches.  i.e. for 'cnt' = 32, N_MAPS will be 2.  The
> batch
> - * size 16 was experimentally found faster than 8 or 32. */
> -typedef uint16_t map_type;
> + * to find a matching entry into the available subtables.
> + * The number of bits in map_type is equal to NETDEV_MAX_BURST. */
> +typedef uint32_t map_type;
>  #define MAP_BITS (sizeof(map_type) * CHAR_BIT)
> +BUILD_ASSERT_DECL(MAP_BITS >= NETDEV_MAX_BURST);
>
> -#if !defined(__CHECKER__) && !defined(_WIN32)
> -const int N_MAPS = DIV_ROUND_UP(cnt, MAP_BITS);
> -#else
> -enum { N_MAPS = DIV_ROUND_UP(NETDEV_MAX_BURST, MAP_BITS) };
> -#endif
> -map_type maps[N_MAPS];
>  struct dpcls_subtable *subtable;
>
> -memset(maps, 0xff, sizeof maps);
> -if (cnt % MAP_BITS) {
> -maps[N_MAPS - 1] >>= MAP_BITS - cnt % MAP_BITS; /* Clear extra
> bits. */
> +map_type keys_map = 0x;
>

Can we use TYPE_MAXIMUM(map_type) instead of 0x?


> +map_type found_map;
> +uint32_t hashes[MAP_BITS];
> +const struct cmap_node *nodes[MAP_BITS];
>

The above three variables can be moved inside the look


> +
> +if (OVS_UNLIKELY(cnt != NETDEV_MAX_BURST)) {
>

I'm not sure the OVS_UNLIKELY is important here.

I'll leave it up to you.


> +keys_map >>= NETDEV_MAX_BURST - cnt; /* Clear extra bits. */
>  }
>  memset(rules, 0, cnt * sizeof *rules);
>
> @@ -5017,59 +5015,45 @@ dpcls_lookup(struct dpcls *cls, const struct
> netdev_flow_key keys[],
>  PVECTOR_FOR_EACH (subtable, >subtables) {
>  const struct netdev_flow_key *mkeys = keys;
>  struct dpcls_rule **mrules = rules;
>

Are the two variables above still necessary?  I think they're not


> -map_type remains = 0;
> -int m;
> -
> -BUILD_ASSERT_DECL(sizeof remains == sizeof *maps);
> -
> -/* Loops on each batch of 16 search-keys. */
> -for (m = 0; m < N_MAPS; m++, mkeys += MAP_BITS, mrules +=
> MAP_BITS) {
> -uint32_t hashes[MAP_BITS];
> -const struct cmap_node *nodes[MAP_BITS];
> -unsigned long map = maps[m];
> -int i;
> -
> -if (!map) {
> -continue; /* Skip empty maps. */
> -}
> -
> -/* Compute hashes for the remaining keys.  Each search-key is
> - * masked with the subtable's mask to avoid hashing the
> wildcarded
> - * bits. */
> -ULLONG_FOR_EACH_1(i, map) {
> -hashes[i] = netdev_flow_key_hash_in_mask([i],
> - >mask);
> -}
> -/* Lookup. */
> -map = cmap_find_batch(>rules, map, hashes, nodes);
> -/* Check results.  When the i-th bit of map is set, it means
> that a
> - * set of nodes with a matching hash value was found for the
> i-th
> - * search-key.  Due to possible hash collisions we need to
> check
> - * which of the found rules, if any, really matches our masked
> - * search-key. */
> -ULLONG_FOR_EACH_1(i, map) {
> -struct dpcls_rule *rule;
> -
> -CMAP_NODE_FOR_EACH (rule, cmap_node, nodes[i]) {
> -if (OVS_LIKELY(dpcls_rule_matches_key(rule,
> [i]))) {
> -mrules[i] = rule;
> -/* Even at 20 Mpps the 32-bit hit_cnt cannot wrap
> - * within one second optimization interval  */
> -subtable->hit_cnt++;
> -lookups_match += subtable_pos;
> -goto next;
> -}
> +int i;
>



> +found_map = keys_map;
>

I believe the above to be not necessary.  We can pass keys_map directly to
cmap_find_batch()


> +
> +   

Re: [ovs-dev] [PATCH v2 07/12] dpif-netdev: Cache align netdev_flow_keys.

[Daniele Di Proietto]

Acked-by: Daniele Di Proietto 

2016-10-13 2:58 GMT-07:00 Bhanuprakash Bodireddy <
bhanuprakash.bodire...@intel.com>:

> Aligning the 'keys' array seems to have positive performance impact.
>
> Signed-off-by: Bhanuprakash Bodireddy 
> Co-authored-by: Antonio Fischetti 
> ---
>  lib/dpif-netdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 84aa570..93ea49d 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4166,7 +4166,7 @@ dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
>  /* Sparse or MSVC doesn't like variable length array. */
>  enum { PKT_ARRAY_SIZE = NETDEV_MAX_BURST };
>  #endif
> -struct netdev_flow_key keys[PKT_ARRAY_SIZE];
> +struct netdev_flow_key keys[PKT_ARRAY_SIZE]
> OVS_ALIGNED_VAR(CACHE_LINE_SIZE);
>  struct packet_batch_per_flow batches[PKT_ARRAY_SIZE];
>  long long now = time_msec();
>  size_t newcnt, n_batches, i;
> --
> 2.4.11
>
> ___
> 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 1/1] doc: Update DPDK pdump documentation

[Daniele Di Proietto]

2016-10-06 3:25 GMT-07:00 Mark Kavanagh :

> The DPDK pdump sample app was renamed from 'dpdk_pdump' to
> 'dpdk-pdump'. Update references to same within
> INSTALL.DPDK-ADVANCED.md.
>
> Add an additional sample command line that shows how to capture all
> traffic traversing an interface within a single pcap file - a useful
> tool for debugging DPDK-related issues.
>
> Signed-off-by: Mark Kavanagh 
>

Thanks, applied to master and branch-2.6
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 05/12] dpif-netdev: Add comments to dp_netdev_input__().

[Daniele Di Proietto]

Acked-by: Daniele Di Proietto 

2016-10-13 2:58 GMT-07:00 Bhanuprakash Bodireddy <
bhanuprakash.bodire...@intel.com>:

> Add comments in dp_netdev_input__() to explain the reason behind
> clearing the flow batches before packet_batch_execute().
>
> Signed-off-by: Bhanuprakash Bodireddy 
> Co-authored-by: Antonio Fischetti 
> ---
>  lib/dpif-netdev.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 3b3556a..84aa570 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4182,6 +4182,15 @@ dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
>  fast_path_processing(pmd, packets, keys, batches, _batches,
> in_port, now);
>  }
>
> +/* All the flow batches need to be reset before any call to
> + * packet_batch_per_flow_execute() as it could potentially trigger
> + * recirculation. When a packet matching flow ‘j’ happens to be
> + * recirculated, the nested call to dp_netdev_input__() could
> potentially
> + * classify the packet as matching another flow - say 'k'. It could
> happen
> + * that in the previous call to dp_netdev_input__() that same flow
> 'k' had
> + * already its own batches[k] still waiting to be served.  So if its
> + * ‘batch’ member is not reset, the recirculated packet would be
> wrongly
> + * appended to batches[k] of the 1st call to dp_netdev_input__(). */
>  for (i = 0; i < n_batches; i++) {
>  batches[i].flow->batch = NULL;
>  }
> --
> 2.4.11
>
> ___
> 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 v2 00/12] Improve performance of OVS-DPDK classifier.

[Daniele Di Proietto]

Thanks for v2.

I had a few extra comments on patch one

Sorry for nitpicking on this and sorry I wasn't clear enough on the signoff
requirements.
From CONTRIBUTING.md:

Co-authored-by: Author Name 

Git can only record a single person as the author of a given
patch.  In the rare event that a patch has multiple authors,
one must be given the credit in Git and the others must be
credited via Co-authored-by: tags.  (All co-authors must also
sign off.)

There should be one signoff for the main author, plus one extra for
each coauthor. I always mix this up myself, but it would help if you
could send another version with the amended tags.

Daniele

2016-10-13 2:58 GMT-07:00 Bhanuprakash Bodireddy <
bhanuprakash.bodire...@intel.com>:

> This patch series is aimed at improving the performance of OVS-DPDK
> dpcls.
>
> With few thousand flows installed, the EMC becomes inefficient due
> to thrashing and the bottleneck moves to the dpcls. In EMC disabled
> case, through VTune we found that significant performance degradation
> is due to LLC thrashing, memory latency, machine clears and expensive
> hash computation.
>
> This first patch-set improves the dpcls performance by 15% (+1 Mpps)
> when EMC is disabled and OVS-DPDK built with CFLAGS="-O2 -g".
>
> Bhanuprakash Bodireddy (12):
>   dpcls: Use 32 packet batches for lookups.
> Comment: ~120k performance throughput improvement.
>
>   flow: Add comments to mf_get_next_in_map().
> Comment: Add comments to the function.
>
>   flow: Skip invoking expensive count_1bits() with zero input.
> Comment: ~630k performance throughput improvement.
>
>   hash: Skip invoking mhash_add__() with zero input.
> Comment: ~150k performance throughput improvement.
>
>   dpif-netdev: Add comments to dp_netdev_input__().
> Comment: Add comments to the function.
>
>   cmap: Remove prefetching in cmap_find_batch().
> Comment: ~39k performance throughput improvement.
>
>   dpif-netdev: Cache align netdev_flow_keys.
> Comment: ~170k performance throughput improvement in EMC enabled
> case.
>
>   dpif-netdev: Reorder elements in dp_netdev_port structure.
>   dpif: Reorder elements in dpif_upcall structure.
>   ovsdb: Reorder elements in ovsdb_table_schema structure.
>   netlink-socket: Reorder elements in nl_dump structure.
>   timeval: Reorder elements in clock structure.
> Comment: Reorder memeber variables of the structures to reduce
>  pad bytes and there by the memory footprint.
>
>  lib/cmap.c   |   8 +---
>  lib/dpif-netdev.c| 123 --
> -
>  lib/dpif.h   |   5 ++-
>  lib/flow.h   |  47 +++-
>  lib/hash.h   |   5 +++
>  lib/netlink-socket.h |   6 +--
>  lib/timeval.c|   4 +-
>  ovsdb/table.h|   4 +-
>  8 files changed, 113 insertions(+), 89 deletions(-)
>
> --
> 2.4.11
>
> ___
> 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] The Patch netdev-dpdk: add TSO support for vhost-user ports

[Michael Qiu]

Hi, Mark


I have reviewed your RFC patch, I found one place could have issue, see 
below:



+if (v > ETHER_MAX_LEN) {
+b->mbuf.data_len =
+(uint16_t)(b->mbuf.buf_len - b->mbuf.data_off);
+} else {
+b->mbuf.data_len = (uint16_t)v;
+}
+

Here, what will happen if v > ETHER_MAX_LEN but v < (b->mbuf.buf_len - 
b->mbuf.data_off) ?


The driver will be get wrong data lenth in my option.

probably, below is right :
if (v > (b->mbuf.buf_len - b->mbuf.data_off))
b->mbuf.data_len = (uint16_t)(b->mbuf.buf_len - b->mbuf.data_off);


Thanks,
Michael

在 2016/10/11 22:34, Kavanagh, Mark B 写道:

Hi,  all


This patch is very important for users want to improve the performance
of the large packets.


But you know, in data center, lots of networks using vxlan, so if it
supports vxlan, then it will be very useful.


Would you guys has a plan to support it? I would like to help test it,
or work together on it?

Hi Michael,

Some work has already been done to enable TSO over VxLAN, but it was blocked 
until recently, on account of gaps in the DPKD i40e PMD's support for tunnel 
offload.

We believe that those gaps have now been addressed; our current focus is on 
ironing out the issues in TSO support over flat and VLAN networks - once that's 
done, we plan to resume work on VxLAN support (and GRE, IPinIP).

Hope this helps,
Mark
 


--
Thanks,
Michael



--
Thanks,
Michael


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

Re: [ovs-dev] [PATCH ovs RFC 0/9] Introducing HW offload support for openvswitch

[Simon Horman]

On Wed, Oct 12, 2016 at 01:36:44PM -0700, Pravin Shelar wrote:
> Sorry for jumping in a bit late. I have couple of high level comments below.
> 
> On Thu, Oct 6, 2016 at 10:10 AM, Rony Efraim  wrote:
> > From: Joe Stringer [mailto:j...@ovn.org]  Sent: Thursday, October 06, 2016 
> > 5:06 AM

...

> >> Other considerations
> >> * Is tc flower filter setup rate and stats dump fast enough? How does it
> >> compare to existing kernel datapath flow setup rate? Multiple threads 
> >> inserting
> >> at once? How many filters can be dumped per second?
> >> etc.
> > [RONY] we will test it, and will try to improve the TC if it will be needed
> >
> I think there are two part in flow offloading.
> 1. Time spent to Add the flow to TC.
> 2. Time spent on pushing the flow to hardware.
> 
> It would be interesting to know which one is dominant in this case.

I agree that the problem should be quantified but I expect the answer will
depend on the hardware in use. And I entirely expect there are worthwhile
gains to be had on the software side.

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

[ovs-dev] [PATCH v2 00/12] Improve performance of OVS-DPDK classifier.

[Bhanuprakash Bodireddy]

This patch series is aimed at improving the performance of OVS-DPDK
dpcls.

With few thousand flows installed, the EMC becomes inefficient due
to thrashing and the bottleneck moves to the dpcls. In EMC disabled
case, through VTune we found that significant performance degradation
is due to LLC thrashing, memory latency, machine clears and expensive
hash computation.

This first patch-set improves the dpcls performance by 15% (+1 Mpps)
when EMC is disabled and OVS-DPDK built with CFLAGS="-O2 -g".

Bhanuprakash Bodireddy (12):
  dpcls: Use 32 packet batches for lookups.
Comment: ~120k performance throughput improvement.

  flow: Add comments to mf_get_next_in_map().
Comment: Add comments to the function.

  flow: Skip invoking expensive count_1bits() with zero input.
Comment: ~630k performance throughput improvement.

  hash: Skip invoking mhash_add__() with zero input.
Comment: ~150k performance throughput improvement.

  dpif-netdev: Add comments to dp_netdev_input__().
Comment: Add comments to the function.

  cmap: Remove prefetching in cmap_find_batch().
Comment: ~39k performance throughput improvement.

  dpif-netdev: Cache align netdev_flow_keys.
Comment: ~170k performance throughput improvement in EMC enabled case.

  dpif-netdev: Reorder elements in dp_netdev_port structure.
  dpif: Reorder elements in dpif_upcall structure.
  ovsdb: Reorder elements in ovsdb_table_schema structure.
  netlink-socket: Reorder elements in nl_dump structure.
  timeval: Reorder elements in clock structure.
Comment: Reorder memeber variables of the structures to reduce
 pad bytes and there by the memory footprint.

 lib/cmap.c   |   8 +---
 lib/dpif-netdev.c| 123 ---
 lib/dpif.h   |   5 ++-
 lib/flow.h   |  47 +++-
 lib/hash.h   |   5 +++
 lib/netlink-socket.h |   6 +--
 lib/timeval.c|   4 +-
 ovsdb/table.h|   4 +-
 8 files changed, 113 insertions(+), 89 deletions(-)

-- 
2.4.11

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

[ovs-dev] [PATCH v2 02/12] flow: Add comments to mf_get_next_in_map().

[Bhanuprakash Bodireddy]

This patch adds comments to mf_get_next_in_map() to make it more
comprehensible.

Signed-off-by: Jarno Rajahalme 
Acked-by: Bhanuprakash Bodireddy 
Acked-by: Antonio Fischetti 
---
 lib/flow.h | 32 +++-
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/lib/flow.h b/lib/flow.h
index ea24e28..5a14941 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -564,12 +564,27 @@ flow_values_get_next_in_maps(struct 
flow_for_each_in_maps_aux *aux,
  flow_values_get_next_in_maps(__, &(VALUE));)
 
 struct mf_for_each_in_map_aux {
-size_t unit;
-struct flowmap fmap;
-struct flowmap map;
-const uint64_t *values;
+size_t unit; /* Current 64-bit unit of the flowmaps
+   being processed. */
+struct flowmap fmap; /* Remaining 1-bits corresponding to the
+   64-bit words in ‘values’ */
+struct flowmap map;  /* Remaining 1-bits corresponding to the
+   64-bit words of interest. */
+const uint64_t *values;  /* 64-bit words corresponding to the
+   1-bits in ‘fmap’. */
 };
 
+/* Get the data from ‘aux->values’ corresponding to the next lowest 1-bit
+ * in ‘aux->map’, given that ‘aux->values’ points to an array of 64-bit
+ * words corresponding to the 1-bits in ‘aux->fmap’, starting from the
+ * rightmost 1-bit.
+ *
+ * Returns ’true’ if the traversal is incomplete, ‘false’ otherwise.
+ * ‘aux’ is prepared for the next iteration after each call.
+ *
+ * This is used to traverse through, for example, the values in a miniflow
+ * representation of a flow key selected by non-zero 64-bit words in a
+ * corresponding subtable mask. */
 static inline bool
 mf_get_next_in_map(struct mf_for_each_in_map_aux *aux,
uint64_t *value)
@@ -577,8 +592,10 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux,
 map_t *map, *fmap;
 map_t rm1bit;
 
+/* Skip empty map units. */
 while (OVS_UNLIKELY(!*(map = >map.bits[aux->unit]))) {
-/* Skip remaining data in the previous unit. */
+/* Skip remaining data in the current unit before advancing
+ * to the next. */
 aux->values += count_1bits(aux->fmap.bits[aux->unit]);
 if (++aux->unit == FLOWMAP_UNITS) {
 return false;
@@ -589,7 +606,12 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux,
 *map -= rm1bit;
 fmap = >fmap.bits[aux->unit];
 
+/* If the rightmost 1-bit found from the current unit in ‘aux->map’
+ * (‘rm1bit’) is also present in ‘aux->fmap’, store the corresponding
+ * value from ‘aux->values’ to ‘*value', otherwise store 0. */
 if (OVS_LIKELY(*fmap & rm1bit)) {
+/* Skip all 64-bit words in ‘values’ preceding the one corresponding
+ * to ‘rm1bit’. */
 map_t trash = *fmap & (rm1bit - 1);
 
 *fmap -= trash;
-- 
2.4.11

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

[ovs-dev] [PATCH v2 03/12] flow: Skip invoking expensive count_1bits() with zero input.

[Bhanuprakash Bodireddy]

This patch checks if trash is non-zero and only then resets the flowmap
bit and increment the pointer by set bits as found in trash.

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
Acked-by: Jarno Rajahalme 
---
 lib/flow.h | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/flow.h b/lib/flow.h
index 5a14941..74e75d6 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -614,11 +614,16 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux,
  * to ‘rm1bit’. */
 map_t trash = *fmap & (rm1bit - 1);
 
-*fmap -= trash;
-/* count_1bits() is fast for systems where speed matters (e.g.,
- * DPDK), so we don't try avoid using it.
- * Advance 'aux->values' to point to the value for 'rm1bit'. */
-aux->values += count_1bits(trash);
+/* Avoid resetting 'fmap' and calling count_1bits() when trash is
+ * zero. */
+if (trash) {
+*fmap -= trash;
+/* count_1bits() is fast for systems where speed matters (e.g.,
+ * DPDK), so we don't try avoid using it.
+ * Advance 'aux->values' to point to the value for 'rm1bit' only.
+ */
+aux->values += count_1bits(trash);
+}
 
 *value = *aux->values;
 } else {
-- 
2.4.11

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

Re: [ovs-dev] [PATCH 0/9 md->rst] Convert most INSTALL guides

[Stephen Finucane]

On 2016-10-12 20:20, Russell Bryant wrote:

On Sat, Oct 8, 2016 at 12:30 PM, Stephen Finucane 
wrote:


I'm going to dripfeed the conversion patches to avoid overloading
peoples (there are ~25 of them so far). This is the first batch.

Stephen Finucane (9):
dist-docs: Add support for rST
doc: Convert INSTALL to rST
doc: Convert INSTALL.DPDK to rST
doc: Convert INSTALL.Debian to rST
doc: Convert INSTALL.Docker to rST
doc: Convert INSTALL.Windows to rST
doc: Convert INSTALL.userspace to rST
doc: Convert INSTALL.XenServer to rST
doc: Convert INSTALL.KVM to rST


Do you have the end result published anywhere?  It would be nice to
see the before and after before diving into the detailed reviews.


I visually inspected them myself but I haven't published them anywhere. 
You can run 'make dist-docs' and visually compare the resulting HTML 
files to what's on openvswitch.org (remembering that the 
reStructuredText versions should actually be more complete due to latent 
bugs in the Markdown versions). I could find somewhere to publish them 
if it would help but I don't know if it's really necessary :)


To be honest though, I don't know how detailed we need to be right now. 
IMO if the functional aspects of these patches work (i.e. the 
'dist-docs' target generates sane HTML for all rST files) and we haven't 
lost any of the raw information then any stylistic issues I may have 
introduced can be fixed in follow up patches. Most of these docs are 
going to get shuffled around or rewritten when we start moving to sphinx 
anyway, so we'll have the chance to really review the content itself 
then.


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

[ovs-dev] [PATCH v2 06/12] cmap: Remove prefetching in cmap_find_batch().

[Bhanuprakash Bodireddy]

prefetching the data in to the caches isn't improving the performance in
cmap_find_batch(). Moreover its found that there is slight improvement
in performance with out prefetching.

This patch removes prefetching from cmap_find_batch().

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
---
 lib/cmap.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/lib/cmap.c b/lib/cmap.c
index 8c7312d..8097b56 100644
--- a/lib/cmap.c
+++ b/lib/cmap.c
@@ -393,11 +393,10 @@ cmap_find_batch(const struct cmap *cmap, unsigned long 
map,
 const struct cmap_bucket *b2s[sizeof map * CHAR_BIT];
 uint32_t c1s[sizeof map * CHAR_BIT];
 
-/* Compute hashes and prefetch 1st buckets. */
+/* Compute hashes. */
 ULLONG_FOR_EACH_1(i, map) {
 h1s[i] = rehash(impl, hashes[i]);
 b1s[i] = >buckets[h1s[i] & impl->mask];
-OVS_PREFETCH(b1s[i]);
 }
 /* Lookups, Round 1. Only look up at the first bucket. */
 ULLONG_FOR_EACH_1(i, map) {
@@ -411,15 +410,13 @@ cmap_find_batch(const struct cmap *cmap, unsigned long 
map,
 } while (OVS_UNLIKELY(counter_changed(b1, c1)));
 
 if (!node) {
-/* Not found (yet); Prefetch the 2nd bucket. */
+/* Not found (yet). */
 b2s[i] = >buckets[other_hash(h1s[i]) & impl->mask];
-OVS_PREFETCH(b2s[i]);
 c1s[i] = c1; /* We may need to check this after Round 2. */
 continue;
 }
 /* Found. */
 ULLONG_SET0(map, i); /* Ignore this on round 2. */
-OVS_PREFETCH(node);
 nodes[i] = node;
 }
 /* Round 2. Look into the 2nd bucket, if needed. */
@@ -453,7 +450,6 @@ cmap_find_batch(const struct cmap *cmap, unsigned long map,
 continue;
 }
 found:
-OVS_PREFETCH(node);
 nodes[i] = node;
 }
 return result;
-- 
2.4.11

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

[ovs-dev] [PATCH v2 07/12] dpif-netdev: Cache align netdev_flow_keys.

[Bhanuprakash Bodireddy]

Aligning the 'keys' array seems to have positive performance impact.

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
---
 lib/dpif-netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 84aa570..93ea49d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4166,7 +4166,7 @@ dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
 /* Sparse or MSVC doesn't like variable length array. */
 enum { PKT_ARRAY_SIZE = NETDEV_MAX_BURST };
 #endif
-struct netdev_flow_key keys[PKT_ARRAY_SIZE];
+struct netdev_flow_key keys[PKT_ARRAY_SIZE] 
OVS_ALIGNED_VAR(CACHE_LINE_SIZE);
 struct packet_batch_per_flow batches[PKT_ARRAY_SIZE];
 long long now = time_msec();
 size_t newcnt, n_batches, i;
-- 
2.4.11

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

[ovs-dev] [PATCH v2 08/12] dpif-netdev: Reorder elements in dp_netdev_port structure.

[Bhanuprakash Bodireddy]

By reordering the data elements in dp_netdev_port structure, pad bytes
can be reduced and there by saving a cache line.

Before: structure size:136, holes:3, sum padbytes:15, cachelines:3
After: structure size:128, holes:2, sum padbytes:7, cachelines:2

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
---
 lib/dpif-netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 93ea49d..c079ab8 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -290,8 +290,8 @@ struct dp_netdev_port {
 struct netdev *netdev;
 struct hmap_node node;  /* Node in dp_netdev's 'ports'. */
 struct netdev_saved_flags *sf;
-unsigned n_rxq; /* Number of elements in 'rxq' */
 struct dp_netdev_rxq *rxqs;
+unsigned n_rxq; /* Number of elements in 'rxq' */
 bool dynamic_txqs;  /* If true XPS will be used. */
 unsigned *txq_used; /* Number of threads that uses each tx queue. 
*/
 struct ovs_mutex txq_used_mutex;
-- 
2.4.11

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

[ovs-dev] [PATCH v2 01/12] dpcls: Use 32 packet batches for lookups.

[Bhanuprakash Bodireddy]

This patch increases the number of packets processed in a batch during a
lookup from 16 to 32. Processing batches of 32 packets improves
performance and also one of the internal loops can be avoided here.

Signed-off-by: Antonio Fischetti 
Co-authored-by: Bhanuprakash Bodireddy 
---
 lib/dpif-netdev.c | 110 +++---
 1 file changed, 47 insertions(+), 63 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index eb9f764..3b3556a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4985,23 +4985,21 @@ dpcls_lookup(struct dpcls *cls, const struct 
netdev_flow_key keys[],
  int *num_lookups_p)
 {
 /* The received 'cnt' miniflows are the search-keys that will be processed
- * in batches of 16 elements.  N_MAPS will contain the number of these
- * 16-elements batches.  i.e. for 'cnt' = 32, N_MAPS will be 2.  The batch
- * size 16 was experimentally found faster than 8 or 32. */
-typedef uint16_t map_type;
+ * to find a matching entry into the available subtables.
+ * The number of bits in map_type is equal to NETDEV_MAX_BURST. */
+typedef uint32_t map_type;
 #define MAP_BITS (sizeof(map_type) * CHAR_BIT)
+BUILD_ASSERT_DECL(MAP_BITS >= NETDEV_MAX_BURST);
 
-#if !defined(__CHECKER__) && !defined(_WIN32)
-const int N_MAPS = DIV_ROUND_UP(cnt, MAP_BITS);
-#else
-enum { N_MAPS = DIV_ROUND_UP(NETDEV_MAX_BURST, MAP_BITS) };
-#endif
-map_type maps[N_MAPS];
 struct dpcls_subtable *subtable;
 
-memset(maps, 0xff, sizeof maps);
-if (cnt % MAP_BITS) {
-maps[N_MAPS - 1] >>= MAP_BITS - cnt % MAP_BITS; /* Clear extra bits. */
+map_type keys_map = 0x;
+map_type found_map;
+uint32_t hashes[MAP_BITS];
+const struct cmap_node *nodes[MAP_BITS];
+
+if (OVS_UNLIKELY(cnt != NETDEV_MAX_BURST)) {
+keys_map >>= NETDEV_MAX_BURST - cnt; /* Clear extra bits. */
 }
 memset(rules, 0, cnt * sizeof *rules);
 
@@ -5017,59 +5015,45 @@ dpcls_lookup(struct dpcls *cls, const struct 
netdev_flow_key keys[],
 PVECTOR_FOR_EACH (subtable, >subtables) {
 const struct netdev_flow_key *mkeys = keys;
 struct dpcls_rule **mrules = rules;
-map_type remains = 0;
-int m;
-
-BUILD_ASSERT_DECL(sizeof remains == sizeof *maps);
-
-/* Loops on each batch of 16 search-keys. */
-for (m = 0; m < N_MAPS; m++, mkeys += MAP_BITS, mrules += MAP_BITS) {
-uint32_t hashes[MAP_BITS];
-const struct cmap_node *nodes[MAP_BITS];
-unsigned long map = maps[m];
-int i;
-
-if (!map) {
-continue; /* Skip empty maps. */
-}
-
-/* Compute hashes for the remaining keys.  Each search-key is
- * masked with the subtable's mask to avoid hashing the wildcarded
- * bits. */
-ULLONG_FOR_EACH_1(i, map) {
-hashes[i] = netdev_flow_key_hash_in_mask([i],
- >mask);
-}
-/* Lookup. */
-map = cmap_find_batch(>rules, map, hashes, nodes);
-/* Check results.  When the i-th bit of map is set, it means that a
- * set of nodes with a matching hash value was found for the i-th
- * search-key.  Due to possible hash collisions we need to check
- * which of the found rules, if any, really matches our masked
- * search-key. */
-ULLONG_FOR_EACH_1(i, map) {
-struct dpcls_rule *rule;
-
-CMAP_NODE_FOR_EACH (rule, cmap_node, nodes[i]) {
-if (OVS_LIKELY(dpcls_rule_matches_key(rule, [i]))) {
-mrules[i] = rule;
-/* Even at 20 Mpps the 32-bit hit_cnt cannot wrap
- * within one second optimization interval  */
-subtable->hit_cnt++;
-lookups_match += subtable_pos;
-goto next;
-}
+int i;
+found_map = keys_map;
+
+/* Compute hashes for the remaining keys.  Each search-key is
+ * masked with the subtable's mask to avoid hashing the wildcarded
+ * bits. */
+ULLONG_FOR_EACH_1(i, keys_map) {
+hashes[i] = netdev_flow_key_hash_in_mask([i],
+ >mask);
+}
+/* Lookup. */
+found_map = cmap_find_batch(>rules, found_map, hashes,
+nodes);
+/* Check results.  When the i-th bit of found_map is set, it means
+ * that a set of nodes with a matching hash value was found for the
+ * i-th search-key.  Due to possible hash collisions we need to check
+ * which of the found rules, if any, really matches our masked
+ 

[ovs-dev] [PATCH v2 04/12] hash: Skip Invoking mhash_add__() with zero input.

[Bhanuprakash Bodireddy]

mhash_add__() is expensive and should be only called with valid input.
zero-valued 'data' will not affect the 'hash' value and expensive hash
computation can be skipped when input is zero.

This patch will validate the input in mhash_add__ to save some cpu
cycles.

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
---
 lib/hash.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/lib/hash.h b/lib/hash.h
index 114a419..f2dd510 100644
--- a/lib/hash.h
+++ b/lib/hash.h
@@ -62,6 +62,11 @@ static inline uint32_t hash_string(const char *, uint32_t 
basis);
 
 static inline uint32_t mhash_add__(uint32_t hash, uint32_t data)
 {
+/* zero-valued 'data' will not change the 'hash' value */
+if (!data) {
+return hash;
+}
+
 data *= 0xcc9e2d51;
 data = hash_rot(data, 15);
 data *= 0x1b873593;
-- 
2.4.11

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

[ovs-dev] [PATCH v2 05/12] dpif-netdev: Add comments to dp_netdev_input__().

[Bhanuprakash Bodireddy]

Add comments in dp_netdev_input__() to explain the reason behind
clearing the flow batches before packet_batch_execute().

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
---
 lib/dpif-netdev.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3b3556a..84aa570 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4182,6 +4182,15 @@ dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
 fast_path_processing(pmd, packets, keys, batches, _batches, in_port, 
now);
 }
 
+/* All the flow batches need to be reset before any call to
+ * packet_batch_per_flow_execute() as it could potentially trigger
+ * recirculation. When a packet matching flow ‘j’ happens to be
+ * recirculated, the nested call to dp_netdev_input__() could potentially
+ * classify the packet as matching another flow - say 'k'. It could happen
+ * that in the previous call to dp_netdev_input__() that same flow 'k' had
+ * already its own batches[k] still waiting to be served.  So if its
+ * ‘batch’ member is not reset, the recirculated packet would be wrongly
+ * appended to batches[k] of the 1st call to dp_netdev_input__(). */
 for (i = 0; i < n_batches; i++) {
 batches[i].flow->batch = NULL;
 }
-- 
2.4.11

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

[ovs-dev] [PATCH v2 10/12] ovsdb: Reorder elements in ovsdb_table_schema structure.

[Bhanuprakash Bodireddy]

By reordering the elements in ovsdb_table_schema structure, pad bytes
can be reduced and also a cache line is saved.

Before: structure size:72, holes:2, sum padbytes:10, cachelines:2
After: structure size:64, holes:1, sum padbytes:2, cachelines:1

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
Acked-by: Jarno Rajahalme 
---
 ovsdb/table.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ovsdb/table.h b/ovsdb/table.h
index f910d18..69dd649 100644
--- a/ovsdb/table.h
+++ b/ovsdb/table.h
@@ -28,9 +28,9 @@ struct uuid;
 struct ovsdb_table_schema {
 char *name;
 bool mutable;
-struct shash columns;   /* Contains "struct ovsdb_column *"s. */
-unsigned int max_rows;  /* Maximum number of rows. */
 bool is_root;   /* Part of garbage collection root set? */
+unsigned int max_rows;  /* Maximum number of rows. */
+struct shash columns;   /* Contains "struct ovsdb_column *"s. */
 struct ovsdb_column_set *indexes;
 size_t n_indexes;
 };
-- 
2.4.11

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

[ovs-dev] [PATCH v2 11/12] netlink-socket: Reorder elements in nl_dump structure.

[Bhanuprakash Bodireddy]

By reordering the elements in nl_dump structure, pad bytes can be
reduced there by saving a cache line.

Before: structure size:72, holes:1, sum padbytes:4, cachelines:2
After: structure size:64, holes:0, sum padbytes:0, cachelines:1

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
Acked-by: Jarno Rajahalme 
---
 lib/netlink-socket.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/netlink-socket.h b/lib/netlink-socket.h
index f73fc7d..d3cc642 100644
--- a/lib/netlink-socket.h
+++ b/lib/netlink-socket.h
@@ -260,12 +260,12 @@ struct nl_dump {
 /* These members are immutable during the lifetime of the nl_dump. */
 struct nl_sock *sock;   /* Socket being dumped. */
 uint32_t nl_seq;/* Expected nlmsg_seq for replies. */
-
-/* 'mutex' protects 'status' and serializes access to 'sock'. */
-struct ovs_mutex mutex; /* Protects 'status', synchronizes recv(). */
 int status OVS_GUARDED; /* 0: dump in progress,
  * positive errno: dump completed with error,
  * EOF: dump completed successfully. */
+
+/* 'mutex' protects 'status' and serializes access to 'sock'. */
+struct ovs_mutex mutex; /* Protects 'status', synchronizes recv(). */
 };
 
 void nl_dump_start(struct nl_dump *, int protocol,
-- 
2.4.11

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

[ovs-dev] [PATCH v2 09/12] dpif: Reorder elements in dpif_upcall structure.

[Bhanuprakash Bodireddy]

By reordering the data elements in dpif_upcall structure, pad bytes can
be reduced and also a cache line. Also dp_packet should be the first
member of the structure because rte_mbuf, the first member of dp_packet
should be aligned atleast on a 64-byte boundary.

Before: structure size:768, holes:1, sum padbytes:60, cachelines:12
After: structure size:704, holes:1, sum padbytes:4, cachelines:11

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
Acked-by: Jarno Rajahalme 
---
 lib/dpif.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/dpif.h b/lib/dpif.h
index 8a68d5d..cade046 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -780,8 +780,11 @@ const char *dpif_upcall_type_to_string(enum 
dpif_upcall_type);
  */
 struct dpif_upcall {
 /* All types. */
+struct dp_packet packet;/* Packet data,'dp_packet' should be the first
+  member to avoid a hole. This is because
+  'rte_mbuf' in dp_packet is aligned atleast
+  on a 64-byte boundary */
 enum dpif_upcall_type type;
-struct dp_packet packet;   /* Packet data. */
 struct nlattr *key; /* Flow key. */
 size_t key_len; /* Length of 'key' in bytes. */
 ovs_u128 ufid;  /* Unique flow identifier for 'key'. */
-- 
2.4.11

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

[ovs-dev] [PATCH v2 12/12] timeval: Reorder elements in clock structure.

[Bhanuprakash Bodireddy]

By reordering the elements in clock structure, pad bytes
can be reduced and also a cache line is saved.

Before: structure size:136, holes:3, sum padbytes:18, cachelines:3
After: structure size:120, holes:1, sum padbytes:2, cachelines:2

Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Antonio Fischetti 
Acked-by: Jarno Rajahalme 
---
 lib/timeval.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/timeval.c b/lib/timeval.c
index 0e8709a..dd63f03 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -71,10 +71,10 @@ struct clock {
 clockid_t id;   /* CLOCK_MONOTONIC or CLOCK_REALTIME. */
 
 /* Features for use by unit tests.  Protected by 'mutex'. */
-struct ovs_mutex mutex;
 atomic_bool slow_path; /* True if warped or stopped. */
-struct timespec warp OVS_GUARDED;  /* Offset added for unit tests. */
 bool stopped OVS_GUARDED;  /* Disable real-time updates if true. */
+struct ovs_mutex mutex;
+struct timespec warp OVS_GUARDED;  /* Offset added for unit tests. */
 struct timespec cache OVS_GUARDED; /* Last time read from kernel. */
 struct large_warp large_warp OVS_GUARDED; /* Connection information waiting
  for warp response. */
-- 
2.4.11

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

Re: [ovs-dev] [PATCH] rhel-systemd: Delay shutting down the services

[Flavio Leitner]

On Wed, Oct 12, 2016 at 11:56:51AM -0400, Russell Bryant wrote:
> On Fri, Oct 7, 2016 at 1:36 PM, Aaron Conole  wrote:
> 
> > During testing it was found that systemd would consider the openvswitch
> > service as a part of networking component, but the dependent services of
> > ovs-vswitchd and ovsdb-server were not likewise considered.  This leads
> > to some strange race conditions, observed when using NFS over TCP, while
> > shutting down systems.
> >
> > Fixes: 84ad12083491 ("rhel: Improved Systemd Integration")
> > Co-authored-by: Flavio Leitner 
> > Signed-off-by: Aaron Conole 
> >
> 
> This looks fine to me.  Flavio, can you provide a Signed-off-by for this
> patch, please?

Signed-off-by: Flavio Leitner 

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

Re: [ovs-dev] 答复: Re: [PATCH] netdev: Initialize netdev's features before getting them

[Aaron Conole]

xu.binb...@zte.com.cn writes:

> Hi Aaron, 
>
> Thanks for your suggestion, I'll resubmit a patch for DPDK, and take a look 
> at 
> lib/netdev-bsd.c to confirm that the additional patch is needed or not. 
>
> Thanks, 
>
> Aaron Conole  写于 2016/10/12 21:26:14:
>
>> 发件人:  Aaron Conole  
>> 收件人:  Binbin Xu , 
>> 抄送: dev@openvswitch.org 
>> 日期:  2016/10/12 21:26 
>> 主题: Re: [ovs-dev] [PATCH] netdev: Initialize netdev's features 
>> before getting them 
>> 
>> Hi Binbin,
>> 
>> Binbin Xu  writes:
>> 
>> > When OVS is used, DPDK doesn't support features 'advertised',
>> > 'supported' and 'peer'. If a physical port added to bridge, features
>> > descirbed above can't be assigned, and the values are random.
>> >
>> > Signed-off-by: Binbin Xu 
>> > ---
>> 
>> Thanks for reporting this.  I consider this a bug in dpdk, not
>> something that requires changing the netdev framework.  A look at other
>> netdev classes confirms this.
>> 
>> Please re-submit something that fixes DPDK, like the following.  If I
>> read it correctly, you might also submit an additional patch to cleanup
>> the interface in lib/netdev-bsd.c (which seems to be 'mismatched').
>> 
>> NOTE patch is *completely* untested, not even compile tested. 
>
> I have tested the patch in my environment, you means that i didn't make the 
> unit test? 

I meant I have not tested the code I posted.

>> 
>> ---
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 39bf930..6f5ec43 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1978,13 +1978,15 @@ out:
>>  static int
>>  netdev_dpdk_get_features(const struct netdev *netdev,
>>   enum netdev_features *current,
>> - enum netdev_features *advertised OVS_UNUSED,
>> - enum netdev_features *supported OVS_UNUSED,
>> - enum netdev_features *peer OVS_UNUSED)
>> + enum netdev_features *advertised,
>> + enum netdev_features *supported,
>> + enum netdev_features *peer)
>>  {
>>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>  struct rte_eth_link link;
>>  
>> +*advertised = *peer = *supported = 0;
>> +
>>  ovs_mutex_lock(>mutex);
>>  link = dev->link;
>>  ovs_mutex_unlock(>mutex);
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/9 md->rst] Convert most INSTALL guides

[Ben Pfaff]

On Thu, Oct 13, 2016 at 09:10:58AM -0400, Russell Bryant wrote:
> On Thu, Oct 13, 2016 at 6:00 AM, Stephen Finucane  wrote:
> 
> > On 2016-10-12 20:20, Russell Bryant wrote:
> >
> >> On Sat, Oct 8, 2016 at 12:30 PM, Stephen Finucane 
> >> wrote:
> >>
> >> I'm going to dripfeed the conversion patches to avoid overloading
> >>> peoples (there are ~25 of them so far). This is the first batch.
> >>>
> >>> Stephen Finucane (9):
> >>> dist-docs: Add support for rST
> >>> doc: Convert INSTALL to rST
> >>> doc: Convert INSTALL.DPDK to rST
> >>> doc: Convert INSTALL.Debian to rST
> >>> doc: Convert INSTALL.Docker to rST
> >>> doc: Convert INSTALL.Windows to rST
> >>> doc: Convert INSTALL.userspace to rST
> >>> doc: Convert INSTALL.XenServer to rST
> >>> doc: Convert INSTALL.KVM to rST
> >>>
> >>
> >> Do you have the end result published anywhere?  It would be nice to
> >> see the before and after before diving into the detailed reviews.
> >>
> >
> > I visually inspected them myself but I haven't published them anywhere.
> > You can run 'make dist-docs' and visually compare the resulting HTML files
> > to what's on openvswitch.org (remembering that the reStructuredText
> > versions should actually be more complete due to latent bugs in the
> > Markdown versions). I could find somewhere to publish them if it would help
> > but I don't know if it's really necessary :)
> >
> > To be honest though, I don't know how detailed we need to be right now.
> > IMO if the functional aspects of these patches work (i.e. the 'dist-docs'
> > target generates sane HTML for all rST files) and we haven't lost any of
> > the raw information then any stylistic issues I may have introduced can be
> > fixed in follow up patches. Most of these docs are going to get shuffled
> > around or rewritten when we start moving to sphinx anyway, so we'll have
> > the chance to really review the content itself then.
> >
> 
> OK, sounds good.  I also see the previous conversation about this work
> now.  Sorry about that.  I'm on board with all of this.
> 
> Ben, I'm happy to review all of the patches for this conversion if you'd
> like.

That would be really valuable, thanks Russell!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Native DNS support proposal in OVN for internal DNS resolution

[Numan Siddique]

Below is the proposal to add native DNS support in OVN for internal DNS
resolution. This will be useful If a VM sends a DNS lookup request for
another VM belonging to the same virtual network.

 - hostname of the logical ports will be stored in the north db lsp row

 - ovn-northd will add the below logical flows for each of the lsp's having
hostname defined. (3 new logical stages "ls_in_l7_parse", "ls_in_l7_match",
"ls_in_l7_response" will be defined).


   table=13(ls_in_l7_parse ), priority=100  , match=(ip4 && udp.dst ==
53), action=(reg0[4] = extract_dns_packet(); next;)
  table=13(ls_in_l7_parse ), priority=0, match=(1), action=(next;)
  table=14(ls_in_l7_match ), priority=90   , match=(dns.query ==
"vm1"), action=(put_dns_answer(10.0.0.20);)
  table=14(ls_in_l7_match ), priority=90   , match=(dns.query ==
"vm2"), action=(put_dns_answer(10.0.0.21);)


  table=14(ls_in_l7_match ), priority=0, match=(1), action=(next;)
  table=15(ls_in_l7_response  ), priority=100  , match=(ip4 && udp.dst ==
53 && reg0[4]), action=(eth.dst <-> eth.src; ip4.src <-> ip4.dst; udp.dst
<-> udp.src; outport = inport; flags.loopback = 1; output;)
  table=15(ls_in_l7_response  ), priority=0, match=(1), action=(next;)


 - ovn-controller will translate these into below OF Flows

   cookie=0x0, duration=631.516s, table=29, n_packets=8, n_bytes=504,
idle_age=585, priority=100,udp,metadata=0x1,tp_dst=53
actions=controller(userdata=00.00.00.06.00.00.00.00.00.01.
de.10.00.00.00.64,pause),resubmit(,30)

 cookie=0x0, duration=631.517s, table=29, n_packets=0, n_bytes=0,
idle_age=631, priority=0,metadata=0x1 actions=resubmit(,30)

 cookie=0x0, duration=631.523s, table=30, n_packets=0, n_bytes=0,
idle_age=631, priority=0,metadata=0x1 actions=resubmit(,31)
 cookie=0x0, duration=631.521s, table=31, n_packets=0, n_bytes=0,
idle_age=631, priority=100,udp,reg0=0x10/0x10,metadata=0x1,tp_dst=53
actions=push:NXM_OF_ETH_SRC[],push:NXM_OF_ETH_DST[],pop:NXM_
OF_ETH_SRC[],pop:NXM_OF_ETH_DST[],push:NXM_OF_IP_DST[],
push:NXM_OF_IP_SRC[],pop:NXM_OF_IP_DST[],pop:NXM_OF_IP_SRC[
],push:NXM_OF_UDP_SRC[],push:NXM_OF_UDP_DST[],pop:NXM_OF_
UDP_SRC[],pop:NXM_OF_UDP_DST[],move:NXM_NX_REG14[]->NXM_NX_
REG15[],load:0x1->NXM_NX_REG10[0],resubmit(,32)


​ovn-controller will translate the "extract_dns_packet" ovn action to
"controller action with pause flag set.
For the flows in the table "​ls_in_l7_match", ovn-controller will NOT
translate it into any OF Flow, instead it will
   - maintain a hash map for each logical datapath (with datapath_key as
key) - "l7_dp_flows"
   - store the (dns query value, ip address) pair in the hash map of the
datapath.

​Below is what happens when a dns request packet is received​

  - ovs-vswitchd will send it to ovn-controller.
  - ovn-controller  will parse the dns packet and extract the host name
  - ovn-controller will extract the datapath key from the packet metadata
and looks up the ip address for the host name in the hash map (l7_dp_flows).
  - If the match is found, it will generate a dns response packet, sets 1
bit in the result register bit and resumes the packet.
  - on resuming the packet, the flow in "ls_in_l7_response" will reply back
if the result register bit is set. otherwise the packet will continue
further down in the pipeline.

- It will handle both queries for A (IPv4) and  (IPv6) records.

I want to get the feedback and see if this approach is reasonable ? If so,
I will continue with the development.

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

Re: [ovs-dev] [PATCH 0/9 md->rst] Convert most INSTALL guides

[Russell Bryant]

On Thu, Oct 13, 2016 at 6:00 AM, Stephen Finucane  wrote:

> On 2016-10-12 20:20, Russell Bryant wrote:
>
>> On Sat, Oct 8, 2016 at 12:30 PM, Stephen Finucane 
>> wrote:
>>
>> I'm going to dripfeed the conversion patches to avoid overloading
>>> peoples (there are ~25 of them so far). This is the first batch.
>>>
>>> Stephen Finucane (9):
>>> dist-docs: Add support for rST
>>> doc: Convert INSTALL to rST
>>> doc: Convert INSTALL.DPDK to rST
>>> doc: Convert INSTALL.Debian to rST
>>> doc: Convert INSTALL.Docker to rST
>>> doc: Convert INSTALL.Windows to rST
>>> doc: Convert INSTALL.userspace to rST
>>> doc: Convert INSTALL.XenServer to rST
>>> doc: Convert INSTALL.KVM to rST
>>>
>>
>> Do you have the end result published anywhere?  It would be nice to
>> see the before and after before diving into the detailed reviews.
>>
>
> I visually inspected them myself but I haven't published them anywhere.
> You can run 'make dist-docs' and visually compare the resulting HTML files
> to what's on openvswitch.org (remembering that the reStructuredText
> versions should actually be more complete due to latent bugs in the
> Markdown versions). I could find somewhere to publish them if it would help
> but I don't know if it's really necessary :)
>
> To be honest though, I don't know how detailed we need to be right now.
> IMO if the functional aspects of these patches work (i.e. the 'dist-docs'
> target generates sane HTML for all rST files) and we haven't lost any of
> the raw information then any stylistic issues I may have introduced can be
> fixed in follow up patches. Most of these docs are going to get shuffled
> around or rewritten when we start moving to sphinx anyway, so we'll have
> the chance to really review the content itself then.
>

OK, sounds good.  I also see the previous conversation about this work
now.  Sorry about that.  I'm on board with all of this.

Ben, I'm happy to review all of the patches for this conversion if you'd
like.

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

Re: [ovs-dev] ovn: Improving southbound database security

[Numan Siddique]

We may have to add one more item in the task breakdown list. Please see
below


On Wed, Oct 12, 2016 at 11:21 PM, Russell Bryant  wrote:

> Hello, I'm back to looking at southbound database security concerns in
> OVN.  A previous thread discussing approaches was here:
>
> http://openvswitch.org/pipermail/dev/2016-August/078106.html
>
> I'm now working with a few others on implementing a proposed solution.
> The overview is that we'd like to make ovn-controller a read-only client of
> the southbound database.
>
> The task breakdown is:
>
> 1) Add support to ovsdb-server for read-only remotes.  The port reachable
> by ovn-controller would only accept read-only connections.
>
> 2) Remove support from ovn-controller for automatically creating chassis
> and encap records.  Document this as an administrative step for adding a
> new chassis to the system, instead.
>
> 3) Remove support from ovn-controller for setting the chassis column of
> Port_Binding records.  Instead, have this handled by ovn-northd based on
> binding instructions given in the northbound database.
>
> As a nice side effect, this helps solve one of the difficulties with live
> migration (two chassis fighting to own a port while the migration is in
> progress).  Instead, we would update OVN when we know the migration is
> complete.
>
> I was originally thinking we may need an upgrade utility to help existing
> environments, but I think ovn-northd can handle this automatically.
>
> For the northbound database, I was thinking of adding a new option for
> logical ports called "chassis" with a value of the hostname of the chassis
> the port should be bound to.
>
> 4) Remove use of MAC_bindings table from ovn-controller.  Replace it with
> a local cache, instead.  I'm proposing just keeping it in memory in
> ovn-controller, but we could also make use of the openvswitch db.
>
> One detail I haven't fully thought through: what should happen to the
> MAC_Bindings table?  Dropping the table seems not backwards compatible and
> would break a rolling upgrade scenario.  Should we leave it as unused for
> one release, and then remove it in the next release?  More generally, I
> think we need to document our upgrade strategy and related rules for
> database schema changes.
>
>
​5) Remove support from ovn-controller updating the 'Chassis.hv_cfg'
column​ and handle the side effect in "--wait=hv" in ovn-nbctl.


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

[ovs-dev] Delivery reports about your e-mail

[Mail Administrator]

Your message was undeliverable due to the following reason(s):

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 was not delivered within 3 days:
Mail server 40.12.48.232 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

Re: [ovs-dev] Subject: [PATCH] ovs-monitor-ipsec: ipsec_vxlan support and IPsec tunnel mode options support.

[Muthukrishnan Thangasamy]

Dear Ben ,


Thank you for your response.


Could you please throw some light on ,any specific reason why OVS stopped 
support for ovs-monitor-ipsec and decided to remove support for ipsec 
altogether in ovs.


Regards

Muthukrishnan

9952012433


From: Ben Pfaff 
Sent: Thursday, September 29, 2016 8:43:43 PM
To: Muthukrishnan Thangasamy
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] Subject: [PATCH] ovs-monitor-ipsec: ipsec_vxlan support 
and IPsec tunnel mode options support.

On Thu, Sep 29, 2016 at 07:13:55AM +, Muthukrishnan Thangasamy wrote:
> From 76e504a38a3c556884782323459cc6862a38747c Mon Sep 17 00:00:00 2001
> From: Muthukrishnan T 
> Date: Thu, 29 Sep 2016 12:21:29 +0530
> Subject: [PATCH] ovs-monitor-ipsec: ipsec_vxlan support and IPsec tunnel mode 
> options support.

We just removed ovs-monitor-ipsec, so this can't be applied.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev