From: Niti Rohilla <niti.rohi...@tcs.com> This patch adds support for Openflow1.4 Group & meter change notification messages. In a multi controller environment, when a controller modifies the state of group and meter table, the request that successfully modifies this state is forwarded to other controllers. Other controllers are informed with the OFPT_REQUESTFORWARD message. Request forwarding is enabled on a per controller channel basis using the Set Asynchronous Configuration Message. --- Comments recevied on 28th July,2015 have been incorporated in this version. Following are the changes between v1 and v2:
1. Modified ofp_print_requestforward(), now the program will not abort if a received OFPT_REQUESTFORWARD message contains any message other than a group mod or a meter mod. 2. Modified ofputil_encode_requestforward(), the final argument to ofpraw_alloc_xid() is the length of 'rf->request'. 3. Added a function ofputil_re_encode_requestforward() to re-encode the inner message for the protocol in use on each 'ofconn'. Modified ofputil_decode_requestforward() to match the versions of OFPT_REQUESTFORWARD and encapsulated "GROUP_MOD" or "METER_MOD" request and reports an error if it differs. include/openflow/openflow-1.4.h | 6 ++++ lib/learning-switch.c | 1 + lib/ofp-msgs.h | 6 ++++ lib/ofp-print.c | 36 +++++++++++++++++++ lib/ofp-util.c | 68 ++++++++++++++++++++++++++++++++++++ lib/ofp-util.h | 14 ++++++++ lib/rconn.c | 1 + ofproto/connmgr.c | 37 ++++++++++++++++++++ ofproto/connmgr.h | 3 ++ ofproto/ofproto.c | 25 +++++++++++--- ovn/controller/ofctrl.c | 1 + tests/ofp-print.at | 46 +++++++++++++++++++++++++ tests/ofproto.at | 76 +++++++++++++++++++++++++++++++++++++++++ 13 files changed, 315 insertions(+), 5 deletions(-) diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h index 37eef4a..5202fe1 100644 --- a/include/openflow/openflow-1.4.h +++ b/include/openflow/openflow-1.4.h @@ -355,6 +355,12 @@ struct ofp14_role_prop_experimenter { }; OFP_ASSERT(sizeof(struct ofp14_role_prop_experimenter) == 12); +/* Group/Meter request forwarding. */ +struct ofp14_requestforward { + struct ofp_header request; /* Request being forwarded. */ +}; +OFP_ASSERT(sizeof(struct ofp14_requestforward) == 8); + /* Bundle control message types */ enum ofp14_bundle_ctrl_type { OFPBCT_OPEN_REQUEST = 0, diff --git a/lib/learning-switch.c b/lib/learning-switch.c index 1753cea..7ddf69b 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -419,6 +419,7 @@ lswitch_process_packet(struct lswitch *sw, const struct ofpbuf *msg) case OFPTYPE_ROLE_REQUEST: case OFPTYPE_ROLE_REPLY: case OFPTYPE_ROLE_STATUS: + case OFPTYPE_REQUESTFORWARD: case OFPTYPE_SET_FLOW_FORMAT: case OFPTYPE_FLOW_MOD_TABLE_ID: case OFPTYPE_SET_PACKET_IN_FORMAT: diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h index 1358b21..52a74d1 100644 --- a/lib/ofp-msgs.h +++ b/lib/ofp-msgs.h @@ -247,6 +247,9 @@ enum ofpraw { /* OFPT 1.4+ (30): struct ofp14_role_status, uint8_t[8][]. */ OFPRAW_OFPT14_ROLE_STATUS, + /* OFPT 1.4+ (32): struct ofp14_requestforward, uint8_t[8][]. */ + OFPRAW_OFPT14_REQUESTFORWARD, + /* OFPT 1.4+ (33): struct ofp14_bundle_ctrl_msg, uint8_t[8][]. */ OFPRAW_OFPT14_BUNDLE_CONTROL, @@ -559,6 +562,9 @@ enum ofptype { /* Controller role change event messages. */ OFPTYPE_ROLE_STATUS, /* OFPRAW_OFPT14_ROLE_STATUS. */ + /* Request forwarding by the switch. */ + OFPTYPE_REQUESTFORWARD, /* OFPRAW_OFPT14_REQUESTFORWARD. */ + OFPTYPE_BUNDLE_CONTROL, /* OFPRAW_OFPT14_BUNDLE_CONTROL. */ OFPTYPE_BUNDLE_ADD_MESSAGE, /* OFPRAW_OFPT14_BUNDLE_ADD_MESSAGE. */ diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 6db32d1..f6e665d 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2960,6 +2960,38 @@ ofp_print_geneve_table_reply(struct ds *s, const struct ofp_header *oh) ofputil_uninit_geneve_table(>r.mappings); } +/* This function will print the request forward message. The reason for + * request forward is taken from rf.request.type */ +static void +ofp_print_requestforward(struct ds *string, const struct ofp_header *oh) +{ + struct ofputil_requestforward rf; + enum ofperr error; + enum ofptype type; + + error = ofputil_decode_requestforward(oh, &rf); + if (error) { + ofp_print_error(string, error); + return; + } + + error = ofptype_decode(&type, rf.request); + if (error) { + ofp_print_error(string, error); + return; + } + + ds_put_cstr(string, " reason="); + + if (type == OFPTYPE_GROUP_MOD) { + ds_put_cstr(string, "group_mod"); + ofp_print_group_mod(string, rf.request); + } else if (type == OFPTYPE_METER_MOD) { + ds_put_cstr(string, "meter_mod"); + ofp_print_meter_mod(string, rf.request); + } +} + static void ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw, struct ds *string, int verbosity) @@ -3089,6 +3121,10 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw, ofp_print_role_status_message(string, oh); break; + case OFPTYPE_REQUESTFORWARD: + ofp_print_requestforward(string, oh); + break; + case OFPTYPE_METER_STATS_REQUEST: case OFPTYPE_METER_CONFIG_STATS_REQUEST: ofp_print_stats(string, oh); diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 9996e84..4f763b9 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -5392,6 +5392,73 @@ ofputil_decode_role_status(const struct ofp_header *oh, return 0; } +/* Re-encodes the "GROUP_MOD" or "METER_MOD" request header for the + * 'protocol' in the ofconn. */ +struct ofp_header * +ofputil_re_encode_requestforward(const struct ofp_header *request, + enum ofputil_protocol protocol) +{ + enum ofp_version version; + struct ofp_header *rf; + struct ofpbuf *buf = ofpbuf_new(0); + + version = ofputil_protocol_to_ofp_version(protocol); + ofpbuf_put(buf, request, ntohs(request->length)); + rf = buf->data; + rf->version = version; + return rf; +} + +/* Encodes "request forward" message encapsulating "GROUP_MOD" or "METER_MOD" + * request in 'rf'. The OpenFlow version of OFPT_REQUESTFORWARD message and + * 'rf->request' should be same. + * 'rf->request' is not converted from host to network byte order because + * "GROUP_MOD" or "METER_MOD" request header in 'rf' is directly encapsulated + * in request forward message. */ +struct ofpbuf * +ofputil_encode_requestforward(const struct ofputil_requestforward *rf) +{ + struct ofpbuf *buf; + + buf = ofpraw_alloc_xid(OFPRAW_OFPT14_REQUESTFORWARD, rf->request->version, + htonl(0), ntohs(rf->request->length)); + ofpbuf_put(buf, rf->request, ntohs(rf->request->length)); + return buf; +} + +/* Converts OpenFlow request forward message 'oh' into an abstract request + * forward in 'rf'. Returns 0 if successful, otherwise an OpenFlow error + * code. */ +enum ofperr +ofputil_decode_requestforward(const struct ofp_header *oh, + struct ofputil_requestforward *rf) +{ + struct ofpbuf b; + enum ofpraw raw; + enum ofperr error; + size_t inner_len; + + ofpbuf_use_const(&b, oh, ntohs(oh->length)); + error = ofpraw_pull(&raw, &b); + if (error) { + return error; + } + + ovs_assert(raw == OFPRAW_OFPT14_REQUESTFORWARD); + + rf->request = b.data; + + if (rf->request->version != oh->version) { + return OFPERR_OFPBRC_BAD_VERSION; + } + inner_len = ntohs(rf->request->length); + if (inner_len < sizeof(struct ofp_header) || inner_len > b.size) { + return OFPERR_OFPBFC_MSG_BAD_LEN; + } + + return 0; +} + /* Table stats. */ /* OpenFlow 1.0 and 1.1 don't distinguish between a field that cannot be @@ -9067,6 +9134,7 @@ ofputil_is_bundlable(enum ofptype type) case OFPTYPE_TABLE_FEATURES_STATS_REPLY: case OFPTYPE_TABLE_DESC_REPLY: case OFPTYPE_ROLE_STATUS: + case OFPTYPE_REQUESTFORWARD: case OFPTYPE_NXT_GENEVE_TABLE_REQUEST: case OFPTYPE_NXT_GENEVE_TABLE_REPLY: break; diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 2668e75..827a1aa 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -861,6 +861,20 @@ struct ofpbuf *ofputil_encode_role_status( enum ofperr ofputil_decode_role_status(const struct ofp_header *oh, struct ofputil_role_status *rs); +struct ofputil_requestforward { + const struct ofp_header *request; +}; + +struct ofp_header * +ofputil_re_encode_requestforward(const struct ofp_header *request, + enum ofputil_protocol protocol); +struct ofpbuf * +ofputil_encode_requestforward(const struct ofputil_requestforward *rf); + +enum ofperr +ofputil_decode_requestforward(const struct ofp_header *oh, + struct ofputil_requestforward *rf); + /* Abstract table stats. * * This corresponds to the OpenFlow 1.3 table statistics structure, which only diff --git a/lib/rconn.c b/lib/rconn.c index 37adfa2..0a9966a 100644 --- a/lib/rconn.c +++ b/lib/rconn.c @@ -1403,6 +1403,7 @@ is_admitted_msg(const struct ofpbuf *b) case OFPTYPE_ROLE_REQUEST: case OFPTYPE_ROLE_REPLY: case OFPTYPE_ROLE_STATUS: + case OFPTYPE_REQUESTFORWARD: case OFPTYPE_SET_FLOW_FORMAT: case OFPTYPE_FLOW_MOD_TABLE_ID: case OFPTYPE_SET_PACKET_IN_FORMAT: diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index b1ba0c6..588a5cc 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -1709,6 +1709,43 @@ connmgr_send_port_status(struct connmgr *mgr, struct ofconn *source, } } +/* Sends an OFPT_REQUESTFORWARD message with 'request' and 'reason' to + * appropriate controllers managed by 'mgr'. For messages caused by a + * controller OFPT_GROUP_MOD and OFPT_METER_MOD, specify 'source' as the + * controller connection that sent the request; otherwise, specify 'source' + * as NULL. */ +void +connmgr_send_requestforward(struct connmgr *mgr, struct ofconn *source, + uint8_t reason, const struct ofp_header *request) +{ + struct ofputil_requestforward rf; + struct ofconn *ofconn; + struct ofp_header *requestp = NULL; + + LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { + if (ofconn_receives_async_msg(ofconn, OAM_REQUESTFORWARD, reason)) { + struct ofpbuf *msg; + + /* For OpenFlow 1.4 and later, it generates OFPT_REQUESTFORWARD + * for OFPT_GROUP_MOD and OFPT_METER_MOD, but not back to the + * originating controller. In a single-controller environment, in + * particular, this means that it will never generate + * OFPT_REQUESTFORWARD for OFPT_GROUP_MOD and OFPT_METER_MOD at + * all. */ + if (rconn_get_version(ofconn->rconn) < OFP14_VERSION + || ofconn == source) { + continue; + } + + requestp = ofputil_re_encode_requestforward(request, + ofconn_get_protocol(ofconn)); + rf.request = requestp; + msg = ofputil_encode_requestforward(&rf); + ofconn_send(ofconn, msg, NULL); + } + } +} + /* Sends an OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED message based on 'fr' to * appropriate controllers managed by 'mgr'. */ void diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index 7ef583a..470e421 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -168,6 +168,9 @@ void connmgr_send_packet_in(struct connmgr *, void ofconn_send_role_status(struct ofconn *ofconn, uint32_t role, uint8_t reason); +void connmgr_send_requestforward(struct connmgr *, struct ofconn *source, + uint8_t reason, const struct ofp_header *); + /* Fail-open settings. */ enum ofproto_fail_mode connmgr_get_fail_mode(const struct connmgr *); void connmgr_set_fail_mode(struct connmgr *, enum ofproto_fail_mode); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index a724071..306a1d0 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -5907,6 +5907,10 @@ handle_meter_mod(struct ofconn *ofconn, const struct ofp_header *oh) break; } + if (!error) { + connmgr_send_requestforward(ofproto->connmgr, ofconn, OFPRFR_METER_MOD, oh); + } + exit_free_bands: ofpbuf_uninit(&bands); return error; @@ -6574,20 +6578,25 @@ handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh) switch (gm.command) { case OFPGC11_ADD: - return add_group(ofproto, &gm); + error = add_group(ofproto, &gm); + break; case OFPGC11_MODIFY: - return modify_group(ofproto, &gm); + error = modify_group(ofproto, &gm); + break; case OFPGC11_DELETE: delete_group(ofproto, gm.group_id); - return 0; + error = 0; + break; case OFPGC15_INSERT_BUCKET: - return modify_group(ofproto, &gm); + error = modify_group(ofproto, &gm); + break; case OFPGC15_REMOVE_BUCKET: - return modify_group(ofproto, &gm); + error = modify_group(ofproto, &gm); + break; default: if (gm.command > OFPGC11_DELETE) { @@ -6596,6 +6605,11 @@ handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh) } return OFPERR_OFPGMFC_BAD_COMMAND; } + + if (!error) { + connmgr_send_requestforward(ofproto->connmgr, ofconn, OFPRFR_GROUP_MOD, oh); + } + return error; } enum ofputil_table_miss @@ -7210,6 +7224,7 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg) case OFPTYPE_TABLE_FEATURES_STATS_REPLY: case OFPTYPE_TABLE_DESC_REPLY: case OFPTYPE_ROLE_STATUS: + case OFPTYPE_REQUESTFORWARD: case OFPTYPE_NXT_GENEVE_TABLE_REPLY: default: if (ofpmsg_is_stat_request(oh)) { diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c index 843e1a1..3732e90 100644 --- a/ovn/controller/ofctrl.c +++ b/ovn/controller/ofctrl.c @@ -197,6 +197,7 @@ ofctrl_recv(const struct ofpbuf *msg) case OFPTYPE_ROLE_REQUEST: case OFPTYPE_ROLE_REPLY: case OFPTYPE_ROLE_STATUS: + case OFPTYPE_REQUESTFORWARD: case OFPTYPE_SET_FLOW_FORMAT: case OFPTYPE_FLOW_MOD_TABLE_ID: case OFPTYPE_SET_PACKET_IN_FORMAT: diff --git a/tests/ofp-print.at b/tests/ofp-print.at index 127bcf1..a1035ac 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -2676,6 +2676,52 @@ OFPT_ROLE_STATUS (OF1.4) (xid=0xa): role=master generation_id=16 reason=configur ]) AT_CLEANUP +AT_SETUP([OFP_REQUESTFORWARD - OF1.4]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +05 20 00 18 00 00 00 02 \ +05 0f 00 10 02 00 00 00 \ +00 00 00 00 00 00 00 01 \ +"], [0], [dnl +OFPT_REQUESTFORWARD (OF1.4) (xid=0x2): reason=group_mod + ADD group_id=1,type=all +]) +AT_CLEANUP + +AT_SETUP([OFP_REQUESTFORWARD - OF1.4]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +05 20 00 18 00 00 00 02 \ +05 0f 00 10 02 00 00 00 \ +00 01 01 00 00 00 00 01 \ +"], [0], [dnl +OFPT_REQUESTFORWARD (OF1.4) (xid=0x2): reason=group_mod + MOD group_id=1,type=select +]) +AT_CLEANUP + +AT_SETUP([OFP_REQUESTFORWARD - OF1.4]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +05 20 00 18 00 00 00 02 \ +05 1d 00 10 02 00 00 00 \ +00 00 00 00 00 00 00 01 \ +"], [0], [dnl +OFPT_REQUESTFORWARD (OF1.4) (xid=0x2): reason=meter_mod ADD meter=1 bands= +]) +AT_CLEANUP + +AT_SETUP([OFP_REQUESTFORWARD - OF1.4]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +05 20 00 18 00 00 00 02 \ +05 1d 00 10 02 00 00 00 \ +00 01 01 00 00 00 00 01 \ +"], [0], [dnl +OFPT_REQUESTFORWARD (OF1.4) (xid=0x2): reason=meter_mod MOD meter=1 flags:0x100 bands= +]) +AT_CLEANUP + AT_SETUP([NXT_SET_PACKET_IN]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl ofp-print "\ diff --git a/tests/ofproto.at b/tests/ofproto.at index cf5ab9f..7b0255b 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -2843,6 +2843,82 @@ done OVS_VSWITCHD_STOP AT_CLEANUP +dnl This test checks the Group and meter notifications when a group mod +dnl command is sent from one controller and the reply is received by +dnl other controller. +AT_SETUP([ofproto - requestforward (OpenFlow 1.4)]) +OVS_VSWITCHD_START +ON_EXIT([kill `cat c1.pid c2.pid`]) + +# Start two ovs-ofctl controller processes. +AT_CAPTURE_FILE([monitor1.log]) +AT_CAPTURE_FILE([expout1]) +AT_CAPTURE_FILE([monitor2.log]) +AT_CAPTURE_FILE([expout2]) +for i in 1 2; do + AT_CHECK([ovs-ofctl -O OpenFlow14 monitor br0 --detach --no-chdir --pidfile=`pwd`/c$i.pid --unixctl=`pwd`/c$i]) +done + +check_async () { + for i in 1 2; do + ovs-appctl -t `pwd`/c$i ofctl/barrier + ovs-appctl -t `pwd`/c$i ofctl/set-output-file monitor$i.log + : > expout$i + done + + printf '\n\n--- check_async %d ---\n\n\n' $1 + INDEX=$1 + shift + + # OFPGC_ADD + ovs-appctl -t `pwd`/c2 ofctl/send 050f0010000000020000000000000001 + if test X"$1" = X"OFPGC_ADD"; then shift; + echo >>expout2 "send: OFPT_GROUP_MOD (OF1.4): + ADD group_id=1,type=all" + echo >>expout1 "OFPT_REQUESTFORWARD (OF1.4): reason=group_mod + ADD group_id=1,type=all" + fi + + # OFPGC_MODIFY + ovs-appctl -t `pwd`/c2 ofctl/send 050f0010000000020001010000000001 + if test X"$1" = X"OFPGC_MODIFY"; then shift; + echo >>expout2 "send: OFPT_GROUP_MOD (OF1.4): + MOD group_id=1,type=select" + echo >>expout1 "OFPT_REQUESTFORWARD (OF1.4): reason=group_mod + MOD group_id=1,type=select" + fi + + for i in 1 2; do + ovs-appctl -t `pwd`/c$i ofctl/barrier + echo >>expout$i "OFPT_BARRIER_REPLY (OF1.4):" + done + + # Check output. + for i in 1 2; do + cp expout$i expout + AT_CHECK( + [[sed ' +s/ (xid=0x[0-9a-fA-F]*)//'< monitor$i.log]], + [0], [expout]) + done +} + +# controller 1: Become slave +ovs-appctl -t `pwd`/c1 ofctl/send 051800180000000300000003000000008000000000000002 + +# controller 2: Become master +ovs-appctl -t `pwd`/c2 ofctl/send 051800180000000300000002000000008000000000000003 + +# controller 1: Enabled requestforward using set Asynchronous message +ovs-appctl -t `pwd`/c1 ofctl/send 051c00280000000200000008000000050002000800000002000400080000001a000a000800000003 + +# controller 2: Enabled requestforward using set Asynchronous message +ovs-appctl -t `pwd`/c2 ofctl/send 051c002800000002000100080000000200030008000000050005000800000005000b000800000003 +check_async 1 OFPGC_ADD OFPGC_MODIFY + +OVS_VSWITCHD_STOP +AT_CLEANUP + dnl This test checks that OFPT_PACKET_OUT accepts both OFPP_NONE (as dnl specified by OpenFlow 1.0) and OFPP_CONTROLLER (used by some dnl controllers despite the spec) as meaning a packet that was generated -- 2.4.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev