[ovs-dev] [PATCH 3/5] ofp-util: Add ofputil_switch_features_ports_trunc function.
Justin Pettit
jpettit at nicira.com
Tue May 8 07:10:41 UTC 2012
On May 7, 2012, at 4:59 PM, Ben Pfaff wrote:
> On Mon, May 07, 2012 at 11:56:50AM -0700, Justin Pettit wrote:
>> Add function to determine whether the max number of ports are contains
>> in a Features Reply. If so, it removes the port list, since it may be
>> incomplete. This function will be used in a later commit.
>>
>> Signed-off-by: Justin Pettit <jpettit at nicira.com>
>
> Now we have several bits of code that translate an ofp version into a
> struct size. I count three, in ofputil_decode_switch_features(),
> max_ports_in_features(), and ofputil_count_phy_ports(). It'd be
> better to just have a single helper to do that.
Yeah, the thought had crossed my mind. :-) I've attached a commit to do that, which would follow this one in the series. (Congratulations on finally getting referenced in the commit log!)
> In ofputil_switch_features_ports_trunc() you can just call
> update_openflow_length() instead of inlining it as:
>> + osf->header.length = htons(sizeof(*osf));
Nice. Thanks.
--Justin
-----------------
commit cb686d6b757eb4000a27b920cc65721753462877
Author: Justin Pettit <jpettit at nicira.com>
Date: Tue May 8 00:01:11 2012 -0700
ofp-util: Factor out determining physical port size.
There are a few places where we determine the size of a physical port
structure based on the OpenFlow version. Use a helper function to do
that.
Suggested-by: Ben Pfaff <blp at nicira.com>
Signed-off-by: Justin Pettit <jpettit at nicira.com>
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 69e2b17..48be774 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2371,6 +2371,13 @@ ofputil_decode_ofp11_port(struct ofputil_phy_port *pp,
return 0;
}
+static size_t
+ofputil_get_phy_port_size(uint8_t ofp_version)
+{
+ return ofp_version == OFP10_VERSION ? sizeof(struct ofp10_phy_port)
+ : sizeof(struct ofp11_port);
+}
+
static void
ofputil_encode_ofp10_phy_port(const struct ofputil_phy_port *pp,
struct ofp10_phy_port *opp)
@@ -2543,20 +2550,17 @@ ofputil_decode_switch_features(const struct ofp_switch_f
features->n_tables = osf->n_tables;
features->capabilities = ntohl(osf->capabilities) & OFPC_COMMON;
- if (osf->header.version == OFP10_VERSION) {
- if (b->size % sizeof(struct ofp10_phy_port)) {
- return OFPERR_OFPBRC_BAD_LEN;
- }
+ if (b->size % ofputil_get_phy_port_size(osf->header.version)) {
+ return OFPERR_OFPBRC_BAD_LEN;
+ }
+
+ if (osf->header.version == OFP10_VERSION) {
if (osf->capabilities & htonl(OFPC10_STP)) {
features->capabilities |= OFPUTIL_C_STP;
}
features->actions = decode_action_bits(osf->actions, of10_action_bits);
} else if (osf->header.version == OFP11_VERSION) {
- if (b->size % sizeof(struct ofp11_port)) {
- return OFPERR_OFPBRC_BAD_LEN;
- }
-
if (osf->capabilities & htonl(OFPC11_GROUP_STATS)) {
features->capabilities |= OFPUTIL_C_GROUP_STATS;
}
@@ -2572,10 +2576,7 @@ ofputil_decode_switch_features(const struct ofp_switch_fe
static bool
max_ports_in_features(const struct ofp_switch_features *osf)
{
- size_t pp_size = osf->header.version == OFP10_VERSION ?
- sizeof(struct ofp10_phy_port) :
- sizeof(struct ofp11_port);
-
+ size_t pp_size = ofputil_get_phy_port_size(osf->header.version);
return ntohs(osf->header.length) + pp_size > UINT16_MAX;
}
@@ -3403,9 +3404,7 @@ ofputil_pull_phy_port(uint8_t ofp_version, struct ofpbuf *
* 'ofp_version', returns the number of elements. */
size_t ofputil_count_phy_ports(uint8_t ofp_version, struct ofpbuf *b)
{
- return (ofp_version == OFP10_VERSION
- ? b->size / sizeof(struct ofp10_phy_port)
- : b->size / sizeof(struct ofp11_port));
+ return b->size / ofputil_get_phy_port_size(ofp_version);
}
static enum ofperr
More information about the dev
mailing list