[ovs-dev] [PATCH v2] dpctl: add ovs-appctl dpctl/* commands to talk to dpif-netdev

Ben Pfaff blp at nicira.com
Mon Aug 4 21:31:55 UTC 2014


On Thu, Jul 17, 2014 at 05:26:00PM -0700, Daniele Di Proietto wrote:
> This commit introduces multiple appctl commands (dpctl/*)
> 
> They are needed to interact with userspace datapaths (dpif-netdev), because the
> ovs-dpctl command runs in a separate process and cannot see the userspace
> datapaths inside vswitchd.
> 
> This change moves most of the code of utilities/ovs-dpctl.c in lib/dpctl.c.
> 
> Both the ovs-dpctl command and the ovs-appctl dpctl/* commands make calls to
> lib/dpctl.c functions, to interact with datapaths.
> 
> The code from utilities/ovs-dpctl.c has been moved to lib/dpctl.c and has been
> changed for different reasons:
>    - An exit() call in the old code made perfectly sense. Now (since the code
>      can be run inside vswitchd) it would terminate the daemon. Same reasoning
>      can be applied to ovs_fatal_*() calls.
>    - The lib/dpctl.c code _should_ not leak memory.
>    - All the print* have been replaced with a function pointer provided by the
>      caller, since this code can be run in the ovs-dpctl process (in which
>      case we need to print to stdout) or in response to a unixctl request (and
>      in this case we need to send everything through a socket, using JSON
>      encapsulation).
> 
> The syntax is
>    ovs-appctl dpctl/(COMMAND) [OPTIONS] [PARAMETERS]
> while the ovs-dpctl syntax (which _should_ remain the same after this change)
> is
>    ovs-dpctl [OPTIONS] (COMMAND) [PARAMETERS]
> 
> Signed-off-by: Daniele Di Proietto <ddiproietto at vmware.com>

I'm happy with this.  I folded in the following minor changes and I'll
apply this to master in a minute.

diff --git a/NEWS b/NEWS
index 742ae22..bf7eb2f 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,10 @@ Post-v2.3.0
      release. The protocol is documented at
      http://tools.ietf.org/html/draft-gross-geneve-00
    - The OVS database now reports controller rate limiting statistics.
+   - ovs-dpctl functionality is now available for datapaths integrated
+     into ovs-vswitchd, via ovs-appctl.  Some existing ovs-appctl
+     commands are now redundant and will be removed in a future
+     release.  See ovs-vswitchd(8) for details.
    - OpenFlow:
      * OpenFlow 1.5 (draft) extended registers are now supported.
 
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 17d8b56..623f5b1 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -92,17 +92,17 @@ dpctl_error(struct dpctl_params* dpctl_p, int err_no, const char *fmt, ...)
     }
     ds_put_cstr(&ds, "\n");
 
-    dpctl_puts(dpctl_p, true,  ds_cstr(&ds));
+    dpctl_puts(dpctl_p, true, ds_cstr(&ds));
 
     ds_destroy(&ds);
 
     errno = save_errno;
 }
 
-static int dpctl_add_if(int argc, const char *argv[],
-                         struct dpctl_params *dpctl_p);
+static int dpctl_add_if(int argc, const char *argv[], struct dpctl_params *);
 
-static int if_up(struct netdev *netdev)
+static int
+if_up(struct netdev *netdev)
 {
     return netdev_turn_flags_on(netdev, NETDEV_UP, NULL);
 }
@@ -1154,7 +1154,7 @@ dpctl_normalize_actions(int argc, const char *argv[],
     ds_clear(&s);
     odp_flow_format(ofpbuf_data(&keybuf), ofpbuf_size(&keybuf), NULL, 0, NULL,
                     &s, dpctl_p->verbosity);
-    printf("input flow: %s\n", ds_cstr(&s));
+    dpctl_print(dpctl_p, "input flow: %s\n", ds_cstr(&s));
 
     error = odp_flow_key_to_flow(ofpbuf_data(&keybuf), ofpbuf_size(&keybuf),
                                  &flow);
@@ -1221,7 +1221,7 @@ dpctl_normalize_actions(int argc, const char *argv[],
                         vlan_tci_to_vid(af->flow.vlan_tci),
                         vlan_tci_to_pcp(af->flow.vlan_tci));
         } else {
-            printf("no vlan: ");
+            dpctl_print(dpctl_p, "no vlan: ");
         }
 
         if (eth_type_mpls(af->flow.dl_type)) {
@@ -1289,8 +1289,8 @@ static const struct dpctl_command all_commands[] = {
 /* Runs the command designated by argv[0] within the command table specified by
  * 'commands', which must be terminated by a command whose 'name' member is a
  * null pointer. */
-int dpctl_run_command(int argc, const char *argv[],
-                      struct dpctl_params *dpctl_p)
+int
+dpctl_run_command(int argc, const char *argv[], struct dpctl_params *dpctl_p)
 {
     const struct dpctl_command *p;
 
diff --git a/lib/dpctl.h b/lib/dpctl.h
index f97b7b7..41186f6 100644
--- a/lib/dpctl.h
+++ b/lib/dpctl.h
@@ -21,13 +21,15 @@
 #include "compiler.h"
 
 struct dpctl_params {
-    /* Options */
     /* -s, --statistics: Print port/flow statistics? */
     bool print_statistics;
+
     /* --clear: Reset existing statistics to zero when modifying a flow? */
     bool zero_statistics;
+
     /* --may-create: Allow mod-flows command to create a new flow? */
     bool may_create;
+
     /* -m, --more: Increase output verbosity. */
     int verbosity;
 
diff --git a/lib/dpctl.man b/lib/dpctl.man
index 98cd8af..c678576 100644
--- a/lib/dpctl.man
+++ b/lib/dpctl.man
@@ -3,7 +3,7 @@
 Creates datapath \fIdp\fR, with a local port also named \fIdp\fR.
 This will fail if a network device \fIdp\fR already exists.
 .IP
-If \fInetdev\fRs are specified, \fBovs\-dpctl\fR adds them to the
+If \fInetdev\fRs are specified, \fB\*(PN\fR adds them to the
 new datapath, just as if \fBadd\-if\fR was specified.
 .
 .TP
@@ -82,9 +82,9 @@ visited per packet; the ratio between "hit" and total number of
 packets processed by the datapath".
 .IP
 If one or more datapaths are specified, information on only those
-datapaths are displayed.  Otherwise, \fBovs\-dpctl\fR displays information
+datapaths are displayed.  Otherwise, \fB\*(PN\fR displays information
 about all configured datapaths.
-.SS "DPCTL DEBUGGING COMMANDS"
+.SS "DATAPATH FLOW TABLE DEBUGGING COMMANDS"
 The following commands are primarily useful for debugging Open
 vSwitch.  The flow table entries (both matches and actions) that they
 work with are not OpenFlow flow entries.  Instead, they are different
diff --git a/ofproto/ofproto-dpif-unixctl.man b/ofproto/ofproto-dpif-unixctl.man
index 14e9ff2..bbf7fbb 100644
--- a/ofproto/ofproto-dpif-unixctl.man
+++ b/ofproto/ofproto-dpif-unixctl.man
@@ -1,6 +1,9 @@
-.SS "DATAPATH COMMANDS"
-These commands manage logical datapaths.  They are are similar to the
-equivalent \fBovs\-dpctl\fR commands.
+.SS "DATAPATH DEBUGGING COMMANDS"
+These commands query and modify datapaths.  They are are similar to
+\fBovs\-dpctl\fR(8) commands.  \fBdpif/show\fR has the additional
+functionality, beyond \fBdpctl/show\fR of printing OpenFlow port
+numbers.  The other commands are redundant and will be removed in a
+future release.
 .
 .IP "\fBdpif/dump\-dps\fR"
 Prints the name of each configured datapath on a separate line.
diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index d58b0d5..0fd13e3 100644
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -216,12 +216,17 @@ whether it is attached or detached, port id and priority, actor
 information, and partner information.  If \fIport\fR is not specified,
 then displays detailed information about all interfaces with CFM
 enabled.
-.SS "DPCTL COMMANDS"
-These commands can be used to manage datapaths. They implement the same
-features (and syntax) as \fBovs\-dpctl\fR commands, but they work also with
-non-kernel datapaths (dpif-netdev), which \fBovs\-dpctl\fR cannot control.
+.SS "DPCTL DATAPATH DEBUGGING COMMANDS"
+The primary way to configure \fBovs\-vswitchd\fR is through the Open
+vSwitch database, e.g. using \fBovs\-vsctl\fR(8).  These commands
+provide a debugging interface for managing datapaths.  They implement
+the same features (and syntax) as \fBovs\-dpctl\fR(8).  Unlike
+\fBovs\-dpctl\fR(8), these commands work with datapaths that are
+integrated into \fBovs\-vswitchd\fR (e.g. the \fBnetdev\fR datapath
+type).
+.PP
 .
-.ds DX "\fBdpctl/\fR
+.ds DX \fBdpctl/\fR
 .de DO
 \\$2 \\$1 \\$3
 ..



More information about the dev mailing list