Daniele,
See comments below.
Also, it would be preferable to send related changes, or multiple unrelated
changes to any given subsystem, as a series of patches instead of individual
ones. “git format-patch” does that automatically.
Jarno
On Sep 10, 2014, at 1:28 AM, Daniele Venturino <daniele.ventur...@m3s.it> wrote:
> See commit bacdb85ad82f981697245eefb40a3b360cfe379b.
>
> Signed-off by: Daniele Venturino <daniele.ventur...@m3s.it>
>
> ---
> lib/rstp.h | 42 +++++++++++++++++++++++++++++++++++++++---
> ofproto/ofproto-dpif-xlate.c | 6 +++---
> 2 files changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/lib/rstp.h b/lib/rstp.h
> index 364a181..b15d22f 100644
> --- a/lib/rstp.h
> +++ b/lib/rstp.h
> @@ -99,6 +99,38 @@ typedef uint64_t rstp_identifier;
>
> #define RSTP_PORT_ID_FMT "%04"PRIx16
>
> +/* State of an RSTP port.
> + *
> + * The RSTP_DISABLED state means that the port is disabled by management.
> + * In our implementation, this state means that the port does not
> + * participate in the spanning tree, but it still forwards traffic as if
> + * it were in the RSTP_FORWARDING state. This may be different from
> + * other implementations.
> + *
> + * The following diagram describes the various states and what they are
> + * allowed to do in OVS:
> + *
> + * FWD LRN TX_BPDU RX_BPDU FWD_BPDU
> + * --- --- ------- ------- --------
> + * Disabled Y - - - Y
> + * Discarding - - Y Y Y
> + * Learning - Y Y Y Y
> + * Forwarding Y Y Y Y Y
> + *
> + *
> + * FWD: the port should forward any incoming non-rstp-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.
> + *
> + */
> +
> enum rstp_state {
> RSTP_DISABLED,
> RSTP_LEARNING,
> @@ -128,7 +160,7 @@ const char *rstp_state_name(enum rstp_state);
> const char *rstp_port_role_name(enum rstp_port_role);
> static inline bool rstp_forward_in_state(enum rstp_state);
> static inline bool rstp_learn_in_state(enum rstp_state);
> -static inline bool rstp_should_manage_bpdu(enum rstp_state state);
> +static inline bool rstp_should_forward_bpdu(enum rstp_state state);
>
> void rstp_init(void)
> OVS_EXCLUDED(rstp_mutex);
> @@ -255,12 +287,16 @@ void rstp_port_set_state(struct rstp_port *p, enum
> rstp_state state)
> /* Inline functions. */
> /* Returns true if 'state' is one in which BPDU packets should be received
> * and transmitted on a port, false otherwise.
> + *
> + * Returns true if 'state' is RSTP_DISABLED, since in that case the port does
> + * not generate the bpdu and should just forward it (e.g. patch port on pif
> + * bridge).
> */
> static inline bool
> -rstp_should_manage_bpdu(enum rstp_state state)
> +rstp_should_forward_bpdu(enum rstp_state state)
> {
> return (state == RSTP_DISCARDING || state == RSTP_LEARNING ||
> - state == RSTP_FORWARDING);
> + state == RSTP_FORWARDING || state == RSTP_DISABLED);
> }
This now returns true for all the possible states, so this should be removed.
>
> /* Returns true if 'state' is one in which packets received on a port should
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 786494d..51d25d4 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -1193,9 +1193,9 @@ xport_rstp_forward_state(const struct xport *xport)
> }
>
> static bool
> -xport_rstp_should_manage_bpdu(const struct xport *xport)
> +xport_rstp_should_forward_bpdu(const struct xport *xport)
> {
> - return rstp_should_manage_bpdu(xport_get_rstp_port_state(xport));
> + return rstp_should_forward_bpdu(xport_get_rstp_port_state(xport));
> }
>
Ditto.
> static void
> @@ -2493,7 +2493,7 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
> } else if (check_stp) {
> if (is_stp(&ctx->base_flow)) {
> if (!xport_stp_should_forward_bpdu(xport) &&
> - !xport_rstp_should_manage_bpdu(xport)) {
> + !xport_rstp_should_forward_bpdu(xport)) {
Maybe the same is true for sport_stp_should_forward_bpdu()? At least the
comment in stp.h states that BPDUs should always be forwarded.
> if (ctx->xbridge->stp != NULL) {
> xlate_report(ctx, "STP not in listening state, "
> "skipping bpdu output");
> --
> 1.8.1.2
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
