[ovs-dev] [PATCH 10/17] rstp: disable learning and forwarding in STP/RSTP disabled state.

[Jarno Rajahalme]

This patch also changes STP, so the subject line should start with “stp, rstp:” and the commit message should not have indentation. Otherwise:

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>

Thanks!

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

>      There is a difference between a port with STP/RSTP protocol enabled and a
>      disabled role and a port which has a disabled role because STP/RSTP is
>      not active. This commit ensure to make such distinction.
> 
>      Standard 802.1D claims that the Topology Change state machine (17.31)
>      treats a Port as no longer active when it becomes an Alternate, Backup,
>      or Disabled Port and stops learning from received frames.
> 
> Signed-off-by: Daniele Venturino <daniele.venturino at m3s.it>
> ---
> lib/rstp.h                   | 12 +++---------
> lib/stp.c                    | 12 ++++--------
> ofproto/ofproto-dpif-xlate.c | 16 ++++++++++++----
> ofproto/ofproto-dpif.c       |  6 ++++--
> 4 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/lib/rstp.h b/lib/rstp.h
> index 458aecf..b429e35 100644
> --- a/lib/rstp.h
> +++ b/lib/rstp.h
> @@ -274,26 +274,20 @@ rstp_should_manage_bpdu(enum rstp_state state)
> 
> /* Returns true if 'state' is one in which packets received on a port should
>  * be forwarded, false otherwise.
> - *
> - * Returns true if 'state' is RSTP_DISABLED, since presumably in that case the
> - * port should still work, just not have RSTP applied to it.
>  */
> static inline bool
> rstp_forward_in_state(enum rstp_state state)
> {
> -    return (state == RSTP_DISABLED || state == RSTP_FORWARDING);
> +    return (state == RSTP_FORWARDING);
> }
> 
> /* Returns true if 'state' is one in which MAC learning should be done on
>  * packets received on a port, false otherwise.
> - *
> - * Returns true if 'state' is RSTP_DISABLED, since presumably in that case the
> - * port should still work, just not have RSTP applied to it. */
> + */
> static inline bool
> rstp_learn_in_state(enum rstp_state state)
> {
> -    return (state == RSTP_DISABLED || state == RSTP_LEARNING ||
> -            state == RSTP_FORWARDING);
> +    return (state == RSTP_LEARNING || state == RSTP_FORWARDING);
> }
> 
> #endif /* rstp.h */
> diff --git a/lib/stp.c b/lib/stp.c
> index 8f904c0..9055922 100644
> --- a/lib/stp.c
> +++ b/lib/stp.c
> @@ -673,24 +673,20 @@ stp_state_name(enum stp_state state)
> 
> /* Returns true if 'state' is one in which packets received on a port should
>  * be forwarded, false otherwise.
> - *
> - * Returns true if 'state' is STP_DISABLED, since presumably in that case the
> - * port should still work, just not have STP applied to it. */
> + */
> bool
> stp_forward_in_state(enum stp_state state)
> {
> -    return (state & (STP_DISABLED | STP_FORWARDING)) != 0;
> +    return (state & STP_FORWARDING) != 0;
> }
> 
> /* Returns true if 'state' is one in which MAC learning should be done on
>  * packets received on a port, false otherwise.
> - *
> - * Returns true if 'state' is STP_DISABLED, since presumably in that case the
> - * port should still work, just not have STP applied to it. */
> + */
> bool
> stp_learn_in_state(enum stp_state state)
> {
> -    return (state & (STP_DISABLED | STP_LEARNING | STP_FORWARDING)) != 0;
> +    return (state & (STP_LEARNING | STP_FORWARDING)) != 0;
> }
> 
> /* Returns true if 'state' is one in which bpdus should be forwarded on a
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 53ec297..b230c52 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -1132,14 +1132,18 @@ static bool
> xport_stp_learn_state(const struct xport *xport)
> {
>     struct stp_port *sp = xport_get_stp_port(xport);
> -    return stp_learn_in_state(sp ? stp_port_get_state(sp) : STP_DISABLED);
> +    return sp
> +        ? stp_learn_in_state(stp_port_get_state(sp))
> +        : true;
> }
> 
> static bool
> xport_stp_forward_state(const struct xport *xport)
> {
>     struct stp_port *sp = xport_get_stp_port(xport);
> -    return stp_forward_in_state(sp ? stp_port_get_state(sp) : STP_DISABLED);
> +    return sp
> +        ? stp_forward_in_state(stp_port_get_state(sp))
> +        : true;
> }
> 
> static bool
> @@ -1193,13 +1197,17 @@ xport_get_rstp_port_state(const struct xport *xport)
> static bool
> xport_rstp_learn_state(const struct xport *xport)
> {
> -    return rstp_learn_in_state(xport_get_rstp_port_state(xport));
> +    return xport->xbridge->rstp && xport->rstp_port
> +        ? rstp_learn_in_state(xport_get_rstp_port_state(xport))
> +        : true;
> }
> 
> static bool
> xport_rstp_forward_state(const struct xport *xport)
> {
> -    return rstp_forward_in_state(xport_get_rstp_port_state(xport));
> +    return xport->xbridge->rstp && xport->rstp_port
> +        ? rstp_forward_in_state(xport_get_rstp_port_state(xport))
> +        : true;
> }
> 
> static bool
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 95298f2..dc0ecdf 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2520,7 +2520,8 @@ bundle_update(struct ofbundle *bundle)
>     LIST_FOR_EACH (port, bundle_node, &bundle->ports) {
>         if (port->up.pp.config & OFPUTIL_PC_NO_FLOOD
>             || port->is_layer3
> -            || !stp_forward_in_state(port->stp_state)) {
> +            || (bundle->ofproto->stp && !stp_forward_in_state(port->stp_state))
> +            || (bundle->ofproto->rstp && !rstp_forward_in_state(port->rstp_state))) {
>             bundle->floodable = false;
>             break;
>         }
> @@ -2568,7 +2569,8 @@ bundle_add_port(struct ofbundle *bundle, ofp_port_t ofp_port,
>         list_push_back(&bundle->ports, &port->bundle_node);
>         if (port->up.pp.config & OFPUTIL_PC_NO_FLOOD
>             || port->is_layer3
> -            || !stp_forward_in_state(port->stp_state)) {
> +            || (bundle->ofproto->stp && !stp_forward_in_state(port->stp_state))
> +            || (bundle->ofproto->rstp && !rstp_forward_in_state(port->rstp_state))) {
>             bundle->floodable = false;
>         }
>     }
> -- 
> 1.8.1.2
>