[ovs-dev] [PATCH] lib/rstp: Make rstp-disabled port forward stp bpdu packets.
Daniele Venturino
daniele.venturino at m3s.it
Thu Nov 20 19:35:04 UTC 2014
We didn’t merge this patch back in september.
Were you suggesting to remove both stp_should_forward_bpdu() and rstp_should_manage_bpdu() ?
Regards,
Daniele
> Il giorno 11/set/2014, alle ore 16:11, Jarno Rajahalme <jrajahalme at nicira.com> ha scritto:
>
>
>
> Sent from my iPhone
>
> On Sep 11, 2014, at 2:08 AM, Daniele Venturino <daniele.venturino at m3s.it <mailto:daniele.venturino at m3s.it>> wrote:
>
>>
>> Il giorno 10/set/2014, alle ore 18:54, Jarno Rajahalme <jrajahalme at nicira.com <mailto:jrajahalme at nicira.com>> ha scritto:
>>
>>> 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
>>>
>>
>> I’ll send the next group of patches as a series.
>>
>>> On Sep 10, 2014, at 1:28 AM, Daniele Venturino <daniele.venturino at m3s.it <mailto:daniele.venturino at m3s.it>> wrote:
>>>
>>>> See commit bacdb85ad82f981697245eefb40a3b360cfe379b.
>>>>
>>>> Signed-off by: Daniele Venturino <daniele.venturino at m3s.it <mailto:daniele.venturino at 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.
>>
>> You’re right. I left it there to show that the check was done on both protocols, but I guess it can be removed since it’s always true.
>>
>>>
>>>> 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.
>>
>> Right now STP forwards BPDUs in all states but the initial state STP_BLOCKING.
>
> I don't recall it now offhand, but that state could be transitory and in effect only during initialization, I.e., never when BPDUs are received.
>
> Jarno
>
>>
>>>
>>>> if (ctx->xbridge->stp != NULL) {
>>>> xlate_report(ctx, "STP not in listening state, "
>>>> "skipping bpdu output");
>>>> --
>>>> 1.8.1.2
>>
>> Daniele
More information about the dev
mailing list