From 3cbd99318d04d07cfce8469711fb0398a832c8a7 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 7 Aug 2012 11:30:46 -0700 Subject: [PATCH] learning-switch: Don't use exact-match on every field by default. OVS has all kinds of odd fields, e.g. registers, and it doesn't make sense to try to match on all of them. This commit changes learning-switch to only try to match on the fields defined by OpenFlow 1.0. That's still not minimal, but it's more reasonable. This commit should not have an immediately visible effect since ovs-controller always sends OF1.0 format flows to the switch, and OF1.0 format flows don't have these extra fields. But in the future when we add support for new protocols and flow formats to ovs-controller, it will make a difference. Signed-off-by: Ben Pfaff --- lib/learning-switch.c | 35 +++++++++++++++------------- lib/ofp-util.c | 54 ++++++++++++++++++++++++++++--------------- lib/ofp-util.h | 1 + 3 files changed, 56 insertions(+), 34 deletions(-) diff --git a/lib/learning-switch.c b/lib/learning-switch.c index f52d3c9d..e4287c4e 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -112,6 +112,7 @@ struct lswitch * lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg) { struct lswitch *sw; + uint32_t ofpfw; sw = xzalloc(sizeof *sw); sw->rconn = rconn; @@ -123,24 +124,26 @@ lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg) : NULL); sw->action_normal = cfg->mode == LSW_NORMAL; - flow_wildcards_init_exact(&sw->wc); - if (cfg->wildcards) { - uint32_t ofpfw; - - if (cfg->wildcards == UINT32_MAX) { - /* Try to wildcard as many fields as possible, but we cannot - * wildcard all fields. We need in_port to detect moves. We need - * Ethernet source and dest and VLAN VID to do L2 learning. */ - ofpfw = (OFPFW10_DL_TYPE | OFPFW10_DL_VLAN_PCP - | OFPFW10_NW_SRC_ALL | OFPFW10_NW_DST_ALL - | OFPFW10_NW_TOS | OFPFW10_NW_PROTO - | OFPFW10_TP_SRC | OFPFW10_TP_DST); - } else { - ofpfw = cfg->wildcards; - } + switch (cfg->wildcards) { + case 0: + ofpfw = 0; + break; - ofputil_wildcard_from_ofpfw10(ofpfw, &sw->wc); + case UINT32_MAX: + /* Try to wildcard as many fields as possible, but we cannot + * wildcard all fields. We need in_port to detect moves. We need + * Ethernet source and dest and VLAN VID to do L2 learning. */ + ofpfw = (OFPFW10_DL_TYPE | OFPFW10_DL_VLAN_PCP + | OFPFW10_NW_SRC_ALL | OFPFW10_NW_DST_ALL + | OFPFW10_NW_TOS | OFPFW10_NW_PROTO + | OFPFW10_TP_SRC | OFPFW10_TP_DST); + break; + + default: + ofpfw = cfg->wildcards; + break; } + ofputil_wildcard_from_ofpfw10(ofpfw, &sw->wc); sw->default_queue = cfg->default_queue; hmap_init(&sw->queue_numbers); diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 1fe30b46..030c8129 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -3319,23 +3319,8 @@ ofputil_put_action(enum ofputil_action_code code, struct ofpbuf *buf) } #include "ofp-util.def" -/* "Normalizes" the wildcards in 'rule'. That means: - * - * 1. If the type of level N is known, then only the valid fields for that - * level may be specified. For example, ARP does not have a TOS field, - * so nw_tos must be wildcarded if 'rule' specifies an ARP flow. - * Similarly, IPv4 does not have any IPv6 addresses, so ipv6_src and - * ipv6_dst (and other fields) must be wildcarded if 'rule' specifies an - * IPv4 flow. - * - * 2. If the type of level N is not known (or not understood by Open - * vSwitch), then no fields at all for that level may be specified. For - * example, Open vSwitch does not understand SCTP, an L4 protocol, so the - * L4 fields tp_src and tp_dst must be wildcarded if 'rule' specifies an - * SCTP flow. - */ -void -ofputil_normalize_rule(struct cls_rule *rule) +static void +ofputil_normalize_rule__(struct cls_rule *rule, bool may_log) { enum { MAY_NW_ADDR = 1 << 0, /* nw_src, nw_dst */ @@ -3409,7 +3394,7 @@ ofputil_normalize_rule(struct cls_rule *rule) /* Log any changes. */ if (!flow_wildcards_equal(&wc, &rule->wc)) { - bool log = !VLOG_DROP_INFO(&bad_ofmsg_rl); + bool log = may_log && !VLOG_DROP_INFO(&bad_ofmsg_rl); char *pre = log ? cls_rule_to_string(rule) : NULL; rule->wc = wc; @@ -3426,6 +3411,39 @@ ofputil_normalize_rule(struct cls_rule *rule) } } +/* "Normalizes" the wildcards in 'rule'. That means: + * + * 1. If the type of level N is known, then only the valid fields for that + * level may be specified. For example, ARP does not have a TOS field, + * so nw_tos must be wildcarded if 'rule' specifies an ARP flow. + * Similarly, IPv4 does not have any IPv6 addresses, so ipv6_src and + * ipv6_dst (and other fields) must be wildcarded if 'rule' specifies an + * IPv4 flow. + * + * 2. If the type of level N is not known (or not understood by Open + * vSwitch), then no fields at all for that level may be specified. For + * example, Open vSwitch does not understand SCTP, an L4 protocol, so the + * L4 fields tp_src and tp_dst must be wildcarded if 'rule' specifies an + * SCTP flow. + * + * If this function changes 'rule', it logs a rate-limited informational + * message. */ +void +ofputil_normalize_rule(struct cls_rule *rule) +{ + ofputil_normalize_rule__(rule, true); +} + +/* Same as ofputil_normalize_rule() without the logging. Thus, this function + * is suitable for a program's internal use, whereas ofputil_normalize_rule() + * sense for use on flows received from elsewhere (so that a bug in the program + * that sent them can be reported and corrected). */ +void +ofputil_normalize_rule_quiet(struct cls_rule *rule) +{ + ofputil_normalize_rule__(rule, false); +} + /* Parses a key or a key-value pair from '*stringp'. * * On success: Stores the key into '*keyp'. Stores the value, if present, into diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 64f6c39c..7b30e878 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -114,6 +114,7 @@ void ofputil_cls_rule_from_ofp10_match(const struct ofp10_match *, unsigned int priority, struct cls_rule *); void ofputil_normalize_rule(struct cls_rule *); +void ofputil_normalize_rule_quiet(struct cls_rule *); void ofputil_cls_rule_to_ofp10_match(const struct cls_rule *, struct ofp10_match *); -- 2.30.2