On Thu, Mar 5, 2015 at 11:12 AM, Ben Pfaff <b...@nicira.com> wrote:
> Until now, if both STP and RSTP were enabled, ovs-vswitchd would actually
> enable only the one it first noticed to be enabled, and actually turn off
> the setting for the other one in the database (!).  This doesn't match
> ovs-vswitchd behavior for other contradictory configurations, so this
> commit changes its behavior so that, if both are enabled, RSTP takes
> precedence.
>
> Reported-by: Daniele Venturino <daniele.ventur...@m3s.it>
> Signed-off-by: Ben Pfaff <b...@nicira.com>

Acked-by: Ansis Atteka <aatt...@nicira.com>

One small comment below.
> ---
>  vswitchd/bridge.c |   46 ++++++++++++++++++++++------------------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index f4ed174..dd622dc 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -248,8 +248,7 @@ static void bridge_configure_mac_table(struct bridge *);
>  static void bridge_configure_mcast_snooping(struct bridge *);
>  static void bridge_configure_sflow(struct bridge *, int 
> *sflow_bridge_number);
>  static void bridge_configure_ipfix(struct bridge *);
> -static void bridge_configure_stp(struct bridge *);
> -static void bridge_configure_rstp(struct bridge *);
> +static void bridge_configure_spanning_tree(struct bridge *);
>  static void bridge_configure_tables(struct bridge *);
>  static void bridge_configure_dp_desc(struct bridge *);
>  static void bridge_configure_aa(struct bridge *);
> @@ -668,8 +667,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch 
> *ovs_cfg)
>          bridge_configure_netflow(br);
>          bridge_configure_sflow(br, &sflow_bridge_number);
>          bridge_configure_ipfix(br);
> -        bridge_configure_stp(br);
> -        bridge_configure_rstp(br);
> +        bridge_configure_spanning_tree(br);
>          bridge_configure_tables(br);
>          bridge_configure_dp_desc(br);
>          bridge_configure_aa(br);
> @@ -1444,18 +1442,10 @@ port_configure_rstp(const struct ofproto *ofproto, 
> struct port *port,
>
>  /* Set spanning tree configuration on 'br'. */
>  static void
> -bridge_configure_stp(struct bridge *br)
> +bridge_configure_stp(struct bridge *br, bool enable_stp)
>  {
> -    struct ofproto_rstp_status rstp_status;
> -
> -    ofproto_get_rstp_status(br->ofproto, &rstp_status);
> -    if (!br->cfg->stp_enable) {
> -        ofproto_set_stp(br->ofproto, NULL);
> -    } else if (rstp_status.enabled) {
> -        /* Do not activate STP if RSTP is enabled. */
> -        VLOG_ERR("STP cannot be enabled if RSTP is running.");
> +    if (!enable_stp) {
>          ofproto_set_stp(br->ofproto, NULL);
> -        ovsrec_bridge_set_stp_enable(br->cfg, false);
>      } else {
>          struct ofproto_stp_settings br_s;
>          const char *config_str;
> @@ -1546,18 +1536,10 @@ bridge_configure_stp(struct bridge *br)
>  }
>
>  static void
> -bridge_configure_rstp(struct bridge *br)
> +bridge_configure_rstp(struct bridge *br, bool enable_rstp)
>  {
> -    struct ofproto_stp_status stp_status;
> -
> -    ofproto_get_stp_status(br->ofproto, &stp_status);
> -    if (!br->cfg->rstp_enable) {
> +    if (!enable_rstp) {
>          ofproto_set_rstp(br->ofproto, NULL);
> -    } else if (stp_status.enabled) {
> -        /* Do not activate RSTP if STP is enabled. */
> -        VLOG_ERR("RSTP cannot be enabled if STP is running.");
> -        ofproto_set_rstp(br->ofproto, NULL);
> -        ovsrec_bridge_set_rstp_enable(br->cfg, false);
>      } else {
>          struct ofproto_rstp_settings br_s;
>          const char *config_str;
> @@ -1652,6 +1634,22 @@ bridge_configure_rstp(struct bridge *br)
>      }
>  }
>
> +static void
> +bridge_configure_spanning_tree(struct bridge *br)
> +{
> +    bool enable_rstp = br->cfg->rstp_enable;
> +    bool enable_stp = br->cfg->stp_enable;
> +
> +    if (enable_rstp && enable_stp) {
> +        VLOG_WARN("%s: RSTP and STP are mutually exclusive but both are "
> +                  "configured; enabling RSTP", br->name);
Would you need to rate limit this VLOG_WARN() now?
> +        enable_stp = false;
> +    }
> +
> +    bridge_configure_stp(br, enable_stp);
> +    bridge_configure_rstp(br, enable_rstp);
> +}
> +
>  static bool
>  bridge_has_bond_fake_iface(const struct bridge *br, const char *name)
>  {
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to