Acked-by: Joe Stringer <joestrin...@nicira.com>
On 16 July 2014 17:21, Alex Wang <al...@nicira.com> wrote:
> Commit 0d1cee123a84 (stp: Fix bpdu tx problem in listening state)
> makes ovs drop the stp bpdu packets if stp is not enabled on the
> input port. However, when pif bridge is used and stp is enabled
> on the integration bridge. The flow translation of stp bpdu
> packets will go through a level of resubmission which changes
> the input port to the corresponding peer port. Since, the
> patch port on the pif bridge does not have stp enabled, the
> flow translation will drop the bpdu packets.
>
> This commit fixes the issue by making ovs forward stp bpdu packets
> on stp-disabled port.
>
> VMware-BZ: #1284695
>
> Signed-off-by: Alex Wang <al...@nicira.com>
> Acked-by: Ben Pfaff <b...@nicira.com>
>
> ---
> PATCH->V2:
> - rename stp_listen_in_state() to stp_should_forward_bpdu().
> - add comments to explain the function and the forwarding of
> bpdus.
> ---
> lib/stp.c | 13 +++++++++----
> lib/stp.h | 28 ++++++++++++++++++++--------
> ofproto/ofproto-dpif-xlate.c | 6 +++---
> 3 files changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/lib/stp.c b/lib/stp.c
> index 218eb9a..92948ef 100644
> --- a/lib/stp.c
> +++ b/lib/stp.c
> @@ -684,13 +684,18 @@ stp_learn_in_state(enum stp_state state)
> return (state & (STP_DISABLED | STP_LEARNING | STP_FORWARDING)) != 0;
> }
>
> -/* Returns true if 'state' is one in which rx&tx bpdu should be done on
> - * on a port, false otherwise. */
> +/* Returns true if 'state' is one in which bpdus should be forwarded on a
> + * port, false otherwise.
> + *
> + * Returns true if 'state' is STP_DISABLED, since in that case the port
> does
> + * not generate the bpdu and should just forward it (e.g. patch port on
> pif
> + * bridge). */
> bool
> -stp_listen_in_state(enum stp_state state)
> +stp_should_forward_bpdu(enum stp_state state)
> {
> return (state &
> - (STP_LISTENING | STP_LEARNING | STP_FORWARDING)) != 0;
> + ( STP_DISABLED | STP_LISTENING | STP_LEARNING
> + | STP_FORWARDING)) != 0;
> }
>
> /* Returns the name for the given 'role' (for use in debugging and log
> diff --git a/lib/stp.h b/lib/stp.h
> index c0175cd..f0d2198 100644
> --- a/lib/stp.h
> +++ b/lib/stp.h
> @@ -99,13 +99,25 @@ bool stp_get_changed_port(struct stp *, struct
> stp_port **portp);
> * The following diagram describes the various states and what they are
> * allowed to do in OVS:
> *
> - * FWD LRN TX_BPDU RX_BPDU
> - * --- --- ------- -------
> - * Disabled Y - - -
> - * Blocking - - - Y
> - * Listening - - Y Y
> - * Learning - Y Y Y
> - * Forwarding Y Y Y Y
> + * FWD LRN TX_BPDU RX_BPDU FWD_BPDU
> + * --- --- ------- ------- --------
> + * Disabled Y - - - Y
> + * Blocking - - - Y -
> + * Listening - - Y Y Y
> + * Learning - Y Y Y Y
> + * Forwarding Y Y Y Y Y
> + *
> + *
> + * FWD: the port should forward any incomging non-stp-bpdu
> + * packets.
> + *
> + * LRN: the port should conduct MAC learning on packets
> received.
> + *
> + * TX_BPDU/RX_BPDU: the port could generate/consume bpdus.
> + *
> + * FWD_BPDU: the port should should always forward the bpdus,
> + * whether they are generated by the port or received
> + * as incoming packets.
> *
> * Once again, note that the disabled state forwards traffic, which is
> * likely different than the spec would indicate.
> @@ -120,7 +132,7 @@ enum stp_state {
> const char *stp_state_name(enum stp_state);
> bool stp_forward_in_state(enum stp_state);
> bool stp_learn_in_state(enum stp_state);
> -bool stp_listen_in_state(enum stp_state);
> +bool stp_should_forward_bpdu(enum stp_state);
>
> /* Role of an STP port. */
> enum stp_role {
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index e930521..4aedb59 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -1101,10 +1101,10 @@ xport_stp_forward_state(const struct xport *xport)
> }
>
> static bool
> -xport_stp_listen_state(const struct xport *xport)
> +xport_stp_should_forward_bpdu(const struct xport *xport)
> {
> struct stp_port *sp = xport_get_stp_port(xport);
> - return stp_listen_in_state(sp ? stp_port_get_state(sp) :
> STP_DISABLED);
> + return stp_should_forward_bpdu(sp ? stp_port_get_state(sp) :
> STP_DISABLED);
> }
>
> /* Returns true if STP should process 'flow'. Sets fields in 'wc' that
> @@ -2376,7 +2376,7 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
> return;
> } else if (check_stp) {
> if (is_stp(&ctx->base_flow)) {
> - if (!xport_stp_listen_state(xport)) {
> + if (!xport_stp_should_forward_bpdu(xport)) {
> xlate_report(ctx, "STP not in listening state, "
> "skipping bpdu output");
> return;
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
