[ovs-dev] [STP 6/9] Various bug fixes and cleanups to STP library.

[Ben Pfaff]

On Mon, Oct 17, 2011 at 10:31:46PM -0700, Justin Pettit wrote:
>     - Don't apply endian conversions to flags, which are 8 bits.
>     - Use #defines for default times for use outside library.
>     - Clarify our behavior when in STP_DISABLED state.
>     - Add "aux" member to STP port struct to be able to refer back to
>       the owning port.
>     - Define macros to print STP bridge and port ids.
>     - New helper function to get port id.
>     - New helper function to convert speed to cost.
>     - New functions to describe current role of port.

STP_ID_ARGS should parenthesize the name of its argument, in case
someone passes in a complicated expression (unlikely as that is).

But can't the ID formatting macros be simplified to:

#define STP_ID_FMT "%04"PRIx16"."012"PRIx64
#define STP_ID_ARG(stp_id) \
    (uint16_t)((stp_id) >> 48), \
    (uint64_t)((stp_id) & 0xffffffffffffULL)

and I'm not sure that we really need port id macros at all.  I think
that "%04"PRIx16 works OK.  (But it might make sense to use macros for
that; it's a reasonable choice.)

Can the STP_ROLE_* constants have some comments?  Maybe "Port closest
to root bridge.", "Forwards frames toward/away from root.", "Not used
for forwarding." or similar.

Should there be a role for "disabled" ports?  I think that they are
technically different from alternate ports.

Thanks,

Ben.