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

Daniele Di Proietto ddiproietto at vmware.com
Tue Aug 5 16:48:31 UTC 2014


Thanks for the review and for the fixes!

Daniele

On 8/4/14, 2:31 PM, "Ben Pfaff" <blp at nicira.com> wrote:

>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
>      
>https://urldefense.proofpoint.com/v1/url?u=http://tools.ietf.org/html/draf
>t-gross-geneve-00&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=MV9BdLjtFIdhBDBaw5
>z%2BU6SSA2gAfY4L%2F1HCy3VjlKU%3D%0A&m=UnG1y54Wx9Bs4iOLlUJN8aYOQyakvbhFglUN
>PGlhQ1A%3D%0A&s=9d7ac58ffcf650ccd68af12f749d89fc7f187f3d8d83edd7874ae8d736
>d457a8
>    - 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