Looks Good. Ethan
On Mon, May 2, 2011 at 12:58, Ben Pfaff <[email protected]> wrote: > The normalize_match() function does more work than really needed. It goes > to some trouble to zero out fields that are wildcarded. This is not > necessary, because cls_rule_from_match() will take care of it later. > > Also make normalize_match() private to ofp-util.c, since it has no other > users now and I don't expect more later. > --- > lib/ofp-util.c | 58 +++++++------------------------------------------------ > lib/ofp-util.h | 1 - > 2 files changed, 8 insertions(+), 51 deletions(-) > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index ade0756..7772c47 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -36,6 +36,8 @@ > > VLOG_DEFINE_THIS_MODULE(ofp_util); > > +static void normalize_match(struct ofp_match *); > + > /* Rate limit for OpenFlow message parse errors. These always indicate a bug > * in the peer and so there's not much point in showing a lot of them. */ > static struct vlog_rate_limit bad_ofmsg_rl = VLOG_RATE_LIMIT_INIT(1, 5); > @@ -2028,7 +2030,7 @@ actions_next(struct actions_iterator *iter) > } > } > > -void > +static void > normalize_match(struct ofp_match *m) > { > enum { OFPFW_NW = (OFPFW_NW_SRC_ALL | OFPFW_NW_DST_ALL | OFPFW_NW_PROTO > @@ -2038,74 +2040,30 @@ normalize_match(struct ofp_match *m) > > wc = ntohl(m->wildcards) & OFPFW_ALL; > if (wc & OFPFW_DL_TYPE) { > - m->dl_type = 0; > - > /* Can't sensibly match on network or transport headers if the > * data link type is unknown. */ > wc |= OFPFW_NW | OFPFW_TP; > - m->nw_src = m->nw_dst = m->nw_proto = m->nw_tos = 0; > - m->tp_src = m->tp_dst = 0; > } else if (m->dl_type == htons(ETH_TYPE_IP)) { > if (wc & OFPFW_NW_PROTO) { > - m->nw_proto = 0; > - > /* Can't sensibly match on transport headers if the network > * protocol is unknown. */ > wc |= OFPFW_TP; > - m->tp_src = m->tp_dst = 0; > - } else if (m->nw_proto == IPPROTO_TCP || > - m->nw_proto == IPPROTO_UDP || > - m->nw_proto == IPPROTO_ICMP) { > - if (wc & OFPFW_TP_SRC) { > - m->tp_src = 0; > - } > - if (wc & OFPFW_TP_DST) { > - m->tp_dst = 0; > - } > - } else { > + } else if (m->nw_proto != IPPROTO_TCP && > + m->nw_proto != IPPROTO_UDP && > + m->nw_proto != IPPROTO_ICMP) { > /* Transport layer fields will always be extracted as zeros, so we > * can do an exact-match on those values. */ > wc &= ~OFPFW_TP; > m->tp_src = m->tp_dst = 0; > } > - if (wc & OFPFW_NW_SRC_MASK) { > - m->nw_src &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_SRC_SHIFT); > - } > - if (wc & OFPFW_NW_DST_MASK) { > - m->nw_dst &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_DST_SHIFT); > - } > - if (wc & OFPFW_NW_TOS) { > - m->nw_tos = 0; > - } else { > - m->nw_tos &= IP_DSCP_MASK; > - } > - } else if (m->dl_type == htons(ETH_TYPE_ARP)) { > - if (wc & OFPFW_NW_PROTO) { > - m->nw_proto = 0; > - } > - if (wc & OFPFW_NW_SRC_MASK) { > - m->nw_src &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_SRC_SHIFT); > - } > - if (wc & OFPFW_NW_DST_MASK) { > - m->nw_dst &= ofputil_wcbits_to_netmask(wc >> OFPFW_NW_DST_SHIFT); > - } > - m->tp_src = m->tp_dst = m->nw_tos = 0; > - } else if (m->dl_type == htons(ETH_TYPE_IPV6)) { > - /* Don't normalize IPv6 traffic, since OpenFlow doesn't have a > - * way to express it. */ > - } else { > + } else if (m->dl_type != htons(ETH_TYPE_ARP) && > + m->dl_type != htons(ETH_TYPE_IPV6)) { > /* Network and transport layer fields will always be extracted as > * zeros, so we can do an exact-match on those values. */ > wc &= ~(OFPFW_NW | OFPFW_TP); > m->nw_proto = m->nw_src = m->nw_dst = m->nw_tos = 0; > m->tp_src = m->tp_dst = 0; > } > - if (wc & OFPFW_DL_SRC) { > - memset(m->dl_src, 0, sizeof m->dl_src); > - } > - if (wc & OFPFW_DL_DST) { > - memset(m->dl_dst, 0, sizeof m->dl_dst); > - } > m->wildcards = htonl(wc); > } > > diff --git a/lib/ofp-util.h b/lib/ofp-util.h > index 6e69bae..8904404 100644 > --- a/lib/ofp-util.h > +++ b/lib/ofp-util.h > @@ -101,7 +101,6 @@ int ofputil_netmask_to_wcbits(ovs_be32 netmask); > void ofputil_cls_rule_from_match(const struct ofp_match *, > unsigned int priority, struct cls_rule *); > void ofputil_cls_rule_to_match(const struct cls_rule *, struct ofp_match *); > -void normalize_match(struct ofp_match *); > char *ofp_match_to_literal_string(const struct ofp_match *match); > > /* dl_type translation between OpenFlow and 'struct flow' format. */ > -- > 1.7.4.4 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
