On May 15, 2015 at 7:45:31 AM, Jarno Rajahalme (jrajaha...@nicira.com(mailto:jrajaha...@nicira.com)) wrote:
> > > On May 14, 2015, at 23:44, Romain Lenglet wrote: > > > > Thanks Jarno! > > > >> On May 14, 2015 at 2:11:50 PM, Jarno Rajahalme > >> (jrajaha...@nicira.com(mailto:jrajaha...@nicira.com)) wrote: > >> The new ovs-ofctl 'bundle' command accepts files similar to > >> 'add-flows', but each line can optionally start with 'add', 'modify', > >> 'delete', 'modify_strict', or 'delete_strict' keyword, so that > >> arbitrary flow table modifications may be specified in a single file. > >> The modifications in the file are executed as a single transaction > >> using a OpenFlow 1.4 bundle. > > > > I would suggest a different design for the ovs-ofctl support: add a > > --bundle option to the ovs-ofctl’s add-flows, del-flows and replace-flows, > > so that all the flow mods generated by each execution would be sent in a > > bundle. This option would be accepted only when OF1.4 or later is used. > > > > Unfortunately this does not work, as bundles are controller (connection) > specific, and each ovs-ofctl invocation is seen as a separate controller. Yes, that’s what I had in mind: one bundle per invocation of add-flows, del-flows, diff-flows, or replace-flows. > > This said, it could be possible to come up with an extension that would > circumvent this restriction between ovs-ofctl and OVS. > > > This design would allow using the current flows file format unmodified, and > > seems much more useful to me. I understand that this design wouldn’t > > support port mods, but adding support for bundles to replace-flows would be > > extremely useful. > > > > This should be doable, as replace-flows runs as a single ovs-ofctl > invocation. I'll look into this. Thanks, I’m looking forward to this! -- Romain Lenglet > > Jarno > > >> OpenFlow 1.4 requires bundles to support at least flow and port mods. > >> This implementation does not yet support port mods in bundles. > >> > >> Another restriction is that the 'atomic' keyword is not yet supported. > >> > >> Signed-off-by: Jarno Rajahalme > >> --- > >> NEWS | 10 +- > >> include/openvswitch/vconn.h | 6 +- > >> lib/ofp-parse.c | 95 +++++++++++++++++- > >> lib/ofp-parse.h | 12 ++- > >> lib/ofp-util.c | 30 ++++++ > >> lib/ofp-util.h | 2 + > >> lib/vconn.c | 27 ++++-- > >> tests/ofproto.at | 110 +++++++++++++++++++++ > >> utilities/ovs-ofctl.8.in | 32 +++++++ > >> utilities/ovs-ofctl.c | 222 +++++++++++++++++++++++++++++++++++++++++-- > >> 10 files changed, 523 insertions(+), 23 deletions(-) > >> > >> diff --git a/NEWS b/NEWS > >> index a480607..a7f9369 100644 > >> --- a/NEWS > >> +++ b/NEWS > >> @@ -1,6 +1,6 @@ > >> Post-v2.3.0 > >> --------------------- > >> - - Added support for SFQ, FQ_CoDel and CoDel qdiscs. > >> + - Added support for SFQ, FQ_CoDel and CoDel qdiscs. > >> - Add bash command-line completion support for ovs-vsctl Please check > >> utilities/ovs-command-compgen.INSTALL.md for how to use. > >> - The MAC learning feature now includes per-port fairness to mitigate > >> @@ -27,6 +27,11 @@ Post-v2.3.0 > >> commands are now redundant and will be removed in a future > >> release. See ovs-vswitchd(8) for details. > >> - OpenFlow: > >> + * OpenFlow 1.4 bundles are now supported, but for flow mod > >> + messages only. 'atomic' bundles are not yet supported, and > >> + 'ordered' bundles are trivially supported, as all bundled > >> + messages are executed in the order they were added to the > >> + bundle regardless of the presence of the 'ordered' flag. > >> * IPv6 flow label and neighbor discovery fields are now modifiable. > >> * OpenFlow 1.5 extended registers are now supported. > >> * The OpenFlow 1.5 actset_output field is now supported. > >> @@ -41,6 +46,9 @@ Post-v2.3.0 > >> * A new Netronome extension to OpenFlow 1.5+ allows control over the > >> fields hashed for OpenFlow select groups. See "selection_method" and > >> related options in ovs-ofctl(8) for details. > >> + - ovs-ofctl has a new 'bundle' command that accepts a file of > >> + arbitrary flow mods as an input that are executed as a single > >> + transaction using the new OpenFlow 1.4 bundles support. > >> - ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because > >> MD5 is no longer secure and some operating systems have started to disable > >> it in OpenSSL. > >> diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h > >> index 3b157e1..a8d7297 100644 > >> --- a/include/openvswitch/vconn.h > >> +++ b/include/openvswitch/vconn.h > >> @@ -50,8 +50,10 @@ void vconn_set_recv_any_version(struct vconn *); > >> int vconn_connect(struct vconn *); > >> int vconn_recv(struct vconn *, struct ofpbuf **); > >> int vconn_send(struct vconn *, struct ofpbuf *); > >> -int vconn_recv_xid(struct vconn *, ovs_be32 xid, struct ofpbuf **); > >> -int vconn_transact(struct vconn *, struct ofpbuf *, struct ofpbuf **); > >> +int vconn_recv_xid(struct vconn *, ovs_be32 xid, struct ofpbuf **, > >> + void (*error_reporter)(const struct ofp_header *)); > >> +int vconn_transact(struct vconn *, struct ofpbuf *, struct ofpbuf **, > >> + void (*error_reporter)(const struct ofp_header *)); > >> int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf > >> **); > >> int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list > >> *requests, > >> struct ofpbuf **replyp); > >> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c > >> index 126980c..3e746ea 100644 > >> --- a/lib/ofp-parse.c > >> +++ b/lib/ofp-parse.c > >> @@ -257,6 +257,29 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int > >> command, char *string, > >> > >> *usable_protocols = OFPUTIL_P_ANY; > >> > >> + if (command == -2) { > >> + size_t len; > >> + > >> + string += strspn(string, " \t\r\n"); /* Skip white space. */ > >> + len = strcspn(string, ", \t\r\n"); /* Get length of the first token. */ > >> + > >> + if (!strncmp(string, "add", len)) { > >> + command = OFPFC_ADD; > >> + } else if (!strncmp(string, "delete", len)) { > >> + command = OFPFC_DELETE; > >> + } else if (!strncmp(string, "delete_strict", len)) { > >> + command = OFPFC_DELETE_STRICT; > >> + } else if (!strncmp(string, "modify", len)) { > >> + command = OFPFC_MODIFY; > >> + } else if (!strncmp(string, "modify_strict", len)) { > >> + command = OFPFC_MODIFY_STRICT; > >> + } else { > >> + len = 0; > >> + command = OFPFC_ADD; > >> + } > >> + string += len; > >> + } > >> + > >> switch (command) { > >> case -1: > >> fields = 0; > >> @@ -485,6 +508,10 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int > >> command, char *string, > >> * constant for 'command'. To parse syntax for an OFPST_FLOW or > >> * OFPST_AGGREGATE (or NXST_FLOW or NXST_AGGREGATE), use -1 for 'command'. > >> * > >> + * if 'command' is given as -2, 'str_' may begin with a command name > >> ("add", > >> + * "modify", "delete", "modify_strict", or "delete_strict"). A missing > >> command > >> + * name is treated as "add". > >> + * > >> * Returns NULL if successful, otherwise a malloc()'d string describing the > >> * error. The caller is responsible for freeing the returned string. */ > >> char * OVS_WARN_UNUSED_RESULT > >> @@ -817,14 +844,19 @@ parse_flow_monitor_request(struct > >> ofputil_flow_monitor_request *fmr, > >> /* Parses 'string' as an OFPT_FLOW_MOD or NXT_FLOW_MOD with command > >> 'command' > >> * (one of OFPFC_*) into 'fm'. > >> * > >> + * if 'command' is given as -2, 'string' may begin with a command name > >> ("add", > >> + * "modify", "delete", "modify_strict", or "delete_strict"). A missing > >> command > >> + * name is treated as "add". > >> + * > >> * Returns NULL if successful, otherwise a malloc()'d string describing the > >> * error. The caller is responsible for freeing the returned string. */ > >> char * OVS_WARN_UNUSED_RESULT > >> parse_ofp_flow_mod_str(struct ofputil_flow_mod *fm, const char *string, > >> - uint16_t command, > >> + int command, > >> enum ofputil_protocol *usable_protocols) > >> { > >> char *error = parse_ofp_str(fm, command, string, usable_protocols); > >> + > >> if (!error) { > >> /* Normalize a copy of the match. This ensures that non-normalized > >> * flows get logged but doesn't affect what gets sent to the switch, so > >> @@ -882,10 +914,14 @@ parse_ofp_table_mod(struct ofputil_table_mod *tm, > >> const char *table_id, > >> * type (one of OFPFC_*). Stores each flow_mod in '*fm', an array allocated > >> * on the caller's behalf, and the number of flow_mods in '*n_fms'. > >> * > >> + * if 'command' is given as -2, each line may start with a command name > >> + * ("add", "modify", "delete", "modify_strict", or "delete_strict"). A > >> missing > >> + * command name is treated as "add". > >> + * > >> * Returns NULL if successful, otherwise a malloc()'d string describing the > >> * error. The caller is responsible for freeing the returned string. */ > >> char * OVS_WARN_UNUSED_RESULT > >> -parse_ofp_flow_mod_file(const char *file_name, uint16_t command, > >> +parse_ofp_flow_mod_file(const char *file_name, int command, > >> struct ofputil_flow_mod **fms, size_t *n_fms, > >> enum ofputil_protocol *usable_protocols) > >> { > >> @@ -1547,3 +1583,58 @@ parse_ofp_group_mod_file(const char *file_name, > >> uint16_t command, > >> } > >> return NULL; > >> } > >> + > >> +static char * OVS_WARN_UNUSED_RESULT > >> +parse_ofp_bundle_control_str__(struct ofputil_bundle_ctrl_msg *bc, > >> + char *string, > >> + enum ofputil_protocol *usable_protocols) > >> +{ > >> + char *save_ptr = NULL; > >> + char *name; > >> + > >> + /* Bundles require at least OF 1.4. */ > >> + *usable_protocols = OFPUTIL_P_OF14_UP; > >> + > >> + memset(bc, 0, sizeof *bc); > >> + > >> + for (name = strtok_r(string, "=, \t\r\n", &save_ptr); name; > >> + name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) { > >> + > >> + if (!strcmp(name, "atomic")) { > >> + bc->flags |= OFPBF_ATOMIC; > >> + } else if (!strcmp(name, "ordered")) { > >> + bc->flags |= OFPBF_ORDERED; > >> + } else { > >> + char *value; > >> + > >> + value = strtok_r(NULL, ", \t\r\n", &save_ptr); > >> + if (!value) { > >> + return xasprintf("field %s missing value", name); > >> + } > >> + > >> + if (!strcmp(name, "bundle")) { > >> + char *error = str_to_u32(value, &bc->bundle_id); > >> + > >> + if (error) { > >> + return error; > >> + } > >> + } else { > >> + return xasprintf("unknown keyword %s", name); > >> + } > >> + } > >> + } > >> + > >> + return NULL; > >> +} > >> + > >> +char * OVS_WARN_UNUSED_RESULT > >> +parse_ofp_bundle_control_str(struct ofputil_bundle_ctrl_msg *bc, > >> + const char *str_, > >> + enum ofputil_protocol *usable_protocols) > >> +{ > >> + char *string = xstrdup(str_); > >> + char *error = parse_ofp_bundle_control_str__(bc, string, > >> usable_protocols); > >> + > >> + free(string); > >> + return error; > >> +} > >> diff --git a/lib/ofp-parse.h b/lib/ofp-parse.h > >> index db30f43..f39c323 100644 > >> --- a/lib/ofp-parse.h > >> +++ b/lib/ofp-parse.h > >> @@ -1,5 +1,5 @@ > >> /* > >> - * Copyright (c) 2010, 2011, 2012, 2013, 2014 Nicira, Inc. > >> + * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. > >> * > >> * Licensed under the Apache License, Version 2.0 (the "License"); > >> * you may not use this file except in compliance with the License. > >> @@ -32,6 +32,7 @@ struct ofputil_flow_mod; > >> struct ofputil_flow_monitor_request; > >> struct ofputil_flow_stats_request; > >> struct ofputil_group_mod; > >> +struct ofputil_bundle_ctrl_msg; > >> struct ofputil_meter_mod; > >> struct ofputil_table_mod; > >> struct simap; > >> @@ -42,7 +43,7 @@ char *parse_ofp_str(struct ofputil_flow_mod *, int > >> command, const char *str_, > >> OVS_WARN_UNUSED_RESULT; > >> > >> char *parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char *string, > >> - uint16_t command, > >> + int command, > >> enum ofputil_protocol *usable_protocols) > >> OVS_WARN_UNUSED_RESULT; > >> > >> @@ -51,7 +52,7 @@ char *parse_ofp_table_mod(struct ofputil_table_mod *, > >> enum ofputil_protocol *usable_protocols) > >> OVS_WARN_UNUSED_RESULT; > >> > >> -char *parse_ofp_flow_mod_file(const char *file_name, uint16_t command, > >> +char *parse_ofp_flow_mod_file(const char *file_name, int command, > >> struct ofputil_flow_mod **fms, size_t *n_fms, > >> enum ofputil_protocol *usable_protocols) > >> OVS_WARN_UNUSED_RESULT; > >> @@ -84,6 +85,11 @@ char *parse_ofp_group_mod_str(struct ofputil_group_mod > >> *, uint16_t command, > >> enum ofputil_protocol *usable_protocols) > >> OVS_WARN_UNUSED_RESULT; > >> > >> +char *parse_ofp_bundle_control_str(struct ofputil_bundle_ctrl_msg *, > >> + const char *string, > >> + enum ofputil_protocol *usable_protocols) > >> + OVS_WARN_UNUSED_RESULT; > >> + > >> char *str_to_u8(const char *str, const char *name, uint8_t *valuep) > >> OVS_WARN_UNUSED_RESULT; > >> char *str_to_u16(const char *str, const char *name, uint16_t *valuep) > >> diff --git a/lib/ofp-util.c b/lib/ofp-util.c > >> index 17a0c41..d3251ed 100644 > >> --- a/lib/ofp-util.c > >> +++ b/lib/ofp-util.c > >> @@ -8748,6 +8748,36 @@ ofputil_decode_bundle_ctrl(const struct ofp_header > >> *oh, > >> } > >> > >> struct ofpbuf * > >> +ofputil_encode_bundle_ctrl_request(enum ofp_version ofp_version, > >> + struct ofputil_bundle_ctrl_msg *bc) > >> +{ > >> + struct ofpbuf *request; > >> + struct ofp14_bundle_ctrl_msg *m; > >> + > >> + switch (ofp_version) { > >> + case OFP10_VERSION: > >> + case OFP11_VERSION: > >> + case OFP12_VERSION: > >> + case OFP13_VERSION: > >> + ovs_fatal(0, "bundles need OpenFlow 1.4 or later " > >> + "(\'-O OpenFlow14\')"); > >> + case OFP14_VERSION: > >> + case OFP15_VERSION: > >> + request = ofpraw_alloc(OFPRAW_OFPT14_BUNDLE_CONTROL, ofp_version, 0); > >> + m = ofpbuf_put_zeros(request, sizeof *m); > >> + > >> + m->bundle_id = htonl(bc->bundle_id); > >> + m->type = htons(bc->type); > >> + m->flags = htons(bc->flags); > >> + break; > >> + default: > >> + OVS_NOT_REACHED(); > >> + } > >> + > >> + return request; > >> +} > >> + > >> +struct ofpbuf * > >> ofputil_encode_bundle_ctrl_reply(const struct ofp_header *oh, > >> struct ofputil_bundle_ctrl_msg *msg) > >> { > >> diff --git a/lib/ofp-util.h b/lib/ofp-util.h > >> index efb5b18..644a679 100644 > >> --- a/lib/ofp-util.h > >> +++ b/lib/ofp-util.h > >> @@ -1114,6 +1114,8 @@ enum ofptype; > >> enum ofperr ofputil_decode_bundle_ctrl(const struct ofp_header *, > >> struct ofputil_bundle_ctrl_msg *); > >> > >> +struct ofpbuf *ofputil_encode_bundle_ctrl_request(enum ofp_version, > >> + struct ofputil_bundle_ctrl_msg *); > >> struct ofpbuf *ofputil_encode_bundle_ctrl_reply(const struct ofp_header *, > >> struct ofputil_bundle_ctrl_msg *); > >> > >> diff --git a/lib/vconn.c b/lib/vconn.c > >> index c033b48..e0329ec 100644 > >> --- a/lib/vconn.c > >> +++ b/lib/vconn.c > >> @@ -1,5 +1,5 @@ > >> /* > >> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. > >> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, > >> Inc. > >> * > >> * Licensed under the Apache License, Version 2.0 (the "License"); > >> * you may not use this file except in compliance with the License. > >> @@ -751,11 +751,14 @@ vconn_recv_block(struct vconn *vconn, struct ofpbuf > >> **msgp) > >> * > >> * 'request' is always destroyed, regardless of the return value. */ > >> int > >> -vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp) > >> +vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp, > >> + void (*error_reporter)(const struct ofp_header *)) > >> { > >> for (;;) { > >> ovs_be32 recv_xid; > >> struct ofpbuf *reply; > >> + const struct ofp_header *oh; > >> + enum ofptype type; > >> int error; > >> > >> error = vconn_recv_block(vconn, &reply); > >> @@ -763,15 +766,21 @@ vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, > >> struct ofpbuf **replyp) > >> *replyp = NULL; > >> return error; > >> } > >> - recv_xid = ((struct ofp_header *) reply->data)->xid; > >> + oh = reply->data; > >> + recv_xid = oh->xid; > >> if (xid == recv_xid) { > >> *replyp = reply; > >> return 0; > >> } > >> > >> - VLOG_DBG_RL(&bad_ofmsg_rl, "%s: received reply with xid %08"PRIx32 > >> - " != expected %08"PRIx32, > >> - vconn->name, ntohl(recv_xid), ntohl(xid)); > >> + error = ofptype_decode(&type, oh); > >> + if (!error && type == OFPTYPE_ERROR && error_reporter) { > >> + error_reporter(oh); > >> + } else { > >> + VLOG_DBG_RL(&bad_ofmsg_rl, "%s: received reply with xid %08"PRIx32 > >> + " != expected %08"PRIx32, > >> + vconn->name, ntohl(recv_xid), ntohl(xid)); > >> + } > >> ofpbuf_delete(reply); > >> } > >> } > >> @@ -788,7 +797,8 @@ vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, > >> struct ofpbuf **replyp) > >> * 'request' is always destroyed, regardless of the return value. */ > >> int > >> vconn_transact(struct vconn *vconn, struct ofpbuf *request, > >> - struct ofpbuf **replyp) > >> + struct ofpbuf **replyp, > >> + void (*error_reporter)(const struct ofp_header *)) > >> { > >> ovs_be32 send_xid = ((struct ofp_header *) request->data)->xid; > >> int error; > >> @@ -798,7 +808,8 @@ vconn_transact(struct vconn *vconn, struct ofpbuf > >> *request, > >> if (error) { > >> ofpbuf_delete(request); > >> } > >> - return error ? error : vconn_recv_xid(vconn, send_xid, replyp); > >> + return error ? error : vconn_recv_xid(vconn, send_xid, replyp, > >> + error_reporter); > >> } > >> > >> /* Sends 'request' followed by a barrier request to 'vconn', then blocks > >> until > >> diff --git a/tests/ofproto.at b/tests/ofproto.at > >> index 9729a7c..20bf5d6 100644 > >> --- a/tests/ofproto.at > >> +++ b/tests/ofproto.at > >> @@ -3450,3 +3450,113 @@ OFPT_BARRIER_REPLY (OF1.4): > >> > >> OVS_VSWITCHD_STOP > >> AT_CLEANUP > >> + > >> + > >> +AT_SETUP([ofproto - bundle with multiple flow mods (OpenFlow 1.4)]) > >> +AT_KEYWORDS([monitor]) > >> +OVS_VSWITCHD_START > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 del-flows br0]) > >> + > >> +AT_DATA([flows.txt], [dnl > >> +add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=1 > >> +add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=2 > >> +add idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=3 > >> +add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=4 > >> +delete > >> +add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=5 > >> +add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=6 > >> +add idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=7 > >> +delete in_port=2 dl_src=00:88:99:aa:bb:cc > >> +]) > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 bundle br0 flows.txt]) > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], > >> [0], [dnl > >> + idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=output:5 > >> + idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:6 > >> +OFPST_FLOW reply (OF1.4): > >> +]) > >> + > >> +AT_DATA([flows.txt], [dnl > >> +modify actions=drop > >> +modify_strict in_port=2 dl_src=00:77:88:99:aa:bb actions=7 > >> +]) > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 bundle br0 flows.txt]) > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], > >> [0], [dnl > >> + idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=drop > >> + idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:7 > >> +OFPST_FLOW reply (OF1.4): > >> +]) > >> + > >> +# Adding an existing flow acts as a modify, and delete_strict also works. > >> +AT_DATA([flows.txt], [dnl > >> +add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=8 > >> +delete_strict in_port=2 dl_src=00:66:77:88:99:aa > >> +add in_port=2 dl_src=00:66:77:88:99:aa actions=drop > >> +]) > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 bundle br0 flows.txt]) > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], > >> [0], [dnl > >> + idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:8 > >> + in_port=2,dl_src=00:66:77:88:99:aa actions=drop > >> +OFPST_FLOW reply (OF1.4): > >> +]) > >> + > >> +OVS_VSWITCHD_STOP > >> +AT_CLEANUP > >> + > >> + > >> +AT_SETUP([ofproto - failing bundle commit (OpenFlow 1.4)]) > >> +AT_KEYWORDS([monitor]) > >> +OVS_VSWITCHD_START > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 del-flows br0]) > >> + > >> +ovs-ofctl add-flows br0 - <> +idle_timeout=50 in_port=2 > >> dl_src=00:66:77:88:99:aa actions=11 > >> +idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=22 > >> +idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=33 > >> +EOF > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], > >> [0], [dnl > >> + idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=output:11 > >> + idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:22 > >> + idle_timeout=70, in_port=2,dl_src=00:88:99:aa:bb:cc actions=output:33 > >> +OFPST_FLOW reply (OF1.4): > >> +]) > >> + > >> +# last line uses illegal table number (OVS internal table) > >> +AT_DATA([flows.txt], [dnl > >> +add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=1 > >> +add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=2 > >> +add idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=3 > >> +modify idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=4 > >> +delete > >> +add idle_timeout=50 in_port=2 dl_src=00:66:77:88:99:aa actions=5 > >> +add idle_timeout=60 in_port=2 dl_src=00:77:88:99:aa:bb actions=6 > >> +add idle_timeout=70 in_port=2 dl_src=00:88:99:aa:bb:cc actions=7 > >> +delete in_port=2 dl_src=00:88:99:aa:bb:cc > >> +add table=254 actions=drop > >> +]) > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 bundle br0 flows.txt], [1], [], [dnl > >> +OFPT_ERROR (OF1.4) (xid=0x15): OFPBRC_EPERM > >> +OFPT_FLOW_MOD (OF1.4) (xid=0x15): ADD table:254 actions=drop > >> +OFPT_ERROR (OF1.4) (xid=0x17): OFPBFC_MSG_FAILED > >> +OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x17): > >> + bundle_id=0 type=COMMIT_REQUEST flags=0 > >> +ovs-ofctl: Bundle commit failed (OFPBFC_MSG_FAILED). > >> +]) > >> + > >> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sort], > >> [0], [dnl > >> + idle_timeout=50, in_port=2,dl_src=00:66:77:88:99:aa actions=output:11 > >> + idle_timeout=60, in_port=2,dl_src=00:77:88:99:aa:bb actions=output:22 > >> + idle_timeout=70, in_port=2,dl_src=00:88:99:aa:bb:cc actions=output:33 > >> +OFPST_FLOW reply (OF1.4): > >> +]) > >> + > >> +OVS_VSWITCHD_STOP > >> +AT_CLEANUP > >> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in > >> index c667aa4..95341ac 100644 > >> --- a/utilities/ovs-ofctl.8.in > >> +++ b/utilities/ovs-ofctl.8.in > >> @@ -318,6 +318,38 @@ Deletes entries from \fIswitch\fR's flow table. With > >> only a > >> entries that match the specified flows. With \fB\-\-strict\fR, > >> wildcards are not treated as active for matching purposes. > >> . > >> +.IQ "\fBbundle \fIswitch [\fBatomic\fR] [\fBordered\fR] \fIfile\fR" > >> +.IQ "\fBbundle \fIswitch [\fBatomic\fR] [\fBordered\fR] \fB\- < > >> \fIfile\fR" > >> +Adds, modifies, and/or deletes entries from \fIswitch\fR's flow table. > >> +\fIfile\fR is a text file where each line contains a flow in the same > >> +syntax as with \fBadd\-flows\fR, \fBmod\-flows\fR, and \fBdel\-flows\fR, > >> +except that the line may start with \fBadd\fR, \fBmodify\fR, > >> +\fBdelete\fR, \fBmodify_strict\fR, or \fBdelete_strict\fR keyword to > >> specify > >> +whether a flow is to be added, modified, or deleted, and whether the > >> +modify or delete is strict or not. A line without one of these > >> +keywords is treated as a flow add. > >> +. > >> +.IP > >> +All flow mods are processed as a single transaction, meaning that if > >> +one of them fails, the whole transaction fails and none of the changes > >> +are made to the \fIswitch\fR's flow table. The \fIswitch\fR may > >> +perform resource management, such as evicting flows, that may not be > >> +unrolled even when the transaction fails. > >> +. > >> +.IP > >> +If an optional \fBatomic\fR keyword is given, the switch is requested > >> +to make the modifications so that no datapath packet sees an > >> +intermediate configuration of the flow tables. The optional > >> +\fBordered\fR keyword requests the modifications to be performed in > >> +the order given in the file. Without this keyword the switch may > >> +apply the modifications in any order. > >> +. > >> +.IP > >> +\fBbundle\fR command requires OpenFlow 1.4 or higher. You may need to > >> +supply an explicit \fB-O OpenFlow14\fR option, and you may need to enable > >> +OVS OpenFlow 1.4 support by setting the OVSDB \fIprotocols\fR column > >> +in the \fIbridge\fR table. > >> +. > >> .IP "[\fB\-\-readd\fR] \fBreplace\-flows \fIswitch file\fR" > >> Reads flow entries from \fIfile\fR (or \fBstdin\fR if \fIfile\fR is > >> \fB\-\fR) and queries the flow table from \fIswitch\fR. Then it fixes > >> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c > >> index 54a5bb8..20abed7 100644 > >> --- a/utilities/ovs-ofctl.c > >> +++ b/utilities/ovs-ofctl.c > >> @@ -506,7 +506,7 @@ dump_transaction(struct vconn *vconn, struct ofpbuf > >> *request) > >> struct ofpbuf *reply; > >> > >> ofpmsg_update_length(request); > >> - run(vconn_transact(vconn, request, &reply), "talking to %s", > >> + run(vconn_transact(vconn, request, &reply, NULL), "talking to %s", > >> vconn_get_name(vconn)); > >> ofp_print(stdout, reply->data, reply->size, verbosity + 1); > >> ofpbuf_delete(reply); > >> @@ -627,7 +627,7 @@ fetch_switch_config(struct vconn *vconn, struct > >> ofp_switch_config *config_) > >> > >> request = ofpraw_alloc(OFPRAW_OFPT_GET_CONFIG_REQUEST, > >> vconn_get_version(vconn), 0); > >> - run(vconn_transact(vconn, request, &reply), > >> + run(vconn_transact(vconn, request, &reply, NULL), > >> "talking to %s", vconn_get_name(vconn)); > >> > >> if (ofptype_pull(&type, reply) || type != OFPTYPE_GET_CONFIG_REPLY) { > >> @@ -664,7 +664,8 @@ ofctl_show(struct ovs_cmdl_context *ctx) > >> open_vconn(vconn_name, &vconn); > >> version = vconn_get_version(vconn); > >> request = ofpraw_alloc(OFPRAW_OFPT_FEATURES_REQUEST, version, 0); > >> - run(vconn_transact(vconn, request, &reply), "talking to %s", vconn_name); > >> + run(vconn_transact(vconn, request, &reply, NULL), "talking to %s", > >> + vconn_name); > >> > >> has_ports = ofputil_switch_features_has_ports(reply); > >> ofp_print(stdout, reply->data, reply->size, verbosity + 1); > >> @@ -731,7 +732,7 @@ fetch_port_by_features(struct vconn *vconn, > >> /* Fetch the switch's ofp_switch_features. */ > >> request = ofpraw_alloc(OFPRAW_OFPT_FEATURES_REQUEST, > >> vconn_get_version(vconn), 0); > >> - run(vconn_transact(vconn, request, &reply), > >> + run(vconn_transact(vconn, request, &reply, NULL), > >> "talking to %s", vconn_get_name(vconn)); > >> > >> oh = reply->data; > >> @@ -1667,7 +1668,8 @@ ofctl_probe(struct ovs_cmdl_context *ctx) > >> > >> open_vconn(ctx->argv[1], &vconn); > >> request = make_echo_request(vconn_get_version(vconn)); > >> - run(vconn_transact(vconn, request, &reply), "talking to %s", > >> ctx->argv[1]); > >> + run(vconn_transact(vconn, request, &reply, NULL), "talking to %s", > >> + ctx->argv[1]); > >> if (reply->size != sizeof(struct ofp_header)) { > >> ovs_fatal(0, "reply does not match request"); > >> } > >> @@ -2022,7 +2024,8 @@ ofctl_ping(struct ovs_cmdl_context *ctx) > >> random_bytes(ofpbuf_put_uninit(request, payload), payload); > >> > >> xgettimeofday(&start); > >> - run(vconn_transact(vconn, ofpbuf_clone(request), &reply), "transact"); > >> + run(vconn_transact(vconn, ofpbuf_clone(request), &reply, NULL), > >> + "transact"); > >> xgettimeofday(&end); > >> > >> rpy_hdr = reply->data; > >> @@ -2075,7 +2078,7 @@ ofctl_benchmark(struct ovs_cmdl_context *ctx) > >> request = ofpraw_alloc(OFPRAW_OFPT_ECHO_REQUEST, > >> vconn_get_version(vconn), payload_size); > >> ofpbuf_put_zeros(request, payload_size); > >> - run(vconn_transact(vconn, request, &reply), "transact"); > >> + run(vconn_transact(vconn, request, &reply, NULL), "transact"); > >> ofpbuf_delete(reply); > >> } > >> xgettimeofday(&end); > >> @@ -2261,6 +2264,210 @@ ofctl_dump_group_features(struct ovs_cmdl_context > >> *ctx) > >> vconn_close(vconn); > >> } > >> > >> +static enum ofperr > >> +bundle_reply_validate(struct ofpbuf *reply, enum ofp_version version, > >> + struct ofputil_bundle_ctrl_msg *request) > >> +{ > >> + const struct ofp_header *oh; > >> + enum ofptype type; > >> + enum ofperr error; > >> + struct ofputil_bundle_ctrl_msg rbc; > >> + > >> + oh = reply->data; > >> + if (oh->version != version) { > >> + return OFPERR_OFPBRC_BAD_VERSION; > >> + } > >> + > >> + error = ofptype_decode(&type, oh); > >> + if (error) { > >> + return error; > >> + } > >> + > >> + if (type == OFPTYPE_ERROR) { > >> + ofp_print(stderr, reply->data, reply->size, verbosity + 1); > >> + fflush(stderr); > >> + error = ofperr_decode_msg(oh, NULL); > >> + return error; > >> + } > >> + if (type != OFPTYPE_BUNDLE_CONTROL) { > >> + ofp_print(stderr, reply->data, reply->size, verbosity + 1); > >> + fflush(stderr); > >> + return OFPERR_OFPBRC_BAD_TYPE; > >> + } > >> + > >> + error = ofputil_decode_bundle_ctrl(oh, &rbc); > >> + if (error) { > >> + return error; > >> + } > >> + > >> + if (rbc.bundle_id != request->bundle_id) { > >> + ofp_print(stderr, reply->data, reply->size, verbosity + 1); > >> + fflush(stderr); > >> + return OFPERR_OFPBFC_BAD_ID; > >> + } > >> + > >> + if (rbc.type != request->type + 1) { > >> + ofp_print(stderr, reply->data, reply->size, verbosity + 1); > >> + fflush(stderr); > >> + return OFPERR_OFPBFC_BAD_TYPE; > >> + } > >> + > >> + return 0; > >> +} > >> + > >> +static void > >> +bundle_error_reporter(const struct ofp_header *oh) > >> +{ > >> + ofp_print(stderr, oh, ntohs(oh->length), verbosity + 1); > >> + fflush(stderr); > >> +} > >> + > >> +static enum ofperr > >> +bundle_control_transact(struct vconn *vconn, enum ofp_version version, > >> + struct ofputil_bundle_ctrl_msg *bc, > >> + uint16_t type) > >> +{ > >> + struct ofpbuf *request, *reply; > >> + enum ofperr error; > >> + > >> + bc->type = type; > >> + request = ofputil_encode_bundle_ctrl_request(version, bc); > >> + ofpmsg_update_length(request); > >> + run(vconn_transact(vconn, request, &reply, bundle_error_reporter), > >> + "talking to %s", vconn_get_name(vconn)); > >> + > >> + error = bundle_reply_validate(reply, version, bc); > >> + ofpbuf_delete(reply); > >> + > >> + return error; > >> +} > >> + > >> +static enum ofperr > >> +bundle_add_msg(struct vconn *vconn, enum ofp_version version, > >> + struct ofputil_bundle_ctrl_msg *bc, struct ofpbuf *msg) > >> +{ > >> + struct ofputil_bundle_add_msg bam; > >> + struct ofpbuf *request, *reply; > >> + > >> + ofpmsg_update_length(msg); > >> + > >> + bam.bundle_id = bc->bundle_id; > >> + bam.flags = bc->flags; > >> + bam.msg = msg->data; > >> + > >> + request = ofputil_encode_bundle_add(version, &bam); > >> + ofpbuf_delete(msg); > >> + ofpmsg_update_length(request); > >> + > >> + run(vconn_transact_noreply(vconn, request, &reply), "talking to %s", > >> + vconn_get_name(vconn)); > >> + > >> + if (reply) { > >> + const struct ofp_header *oh; > >> + enum ofptype type; > >> + enum ofperr error; > >> + > >> + oh = reply->data; > >> + error = ofptype_decode(&type, oh); > >> + if (error) { > >> + goto error_out; > >> + } > >> + > >> + if (type == OFPTYPE_ERROR) { > >> + ofp_print(stderr, reply->data, reply->size, verbosity + 1); > >> + fflush(stderr); > >> + error = ofperr_decode_msg(oh, NULL); > >> + goto error_out; > >> + } > >> + error = OFPERR_OFPBFC_UNKNOWN; > >> +error_out: > >> + ofpbuf_delete(reply); > >> + return error; > >> + } > >> + return 0; > >> +} > >> + > >> +static void > >> +ofctl_bundle(struct ovs_cmdl_context *ctx) > >> +{ > >> + enum ofputil_protocol usable_protocols, usable_protocols2; > >> + enum ofputil_protocol protocol; > >> + enum ofp_version version; > >> + struct vconn *vconn; > >> + const char *file; > >> + struct ofputil_flow_mod *fms = NULL; > >> + size_t n_fms = 0; > >> + struct ofputil_bundle_ctrl_msg bc; > >> + char *error; > >> + enum ofperr ofperr; > >> + size_t i; > >> + > >> + /* Parse bundle options, if any. */ > >> + if (ctx->argc > 3) { > >> + error = parse_ofp_bundle_control_str(&bc, ctx->argv[2], > >> + &usable_protocols); > >> + if (error) { > >> + ovs_fatal(0, "%s", error); > >> + } > >> + file = ctx->argv[3]; /* File name after the options. */ > >> + } else { > >> + memset(&bc, 0, sizeof bc); > >> + file = ctx->argv[2]; > >> + usable_protocols = OFPUTIL_P_OF14_UP; > >> + } > >> + > >> + /* Parse flow mods from the file. */ > >> + error = parse_ofp_flow_mod_file(file, -2, &fms, &n_fms, > >> + &usable_protocols2); > >> + if (error) { > >> + ovs_fatal(0, "%s", error); > >> + } > >> + > >> + usable_protocols &= usable_protocols2; > >> + > >> + protocol = open_vconn_for_flow_mod(ctx->argv[1], &vconn, > >> usable_protocols); > >> + version = ofputil_protocol_to_ofp_version(protocol); > >> + > >> + ofperr = bundle_control_transact(vconn, version, &bc, > >> OFPBCT_OPEN_REQUEST); > >> + if (ofperr) { > >> + ovs_fatal(0, "Bundle open failed (%s).", ofperr_to_string(ofperr)); > >> + } > >> + > >> + for (i = 0; i < n_fms; i++) { > >> + struct ofputil_flow_mod *fm = &fms[i]; > >> + > >> + if (!ofperr) { > >> + ofperr = bundle_add_msg(vconn, version, &bc, > >> + ofputil_encode_flow_mod(fm, protocol)); > >> + if (ofperr) { > >> + fprintf(stderr, "Bundle flow mod #%"PRIuSIZE > >> + " failed (%s), discarding bundle.", > >> + i, ofperr_to_string(ofperr)); > >> + fflush(stderr); > >> + } > >> + } > >> + free(CONST_CAST(struct ofpact *, fm->ofpacts)); > >> + } > >> + free(fms); > >> + > >> + if (!ofperr) { > >> + ofperr = bundle_control_transact(vconn, version, &bc, > >> + OFPBCT_COMMIT_REQUEST); > >> + if (ofperr) { > >> + ovs_fatal(0, "Bundle commit failed (%s).", > >> + ofperr_to_string(ofperr)); > >> + } > >> + } else { > >> + ofperr = bundle_control_transact(vconn, version, &bc, > >> + OFPBCT_DISCARD_REQUEST); > >> + if (ofperr) { > >> + ovs_fatal(0, "Bundle discard failed (%s).", > >> + ofperr_to_string(ofperr)); > >> + } > >> + } > >> + vconn_close(vconn); > >> +} > >> + > >> static void > >> ofctl_help(struct ovs_cmdl_context *ctx OVS_UNUSED) > >> { > >> @@ -3578,6 +3785,7 @@ static const struct ovs_cmdl_command all_commands[] > >> = { > >> 1, 2, ofctl_dump_group_stats }, > >> { "dump-group-features", "switch", > >> 1, 1, ofctl_dump_group_features }, > >> + { "bundle", "switch [options] file", 2, 3, ofctl_bundle }, > >> { "help", NULL, 0, INT_MAX, ofctl_help }, > >> { "list-commands", NULL, 0, INT_MAX, ofctl_list_commands }, > >> > >> -- > >> 1.7.10.4 > > > > -- > > Romain Lenglet > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
