On Mon, Oct 17, 2011 at 10:31:45PM -0700, Justin Pettit wrote: > At one point, the OVS distribution contained an IEEE 802.1D Spanning > Tree Protocol (STP) library written by Ben Pfaff and based on the > 802.1D-1998 reference code. It was never integrated into ovs-vswitchd, > so it was removed as part of commit ba18611 (Remove vestigial support > for Spanning Tree Protocol.) > > This commit reintroduces the library, cleans up a few spots, and makes > it build cleanly against new code. A future commit will have > ovs-vswitchd use this library.
"git am" says: Applying: Add back 802.1D Spanning Tree Protocol (STP) library code. /home/blp/db/.git/rebase-apply/patch:241: trailing whitespace. * arguments to 'send_bpdu' are an STP BPDU encapsulated in 'bpdu', /home/blp/db/.git/rebase-apply/patch:1547: trailing whitespace. # /home/blp/db/.git/rebase-apply/patch:1995: trailing whitespace. dump_lan_tree(struct test_case *tc, struct lan *lan, int level) /home/blp/db/.git/rebase-apply/patch:2259: trailing whitespace. lan = tc->lans[*token - 'a']; /home/blp/db/.git/rebase-apply/patch:1440: new blank line at EOF. + warning: 5 lines add whitespace errors. "sparse" says: ../lib/stp.c:980:1: warning: symbol 'stp_received_config_bpdu' was not declared. Should it be static? ../lib/stp.c:1018:1: warning: symbol 'stp_received_tcn_bpdu' was not declared. Should it be static? You should add 2011 to the copyright notices, at least if you changed anything. You should use ovs_be<N> for big-endian values: diff --git a/lib/stp.c b/lib/stp.c index f58d640..0bdd54b 100644 --- a/lib/stp.c +++ b/lib/stp.c @@ -40,7 +40,7 @@ VLOG_DEFINE_THIS_MODULE(stp); #define STP_TYPE_TCN 0x80 struct stp_bpdu_header { - uint16_t protocol_id; /* STP_PROTOCOL_ID. */ + ovs_be16 protocol_id; /* STP_PROTOCOL_ID. */ uint8_t protocol_version; /* STP_PROTOCOL_VERSION. */ uint8_t bpdu_type; /* One of STP_TYPE_*. */ } __attribute__((packed)); @@ -54,14 +54,14 @@ enum stp_config_bpdu_flags { struct stp_config_bpdu { struct stp_bpdu_header header; /* Type STP_TYPE_CONFIG. */ uint8_t flags; /* STP_CONFIG_* flags. */ - uint64_t root_id; /* 8.5.1.1: Bridge believed to be root. */ - uint32_t root_path_cost; /* 8.5.1.2: Cost of path to root. */ - uint64_t bridge_id; /* 8.5.1.3: ID of transmitting bridge. */ - uint16_t port_id; /* 8.5.1.4: Port transmitting the BPDU. */ - uint16_t message_age; /* 8.5.1.5: Age of BPDU at tx time. */ - uint16_t max_age; /* 8.5.1.6: Timeout for received data. */ - uint16_t hello_time; /* 8.5.1.7: Time between BPDU generation. */ - uint16_t forward_delay; /* 8.5.1.8: State progression delay. */ + ovs_be64 root_id; /* 8.5.1.1: Bridge believed to be root. */ + ovs_be32 root_path_cost; /* 8.5.1.2: Cost of path to root. */ + ovs_be64 bridge_id; /* 8.5.1.3: ID of transmitting bridge. */ + ovs_be16 port_id; /* 8.5.1.4: Port transmitting the BPDU. */ + ovs_be16 message_age; /* 8.5.1.5: Age of BPDU at tx time. */ + ovs_be16 max_age; /* 8.5.1.6: Timeout for received data. */ + ovs_be16 hello_time; /* 8.5.1.7: Time between BPDU generation. */ + ovs_be16 forward_delay; /* 8.5.1.8: State progression delay. */ } __attribute__((packed)); BUILD_ASSERT_DECL(sizeof(struct stp_config_bpdu) == 35); and then we get some useful warnings from sparse (at least with the patch to include/linux/types.h that I posted separately): ../lib/stp.c:718:26: warning: invalid assignment: |= ../lib/stp.c:718:26: left side has type unsigned char ../lib/stp.c:718:26: right side has type restricted __be16 ../lib/stp.c:721:26: warning: invalid assignment: |= ../lib/stp.c:721:26: left side has type unsigned char ../lib/stp.c:721:26: right side has type restricted __be16 ../lib/stp.c:779:49: warning: restricted __be16 degrades to integer ../lib/stp.c:1007:42: warning: restricted __be16 degrades to integer I think I saw that you fixed those in the next commit though. I didn't really read most of this patch, assuming that it was pretty much the same as what was in the tree before. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev