From a47208dc7690dcd37869c696fa949db41a41d2ac Mon Sep 17 00:00:00 2001 From: Yiannis Yiakoumis Date: Mon, 22 Jun 2009 20:17:35 -0700 Subject: [PATCH] Flow mod behavior: Add a new flag to flow mods and flow conflict detection handler for table maintanance Add a CHECK_OVERLAP flag to flow mods which requires the switch to do the (potentially more costly) check that there doesn't already exist a conflicting flow with the same priority. If there is one, the mod fails and an error code is returned. Flow expiry: Record OFPFF_SEND_FLOW_EXP flag in user data path --- datapath/chain.c | 21 +++ datapath/chain.h | 2 + datapath/datapath.c | 24 +-- datapath/forward.c | 66 ++++++-- datapath/private-msg.c | 2 + datapath/table-hash.c | 39 +++++ datapath/table-linear.c | 16 ++ datapath/table.h | 6 + include/openflow/openflow.h | 6 +- lib/ofp-print.c | 4 +- regress/lib/Perl5/OF/OFUtil.pm | 5 +- .../regress/test_flow_mod_check/run.pl | 56 +++++++ regress/projects/black_box/regress/tests.txt | 1 + udatapath/chain.c | 22 ++- udatapath/chain.h | 2 + udatapath/datapath.c | 145 +++++++++++------- udatapath/private-msg.c | 2 + udatapath/table-hash.c | 39 +++++ udatapath/table-linear.c | 17 ++ udatapath/table.h | 8 +- .../openflow/packet-openflow.c | 40 +++-- 21 files changed, 412 insertions(+), 111 deletions(-) create mode 100755 regress/projects/black_box/regress/test_flow_mod_check/run.pl diff --git a/datapath/chain.c b/datapath/chain.c index f74f219..e75f77d 100644 --- a/datapath/chain.c +++ b/datapath/chain.c @@ -153,6 +153,27 @@ chain_modify(struct sw_chain *chain, const struct sw_flow_key *key, return count; } +/* Checks whether the chain has an entry with the same priority which conflicts + * with 'key'. If 'strict' set, wildcards should also match. If 'strict' is not + * set, comparison is done 'module wildcards'. + * + * Returns 'true' if such an entry exists, 'false' otherwise. */ +int +chain_has_conflict(struct sw_chain *chain, const struct sw_flow_key *key, + uint16_t priority, int strict) +{ + int i; + + for (i = 0; i < chain->n_tables; i++) { + struct sw_table *t = chain->tables[i]; + if (t->has_conflict(t, key, priority, strict)) { + return true; + } + } + + return false; +} + /* Deletes from 'chain' any and all flows that match 'key'. If 'out_port' * is not OFPP_NONE, then matching entries must have that port as an * argument for an output action. If 'strict" is set, then wildcards and diff --git a/datapath/chain.h b/datapath/chain.h index 311c225..4b00aef 100644 --- a/datapath/chain.h +++ b/datapath/chain.h @@ -29,6 +29,8 @@ struct sw_flow *chain_lookup(struct sw_chain *, const struct sw_flow_key *, int chain_insert(struct sw_chain *, struct sw_flow *, int); int chain_modify(struct sw_chain *, const struct sw_flow_key *, uint16_t, int, const struct ofp_action_header *, size_t, int); +int chain_has_conflict(struct sw_chain *, const struct sw_flow_key *, + uint16_t, int); int chain_delete(struct sw_chain *, const struct sw_flow_key *, uint16_t, uint16_t, int, int); int chain_timeout(struct sw_chain *); diff --git a/datapath/datapath.c b/datapath/datapath.c index e150bbe..633185c 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -54,18 +54,10 @@ static char hw_desc[DESC_STR_LEN] = "Reference Data plane Kernel Extension for L static char sw_desc[DESC_STR_LEN] = VERSION BUILDNR; static char serial_num[SERIAL_NUM_LEN] = "None"; -#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0) module_param_string(mfr_desc, mfr_desc, sizeof mfr_desc, 0444); module_param_string(hw_desc, hw_desc, sizeof hw_desc, 0444); module_param_string(sw_desc, sw_desc, sizeof sw_desc, 0444); module_param_string(serial_num, serial_num, sizeof serial_num, 0444); -#else -MODULE_PARM(mfr_desc, "s"); -MODULE_PARM(hw_desc, "s"); -MODULE_PARM(sw_desc, "s"); -MODULE_PARM(serial_num, "s"); -#endif - int (*dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd); EXPORT_SYMBOL(dp_ioctl_hook); @@ -544,29 +536,17 @@ do_port_input(struct net_bridge_port *p, struct sk_buff *skb) #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,22) /* Called with rcu_read_lock. */ static struct sk_buff *dp_frame_hook(struct net_bridge_port *p, - struct sk_buff *skb) + struct sk_buff *skb) { do_port_input(p, skb); return NULL; } -#elif LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0) +#else static int dp_frame_hook(struct net_bridge_port *p, struct sk_buff **pskb) { do_port_input(p, *pskb); return 1; } -#else -/* NB: This has only been tested on 2.4.35 */ -static void dp_frame_hook(struct sk_buff *skb) -{ - struct net_bridge_port *p = skb->dev->br_port; - if (p) { - rcu_read_lock(); - do_port_input(p, skb); - rcu_read_unlock(); - } else - kfree_skb(skb); -} #endif /* Forwarding output path. diff --git a/datapath/forward.c b/datapath/forward.c index 4c16510..a72b390 100644 --- a/datapath/forward.c +++ b/datapath/forward.c @@ -202,13 +202,14 @@ recv_echo_reply(struct sw_chain *chain, const struct sender *sender, } static int -add_flow(struct sw_chain *chain, const struct sender *sender, +add_flow(struct sw_chain *chain, const struct sender *sender, const struct ofp_flow_mod *ofm) { int error = -ENOMEM; uint16_t v_code; struct sw_flow *flow; size_t actions_len = ntohs(ofm->header.length) - sizeof *ofm; + int overlap; /* Allocate memory. */ flow = flow_alloc(actions_len, GFP_ATOMIC); @@ -224,8 +225,21 @@ add_flow(struct sw_chain *chain, const struct sender *sender, goto error_free_flow; } - /* Fill out flow. */ flow->priority = flow->key.wildcards ? ntohs(ofm->priority) : -1; + if (ntohs(ofm->flags) & OFPFF_CHECK_OVERLAP) { + /* check whether there is any conflict */ + overlap = chain_has_conflict(chain, &flow->key, flow->priority, + false); + if (overlap){ + dp_send_error_msg(chain->dp, sender, + OFPET_FLOW_MOD_FAILED, + OFPFMFC_OVERLAP, + ofm, ntohs(ofm->header.length)); + goto error_free_flow; + } + } + + /* Fill out flow. */ flow->idle_timeout = ntohs(ofm->idle_timeout); flow->hard_timeout = ntohs(ofm->hard_timeout); flow->send_flow_exp = (ntohs(ofm->flags) & OFPFF_SEND_FLOW_EXP) ? 1 : 0; @@ -240,6 +254,7 @@ add_flow(struct sw_chain *chain, const struct sender *sender, goto error_free_flow; } else if (error) goto error_free_flow; + error = 0; if (ntohl(ofm->buffer_id) != (uint32_t) -1) { struct sk_buff *skb = retrieve_skb(ntohl(ofm->buffer_id)); @@ -269,27 +284,52 @@ mod_flow(struct sw_chain *chain, const struct sender *sender, { int error = -ENOMEM; uint16_t v_code; - size_t actions_len; - struct sw_flow_key key; - uint16_t priority; + size_t actions_len = ntohs(ofm->header.length) - sizeof *ofm; + struct sw_flow *flow; int strict; - flow_extract_match(&key, &ofm->match); + /* Allocate memory. */ + flow = flow_alloc(actions_len,GFP_ATOMIC); + if (flow == NULL) + goto error; - actions_len = ntohs(ofm->header.length) - sizeof *ofm; + flow_extract_match(&flow->key, &ofm->match); - v_code = validate_actions(chain->dp, &key, ofm->actions, actions_len); + v_code = validate_actions(chain->dp, &flow->key, ofm->actions, actions_len); if (v_code != ACT_VALIDATION_OK) { dp_send_error_msg(chain->dp, sender, OFPET_BAD_ACTION, v_code, ofm, ntohs(ofm->header.length)); - goto error; + goto error_free_flow; } - priority = key.wildcards ? ntohs(ofm->priority) : -1; + flow->priority = flow->key.wildcards ? ntohs(ofm->priority) : -1; strict = (ofm->command == htons(OFPFC_MODIFY_STRICT)) ? 1 : 0; - chain_modify(chain, &key, priority, strict, ofm->actions, actions_len, - (ntohs(ofm->flags) & OFPFF_EMERG) ? 1 : 0); + /* First try to modify existing flows if any */ + /* if there is no matching flow, add it */ + if (!chain_modify(chain, &flow->key, flow->priority, strict, + ofm->actions, actions_len, + (ntohs(ofm->flags) & OFPFF_EMERG) ? 1 : 0)) { + /* Fill out flow. */ + flow->idle_timeout = ntohs(ofm->idle_timeout); + flow->hard_timeout = ntohs(ofm->hard_timeout); + flow->send_flow_exp = (ntohs(ofm->flags) & OFPFF_SEND_FLOW_EXP) + ? 1 : 0; + flow_setup_actions(flow, ofm->actions, actions_len); + error = chain_insert(chain, flow, + (ntohs(ofm->flags) & OFPFF_EMERG) ? 1 : 0); + if (error == -ENOBUFS) { + dp_send_error_msg(chain->dp, sender, + OFPET_FLOW_MOD_FAILED, + OFPFMFC_ALL_TABLES_FULL, + ofm, ntohs(ofm->header.length)); + goto error_free_flow; + } else if (error) { + goto error_free_flow; + } + } + + error = 0; if (ntohl(ofm->buffer_id) != (uint32_t) -1) { struct sk_buff *skb = retrieve_skb(ntohl(ofm->buffer_id)); if (skb) { @@ -303,6 +343,8 @@ mod_flow(struct sw_chain *chain, const struct sender *sender, } return error; +error_free_flow: + flow_free(flow); error: if (ntohl(ofm->buffer_id) != (uint32_t) -1) discard_skb(ntohl(ofm->buffer_id)); diff --git a/datapath/private-msg.c b/datapath/private-msg.c index 3fb42d8..e3c58bd 100644 --- a/datapath/private-msg.c +++ b/datapath/private-msg.c @@ -75,9 +75,11 @@ protection_callback(struct sw_flow *flow, void *private_) memset(&match, 0, sizeof(match)); flow_fill_match(&match, &flow->key); flow_extract_match(&tgtflow->key, &match); + /* Fill out flow. */ tgtflow->priority = flow->priority; tgtflow->idle_timeout = 0; tgtflow->hard_timeout = 0; + tgtflow->send_flow_exp = flow->send_flow_exp; flow_setup_actions(tgtflow, actions->actions, actions->actions_len); error = chain_insert(private->chain, tgtflow, 0); diff --git a/datapath/table-hash.c b/datapath/table-hash.c index 9e27b0c..bc8bd3a 100644 --- a/datapath/table-hash.c +++ b/datapath/table-hash.c @@ -101,6 +101,34 @@ static int table_hash_modify(struct sw_table *swt, return count; } +static int table_hash_has_conflict(struct sw_table *swt, + const struct sw_flow_key *key, + uint16_t priority, int strict) +{ + struct sw_table_hash *th = (struct sw_table_hash *)swt; + + if (key->wildcards == 0) { + struct sw_flow **bucket = find_bucket(swt, key); + struct sw_flow *flow = *bucket; + if (flow && flow_matches_desc(&flow->key, key,strict) + && (flow->priority == priority)) { + return true; + } + } else { + unsigned int i; + + for (i = 0; i <= th->bucket_mask; i++) { + struct sw_flow **bucket = &th->buckets[i]; + struct sw_flow *flow = *bucket; + if (flow && flow_matches_desc(&flow->key, key, strict) + && (flow->priority == priority)) { + return true; + } + } + } + return false; +} + /* Caller must update n_flows. */ static int do_delete(struct datapath *dp, struct sw_flow **bucket, struct sw_flow *flow, enum nx_flow_end_reason reason) @@ -252,6 +280,7 @@ struct sw_table *table_hash_create(unsigned int polynomial, swt = &th->swt; swt->lookup = table_hash_lookup; swt->insert = table_hash_insert; + swt->has_conflict = table_hash_has_conflict; swt->delete = table_hash_delete; swt->timeout = table_hash_timeout; swt->destroy = table_hash_destroy; @@ -305,6 +334,15 @@ static int table_hash2_modify(struct sw_table *swt, actions, actions_len)); } +static int table_hash2_has_conflict(struct sw_table *swt, + const struct sw_flow_key *key, + uint16_t priority, int strict) +{ + struct sw_table_hash2 *t2 = (struct sw_table_hash2 *) swt; + return (table_hash_has_conflict(t2->subtable[0], key, priority, strict) || + table_hash_has_conflict(t2->subtable[1], key, priority, strict)); +} + static int table_hash2_delete(struct datapath *dp, struct sw_table *swt, const struct sw_flow_key *key, uint16_t out_port, @@ -393,6 +431,7 @@ struct sw_table *table_hash2_create(unsigned int poly0, unsigned int buckets0, swt->lookup = table_hash2_lookup; swt->insert = table_hash2_insert; swt->modify = table_hash2_modify; + swt->has_conflict = table_hash2_has_conflict; swt->delete = table_hash2_delete; swt->timeout = table_hash2_timeout; swt->destroy = table_hash2_destroy; diff --git a/datapath/table-linear.c b/datapath/table-linear.c index bef58a6..416b008 100644 --- a/datapath/table-linear.c +++ b/datapath/table-linear.c @@ -90,6 +90,21 @@ static int table_linear_modify(struct sw_table *swt, return count; } +static int table_linear_has_conflict(struct sw_table *swt, + const struct sw_flow_key *key, uint16_t priority, int strict) +{ + struct sw_table_linear *tl = (struct sw_table_linear *) swt; + struct sw_flow *flow; + + list_for_each_entry (flow, &tl->flows, node) { + if (flow_matches_desc(&flow->key, key, strict) + && (flow->priority == priority)) { + return true; + } + } + return false; +} + static int do_delete(struct datapath *dp, struct sw_table *swt, struct sw_flow *flow, enum nx_flow_end_reason reason) { @@ -201,6 +216,7 @@ struct sw_table *table_linear_create(unsigned int max_flows) swt->lookup = table_linear_lookup; swt->insert = table_linear_insert; swt->modify = table_linear_modify; + swt->has_conflict = table_linear_has_conflict; swt->delete = table_linear_delete; swt->timeout = table_linear_timeout; swt->destroy = table_linear_destroy; diff --git a/datapath/table.h b/datapath/table.h index 8be895e..1cc90a0 100644 --- a/datapath/table.h +++ b/datapath/table.h @@ -64,6 +64,12 @@ struct sw_table { uint16_t priority, int strict, const struct ofp_action_header *actions, size_t actions_len); + /* Checks whether 'table' has an equal priotiry entry of thethat conflicts + * with 'key'. If 'strict' is set, wildcards must match. + * Returns the number of flows that were modified. */ + int (*has_conflict)(struct sw_table *table, const struct sw_flow_key *key, + uint16_t priority, int strict); + /* Deletes from 'table' any and all flows that match 'key' from * 'table'. If 'out_port' is not OFPP_NONE, then matching entries * must have that port as an argument for an output action. If diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h index 0ff6519..c02786e 100644 --- a/include/openflow/openflow.h +++ b/include/openflow/openflow.h @@ -434,7 +434,7 @@ enum ofp_flow_mod_command { OFPFC_MODIFY, /* Modify all matching flows. */ OFPFC_MODIFY_STRICT, /* Modify entry strictly matching wildcards */ OFPFC_DELETE, /* Delete all matching flows. */ - OFPFC_DELETE_STRICT /* Strictly match wildcards and priority. */ + OFPFC_DELETE_STRICT /* Strictly match wildcards and priority. */ }; /* Flow wildcards. */ @@ -614,7 +614,9 @@ enum ofp_bad_action_code { /* ofp_error_msg 'code' values for OFPET_FLOW_MOD_FAILED. 'data' contains * at least the first 64 bytes of the failed request. */ enum ofp_flow_mod_failed_code { - OFPFMFC_ALL_TABLES_FULL /* Flow not added because of full tables. */ + OFPFMFC_ALL_TABLES_FULL, /* Flow not added because of full tables. */ + OFPFMFC_OVERLAP /* Attempted to add overlapping flow with + * CHECK_OVERLAP flag set. */ }; /* OFPT_ERROR: Error message (datapath -> controller). */ diff --git a/lib/ofp-print.c b/lib/ofp-print.c index b2abcac..7daf865 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -742,10 +742,10 @@ ofp_print_flow_mod(struct ds *string, const void *oh, size_t len, default: ds_put_format(string, " cmd:%d ", ntohs(ofm->command)); } - ds_put_format(string, "idle:%d hard:%d pri:%d buf:%#x", + ds_put_format(string, "idle:%d hard:%d pri:%d buf:%#x flg:%#x", ntohs(ofm->idle_timeout), ntohs(ofm->hard_timeout), ofm->match.wildcards ? ntohs(ofm->priority) : (uint16_t)-1, - ntohl(ofm->buffer_id)); + ntohl(ofm->buffer_id), ntohs(ofm->flags)); ofp_print_actions(string, ofm->actions, len - offsetof(struct ofp_flow_mod, actions)); ds_put_char(string, '\n'); diff --git a/regress/lib/Perl5/OF/OFUtil.pm b/regress/lib/Perl5/OF/OFUtil.pm index 6429146..54a3836 100644 --- a/regress/lib/Perl5/OF/OFUtil.pm +++ b/regress/lib/Perl5/OF/OFUtil.pm @@ -896,7 +896,7 @@ sub create_flow_mod_from_udp_action { if ( $mod_type ne 'drop' && $mod_type ne 'OFPFC_ADD' && $mod_type ne 'OFPFC_DELETE' - && $mod_type ne 'OFPFC_DELETE_STRICT' ) + && $mod_type ne 'OFPFC_DELETE_STRICT') { die "Undefined flow mod type: $mod_type\n"; } @@ -972,7 +972,8 @@ sub create_flow_mod_from_udp_action { flags => $flags, priority => 0, buffer_id => -1, - out_port => $enums{'OFPP_NONE'} + out_port => $enums{'OFPP_NONE'}, + flags => $enums{"$fm_flags"} }; my $flow_mod = $ofp->pack( 'ofp_flow_mod', $flow_mod_args ); my $flow_mod_pkt = combine_args($flow_mod, $mod_type, $out_port, $chg_field, $chg_val); diff --git a/regress/projects/black_box/regress/test_flow_mod_check/run.pl b/regress/projects/black_box/regress/test_flow_mod_check/run.pl new file mode 100755 index 0000000..257f6e8 --- /dev/null +++ b/regress/projects/black_box/regress/test_flow_mod_check/run.pl @@ -0,0 +1,56 @@ +#!/usr/bin/perl -w +# test_flow_mod_check + +use strict; +use OF::Includes; + +sub wait_for_flow_overlap_error { + my ($ofp, $sock, $flow_mod_pkt) = @_; + my $rcvd_msg; + + sysread($sock, $rcvd_msg, 1512); + + my $msg_size = length($rcvd_msg); + my $expected_size = $ofp->sizeof('ofp_error_msg') + length($flow_mod_pkt); + compare("msg size", $msg_size, '==', $expected_size); + + my $msg = $ofp->unpack('ofp_error_msg', $rcvd_msg); + verify_header($msg, 'OFPT_ERROR', $msg_size); + + compare("error type", $$msg{'type'}, '==', $enums{'OFPET_FLOW_MOD_FAILED'}); + compare("error code", $$msg{'code'}, '==', $enums{'OFPFMFC_OVERLAP'}); +} + +sub my_test { + my ($sock, $options_ref) = @_; + + my $in_port = $$options_ref{'port_base'}; + my $out_port_1 = $in_port + 1; + my $out_port_2 = $in_port + 2; + my $out_port_3 = $in_port + 3; + + my $test_pkt = get_default_black_box_pkt($in_port, $out_port_1); + + my $wildcards = 0x03fe; + my $flags = $enums{'OFPFF_CHECK_OVERLAP'}; + my $max_idle = 0x0; # never expire + my $flow_mod_pkt = create_flow_mod_from_udp($ofp, $test_pkt, $in_port, $out_port_1, $max_idle, $flags, $wildcards); + + # Send 'flow_mod' message + print $sock $flow_mod_pkt; + + # Give OF switch time to process the flow mod + usleep($$options_ref{'send_delay'}); + + # change the wildcard and send again. this should fail + $wildcards = 0x03fd; + my $flags = $enums{'OFPFF_CHECK_OVERLAP'}; + $flow_mod_pkt = create_flow_mod_from_udp($ofp, $test_pkt, $in_port, $out_port_2, $max_idle, $flags, $wildcards); + + # Send 'flow_mod' message + print $sock $flow_mod_pkt; + + wait_for_flow_overlap_error($ofp, $sock, $flow_mod_pkt); +} + +run_black_box_test( \&my_test, \@ARGV ); diff --git a/regress/projects/black_box/regress/tests.txt b/regress/projects/black_box/regress/tests.txt index 4adc29d..32dc8e9 100644 --- a/regress/projects/black_box/regress/tests.txt +++ b/regress/projects/black_box/regress/tests.txt @@ -32,6 +32,7 @@ test_drop_exact/run.pl ## Modify Action Tests test_forward_exact_modify_action/run.pl test_forward_wildcard_modify_action/run.pl +test_flow_mod_check/run.pl ## ICMP handling Tests test_forward_exact_icmp_port/run.pl diff --git a/udatapath/chain.c b/udatapath/chain.c index f993416..a5db92f 100644 --- a/udatapath/chain.c +++ b/udatapath/chain.c @@ -167,6 +167,27 @@ chain_modify(struct sw_chain *chain, const struct sw_flow_key *key, return count; } +/* Checks whether the chain has an entry with the same priority which conflicts + * with 'key'. If 'strict' set, wildcards should also match. If 'strict' is not + * set, comparison is done 'module wildcards'. + * + * Returns 'true' if such an entry exists, 'false' otherwise. */ +int +chain_has_conflict(struct sw_chain *chain, const struct sw_flow_key *key, + uint16_t priority, int strict) +{ + int i; + + for (i = 0; i < chain->n_tables; i++) { + struct sw_table *t = chain->tables[i]; + if (t->has_conflict(t, key, priority, strict)) { + return true; + } + } + + return false; +} + /* Deletes from 'chain' any and all flows that match 'key'. If 'out_port' * is not OFPP_NONE, then matching entries must have that port as an * argument for an output action. If 'strict" is set, then wildcards and @@ -193,7 +214,6 @@ chain_delete(struct sw_chain *chain, const struct sw_flow_key *key, } return count; - } /* Deletes timed-out flow entries from all the tables in 'chain' and appends diff --git a/udatapath/chain.h b/udatapath/chain.h index 94793ea..fb4e9c3 100644 --- a/udatapath/chain.h +++ b/udatapath/chain.h @@ -64,6 +64,8 @@ struct sw_flow *chain_lookup(struct sw_chain *, const struct sw_flow_key *, int) int chain_insert(struct sw_chain *, struct sw_flow *, int); int chain_modify(struct sw_chain *, const struct sw_flow_key *, uint16_t, int, const struct ofp_action_header *, size_t, int); +int chain_has_conflict(struct sw_chain *, const struct sw_flow_key *, + uint16_t, int); int chain_delete(struct sw_chain *, const struct sw_flow_key *, uint16_t, uint16_t, int, int); void chain_timeout(struct sw_chain *, struct list *deleted); diff --git a/udatapath/datapath.c b/udatapath/datapath.c index ec77e3e..130bf25 100644 --- a/udatapath/datapath.c +++ b/udatapath/datapath.c @@ -548,8 +548,8 @@ dp_output_port(struct datapath *dp, struct ofpbuf *buffer, case OFPP_TABLE: { struct sw_port *p = lookup_port(dp, in_port); - if (run_flow_through_tables(dp, buffer, p)) { - ofpbuf_delete(buffer); + if (run_flow_through_tables(dp, buffer, p)) { + ofpbuf_delete(buffer); } break; } @@ -833,13 +833,13 @@ int run_flow_through_tables(struct datapath *dp, struct ofpbuf *buffer, ofpbuf_delete(buffer); return 0; } - if (p && p->config & (OFPPC_NO_RECV | OFPPC_NO_RECV_STP) + + if (p && p->config & (OFPPC_NO_RECV | OFPPC_NO_RECV_STP) && p->config & (!eth_addr_equals(key.flow.dl_dst, stp_eth_addr) ? OFPPC_NO_RECV : OFPPC_NO_RECV_STP)) { - ofpbuf_delete(buffer); - return 0; - } - + ofpbuf_delete(buffer); + return 0; + } flow = chain_lookup(dp->chain, &key, 0); if (flow != NULL) { @@ -931,7 +931,7 @@ recv_packet_out(struct datapath *dp, const struct sender *sender, return -ESRCH; } } - + flow_extract(buffer, ntohs(opo->in_port), &key.flow); v_code = validate_actions(dp, &key, opo->actions, actions_len); @@ -969,6 +969,7 @@ add_flow(struct datapath *dp, const struct sender *sender, uint16_t v_code; struct sw_flow *flow; size_t actions_len = ntohs(ofm->header.length) - sizeof *ofm; + int overlap; /* Allocate memory. */ flow = flow_alloc(actions_len); @@ -984,11 +985,23 @@ add_flow(struct datapath *dp, const struct sender *sender, goto error_free_flow; } - /* Fill out flow. */ flow->priority = flow->key.wildcards ? ntohs(ofm->priority) : -1; + + if (ntohs(ofm->flags) & OFPFF_CHECK_OVERLAP) { + /* check whether there is any conflict */ + overlap = chain_has_conflict(dp->chain, &flow->key, flow->priority, + false); + if (overlap){ + dp_send_error_msg(dp, sender, OFPET_FLOW_MOD_FAILED, + OFPFMFC_OVERLAP, ofm, ntohs(ofm->header.length)); + goto error_free_flow; + } + } + + /* Fill out flow. */ flow->idle_timeout = ntohs(ofm->idle_timeout); flow->hard_timeout = ntohs(ofm->hard_timeout); - + flow->send_flow_exp = (ntohs(ofm->flags) & OFPFF_SEND_FLOW_EXP) ? 1 : 0; flow_setup_actions(flow, ofm->actions, actions_len); /* Act. */ @@ -1032,30 +1045,52 @@ mod_flow(struct datapath *dp, const struct sender *sender, { int error = -ENOMEM; uint16_t v_code; - size_t actions_len; - struct sw_flow_key key; - uint16_t priority; + size_t actions_len = ntohs(ofm->header.length) - sizeof *ofm; + struct sw_flow *flow; int strict; - flow_extract_match(&key, &ofm->match); - - actions_len = ntohs(ofm->header.length) - sizeof *ofm; - - v_code = validate_actions(dp, &key, ofm->actions, actions_len); + /* Allocate memory. */ + flow = flow_alloc(actions_len); + if (flow == NULL) + goto error; + + flow_extract_match(&flow->key, &ofm->match); + + v_code = validate_actions(dp, &flow->key, ofm->actions, actions_len); if (v_code != ACT_VALIDATION_OK) { dp_send_error_msg(dp, sender, OFPET_BAD_ACTION, v_code, - ofm, ntohs(ofm->header.length)); + ofm, ntohs(ofm->header.length)); goto error; } - priority = key.wildcards ? ntohs(ofm->priority) : -1; + flow->priority = flow->key.wildcards ? ntohs(ofm->priority) : -1; strict = (ofm->command == htons(OFPFC_MODIFY_STRICT)) ? 1 : 0; - chain_modify(dp->chain, &key, priority, strict, ofm->actions, actions_len, - (ntohs(ofm->flags) & OFPFF_EMERG) ? 1 : 0); + + /* First try to modify existing flows if any */ + /* if there is no matching flow, add it */ + if (!chain_modify(dp->chain, &flow->key, flow->priority, + strict, ofm->actions, actions_len, + (ntohs(ofm->flags) & OFPFF_EMERG) ? 1 : 0)) { + /* Fill out flow. */ + flow->idle_timeout = ntohs(ofm->idle_timeout); + flow->hard_timeout = ntohs(ofm->hard_timeout); + flow->send_flow_exp = (ntohs(ofm->flags) & OFPFF_SEND_FLOW_EXP) ? 1 : 0; + flow_setup_actions(flow, ofm->actions, actions_len); + error = chain_insert(dp->chain, flow, + (ntohs(ofm->flags) & OFPFF_EMERG) ? 1 : 0); + if (error == -ENOBUFS) { + dp_send_error_msg(dp, sender, OFPET_FLOW_MOD_FAILED, + OFPFMFC_ALL_TABLES_FULL, ofm, + ntohs(ofm->header.length)); + goto error; + } else if (error) { + goto error; + } + } if (ntohl(ofm->buffer_id) != UINT32_MAX) { - struct ofpbuf *buffer = retrieve_buffer(ntohl(ofm->buffer_id)); - if (buffer) { + struct ofpbuf *buffer = retrieve_buffer(ntohl(ofm->buffer_id)); + if (buffer) { struct sw_flow_key skb_key; uint16_t in_port = ntohs(ofm->match.in_port); flow_extract(buffer, in_port, &skb_key.flow); @@ -1355,47 +1390,47 @@ static void port_stats_done(void *state) */ static int vendor_stats_init(const void *body, int body_len, - void **state) + void **state) { - /* min_body was checked, this should be safe */ - const uint32_t vendor = ntohl(*((uint32_t *)body)); - int err; + /* min_body was checked, this should be safe */ + const uint32_t vendor = ntohl(*((uint32_t *)body)); + int err; - switch (vendor) { - default: - err = -EINVAL; - } + switch (vendor) { + default: + err = -EINVAL; + } - return err; + return err; } static int vendor_stats_dump(struct datapath *dp, void *state, struct ofpbuf *buffer) { - const uint32_t vendor = *((uint32_t *)state); - int err; + const uint32_t vendor = *((uint32_t *)state); + int err; - switch (vendor) { - default: - /* Should never happen */ - err = 0; - } + switch (vendor) { + default: + /* Should never happen */ + err = 0; + } - return err; + return err; } static void vendor_stats_done(void *state) { - const uint32_t vendor = *((uint32_t *) state); + const uint32_t vendor = *((uint32_t *) state); - switch (vendor) { - default: - /* Should never happen */ - free(state); - } + switch (vendor) { + default: + /* Should never happen */ + free(state); + } - return; + return; } struct stats_type { @@ -1466,12 +1501,12 @@ static const struct stats_type stats[] = { port_stats_done }, { - OFPST_VENDOR, - 8, /* vendor + subtype */ - 32, /* whatever */ - vendor_stats_init, - vendor_stats_dump, - vendor_stats_done + OFPST_VENDOR, + 8, /* vendor + subtype */ + 32, /* whatever */ + vendor_stats_init, + vendor_stats_dump, + vendor_stats_done }, }; @@ -1718,7 +1753,7 @@ uint32_t save_buffer(struct ofpbuf *buffer) /* Don't buffer packet if existing entry is less than * OVERWRITE_SECS old. */ if (time_now() < p->timeout) { /* FIXME */ - return (uint32_t)-1; + return (uint32_t)-1; } else { ofpbuf_delete(p->buffer); } diff --git a/udatapath/private-msg.c b/udatapath/private-msg.c index b87ddcf..7dfce3c 100644 --- a/udatapath/private-msg.c +++ b/udatapath/private-msg.c @@ -78,9 +78,11 @@ protection_callback(struct sw_flow *flow, void *private_) memset(&match, 0, sizeof(match)); flow_fill_match(&match, &flow->key.flow, flow->key.wildcards); flow_extract_match(&tgtflow->key, &match); + /* Fill out flow. */ tgtflow->priority = flow->priority; tgtflow->idle_timeout = 0; tgtflow->hard_timeout = 0; + tgtflow->send_flow_exp = flow->send_flow_exp; flow_setup_actions(tgtflow, actions->actions, actions->actions_len); error = chain_insert(private->dp->chain, tgtflow, 0); diff --git a/udatapath/table-hash.c b/udatapath/table-hash.c index f713df5..36dd435 100644 --- a/udatapath/table-hash.c +++ b/udatapath/table-hash.c @@ -124,6 +124,34 @@ static int table_hash_modify(struct sw_table *swt, return count; } +static int table_hash_has_conflict(struct sw_table *swt, + const struct sw_flow_key *key, + uint16_t priority, int strict) +{ + struct sw_table_hash *th = (struct sw_table_hash *) swt; + + if (key->wildcards == 0) { + struct sw_flow **bucket = find_bucket(swt, key); + struct sw_flow *flow = *bucket; + if (flow && flow_matches_desc(&flow->key, key,strict) + && (flow->priority == priority)) { + return true; + } + } else { + unsigned int i; + + for (i = 0; i <= th->bucket_mask; i++) { + struct sw_flow **bucket = &th->buckets[i]; + struct sw_flow *flow = *bucket; + if (flow && flow_matches_desc(&flow->key, key, strict) + && (flow->priority == priority)) { + return true; + } + } + } + return false; +} + /* Caller must update n_flows. */ static void do_delete(struct sw_flow **bucket) @@ -272,6 +300,7 @@ struct sw_table *table_hash_create(unsigned int polynomial, swt->lookup = table_hash_lookup; swt->insert = table_hash_insert; swt->modify = table_hash_modify; + swt->has_conflict = table_hash_has_conflict; swt->delete = table_hash_delete; swt->timeout = table_hash_timeout; swt->destroy = table_hash_destroy; @@ -324,6 +353,15 @@ static int table_hash2_modify(struct sw_table *swt, actions, actions_len)); } +static int table_hash2_has_conflict(struct sw_table *swt, + const struct sw_flow_key *key, + uint16_t priority, int strict) +{ + struct sw_table_hash2 *t2 = (struct sw_table_hash2 *) swt; + return (table_hash_has_conflict(t2->subtable[0], key, priority, strict) || + table_hash_has_conflict(t2->subtable[1], key, priority, strict)); +} + static int table_hash2_delete(struct datapath *dp, struct sw_table *swt, const struct sw_flow_key *key, uint16_t out_port, @@ -414,6 +452,7 @@ struct sw_table *table_hash2_create(unsigned int poly0, unsigned int buckets0, swt->lookup = table_hash2_lookup; swt->insert = table_hash2_insert; swt->modify = table_hash2_modify; + swt->has_conflict = table_hash2_has_conflict; swt->delete = table_hash2_delete; swt->timeout = table_hash2_timeout; swt->destroy = table_hash2_destroy; diff --git a/udatapath/table-linear.c b/udatapath/table-linear.c index ae960ad..95571ce 100644 --- a/udatapath/table-linear.c +++ b/udatapath/table-linear.c @@ -119,6 +119,22 @@ static int table_linear_modify(struct sw_table *swt, return count; } +static int table_linear_has_conflict(struct sw_table *swt, + const struct sw_flow_key *key, + uint16_t priority, int strict) +{ + struct sw_table_linear *tl = (struct sw_table_linear *) swt; + struct sw_flow *flow; + + LIST_FOR_EACH (flow, struct sw_flow, node, &tl->flows) { + if (flow_matches_desc(&flow->key, key, strict) + && (flow->priority == priority)) { + return true; + } + } + return false; +} + static void do_delete(struct sw_flow *flow) { @@ -229,6 +245,7 @@ struct sw_table *table_linear_create(unsigned int max_flows) swt->lookup = table_linear_lookup; swt->insert = table_linear_insert; swt->modify = table_linear_modify; + swt->has_conflict = table_linear_has_conflict; swt->delete = table_linear_delete; swt->timeout = table_linear_timeout; swt->destroy = table_linear_destroy; diff --git a/udatapath/table.h b/udatapath/table.h index 5d8bc66..1d17c07 100644 --- a/udatapath/table.h +++ b/udatapath/table.h @@ -89,12 +89,18 @@ struct sw_table { int (*insert)(struct sw_table *table, struct sw_flow *flow); /* Modifies the actions in 'table' that match 'key'. If 'strict' - * set, wildcards and priority must match. Returns the number of flows + * set, wildcards and priority must match. Returns the number of flows * that were modified. */ int (*modify)(struct sw_table *table, const struct sw_flow_key *key, uint16_t priority, int strict, const struct ofp_action_header *actions, size_t actions_len); + /* Checks whether 'table' has an equal priotiry entry of thethat conflicts + * with 'key'. If 'strict' is set, wildcards must match. + * Returns the number of flows that were modified. */ + int (*has_conflict)(struct sw_table *table, const struct sw_flow_key *key, + uint16_t priority, int strict); + /* Deletes from 'table' any and all flows that match 'key' from * 'table'. If 'out_port' is not OFPP_NONE, then matching entries * must have that port as an argument for an output action. If diff --git a/utilities/wireshark_dissectors/openflow/packet-openflow.c b/utilities/wireshark_dissectors/openflow/packet-openflow.c index 7c935f9..951504a 100644 --- a/utilities/wireshark_dissectors/openflow/packet-openflow.c +++ b/utilities/wireshark_dissectors/openflow/packet-openflow.c @@ -53,13 +53,13 @@ static dissector_handle_t data_ethernet; /* AM=Async message, CSM=Control/Switch Message, SM=Symmetric Message */ /** names to bind to various values in the type field */ static const value_string names_ofp_type[] = { - /* Immutable messages. */ - { OFPT_HELLO, "Hello (SM)" }, - { OFPT_ERROR, "Error (SM)" }, - { OFPT_ECHO_REQUEST, "Echo Request (SM)" }, - { OFPT_ECHO_REPLY, "Echo Reply (SM)" }, - { OFPT_VENDOR, "Vendor (SM)" }, - + /* Immutable messages. */ + { OFPT_HELLO, "Hello (SM)" }, + { OFPT_ERROR, "Error (SM)" }, + { OFPT_ECHO_REQUEST, "Echo Request (SM)" }, + { OFPT_ECHO_REPLY, "Echo Reply (SM)" }, + { OFPT_VENDOR, "Vendor (SM)" }, + /* Switch configuration messages. */ { OFPT_FEATURES_REQUEST, "Features Request (CSM)" }, { OFPT_FEATURES_REPLY, "Features Reply (CSM)" }, @@ -80,6 +80,7 @@ static const value_string names_ofp_type[] = { /* Statistics messages. */ { OFPT_STATS_REQUEST, "Stats Request (CSM)" }, { OFPT_STATS_REPLY, "Stats Reply (CSM)" }, + { 0, NULL } }; #define OFP_TYPE_MAX_VALUE OFPT_STATS_REPLY @@ -105,7 +106,7 @@ static const value_string names_ofp_action_type[] = { #define NUM_PORT_FEATURES_FLAGS 12 #define NUM_WILDCARDS 11 #define NUM_CAPABILITIES_FLAGS 6 -#define NUM_FLOW_MOD_FLAGS 1 +#define NUM_FLOW_MOD_FLAGS 3 #define NUM_SF_REPLY_FLAGS 1 /** yes/no for bitfields field */ @@ -267,7 +268,8 @@ static const gchar *bad_action_err_str[] = {"Unknown action type", #define N_BADACTION (sizeof bad_action_err_str / sizeof bad_action_err_str[0]) -static const gchar *flow_mod_failed_err_str[] = {"Flow not added because of full tables"}; +static const gchar *flow_mod_failed_err_str[] = {"Flow not added because of full tables", + "Flow not added because of conflicting entry in tables"}; #define N_FLOWMODFAILED (sizeof flow_mod_failed_err_str / sizeof flow_mod_failed_err_str[0]) @@ -664,7 +666,7 @@ void proto_register_openflow() ofp_port_mod_mask[i] = -1; } for( i=0; i