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

2016-10-13 Thread Lance Richardson
> From: "Ben Pfaff" <b...@ovn.org>
> To: "Lance Richardson" <lrich...@redhat.com>
> 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] [RFC] ovsdb: implement read-only remote connection type

2016-10-13 Thread 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


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

2016-10-13 Thread 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,