There's some trailing whitespace added to the ofctl man page.

Otherwise looks good.  May benefit from a unit test.

Ethan



On Wed, May 2, 2012 at 1:36 PM, Ben Pfaff <[email protected]> wrote:
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
> This is a revision of a patch I sent out long ago that was part of
> a series that rotted during review and eventually fell off the end
> of everyone's queue.
>
>  NEWS                     |    2 +
>  utilities/ovs-ofctl.8.in |   58 
> ++++++++++++++++++++++++++++++----------------
>  utilities/ovs-ofctl.c    |   52 ++++++++++++++++++++++++++++------------
>  3 files changed, 76 insertions(+), 36 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 723c256..9cde7d3 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -17,6 +17,8 @@ post-v1.6.0
>     - The "coverage/log" command previously available through ovs-appctl
>       has been replaced by "coverage/show".  The new command replies with
>       coverage counter values, instead of logging them.
> +    - ovs-ofctl:
> +        - "mod-port" command can now control all OpenFlow config flags.
>
>
>  v1.6.0 - xx xxx xxxx
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index fdaa037..891c83c 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -73,31 +73,49 @@ the device name, e.g. \fBeth0\fR.  The \fIaction\fR may 
> be any one of the
>  following:
>  .
>  .RS
> -.IP \fBup\fR
> -Enables the interface.  This is equivalent to ``ifconfig up'' on a Unix
> -system.
> -.
> -.IP \fBdown\fR
> -Disables the interface.  This is equivalent to ``ifconfig down'' on a Unix
> -system.
> +.IQ \fBup\fR
> +.IQ \fBdown\fR
> +Enable or disable the interface.  This is equivalent to \fBifconfig
> +up\fR or \fBifconfig down\fR on a Unix system.
> +.
> +.IP \fBstp\fR
> +.IQ \fBno\-stp\fR
> +Enable or disable 802.1D spanning tree protocol (STP) on the
> +interface.  OpenFlow implementations that don't support STP will
> +refuse to enable it.
> +.
> +.IP \fBreceive\fR
> +.IQ \fBno\-receive\fR
> +.IQ \fBreceive\-stp\fR
> +.IQ \fBno\-receive\-stp\fR
> +Enable or disable OpenFlow processing of packets received on this
> +interface.  When packet processing is disabled, packets will be
> +dropped instead of being processed through the OpenFlow table.  The
> +\fBreceive\fR or \fBno\-receive\fR setting applies to all packets
> +except 802.1D spanning tree packets, which are separately controlled
> +by \fBreceive\-stp\fR or \fBno\-receive\-stp\fR.
>  .
>  .IP \fBforward\fR
> -Allows forwarding of traffic on this interface.  This is the default posture
> -for all ports.
> -.
> -.IP \fBnoforward\fR
> -Disallows forwarding of traffic on this interface.
> +.IQ \fBno\-forward\fR
> +Allow or disallow forwarding of traffic to this interface.  By
> +default, forwarding is enabled.
>  .
>  .IP \fBflood\fR
> -When a \fIflood\fR action is specified, traffic will be sent out this
> -interface.  This is the default posture for monitored ports.
> -.
> -.IP \fBnoflood\fR
> -When a \fIflood\fR action is specified, traffic will not be sent out
> -this interface.  This is primarily useful to prevent loops when a
> -spanning tree protocol is not in use.
> -.
> +.IQ \fBno\-flood\fR
> +Controls whether an OpenFlow \fBflood\fR action will send traffic out
> +this interface.  By default, flooding is enabled.  Disabling flooding
> +is primarily useful to prevent loops when a spanning tree protocol is
> +not in use.
> +.
> +.IP \fBpacket\-in\fR
> +.IQ \fBno\-packet\-in\fR
> +Controls whether packets received on this interface that do not match
> +a flow table entry generate a ``packet in'' message to the OpenFlow
> +controller.  By default, ``packet in'' messages are enabled.
>  .RE
> +.IP
> +The \fBshow\fR command displays (among other information) the
> +configuration that \fBmod\-port\fR changes.
>  .
>  .IP "\fBget\-frags \fIswitch\fR"
>  Prints \fIswitch\fR's fragment handling mode.  See \fBset\-frags\fR,
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 2b91a28..dad1623 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -1142,10 +1142,29 @@ do_packet_out(int argc, char *argv[])
>  static void
>  do_mod_port(int argc OVS_UNUSED, char *argv[])
>  {
> +    struct ofp_config_flag {
> +        const char *name;             /* The flag's name. */
> +        enum ofputil_port_config bit; /* Bit to turn on or off. */
> +        bool on;                      /* Value to set the bit to. */
> +    };
> +    static const struct ofp_config_flag flags[] = {
> +        { "up",          OFPUTIL_PC_PORT_DOWN,    false },
> +        { "down",        OFPUTIL_PC_PORT_DOWN,    true  },
> +        { "stp",         OFPUTIL_PC_NO_STP,       false },
> +        { "receive",     OFPUTIL_PC_NO_RECV,      false },
> +        { "receive-stp", OFPUTIL_PC_NO_RECV_STP,  false },
> +        { "flood",       OFPUTIL_PC_NO_FLOOD,     false },
> +        { "forward",     OFPUTIL_PC_NO_FWD,       false },
> +        { "packet-in",   OFPUTIL_PC_NO_PACKET_IN, false },
> +    };
> +
> +    const struct ofp_config_flag *flag;
>     enum ofputil_protocol protocol;
>     struct ofputil_port_mod pm;
>     struct ofputil_phy_port pp;
>     struct vconn *vconn;
> +    const char *command;
> +    bool not;
>
>     fetch_ofputil_phy_port(argv[1], argv[2], &pp);
>
> @@ -1155,25 +1174,26 @@ do_mod_port(int argc OVS_UNUSED, char *argv[])
>     pm.mask = 0;
>     pm.advertise = 0;
>
> -    if (!strcasecmp(argv[3], "up")) {
> -        pm.mask |= OFPUTIL_PC_PORT_DOWN;
> -    } else if (!strcasecmp(argv[3], "down")) {
> -        pm.mask |= OFPUTIL_PC_PORT_DOWN;
> -        pm.config |= OFPUTIL_PC_PORT_DOWN;
> -    } else if (!strcasecmp(argv[3], "flood")) {
> -        pm.mask |= OFPUTIL_PC_NO_FLOOD;
> -    } else if (!strcasecmp(argv[3], "noflood")) {
> -        pm.mask |= OFPUTIL_PC_NO_FLOOD;
> -        pm.config |= OFPUTIL_PC_NO_FLOOD;
> -    } else if (!strcasecmp(argv[3], "forward")) {
> -        pm.mask |= OFPUTIL_PC_NO_FWD;
> -    } else if (!strcasecmp(argv[3], "noforward")) {
> -        pm.mask |= OFPUTIL_PC_NO_FWD;
> -        pm.config |= OFPUTIL_PC_NO_FWD;
> +    if (!strncasecmp(argv[3], "no-", 3)) {
> +        command = argv[3] + 3;
> +        not = true;
> +    } else if (!strncasecmp(argv[3], "no", 2)) {
> +        command = argv[3] + 2;
> +        not = true;
>     } else {
> -        ovs_fatal(0, "unknown mod-port command '%s'", argv[3]);
> +        command = argv[3];
> +        not = false;
> +    }
> +    for (flag = flags; flag < &flags[ARRAY_SIZE(flags)]; flag++) {
> +        if (!strcasecmp(command, flag->name)) {
> +            pm.mask = flag->bit;
> +            pm.config = flag->on ^ not ? flag->bit : 0;
> +            goto found;
> +        }
>     }
> +    ovs_fatal(0, "unknown mod-port command '%s'", argv[3]);
>
> +found:
>     protocol = open_vconn(argv[1], &vconn);
>     transact_noreply(vconn, ofputil_encode_port_mod(&pm, protocol));
>     vconn_close(vconn);
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to