This patch needs a proper commit message. The user visible changes are in the 
corresponding configuration settings. Maybe briefly introduce them in the 
commit message?

More comments inline below.

I have a working copy with the white-space changes, so if you provide the 
commit message and the answer to the AUTO question below, I can accept and push 
this patch.

Thanks!

  Jarno

On Nov 6, 2014, at 7:31 AM, Daniele Venturino <daniele.ventur...@m3s.it> wrote:

> Signed-off-by: Daniele Venturino <daniele.ventur...@m3s.it>
> ---
> lib/rstp-common.h        |  6 ------
> lib/rstp.c               | 31 ++++++++++++++++++++++++++++++-
> lib/rstp.h               |  9 ++++++++-
> ofproto/ofproto-dpif.c   |  4 +++-
> ofproto/ofproto.h        |  2 ++
> utilities/ovs-vsctl.8.in |  9 +++++++++
> vswitchd/bridge.c        | 10 ++++++++++
> 7 files changed, 62 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/rstp-common.h b/lib/rstp-common.h
> index 587f88c..cd43079 100644
> --- a/lib/rstp-common.h
> +++ b/lib/rstp-common.h
> @@ -240,12 +240,6 @@ struct rstp_bpdu {
>     uint8_t padding[7];
> });
> 
> -enum rstp_admin_point_to_point_mac_state {
> -    RSTP_ADMIN_P2P_MAC_FORCE_TRUE,
> -    RSTP_ADMIN_P2P_MAC_FORCE_FALSE,
> -    RSTP_ADMIN_P2P_MAC_FORCE_AUTO
> -};
> -
> enum rstp_info_is {
>     INFO_IS_DISABLED,
>     INFO_IS_RECEIVED,
> diff --git a/lib/rstp.c b/lib/rstp.c
> index 0f96749..e3007e2 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -110,6 +110,9 @@ static void rstp_port_set_admin_edge__(struct rstp_port 
> *, bool admin_edge)
>     OVS_REQUIRES(rstp_mutex);
> static void rstp_port_set_auto_edge__(struct rstp_port *, bool auto_edge)
>     OVS_REQUIRES(rstp_mutex);
> +static void rstp_port_set_admin_point_to_point_mac__(struct rstp_port *,
> +        enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state)
> +    OVS_REQUIRES(rstp_mutex);
> static void rstp_port_set_mcheck__(struct rstp_port *, bool mcheck)
>     OVS_REQUIRES(rstp_mutex);
> static void reinitialize_port__(struct rstp_port *p)
> @@ -922,6 +925,8 @@ rstp_port_set_administrative_bridge_port__(struct 
> rstp_port *p,
>                                            uint8_t admin_port_state)
>     OVS_REQUIRES(rstp_mutex)
> {
> +    VLOG_DBG("%s, port %u: set RSTP port admin-port-state to %d",
> +             p->rstp->name, p->port_number, admin_port_state);

Add an empty line here.

>     if (admin_port_state == RSTP_ADMIN_BRIDGE_PORT_STATE_DISABLED
>         || admin_port_state == RSTP_ADMIN_BRIDGE_PORT_STATE_ENABLED) {
> 
> @@ -1120,6 +1125,27 @@ rstp_port_set_auto_edge__(struct rstp_port *port, bool 
> auto_edge)
>     }
> }
> 
> +/* Sets the port admin_point_to_point_mac parameter. */
> +static void rstp_port_set_admin_point_to_point_mac__(struct rstp_port *port,
> +        enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state)
> +    OVS_REQUIRES(rstp_mutex)
> +{
> +    VLOG_DBG("%s, port %u: set RSTP port admin-point-to-point-mac to %d",
> +            port->rstp->name, port->port_number, admin_p2p_mac_state);

Add an empty line here.

> +    if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_FORCE_TRUE) {
> +        port->admin_point_to_point_mac =  admin_p2p_mac_state;

Remove extra space after ‘=‘.

> +        rstp_port_set_oper_point_to_point_mac__(port,
> +                RSTP_OPER_P2P_MAC_STATE_ENABLED);
> +    } else if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_FORCE_FALSE) {
> +        port->admin_point_to_point_mac =  admin_p2p_mac_state;

Remove extra space after ‘=‘.

> +        rstp_port_set_oper_point_to_point_mac__(port,
> +                RSTP_OPER_P2P_MAC_STATE_DISABLED);
> +    } else if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_AUTO) {
> +        port->admin_point_to_point_mac = admin_p2p_mac_state;
> +        /* FIXME fix auto behaviour */

Elaborate on this. How the AUTO mode should work? Why not implement it now?

> +    }
> +}
> +
> /* Sets the port mcheck parameter.
>  * [17.19.13] May be set by management to force the Port Protocol Migration
>  * state machine to transmit RST BPDUs for a MigrateTime (17.13.9) period, to
> @@ -1289,7 +1315,8 @@ rstp_port_get_status(const struct rstp_port *p, 
> uint16_t *id,
> void
> rstp_port_set(struct rstp_port *port, uint16_t port_num, int priority,
>               uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
> -              bool do_mcheck, void *aux)
> +              enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state,
> +              bool admin_port_state, bool do_mcheck, void *aux)
>     OVS_EXCLUDED(rstp_mutex)
> {
>     ovs_mutex_lock(&rstp_mutex);
> @@ -1299,6 +1326,8 @@ rstp_port_set(struct rstp_port *port, uint16_t 
> port_num, int priority,
>     rstp_port_set_path_cost__(port, path_cost);
>     rstp_port_set_admin_edge__(port, is_admin_edge);
>     rstp_port_set_auto_edge__(port, is_auto_edge);
> +    rstp_port_set_admin_point_to_point_mac__(port, admin_p2p_mac_state);
> +    rstp_port_set_administrative_bridge_port__(port, admin_port_state);
>     rstp_port_set_mcheck__(port, do_mcheck);
>     ovs_mutex_unlock(&rstp_mutex);
> }
> diff --git a/lib/rstp.h b/lib/rstp.h
> index ccf8292..458aecf 100644
> --- a/lib/rstp.h
> +++ b/lib/rstp.h
> @@ -120,6 +120,12 @@ enum rstp_port_role {
>     ROLE_DISABLED
> };
> 
> +enum rstp_admin_point_to_point_mac_state {
> +    RSTP_ADMIN_P2P_MAC_FORCE_FALSE,
> +    RSTP_ADMIN_P2P_MAC_FORCE_TRUE,
> +    RSTP_ADMIN_P2P_MAC_AUTO
> +};
> +
> struct rstp;
> struct rstp_port;
> struct ofproto_rstp_settings;
> @@ -211,7 +217,8 @@ uint32_t rstp_convert_speed_to_cost(unsigned int speed);
> 
> void rstp_port_set(struct rstp_port *, uint16_t port_num, int priority,
>                    uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
> -                   bool do_mcheck, void *aux)
> +                   enum rstp_admin_point_to_point_mac_state 
> admin_p2p_mac_state,
> +                   bool admin_port_state, bool do_mcheck, void *aux)
>     OVS_EXCLUDED(rstp_mutex);
> 
> enum rstp_state rstp_port_get_state(const struct rstp_port *)
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 2302073..95298f2 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2402,7 +2402,9 @@ set_rstp_port(struct ofport *ofport_,
>     }
> 
>     rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
> -                  s->admin_edge_port, s->auto_edge, s->mcheck, ofport);
> +                  s->admin_edge_port, s->auto_edge,
> +                  s->admin_p2p_mac_state, s->admin_port_state, s->mcheck,
> +                  ofport);
>     update_rstp_port_state(ofport);
>     /* Synchronize operational status. */
>     rstp_port_set_mac_operational(rp, ofport->may_enable);
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 989747d..bb28376 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -126,6 +126,8 @@ struct ofproto_port_rstp_settings {
>     bool admin_edge_port;
>     bool auto_edge;
>     bool mcheck;
> +    uint8_t admin_p2p_mac_state;
> +    bool admin_port_state;
> };
> 
> struct ofproto_stp_settings {
> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> index 8cf13ae..2902a27 100644
> --- a/utilities/ovs-vsctl.8.in
> +++ b/utilities/ovs-vsctl.8.in
> @@ -1073,6 +1073,15 @@ Set the auto edge value of port \fBeth0\fR:
> .IP
> .B "ovs\-vsctl set Port eth0 other_config:rstp-port-auto-edge=true"
> .PP
> +Set the admin point to point mac value  of port \fBeth0\fR.

Remove extra space after ‘value‘.

> +Acceptable values are 0 (force false), 1 (force true) or 2 (auto).
> +.IP
> +.B "ovs\-vsctl set Port eth0 other_config:rstp-admin-p2p-mac=1"
> +.PP
> +Set the admin port state value of port \fBeth0\fR.
> +.IP
> +.B "ovs\-vsctl set Port eth0 other_config:rstp-admin-port-state=true"
> +.PP
> Set the mcheck value of port \fBeth0\fR:
> .IP
> .B "ovs\-vsctl set Port eth0 other_config:rstp-port-mcheck=true"
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 20cfcba..a5fb361 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1398,6 +1398,16 @@ port_configure_rstp(const struct ofproto *ofproto, 
> struct port *port,
>         port_s->priority = RSTP_DEFAULT_PORT_PRIORITY;
>     }
> 
> +    config_str = smap_get(&port->cfg->other_config, "rstp-admin-p2p-mac");
> +    if (config_str) {
> +        port_s->admin_p2p_mac_state = strtoul(config_str, NULL, 0);
> +    } else {
> +        port_s->admin_p2p_mac_state =  RSTP_ADMIN_P2P_MAC_FORCE_TRUE;

Remove extra space after ‘=‘.

> +    }
> +
> +    port_s->admin_port_state = smap_get_bool(&port->cfg->other_config,
> +                                             "rstp-admin-port-state", true);
> +
>     port_s->admin_edge_port = smap_get_bool(&port->cfg->other_config,
>                                             "rstp-port-admin-edge", false);
>     port_s->auto_edge = smap_get_bool(&port->cfg->other_config,
> -- 
> 1.8.1.2
> 

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

Reply via email to