From 6b83a3c5c37a07118681a3738ec776a7d8485a3f Mon Sep 17 00:00:00 2001 From: Simon Horman Date: Wed, 2 Apr 2014 18:04:27 +0900 Subject: [PATCH] ofproto: Support OF version-specific table-miss behaviours OpenFlow 1.1 and 1.2 specify that if a table-miss occurs then the default behaviour is to forward the packet the controller using a packet-in message. And until this patch this is the default behaviour that Open vSwitch uses for all OpenFlow versions. OpenFlow1.3+ specifies that if a table-miss occurs then the default behaviour is simply to drop the packet. This patch implements this behaviour using the following logic: If a table-miss occurs and the table-miss behaviour for the table has not been set using a table_mod (in which case it is no longer the default setting) then: * Installing a facet in the datapath with a drop action in the if there are no pre-OF1.3 controllers connected which would receive an packet_in message. Note that this covers both the case where there are only OF1.3 controllers and the case where there are no controllers at all. * Otherwise sent a packet_in message to all pre-OF1.3 controllers. This covers both the case where there are only pre-OF1.3 controllers and there are both pre-OF1.3 and OF1.3+ controllers. Signed-off-by: Simon Horman Signed-off-by: Ben Pfaff --- OPENFLOW-1.1+ | 7 ---- ofproto/connmgr.c | 62 +++++++++++++++++++++++++++++++++++- ofproto/connmgr.h | 1 + ofproto/ofproto-dpif-xlate.c | 5 +++ ofproto/ofproto-dpif.c | 44 +++++++++++++++++++------ ofproto/ofproto-dpif.h | 8 +++++ ofproto/ofproto-provider.h | 2 +- ofproto/ofproto.c | 8 ++--- ofproto/ofproto.h | 20 ++++++++++-- tests/ofproto-dpif.at | 42 ++++++++++++++++++++---- tests/tunnel.at | 2 +- 11 files changed, 168 insertions(+), 33 deletions(-) diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+ index 6fee77c91..b60b8c1ad 100644 --- a/OPENFLOW-1.1+ +++ b/OPENFLOW-1.1+ @@ -89,13 +89,6 @@ didn't compare the specs carefully yet.) * Add OFPMP_TABLE_FEATURES statistics. Alexander Wu has posted a patch series. [optional for OF1.3+] - * More flexible table miss support. - This requires the following. - - Change the default table-miss action (in the absense of table-miss - entry) from packet_in to drop for OF1.3+. Decide what to do if - a switch is configured to support multiple OF versions. - [required for OF1.3+] - * IPv6 extension header handling support. Fully implementing this requires kernel support. This likely will take some careful and probably time-consuming design work. The actual coding, once diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index 0bcb325a1..383fbdacc 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -1410,6 +1410,65 @@ ofconn_receives_async_msg(const struct ofconn *ofconn, return true; } +/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the + * packet rather than to send the packet to the controller. + * + * This function returns false to indicate the packet should be dropped if + * the controller action was the result of the default table-miss behaviour + * and the controller is using OpenFlow1.3+. + * + * Otherwise true is returned to indicate the packet should be forwarded to + * the controller */ +static bool +ofconn_wants_packet_in_on_miss(struct ofconn *ofconn, + const struct ofproto_packet_in *pin) +{ + if (pin->miss_type == OFPROTO_PACKET_IN_MISS_WITHOUT_FLOW) { + enum ofputil_protocol protocol = ofconn_get_protocol(ofconn); + + if (protocol != OFPUTIL_P_NONE + && ofputil_protocol_to_ofp_version(protocol) >= OFP13_VERSION) { + enum ofproto_table_config config; + + config = ofproto_table_get_config(ofconn->connmgr->ofproto, + pin->up.table_id); + if (config == OFPROTO_TABLE_MISS_DEFAULT) { + return false; + } + } + } + return true; +} + +/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the + * packet rather than to send the packet to the controller. + * + * This function returns false to indicate that a packet_in message + * for a "table-miss" should be sent to at least one controller. + * That is there is at least one controller with controller_id 0 + * which connected using an OpenFlow version earlier than OpenFlow1.3. + * + * False otherwise. + * + * This logic assumes that "table-miss" packet_in messages + * are always sent to controller_id 0. */ +bool +connmgr_wants_packet_in_on_miss(struct connmgr *mgr) +{ + struct ofconn *ofconn; + + LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { + enum ofputil_protocol protocol = ofconn_get_protocol(ofconn); + + if (ofconn->controller_id == 0 && + (protocol == OFPUTIL_P_NONE || + ofputil_protocol_to_ofp_version(protocol) < OFP13_VERSION)) { + return true; + } + } + return false; +} + /* Returns a human-readable name for an OpenFlow connection between 'mgr' and * 'target', suitable for use in log messages for identifying the connection. * @@ -1559,7 +1618,8 @@ connmgr_send_packet_in(struct connmgr *mgr, LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { enum ofp_packet_in_reason reason = wire_reason(ofconn, pin); - if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, reason) + if (ofconn_wants_packet_in_on_miss(ofconn, pin) + && ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->up.reason) && ofconn->controller_id == pin->controller_id) { schedule_packet_in(ofconn, *pin, reason); } diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index c4cfd8337..20c81605e 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -162,6 +162,7 @@ void ofconn_remove_opgroup(struct ofconn *, struct list *, const struct ofp_header *request, int error); /* Sending asynchronous messages. */ +bool connmgr_wants_packet_in_on_miss(struct connmgr *mgr); void connmgr_send_port_status(struct connmgr *, struct ofconn *source, const struct ofputil_phy_port *, uint8_t reason); void connmgr_send_flow_removed(struct connmgr *, diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index a6bfc8702..b8e808480 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1924,6 +1924,11 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id, case RULE_DPIF_LOOKUP_VERDICT_DROP: config = OFPUTIL_PC_NO_PACKET_IN; break; + case RULE_DPIF_LOOKUP_VERDICT_DEFAULT: + if (!ofproto_dpif_wants_packet_in_on_miss(ctx->xbridge->ofproto)) { + config = OFPUTIL_PC_NO_PACKET_IN; + } + break; default: OVS_NOT_REACHED(); } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 9ec0237db..f72d53e18 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -366,6 +366,18 @@ ofproto_dpif_send_packet_in(struct ofproto_dpif *ofproto, free(pin); } } + +/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the + * packet rather than to send the packet to the controller. + * + * This function returns false to indicate that a packet_in message + * for a "table-miss" should be sent to at least one controller. + * False otherwise. */ +bool +ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *ofproto) +{ + return connmgr_wants_packet_in_on_miss(ofproto->up.connmgr); +} /* Factory functions. */ @@ -3108,6 +3120,11 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow, case RULE_DPIF_LOOKUP_VERDICT_DROP: config = OFPUTIL_PC_NO_PACKET_IN; break; + case RULE_DPIF_LOOKUP_VERDICT_DEFAULT: + if (!connmgr_wants_packet_in_on_miss(ofproto->up.connmgr)) { + config = OFPUTIL_PC_NO_PACKET_IN; + } + break; default: OVS_NOT_REACHED(); } @@ -3177,12 +3194,17 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, uint8_t table_id, * * - RULE_DPIF_LOOKUP_VERDICT_MATCH if a rule (in '*rule') was found. * - * - RULE_DPIF_LOOKUP_VERDICT_DROP if no rule was found and a table miss - * configuration specified that the packet should be dropped in this - * case. (This occurs only if 'honor_table_miss' is true, because only in - * this case does the table miss configuration matter.) + * - RULE_OFPTC_TABLE_MISS_CONTROLLER if no rule was found and either: + * + 'honor_table_miss' is false + * + a table miss configuration specified that the packet should be * - * - RULE_DPIF_LOOKUP_VERDICT_CONTROLLER if no rule was found otherwise. */ + * - RULE_DPIF_LOOKUP_VERDICT_DROP if no rule was found, 'honor_table_miss' + * is true and a table miss configuration specified that the packet + * should be dropped in this case. + * + * - RULE_DPIF_LOOKUP_VERDICT_DEFAULT if no rule was found, + * 'honor_table_miss' is true and a table miss configuration has + * not been specified in this case. */ enum rule_dpif_lookup_verdict rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto, const struct flow *flow, @@ -3203,16 +3225,18 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto, } else if (!honor_table_miss) { return RULE_DPIF_LOOKUP_VERDICT_CONTROLLER; } else { - switch (table_get_config(&ofproto->up, *table_id) - & OFPTC11_TABLE_MISS_MASK) { - case OFPTC11_TABLE_MISS_CONTINUE: + switch (ofproto_table_get_config(&ofproto->up, *table_id)) { + case OFPROTO_TABLE_MISS_CONTINUE: break; - case OFPTC11_TABLE_MISS_CONTROLLER: + case OFPROTO_TABLE_MISS_CONTROLLER: return RULE_DPIF_LOOKUP_VERDICT_CONTROLLER; - case OFPTC11_TABLE_MISS_DROP: + case OFPROTO_TABLE_MISS_DROP: return RULE_DPIF_LOOKUP_VERDICT_DROP; + + case OFPROTO_TABLE_MISS_DEFAULT: + return RULE_DPIF_LOOKUP_VERDICT_DEFAULT; } } } diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index 088ff8931..ae6f9b75c 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -42,6 +42,13 @@ enum rule_dpif_lookup_verdict { * the controller. */ RULE_DPIF_LOOKUP_VERDICT_DROP, /* A miss occurred and the packet * should be dropped. */ + RULE_DPIF_LOOKUP_VERDICT_DEFAULT, /* A miss occurred and the packet + * should handled by the default + * miss behaviour. + * For pre-OF1.3 it should be + * forwarded to the controller. + * For OF1.3+ it should be + * dropped. */ }; /* For lock annotation below only. */ @@ -130,6 +137,7 @@ int ofproto_dpif_execute_actions(struct ofproto_dpif *, const struct flow *, OVS_EXCLUDED(xlate_rwlock); void ofproto_dpif_send_packet_in(struct ofproto_dpif *, struct ofproto_packet_in *); +bool ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *); int ofproto_dpif_send_packet(const struct ofport_dpif *, struct ofpbuf *); void ofproto_dpif_flow_mod(struct ofproto_dpif *, struct ofputil_flow_mod *); diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index a7bf4df4f..9f37f7170 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -262,7 +262,7 @@ struct oftable { struct hmap eviction_groups_by_id; struct heap eviction_groups_by_size; - /* Table config: contains enum ofp_table_config; accessed atomically. */ + /* Table config: contains enum ofproto_table_config; accessed atomically. */ atomic_uint config; }; diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 80336de90..677da8c92 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -5786,12 +5786,12 @@ handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh) } } -enum ofp_table_config -table_get_config(const struct ofproto *ofproto, uint8_t table_id) +enum ofproto_table_config +ofproto_table_get_config(const struct ofproto *ofproto, uint8_t table_id) { unsigned int value; atomic_read(&ofproto->tables[table_id].config, &value); - return (enum ofp_table_config)value; + return (enum ofproto_table_config)value; } static enum ofperr @@ -6678,7 +6678,7 @@ oftable_init(struct oftable *table) memset(table, 0, sizeof *table); classifier_init(&table->cls, flow_segment_u32s); table->max_flows = UINT_MAX; - atomic_init(&table->config, (unsigned int)OFPTC11_TABLE_MISS_CONTROLLER); + atomic_init(&table->config, (unsigned int)OFPROTO_TABLE_MISS_DEFAULT); } /* Destroys 'table', including its classifier and eviction groups. diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 309511ca8..3f3557cf6 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -436,8 +436,24 @@ int ofproto_port_set_realdev(struct ofproto *, ofp_port_t vlandev_ofp_port, /* Table configuration */ -enum ofp_table_config table_get_config(const struct ofproto *, - uint8_t table_id); +enum ofproto_table_config { + /* Send to controller. */ + OFPROTO_TABLE_MISS_CONTROLLER = OFPTC11_TABLE_MISS_CONTROLLER, + + /* Continue to the next table in the pipeline (OpenFlow 1.0 behavior). */ + OFPROTO_TABLE_MISS_CONTINUE = OFPTC11_TABLE_MISS_CONTINUE, + + /* Drop the packet. */ + OFPROTO_TABLE_MISS_DROP = OFPTC11_TABLE_MISS_DROP, + + /* The default miss behaviour for the OpenFlow version of the controller a + * packet_in message would be sent to.. For pre-OF1.3 controllers, send + * packet_in to controller. For OF1.3+ controllers, drop. */ + OFPROTO_TABLE_MISS_DEFAULT = 3, +}; + +enum ofproto_table_config ofproto_table_get_config(const struct ofproto *, + uint8_t table_id); #ifdef __cplusplus } diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index fdb4992b8..e78788aa4 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -440,7 +440,7 @@ AT_CHECK([tail -1 stdout], [0], OVS_VSWITCHD_STOP AT_CLEANUP -AT_SETUP([ofproto-dpif - Table Miss - OFPTC_TABLE_MISS_CONTROLLER]) +AT_SETUP([ofproto-dpif - Default Table Miss - OF1.0 (OFPTC_TABLE_MISS_CONTROLLER)]) OVS_VSWITCHD_START([dnl add-port br0 p1 -- set Interface p1 type=dummy ]) @@ -474,6 +474,34 @@ NXST_FLOW reply: OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - Default Table Miss - OF1.3 (OFPTC_TABLE_MISS_DROP)]) +OVS_VSWITCHD_START([dnl + add-port br0 p1 -- set Interface p1 type=dummy +]) +ON_EXIT([kill `cat ovs-ofctl.pid`]) + +AT_CAPTURE_FILE([ofctl_monitor.log]) +AT_CHECK([ovs-ofctl del-flows br0]) + +AT_CHECK([ovs-ofctl monitor -OOpenFlow13 -P openflow10 br0 65534 --detach --no-chdir --pidfile 2> ofctl_monitor.log]) + +dnl Test that missed packets are droped +for i in 1 2 3 ; do + ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=9),tcp_flags(0x010)' +done +OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) + +AT_CHECK([cat ofctl_monitor.log], [0], [dnl +]) + +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore]) +AT_CHECK([ovs-ofctl -OOpenFlow13 dump-flows br0 | ofctl_strip | sort], [0], [dnl +OFPST_FLOW reply (OF1.3): +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - Table Miss - goto table and OFPTC_TABLE_MISS_CONTROLLER]) OVS_VSWITCHD_START([dnl add-port br0 p1 -- set Interface p1 type=dummy @@ -3438,21 +3466,21 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 'in_port(2),eth(src=50:54:00:00:00: AT_CHECK([ovs-appctl netdev-dummy/receive p3 'in_port(3),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) AT_CHECK([ovs-appctl dpif/dump-flows br0 | sort | STRIP_USED], [0], [dnl -skb_priority(0),in_port(1),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) -skb_priority(0),in_port(2),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) +skb_priority(0),in_port(1),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff), packets:0, bytes:0, used:never, actions:drop +skb_priority(0),in_port(2),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff), packets:0, bytes:0, used:never, actions:drop ]) AT_CHECK([ovs-appctl dpif/dump-flows br1 | sort | STRIP_USED], [0], [dnl -skb_priority(0),in_port(3),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) +skb_priority(0),in_port(3),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff), packets:0, bytes:0, used:never, actions:drop ]) AT_CHECK([ovs-appctl dpif/dump-flows -m br0 | sort | STRIP_USED], [0], [dnl -skb_priority(0),skb_mark(0/0),in_port(p1),eth(src=50:54:00:00:00:05/00:00:00:00:00:00,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) -skb_priority(0),skb_mark(0/0),in_port(p2),eth(src=50:54:00:00:00:07/00:00:00:00:00:00,dst=50:54:00:00:00:05/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff),icmp(type=0/0,code=0/0), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) +skb_priority(0),skb_mark(0/0),in_port(p1),eth(src=50:54:00:00:00:05/00:00:00:00:00:00,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:never, actions:drop +skb_priority(0),skb_mark(0/0),in_port(p2),eth(src=50:54:00:00:00:07/00:00:00:00:00:00,dst=50:54:00:00:00:05/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff),icmp(type=0/0,code=0/0), packets:0, bytes:0, used:never, actions:drop ]) AT_CHECK([ovs-appctl dpif/dump-flows -m br1 | sort | STRIP_USED], [0], [dnl -skb_priority(0),skb_mark(0/0),in_port(p3),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller)) +skb_priority(0),skb_mark(0/0),in_port(p3),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0xff),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:never, actions:drop ]) OVS_VSWITCHD_STOP diff --git a/tests/tunnel.at b/tests/tunnel.at index 8bfa94c95..bfb07f4e4 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -300,7 +300,7 @@ Datapath actions: 4,3,5 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(tun_id=0x0,src=1.1.1.1,dst=2.2.2.2,tos=0x0,ttl=64,flags(key)),in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl - - Sends "packet-in" messages to the OpenFlow controller. +Datapath actions: drop ]) OVS_VSWITCHD_STOP -- 2.43.0