| Message ID | 20180208231849.31519-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/4] Implement OF1.3 extension for OF1.4 role status feature. | expand |
On Thu, Feb 8, 2018 at 3:18 PM, Ben Pfaff <blp@ovn.org> wrote: > ONF extension pack 1 for OpenFlow 1.3 defines how to implement the OpenFlow > 1.4 "role status" message in OpenFlow 1.3. This commit implements that > feature. > > ONF-JIRA: EXT-191 > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- Looks good to me. I tested it without seeing any failure. Acked-by: William Tu <u9012063@gmail.com> > Documentation/topics/openflow.rst | 8 ----- > NEWS | 2 ++ > include/openvswitch/ofp-msgs.h | 5 ++- > lib/ofp-util.c | 33 ++++++++++---------- > tests/ofp-print.at | 30 ++++++++++++++++++ > tests/ofproto.at | 66 +++++++++++++++++++++++++++++++++++++++ > 6 files changed, 118 insertions(+), 26 deletions(-) > > diff --git a/Documentation/topics/openflow.rst b/Documentation/topics/openflow.rst > index d82d544d9246..381f94d9c22c 100644 > --- a/Documentation/topics/openflow.rst > +++ b/Documentation/topics/openflow.rst > @@ -159,14 +159,6 @@ in OVS. > (EXT-187) > (optional for OF1.4+) > > -* Role Status > - > - Already implemented as a 1.4 feature. > - > - (EXT-191) > - > - (required for OF1.4+) > - > * Flow entry eviction > > OVS has flow eviction functionality. ``table_mod OFPTC_EVICTION``, > diff --git a/NEWS b/NEWS > index 6973f6b45ebf..4f1b3258d31b 100644 > --- a/NEWS > +++ b/NEWS > @@ -10,6 +10,8 @@ Post-v2.9.0 > default it always accepts names and in interactive use it displays them; > use --names or --no-names to override. See ovs-ofctl(8) for details. > - ovs-vsctl: New commands "add-bond-iface" and "del-bond-iface". > + - OpenFlow: > + * OFPT_ROLE_STATUS is now available in OpenFlow 1.3. > > > v2.9.0 - xx xxx xxxx > diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h > index 5f3815c140ef..3f92f2a67751 100644 > --- a/include/openvswitch/ofp-msgs.h > +++ b/include/openvswitch/ofp-msgs.h > @@ -262,6 +262,8 @@ enum ofpraw { > /* OFPT 1.3+ (29): struct ofp13_meter_mod, uint8_t[8][]. */ > OFPRAW_OFPT13_METER_MOD, > > + /* ONFT 1.3 (1911): struct ofp14_role_status, uint8_t[8][]. */ > + OFPRAW_ONFT13_ROLE_STATUS, > /* OFPT 1.4+ (30): struct ofp14_role_status, uint8_t[8][]. */ > OFPRAW_OFPT14_ROLE_STATUS, > > @@ -615,7 +617,8 @@ enum ofptype { > OFPTYPE_METER_MOD, /* OFPRAW_OFPT13_METER_MOD. */ > > /* Controller role change event messages. */ > - OFPTYPE_ROLE_STATUS, /* OFPRAW_OFPT14_ROLE_STATUS. */ > + OFPTYPE_ROLE_STATUS, /* OFPRAW_ONFT13_ROLE_STATUS. > + * OFPRAW_OFPT14_ROLE_STATUS. */ > > /* Request forwarding by the switch. */ > OFPTYPE_REQUESTFORWARD, /* OFPRAW_OFPT14_REQUESTFORWARD. */ > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index 59fbf5f58425..c3b1222a69fb 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -6201,24 +6201,21 @@ struct ofpbuf * > ofputil_encode_role_status(const struct ofputil_role_status *status, > enum ofputil_protocol protocol) > { > - enum ofp_version version; > - > - version = ofputil_protocol_to_ofp_version(protocol); > - if (version >= OFP14_VERSION) { > - struct ofp14_role_status *rstatus; > - struct ofpbuf *buf; > - > - buf = ofpraw_alloc_xid(OFPRAW_OFPT14_ROLE_STATUS, version, htonl(0), > - 0); > - rstatus = ofpbuf_put_zeros(buf, sizeof *rstatus); > - rstatus->role = htonl(status->role); > - rstatus->reason = status->reason; > - rstatus->generation_id = htonll(status->generation_id); > - > - return buf; > - } else { > + enum ofp_version version = ofputil_protocol_to_ofp_version(protocol); > + if (version < OFP13_VERSION) { > return NULL; > } > + > + enum ofpraw raw = (version >= OFP14_VERSION > + ? OFPRAW_OFPT14_ROLE_STATUS > + : OFPRAW_ONFT13_ROLE_STATUS); > + struct ofpbuf *buf = ofpraw_alloc_xid(raw, version, htonl(0), 0); > + struct ofp14_role_status *rstatus = ofpbuf_put_zeros(buf, sizeof *rstatus); > + rstatus->role = htonl(status->role); > + rstatus->reason = status->reason; > + rstatus->generation_id = htonll(status->generation_id); > + > + return buf; > } > > enum ofperr > @@ -6226,7 +6223,9 @@ ofputil_decode_role_status(const struct ofp_header *oh, > struct ofputil_role_status *rs) > { > struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); > - ovs_assert(ofpraw_pull_assert(&b) == OFPRAW_OFPT14_ROLE_STATUS); > + enum ofpraw raw = ofpraw_pull_assert(&b); > + ovs_assert(raw == OFPRAW_OFPT14_ROLE_STATUS || > + raw == OFPRAW_ONFT13_ROLE_STATUS); > > const struct ofp14_role_status *r = b.msg; > if (r->role != htonl(OFPCR12_ROLE_NOCHANGE) && > diff --git a/tests/ofp-print.at b/tests/ofp-print.at > index 821087ce108f..38b8ac0579ef 100644 > --- a/tests/ofp-print.at > +++ b/tests/ofp-print.at > @@ -2852,6 +2852,36 @@ NXT_ROLE_REPLY (xid=0x2): role=slave > ]) > AT_CLEANUP > > +AT_SETUP([OFP_ROLE_STATUS - master, experimenter - OF1.3]) > +AT_KEYWORDS([ofp-print]) > +AT_CHECK([ovs-ofctl ofp-print "\ > +04 04 00 20 00 00 00 0a 4f 4e 46 00 00 00 07 77 \ > +00 00 00 02 02 00 00 00 ff ff ff ff ff ff ff ff \ > +"], [0], [dnl > +ONFT_ROLE_STATUS (OF1.3) (xid=0xa): role=master reason=experimenter_data_changed > +]) > +AT_CLEANUP > + > +AT_SETUP([OFP_ROLE_STATUS - master, config - OF1.3]) > +AT_KEYWORDS([ofp-print]) > +AT_CHECK([ovs-ofctl ofp-print "\ > +04 04 00 20 00 00 00 0a 4f 4e 46 00 00 00 07 77 \ > +00 00 00 02 01 00 00 00 ff ff ff ff ff ff ff ff \ > +"], [0], [dnl > +ONFT_ROLE_STATUS (OF1.3) (xid=0xa): role=master reason=configuration_changed > +]) > +AT_CLEANUP > + > +AT_SETUP([OFP_ROLE_STATUS - master, config,generation - OF1.3]) > +AT_KEYWORDS([ofp-print]) > +AT_CHECK([ovs-ofctl ofp-print "\ > +04 04 00 20 00 00 00 0a 4f 4e 46 00 00 00 07 77 \ > +00 00 00 02 01 00 00 00 00 00 00 00 00 00 00 10 \ > +"], [0], [dnl > +ONFT_ROLE_STATUS (OF1.3) (xid=0xa): role=master generation_id=16 reason=configuration_changed > +]) > +AT_CLEANUP > + > AT_SETUP([OFP_ROLE_STATUS - master, experimenter - OF1.4]) > AT_KEYWORDS([ofp-print]) > AT_CHECK([ovs-ofctl ofp-print "\ > diff --git a/tests/ofproto.at b/tests/ofproto.at > index 1b0ba94fd837..c1beea7aec89 100644 > --- a/tests/ofproto.at > +++ b/tests/ofproto.at > @@ -3945,6 +3945,72 @@ done > OVS_VSWITCHD_STOP > AT_CLEANUP > > +dnl This test checks that the role request/response messaging works, > +dnl that generation_id is handled properly, and that role status update > +dnl messages are sent when a controller's role gets changed from master > +dnl to slave. > +AT_SETUP([ofproto - controller role (OpenFlow 1.3)]) > +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([experr1]) > +AT_CAPTURE_FILE([monitor2.log]) > +AT_CAPTURE_FILE([expout2]) > +AT_CAPTURE_FILE([experr2]) > +for i in 1 2; do > + AT_CHECK([ovs-ofctl -O OpenFlow13 monitor br0 --detach --no-chdir --pidfile=c$i.pid --unixctl=c$i]) > + ovs-appctl -t `pwd`/c$i ofctl/barrier > + ovs-appctl -t `pwd`/c$i ofctl/set-output-file monitor$i.log > + : > expout$i > + : > experr$i > + > + # find out current role > + ovs-appctl -t `pwd`/c$i ofctl/send 041800180000000200000000000000000000000000000000 > + echo >>experr$i "send: OFPT_ROLE_REQUEST (OF1.3): role=nochange" > + echo >>expout$i "OFPT_ROLE_REPLY (OF1.3): role=equal" > +done > + > +# controller 1: Become slave (generation_id is initially undefined, so > +# 2^63+2 should not be stale) > +ovs-appctl -t `pwd`/c1 ofctl/send 041800180000000300000003000000008000000000000002 > +echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.3): role=slave generation_id=9223372036854775810" > +echo >>expout1 "OFPT_ROLE_REPLY (OF1.3): role=slave generation_id=9223372036854775810" > + > +# controller 2: Become master. > +ovs-appctl -t `pwd`/c2 ofctl/send 041800180000000300000002000000008000000000000003 > +echo >>experr2 "send: OFPT_ROLE_REQUEST (OF1.3): role=master generation_id=9223372036854775811" > +echo >>expout2 "OFPT_ROLE_REPLY (OF1.3): role=master generation_id=9223372036854775811" > + > +# controller 1: Try to become the master using a stale generation ID > +ovs-appctl -t `pwd`/c1 ofctl/send 041800180000000400000002000000000000000000000003 > +echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.3): role=master generation_id=3" > +echo >>expout1 "OFPT_ERROR (OF1.3): OFPRRFC_STALE" > +echo >>expout1 "OFPT_ROLE_REQUEST (OF1.3): role=master generation_id=3" > + > +# controller 1: Become master using a valid generation ID > +ovs-appctl -t `pwd`/c1 ofctl/send 041800180000000500000002000000000000000000000001 > +echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.3): role=master generation_id=1" > +echo >>expout1 "OFPT_ROLE_REPLY (OF1.3): role=master generation_id=1" > +echo >>expout2 "ONFT_ROLE_STATUS (OF1.3): role=slave generation_id=1 reason=master_request" > + > +for i in 1 2; do > + ovs-appctl -t `pwd`/c$i ofctl/barrier > + echo >>expout$i "OFPT_BARRIER_REPLY (OF1.3):" > +done > + > +# Check output. > +for i in 1 2; do > + cp expout$i expout > + AT_CHECK([grep -v '^send:' monitor$i.log | strip_xids], [0], [expout]) > + cp experr$i expout > + AT_CHECK([grep '^send:' monitor$i.log | strip_xids], [0], [expout]) > +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 controllers. > -- > 2.15.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/Documentation/topics/openflow.rst b/Documentation/topics/openflow.rst index d82d544d9246..381f94d9c22c 100644 --- a/Documentation/topics/openflow.rst +++ b/Documentation/topics/openflow.rst @@ -159,14 +159,6 @@ in OVS. (EXT-187) (optional for OF1.4+) -* Role Status - - Already implemented as a 1.4 feature. - - (EXT-191) - - (required for OF1.4+) - * Flow entry eviction OVS has flow eviction functionality. ``table_mod OFPTC_EVICTION``, diff --git a/NEWS b/NEWS index 6973f6b45ebf..4f1b3258d31b 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,8 @@ Post-v2.9.0 default it always accepts names and in interactive use it displays them; use --names or --no-names to override. See ovs-ofctl(8) for details. - ovs-vsctl: New commands "add-bond-iface" and "del-bond-iface". + - OpenFlow: + * OFPT_ROLE_STATUS is now available in OpenFlow 1.3. v2.9.0 - xx xxx xxxx diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h index 5f3815c140ef..3f92f2a67751 100644 --- a/include/openvswitch/ofp-msgs.h +++ b/include/openvswitch/ofp-msgs.h @@ -262,6 +262,8 @@ enum ofpraw { /* OFPT 1.3+ (29): struct ofp13_meter_mod, uint8_t[8][]. */ OFPRAW_OFPT13_METER_MOD, + /* ONFT 1.3 (1911): struct ofp14_role_status, uint8_t[8][]. */ + OFPRAW_ONFT13_ROLE_STATUS, /* OFPT 1.4+ (30): struct ofp14_role_status, uint8_t[8][]. */ OFPRAW_OFPT14_ROLE_STATUS, @@ -615,7 +617,8 @@ enum ofptype { OFPTYPE_METER_MOD, /* OFPRAW_OFPT13_METER_MOD. */ /* Controller role change event messages. */ - OFPTYPE_ROLE_STATUS, /* OFPRAW_OFPT14_ROLE_STATUS. */ + OFPTYPE_ROLE_STATUS, /* OFPRAW_ONFT13_ROLE_STATUS. + * OFPRAW_OFPT14_ROLE_STATUS. */ /* Request forwarding by the switch. */ OFPTYPE_REQUESTFORWARD, /* OFPRAW_OFPT14_REQUESTFORWARD. */ diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 59fbf5f58425..c3b1222a69fb 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -6201,24 +6201,21 @@ struct ofpbuf * ofputil_encode_role_status(const struct ofputil_role_status *status, enum ofputil_protocol protocol) { - enum ofp_version version; - - version = ofputil_protocol_to_ofp_version(protocol); - if (version >= OFP14_VERSION) { - struct ofp14_role_status *rstatus; - struct ofpbuf *buf; - - buf = ofpraw_alloc_xid(OFPRAW_OFPT14_ROLE_STATUS, version, htonl(0), - 0); - rstatus = ofpbuf_put_zeros(buf, sizeof *rstatus); - rstatus->role = htonl(status->role); - rstatus->reason = status->reason; - rstatus->generation_id = htonll(status->generation_id); - - return buf; - } else { + enum ofp_version version = ofputil_protocol_to_ofp_version(protocol); + if (version < OFP13_VERSION) { return NULL; } + + enum ofpraw raw = (version >= OFP14_VERSION + ? OFPRAW_OFPT14_ROLE_STATUS + : OFPRAW_ONFT13_ROLE_STATUS); + struct ofpbuf *buf = ofpraw_alloc_xid(raw, version, htonl(0), 0); + struct ofp14_role_status *rstatus = ofpbuf_put_zeros(buf, sizeof *rstatus); + rstatus->role = htonl(status->role); + rstatus->reason = status->reason; + rstatus->generation_id = htonll(status->generation_id); + + return buf; } enum ofperr @@ -6226,7 +6223,9 @@ ofputil_decode_role_status(const struct ofp_header *oh, struct ofputil_role_status *rs) { struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); - ovs_assert(ofpraw_pull_assert(&b) == OFPRAW_OFPT14_ROLE_STATUS); + enum ofpraw raw = ofpraw_pull_assert(&b); + ovs_assert(raw == OFPRAW_OFPT14_ROLE_STATUS || + raw == OFPRAW_ONFT13_ROLE_STATUS); const struct ofp14_role_status *r = b.msg; if (r->role != htonl(OFPCR12_ROLE_NOCHANGE) && diff --git a/tests/ofp-print.at b/tests/ofp-print.at index 821087ce108f..38b8ac0579ef 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -2852,6 +2852,36 @@ NXT_ROLE_REPLY (xid=0x2): role=slave ]) AT_CLEANUP +AT_SETUP([OFP_ROLE_STATUS - master, experimenter - OF1.3]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +04 04 00 20 00 00 00 0a 4f 4e 46 00 00 00 07 77 \ +00 00 00 02 02 00 00 00 ff ff ff ff ff ff ff ff \ +"], [0], [dnl +ONFT_ROLE_STATUS (OF1.3) (xid=0xa): role=master reason=experimenter_data_changed +]) +AT_CLEANUP + +AT_SETUP([OFP_ROLE_STATUS - master, config - OF1.3]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +04 04 00 20 00 00 00 0a 4f 4e 46 00 00 00 07 77 \ +00 00 00 02 01 00 00 00 ff ff ff ff ff ff ff ff \ +"], [0], [dnl +ONFT_ROLE_STATUS (OF1.3) (xid=0xa): role=master reason=configuration_changed +]) +AT_CLEANUP + +AT_SETUP([OFP_ROLE_STATUS - master, config,generation - OF1.3]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +04 04 00 20 00 00 00 0a 4f 4e 46 00 00 00 07 77 \ +00 00 00 02 01 00 00 00 00 00 00 00 00 00 00 10 \ +"], [0], [dnl +ONFT_ROLE_STATUS (OF1.3) (xid=0xa): role=master generation_id=16 reason=configuration_changed +]) +AT_CLEANUP + AT_SETUP([OFP_ROLE_STATUS - master, experimenter - OF1.4]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl ofp-print "\ diff --git a/tests/ofproto.at b/tests/ofproto.at index 1b0ba94fd837..c1beea7aec89 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -3945,6 +3945,72 @@ done OVS_VSWITCHD_STOP AT_CLEANUP +dnl This test checks that the role request/response messaging works, +dnl that generation_id is handled properly, and that role status update +dnl messages are sent when a controller's role gets changed from master +dnl to slave. +AT_SETUP([ofproto - controller role (OpenFlow 1.3)]) +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([experr1]) +AT_CAPTURE_FILE([monitor2.log]) +AT_CAPTURE_FILE([expout2]) +AT_CAPTURE_FILE([experr2]) +for i in 1 2; do + AT_CHECK([ovs-ofctl -O OpenFlow13 monitor br0 --detach --no-chdir --pidfile=c$i.pid --unixctl=c$i]) + ovs-appctl -t `pwd`/c$i ofctl/barrier + ovs-appctl -t `pwd`/c$i ofctl/set-output-file monitor$i.log + : > expout$i + : > experr$i + + # find out current role + ovs-appctl -t `pwd`/c$i ofctl/send 041800180000000200000000000000000000000000000000 + echo >>experr$i "send: OFPT_ROLE_REQUEST (OF1.3): role=nochange" + echo >>expout$i "OFPT_ROLE_REPLY (OF1.3): role=equal" +done + +# controller 1: Become slave (generation_id is initially undefined, so +# 2^63+2 should not be stale) +ovs-appctl -t `pwd`/c1 ofctl/send 041800180000000300000003000000008000000000000002 +echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.3): role=slave generation_id=9223372036854775810" +echo >>expout1 "OFPT_ROLE_REPLY (OF1.3): role=slave generation_id=9223372036854775810" + +# controller 2: Become master. +ovs-appctl -t `pwd`/c2 ofctl/send 041800180000000300000002000000008000000000000003 +echo >>experr2 "send: OFPT_ROLE_REQUEST (OF1.3): role=master generation_id=9223372036854775811" +echo >>expout2 "OFPT_ROLE_REPLY (OF1.3): role=master generation_id=9223372036854775811" + +# controller 1: Try to become the master using a stale generation ID +ovs-appctl -t `pwd`/c1 ofctl/send 041800180000000400000002000000000000000000000003 +echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.3): role=master generation_id=3" +echo >>expout1 "OFPT_ERROR (OF1.3): OFPRRFC_STALE" +echo >>expout1 "OFPT_ROLE_REQUEST (OF1.3): role=master generation_id=3" + +# controller 1: Become master using a valid generation ID +ovs-appctl -t `pwd`/c1 ofctl/send 041800180000000500000002000000000000000000000001 +echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.3): role=master generation_id=1" +echo >>expout1 "OFPT_ROLE_REPLY (OF1.3): role=master generation_id=1" +echo >>expout2 "ONFT_ROLE_STATUS (OF1.3): role=slave generation_id=1 reason=master_request" + +for i in 1 2; do + ovs-appctl -t `pwd`/c$i ofctl/barrier + echo >>expout$i "OFPT_BARRIER_REPLY (OF1.3):" +done + +# Check output. +for i in 1 2; do + cp expout$i expout + AT_CHECK([grep -v '^send:' monitor$i.log | strip_xids], [0], [expout]) + cp experr$i expout + AT_CHECK([grep '^send:' monitor$i.log | strip_xids], [0], [expout]) +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 controllers.
ONF extension pack 1 for OpenFlow 1.3 defines how to implement the OpenFlow 1.4 "role status" message in OpenFlow 1.3. This commit implements that feature. ONF-JIRA: EXT-191 Signed-off-by: Ben Pfaff <blp@ovn.org> --- Documentation/topics/openflow.rst | 8 ----- NEWS | 2 ++ include/openvswitch/ofp-msgs.h | 5 ++- lib/ofp-util.c | 33 ++++++++++---------- tests/ofp-print.at | 30 ++++++++++++++++++ tests/ofproto.at | 66 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 118 insertions(+), 26 deletions(-)