From 14608a1539b73f8f9812e0e791adb60825fee38b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 11 Oct 2010 13:31:35 -0700 Subject: [PATCH] flow: Separate "flow_t" from "struct odp_flow_key". The "struct odp_flow_key" used in the kernel datapath is conceptually separate from the "flow_t" used in userspace, but until now we have used the latter as a typedef for the former for convenience. This commit separates them. This makes it possible in upcoming commits to change them independently. This is cross-ported from the "wdp" branch, which has had it for months. --- lib/classifier.c | 1 - lib/dpif-netdev.c | 54 +++++++++++++++++++++++------------------ lib/dpif.c | 5 ++-- lib/flow.c | 2 -- lib/flow.h | 31 +++++++++++++++++++---- lib/odp-util.c | 51 +++++++++++++++++++++++++++++++++++++- lib/odp-util.h | 21 ++++++++++++++++ ofproto/ofproto.c | 15 +++++++----- tests/test-classifier.c | 1 - 9 files changed, 140 insertions(+), 41 deletions(-) diff --git a/lib/classifier.c b/lib/classifier.c index 8d711a15..9a8d4944 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -58,7 +58,6 @@ void cls_rule_from_flow(const flow_t *flow, uint32_t wildcards, unsigned int priority, struct cls_rule *rule) { - assert(!flow->reserved[0] && !flow->reserved[1] && !flow->reserved[2]); rule->flow = *flow; flow_wildcards_init(&rule->wc, wildcards); rule->priority = priority; diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 11ad370f..2fb13391 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -94,7 +94,7 @@ struct dp_netdev_port { /* A flow in dp_netdev's 'flow_table'. */ struct dp_netdev_flow { struct hmap_node node; /* Element in dp_netdev's 'flow_table'. */ - flow_t key; + struct flow key; /* Statistics. */ struct timespec used; /* Last used time. */ @@ -135,7 +135,7 @@ static int do_del_port(struct dp_netdev *, uint16_t port_no); static int dp_netdev_output_control(struct dp_netdev *, const struct ofpbuf *, int queue_no, int port_no, uint32_t arg); static int dp_netdev_execute_actions(struct dp_netdev *, - struct ofpbuf *, const flow_t *, + struct ofpbuf *, struct flow *, const union odp_action *, int n); static struct dpif_netdev * @@ -588,11 +588,10 @@ dpif_netdev_port_poll_wait(const struct dpif *dpif_) } static struct dp_netdev_flow * -dp_netdev_lookup_flow(const struct dp_netdev *dp, const flow_t *key) +dp_netdev_lookup_flow(const struct dp_netdev *dp, const struct flow *key) { struct dp_netdev_flow *flow; - assert(!key->reserved[0] && !key->reserved[1] && !key->reserved[2]); HMAP_FOR_EACH_WITH_HASH (flow, node, flow_hash(key, 0), &dp->flow_table) { if (flow_equal(&flow->key, key)) { return flow; @@ -601,12 +600,12 @@ dp_netdev_lookup_flow(const struct dp_netdev *dp, const flow_t *key) return NULL; } +/* The caller must fill in odp_flow->key itself. */ static void answer_flow_query(struct dp_netdev_flow *flow, uint32_t query_flags, struct odp_flow *odp_flow) { if (flow) { - odp_flow->key = flow->key; odp_flow->stats.n_packets = flow->packet_count; odp_flow->stats.n_bytes = flow->byte_count; odp_flow->stats.used_sec = flow->used.tv_sec; @@ -638,7 +637,10 @@ dpif_netdev_flow_get(const struct dpif *dpif, struct odp_flow flows[], int n) for (i = 0; i < n; i++) { struct odp_flow *odp_flow = &flows[i]; - answer_flow_query(dp_netdev_lookup_flow(dp, &odp_flow->key), + struct flow key; + + odp_flow_key_to_flow(&odp_flow->key, &key); + answer_flow_query(dp_netdev_lookup_flow(dp, &key), odp_flow->flags, odp_flow); } return 0; @@ -732,8 +734,7 @@ add_flow(struct dpif *dpif, struct odp_flow *odp_flow) int error; flow = xzalloc(sizeof *flow); - flow->key = odp_flow->key; - memset(flow->key.reserved, 0, sizeof flow->key.reserved); + odp_flow_key_to_flow(&odp_flow->key, &flow->key); error = set_flow_actions(flow, odp_flow); if (error) { @@ -760,8 +761,10 @@ dpif_netdev_flow_put(struct dpif *dpif, struct odp_flow_put *put) { struct dp_netdev *dp = get_dp_netdev(dpif); struct dp_netdev_flow *flow; + struct flow key; - flow = dp_netdev_lookup_flow(dp, &put->flow.key); + odp_flow_key_to_flow(&put->flow.key, &key); + flow = dp_netdev_lookup_flow(dp, &key); if (!flow) { if (put->flags & ODPPF_CREATE) { if (hmap_count(&dp->flow_table) < MAX_FLOWS) { @@ -791,8 +794,10 @@ dpif_netdev_flow_del(struct dpif *dpif, struct odp_flow *odp_flow) { struct dp_netdev *dp = get_dp_netdev(dpif); struct dp_netdev_flow *flow; + struct flow key; - flow = dp_netdev_lookup_flow(dp, &odp_flow->key); + odp_flow_key_to_flow(&odp_flow->key, &key); + flow = dp_netdev_lookup_flow(dp, &key); if (flow) { answer_flow_query(flow, 0, odp_flow); dp_netdev_free_flow(dp, flow); @@ -814,7 +819,10 @@ dpif_netdev_flow_list(const struct dpif *dpif, struct odp_flow flows[], int n) if (i >= n) { break; } - answer_flow_query(flow, 0, &flows[i++]); + + odp_flow_key_from_flow(&flows[i].key, &flow->key); + answer_flow_query(flow, 0, &flows[i]); + i++; } return hmap_count(&dp->flow_table); } @@ -827,7 +835,7 @@ dpif_netdev_execute(struct dpif *dpif, struct dp_netdev *dp = get_dp_netdev(dpif); struct ofpbuf copy; bool mutates; - flow_t flow; + flow_t key; int error; if (packet->size < ETH_HEADER_LEN || packet->size > UINT16_MAX) { @@ -852,8 +860,8 @@ dpif_netdev_execute(struct dpif *dpif, * if we don't. */ copy = *packet; } - flow_extract(©, 0, -1, &flow); - error = dp_netdev_execute_actions(dp, ©, &flow, actions, n_actions); + flow_extract(©, 0, -1, &key); + error = dp_netdev_execute_actions(dp, ©, &key, actions, n_actions); if (mutates) { ofpbuf_uninit(©); } @@ -922,7 +930,7 @@ dpif_netdev_recv_wait(struct dpif *dpif) } static void -dp_netdev_flow_used(struct dp_netdev_flow *flow, const flow_t *key, +dp_netdev_flow_used(struct dp_netdev_flow *flow, struct flow *key, const struct ofpbuf *packet) { time_timespec(&flow->used); @@ -939,7 +947,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port, struct ofpbuf *packet) { struct dp_netdev_flow *flow; - flow_t key; + struct flow key; if (packet->size < ETH_HEADER_LEN) { return; @@ -1075,13 +1083,13 @@ dp_netdev_set_dl_dst(struct ofpbuf *packet, const uint8_t dl_addr[ETH_ADDR_LEN]) } static bool -is_ip(const struct ofpbuf *packet, const flow_t *key) +is_ip(const struct ofpbuf *packet, const struct flow *key) { return key->dl_type == htons(ETH_TYPE_IP) && packet->l4; } static void -dp_netdev_set_nw_addr(struct ofpbuf *packet, const flow_t *key, +dp_netdev_set_nw_addr(struct ofpbuf *packet, struct flow *key, const struct odp_action_nw_addr *a) { if (is_ip(packet, key)) { @@ -1107,7 +1115,7 @@ dp_netdev_set_nw_addr(struct ofpbuf *packet, const flow_t *key, } static void -dp_netdev_set_nw_tos(struct ofpbuf *packet, const flow_t *key, +dp_netdev_set_nw_tos(struct ofpbuf *packet, struct flow *key, const struct odp_action_nw_tos *a) { if (is_ip(packet, key)) { @@ -1124,7 +1132,7 @@ dp_netdev_set_nw_tos(struct ofpbuf *packet, const flow_t *key, } static void -dp_netdev_set_tp_port(struct ofpbuf *packet, const flow_t *key, +dp_netdev_set_tp_port(struct ofpbuf *packet, struct flow *key, const struct odp_action_tp_port *a) { if (is_ip(packet, key)) { @@ -1186,7 +1194,7 @@ dp_netdev_output_control(struct dp_netdev *dp, const struct ofpbuf *packet, * screwy or truncated header fields or one whose inner and outer Ethernet * address differ. */ static bool -dp_netdev_is_spoofed_arp(struct ofpbuf *packet, const struct odp_flow_key *key) +dp_netdev_is_spoofed_arp(struct ofpbuf *packet, const struct flow *key) { struct arp_eth_header *arp; struct eth_header *eth; @@ -1212,7 +1220,7 @@ dp_netdev_is_spoofed_arp(struct ofpbuf *packet, const struct odp_flow_key *key) static int dp_netdev_execute_actions(struct dp_netdev *dp, - struct ofpbuf *packet, const flow_t *key, + struct ofpbuf *packet, struct flow *key, const union odp_action *actions, int n_actions) { int i; @@ -1232,7 +1240,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp, case ODPAT_SET_VLAN_VID: dp_netdev_modify_vlan_tci(packet, ntohs(a->vlan_vid.vlan_vid), VLAN_VID_MASK); - break; + break; case ODPAT_SET_VLAN_PCP: dp_netdev_modify_vlan_tci(packet, diff --git a/lib/dpif.c b/lib/dpif.c index b3e82afc..a7706e4e 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1106,7 +1106,8 @@ should_log_flow_message(int error) static void log_flow_message(const struct dpif *dpif, int error, const char *operation, - const flow_t *flow, const struct odp_flow_stats *stats, + const struct odp_flow_key *flow, + const struct odp_flow_stats *stats, const union odp_action *actions, size_t n_actions) { struct ds ds = DS_EMPTY_INITIALIZER; @@ -1118,7 +1119,7 @@ log_flow_message(const struct dpif *dpif, int error, const char *operation, if (error) { ds_put_format(&ds, "(%s) ", strerror(error)); } - flow_format(&ds, flow); + format_odp_flow_key(&ds, flow); if (stats) { ds_put_cstr(&ds, ", "); format_odp_flow_stats(&ds, stats); diff --git a/lib/flow.c b/lib/flow.c index 462df08c..f2f77270 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -304,8 +304,6 @@ flow_from_match(const struct ofp_match *match, bool tun_id_from_cookie, memcpy(flow->dl_dst, match->dl_dst, ETH_ADDR_LEN); flow->nw_tos = match->nw_tos; flow->nw_proto = match->nw_proto; - memset(flow->reserved, 0, sizeof flow->reserved); - if (flow_wildcards) { *flow_wildcards = wildcards; } diff --git a/lib/flow.h b/lib/flow.h index 603c4ace..7b5f1448 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -31,7 +31,30 @@ struct ds; struct ofp_match; struct ofpbuf; -typedef struct odp_flow_key flow_t; +typedef struct flow flow_t; +struct flow { + uint32_t tun_id; /* Encapsulating tunnel ID. */ + uint32_t nw_src; /* IP source address. */ + uint32_t nw_dst; /* IP destination address. */ + uint16_t in_port; /* Input switch port. */ + uint16_t dl_vlan; /* Input VLAN. */ + uint16_t dl_type; /* Ethernet frame type. */ + uint16_t tp_src; /* TCP/UDP source port. */ + uint16_t tp_dst; /* TCP/UDP destination port. */ + uint8_t dl_src[6]; /* Ethernet source address. */ + uint8_t dl_dst[6]; /* Ethernet destination address. */ + uint8_t nw_proto; /* IP protocol or low 8 bits of ARP opcode. */ + uint8_t dl_vlan_pcp; /* Input VLAN priority. */ + uint8_t nw_tos; /* IP ToS (DSCP field, 6 bits). */ +}; + +/* Assert that there are FLOW_SIG_SIZE bytes of significant data in "struct + * flow", followed by FLOW_PAD_SIZE bytes of padding. */ +#define FLOW_SIG_SIZE 37 +#define FLOW_PAD_SIZE 3 +BUILD_ASSERT_DECL(offsetof(struct flow, nw_tos) == FLOW_SIG_SIZE - 1); +BUILD_ASSERT_DECL(sizeof(((struct flow *)0)->nw_tos) == 1); +BUILD_ASSERT_DECL(sizeof(struct flow) == FLOW_SIG_SIZE + FLOW_PAD_SIZE); int flow_extract(struct ofpbuf *, uint32_t tun_id, uint16_t in_port, flow_t *); void flow_extract_stats(const flow_t *flow, struct ofpbuf *packet, @@ -50,7 +73,7 @@ static inline size_t flow_hash(const flow_t *, uint32_t basis); static inline int flow_compare(const flow_t *a, const flow_t *b) { - return memcmp(a, b, sizeof *a); + return memcmp(a, b, FLOW_SIG_SIZE); } static inline bool @@ -62,9 +85,7 @@ flow_equal(const flow_t *a, const flow_t *b) static inline size_t flow_hash(const flow_t *flow, uint32_t basis) { - BUILD_ASSERT_DECL(!(sizeof *flow % sizeof(uint32_t))); - return hash_words((const uint32_t *) flow, - sizeof *flow / sizeof(uint32_t), basis); + return hash_bytes(flow, FLOW_SIG_SIZE, basis); } /* Information on wildcards for a flow, as a supplement to flow_t. */ diff --git a/lib/odp-util.c b/lib/odp-util.c index 8278c228..49d9d34d 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -39,6 +39,19 @@ odp_actions_add(struct odp_actions *actions, uint16_t type) return a; } +void +format_odp_flow_key(struct ds *ds, const struct odp_flow_key *key) +{ + ds_put_format(ds, "in_port%04x:vlan%d:pcp%d mac"ETH_ADDR_FMT + "->"ETH_ADDR_FMT" type%04x proto%"PRId8" tos%"PRIu8 + " ip"IP_FMT"->"IP_FMT" port%d->%d", + key->in_port, ntohs(key->dl_vlan), key->dl_vlan_pcp, + ETH_ADDR_ARGS(key->dl_src), ETH_ADDR_ARGS(key->dl_dst), + ntohs(key->dl_type), key->nw_proto, key->nw_tos, + IP_ARGS(&key->nw_src), IP_ARGS(&key->nw_dst), + ntohs(key->tp_src), ntohs(key->tp_dst)); +} + void format_odp_action(struct ds *ds, const union odp_action *a) { @@ -134,10 +147,46 @@ format_odp_flow_stats(struct ds *ds, const struct odp_flow_stats *s) void format_odp_flow(struct ds *ds, const struct odp_flow *f) { - flow_format(ds, &f->key); + format_odp_flow_key(ds, &f->key); ds_put_cstr(ds, ", "); format_odp_flow_stats(ds, &f->stats); ds_put_cstr(ds, ", actions:"); format_odp_actions(ds, f->actions, f->n_actions); } + +void +odp_flow_key_from_flow(struct odp_flow_key *key, const struct flow *flow) +{ + key->tun_id = flow->tun_id; + key->nw_src = flow->nw_src; + key->nw_dst = flow->nw_dst; + key->in_port = flow->in_port; + key->dl_vlan = flow->dl_vlan; + key->dl_type = flow->dl_type; + key->tp_src = flow->tp_src; + key->tp_dst = flow->tp_dst; + memcpy(key->dl_src, flow->dl_src, ETH_ADDR_LEN); + memcpy(key->dl_dst, flow->dl_dst, ETH_ADDR_LEN); + key->nw_proto = flow->nw_proto; + key->dl_vlan_pcp = flow->dl_vlan_pcp; + key->nw_tos = flow->nw_tos; + memset(key->reserved, 0, sizeof key->reserved); +} +void +odp_flow_key_to_flow(const struct odp_flow_key *key, struct flow *flow) +{ + flow->tun_id = key->tun_id; + flow->nw_src = key->nw_src; + flow->nw_dst = key->nw_dst; + flow->in_port = key->in_port; + flow->dl_vlan = key->dl_vlan; + flow->dl_type = key->dl_type; + flow->tp_src = key->tp_src; + flow->tp_dst = key->tp_dst; + memcpy(flow->dl_src, key->dl_src, ETH_ADDR_LEN); + memcpy(flow->dl_dst, key->dl_dst, ETH_ADDR_LEN); + flow->nw_proto = key->nw_proto; + flow->dl_vlan_pcp = key->dl_vlan_pcp; + flow->nw_tos = key->nw_tos; +} diff --git a/lib/odp-util.h b/lib/odp-util.h index 420bde53..9cbf7ddb 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -20,11 +20,14 @@ #include #include #include +#include +#include "hash.h" #include "openflow/openflow.h" #include "openvswitch/datapath-protocol.h" #include "util.h" struct ds; +struct flow; /* The kernel datapaths limits actions to those that fit in a single page of * memory, so there is no point in allocating more than that. */ @@ -78,10 +81,28 @@ odp_port_to_ofp_port(uint16_t odp_port) } } +void format_odp_flow_key(struct ds *, const struct odp_flow_key *); void format_odp_action(struct ds *, const union odp_action *); void format_odp_actions(struct ds *, const union odp_action *actions, size_t n_actions); void format_odp_flow_stats(struct ds *, const struct odp_flow_stats *); void format_odp_flow(struct ds *, const struct odp_flow *); +void odp_flow_key_from_flow(struct odp_flow_key *, const struct flow *); +void odp_flow_key_to_flow(const struct odp_flow_key *, struct flow *); + +static inline bool +odp_flow_key_equal(const struct odp_flow_key *a, const struct odp_flow_key *b) +{ + return !memcmp(a, b, sizeof *a); +} + +static inline size_t +odp_flow_key_hash(const struct odp_flow_key *flow, uint32_t basis) +{ + BUILD_ASSERT_DECL(!(sizeof *flow % sizeof(uint32_t))); + return hash_words((const uint32_t *) flow, + sizeof *flow / sizeof(uint32_t), basis); +} + #endif /* odp-util.h */ diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 5e8a1d83..5ac50166 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2120,7 +2120,7 @@ do_put_flow(struct ofproto *ofproto, struct rule *rule, int flags, struct odp_flow_put *put) { memset(&put->flow.stats, 0, sizeof put->flow.stats); - put->flow.key = rule->cr.flow; + odp_flow_key_from_flow(&put->flow.key, &rule->cr.flow); put->flow.actions = rule->odp_actions; put->flow.n_actions = rule->n_odp_actions; put->flow.flags = 0; @@ -2224,7 +2224,7 @@ rule_uninstall(struct ofproto *p, struct rule *rule) if (rule->installed) { struct odp_flow odp_flow; - odp_flow.key = rule->cr.flow; + odp_flow_key_from_flow(&odp_flow.key, &rule->cr.flow); odp_flow.actions = NULL; odp_flow.n_actions = 0; odp_flow.flags = 0; @@ -3199,12 +3199,12 @@ query_stats(struct ofproto *p, struct rule *rule, if (rule->cr.wc.wildcards) { size_t i = 0; LIST_FOR_EACH (subrule, list, &rule->list) { - odp_flows[i++].key = subrule->cr.flow; + odp_flow_key_from_flow(&odp_flows[i++].key, &subrule->cr.flow); packet_count += subrule->packet_count; byte_count += subrule->byte_count; } } else { - odp_flows[0].key = rule->cr.flow; + odp_flow_key_from_flow(&odp_flows[0].key, &rule->cr.flow); } /* Fetch up-to-date statistics from the datapath and add them in. */ @@ -4282,9 +4282,12 @@ ofproto_update_used(struct ofproto *p) for (i = 0; i < n_flows; i++) { struct odp_flow *f = &flows[i]; struct rule *rule; + flow_t flow; + + odp_flow_key_to_flow(&f->key, &flow); rule = rule_from_cls_rule( - classifier_find_rule_exactly(&p->cls, &f->key, 0, UINT16_MAX)); + classifier_find_rule_exactly(&p->cls, &flow, 0, UINT16_MAX)); if (rule && rule->installed) { update_time(p, rule, &f->stats); @@ -4405,7 +4408,7 @@ rule_active_timeout(struct ofproto *ofproto, struct rule *rule) * ofproto_update_used() zeroed TCP flags. */ memset(&odp_flow, 0, sizeof odp_flow); if (rule->installed) { - odp_flow.key = rule->cr.flow; + odp_flow_key_from_flow(&odp_flow.key, &rule->cr.flow); odp_flow.flags = ODPFF_ZERO_TCP_FLAGS; dpif_flow_get(ofproto->dpif, &odp_flow); diff --git a/tests/test-classifier.c b/tests/test-classifier.c index 4227c182..e471582b 100644 --- a/tests/test-classifier.c +++ b/tests/test-classifier.c @@ -380,7 +380,6 @@ compare_classifiers(struct classifier *cls, struct tcls *tcls) ETH_ADDR_LEN); flow.nw_proto = nw_proto_values[get_value(&x, N_NW_PROTO_VALUES)]; flow.nw_tos = nw_tos_values[get_value(&x, N_NW_TOS_VALUES)]; - memset(flow.reserved, 0, sizeof flow.reserved); for (include = 1; include <= 3; include++) { cr0 = lookup_with_include_bits(cls, &flow, include); -- 2.30.2