Romain, Thanks for the review!
I hope Ben finds time for reviewing the rest of the series sometime later. Jarno > On May 20, 2015, at 11:26 AM, Romain Lenglet <romain.leng...@oracle.com> > wrote: > > Jarno, > Thanks a lot for this updated patch, this will be very useful. > Sorry, I don’t feel comfortable reviewing the other patches. > > Acked-by: Romain Lenglet <romain.leng...@oracle.com > <mailto:romain.leng...@oracle.com>> > -- > Romain Lenglet > > On May 18, 2015 at 4:26:12 PM, Jarno Rajahalme (jrajaha...@nicira.com > <mailto:jrajaha...@nicira.com>(mailto: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. >> >> Similarly, all existing ovs-ofctl flow mod commands now take an >> optional '--bundle' argument, which executes the flow mods as a single >> transaction. >> >> 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 transactions are not yet >> supported. >> >> Signed-off-by: Jarno Rajahalme >> --- >> NEWS | 14 ++- >> include/openvswitch/vconn.h | 3 + >> lib/ofp-parse.c | 40 ++++++- >> lib/ofp-parse.h | 6 +- >> lib/ofp-util.c | 30 ++++++ >> lib/ofp-util.h | 2 + >> lib/ofp-version-opt.c | 7 ++ >> lib/ofp-version-opt.h | 1 + >> lib/vconn.c | 236 +++++++++++++++++++++++++++++++++++++---- >> tests/ofproto-macros.at <http://ofproto-macros.at/> | 10 ++ >> tests/ofproto.at <http://ofproto.at/> | 244 >> +++++++++++++++++++++++++++++++++++++++++++ >> tests/ovs-ofctl.at <http://ovs-ofctl.at/> | 107 +++++++++++++++++++ >> utilities/ovs-ofctl.8.in | 67 +++++++++--- >> utilities/ovs-ofctl.c | 110 ++++++++++++++++++- >> 14 files changed, 834 insertions(+), 43 deletions(-) >> >> diff --git a/NEWS b/NEWS >> index a480607..ac60451 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,13 @@ 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-ofctl has a new '--bundle' option that makes the flow mod >> + commands (add-flow, add-flows, mod-flows, del-flows, and >> + replace-flows) use an OpenFlow 1.4 bundle to operate the >> + modifications as a single transaction. >> - 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..f8b6655 100644 >> --- a/include/openvswitch/vconn.h >> +++ b/include/openvswitch/vconn.h >> @@ -55,6 +55,9 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct >> ofpbuf **); >> int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf >> **); >> int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list >> *requests, >> struct ofpbuf **replyp); >> +int vconn_bundle_transact(struct vconn *, struct ovs_list *requests, >> + uint16_t bundle_flags, >> + void (*error_reporter)(const struct ofp_header *)); >> >> void vconn_run(struct vconn *); >> void vconn_run_wait(struct vconn *); >> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c >> index 126980c..3ed9937 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”, > > Nitpick: capitalize If. > >> + * "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”, > > Nitpick: capitalize If. > >> + * "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 > > Nitpick: capitalize If. > >> + * ("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) >> { >> diff --git a/lib/ofp-parse.h b/lib/ofp-parse.h >> index db30f43..f112603 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. >> @@ -42,7 +42,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 +51,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; >> 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/ofp-version-opt.c b/lib/ofp-version-opt.c >> index 46fb45a..1cf57e1 100644 >> --- a/lib/ofp-version-opt.c >> +++ b/lib/ofp-version-opt.c >> @@ -27,6 +27,13 @@ mask_allowed_ofp_versions(uint32_t bitmap) >> } >> >> void >> +add_allowed_ofp_versions(uint32_t bitmap) > > Do you think it would be useful to use this in other places, e.g. limit to > OF1.4+ if a flow mod contains a non-zero importance, OF1.1+ if out_group is > used, etc.? > >> +{ >> + assert_single_threaded(); >> + allowed_versions |= bitmap; >> +} >> + >> +void >> ofp_version_usage(void) >> { >> struct ds msg = DS_EMPTY_INITIALIZER; >> diff --git a/lib/ofp-version-opt.h b/lib/ofp-version-opt.h >> index 6bf5eed..82b4ccc 100644 >> --- a/lib/ofp-version-opt.h >> +++ b/lib/ofp-version-opt.h >> @@ -21,6 +21,7 @@ >> uint32_t get_allowed_ofp_versions(void); >> void set_allowed_ofp_versions(const char *string); >> void mask_allowed_ofp_versions(uint32_t); >> +void add_allowed_ofp_versions(uint32_t); >> void ofp_version_usage(void); >> >> #endif >> diff --git a/lib/vconn.c b/lib/vconn.c >> index c033b48..ee6fb9e 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. >> @@ -744,18 +744,15 @@ vconn_recv_block(struct vconn *vconn, struct ofpbuf >> **msgp) >> return retval; >> } >> >> -/* Waits until a message with a transaction ID matching 'xid' is received on >> - * 'vconn'. Returns 0 if successful, in which case the reply is stored in >> - * '*replyp' for the caller to examine and free. Otherwise returns a >> positive >> - * errno value, or EOF, and sets '*replyp' to null. >> - * >> - * 'request' is always destroyed, regardless of the return value. */ >> -int >> -vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp) >> +static int >> +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,19 +760,54 @@ 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); >> } >> } >> >> +/* Waits until a message with a transaction ID matching 'xid' is received on >> + * 'vconn'. Returns 0 if successful, in which case the reply is stored in >> + * '*replyp' for the caller to examine and free. Otherwise returns a >> positive >> + * errno value, or EOF, and sets '*replyp' to null. >> + * >> + * 'request' is always destroyed, regardless of the return value. */ >> +int >> +vconn_recv_xid(struct vconn *vconn, ovs_be32 xid, struct ofpbuf **replyp) >> +{ >> + return vconn_recv_xid__(vconn, xid, replyp, NULL); >> +} >> + >> +static int >> +vconn_transact__(struct vconn *vconn, struct ofpbuf *request, >> + struct ofpbuf **replyp, >> + void (*error_reporter)(const struct ofp_header *)) >> +{ >> + ovs_be32 send_xid = ((struct ofp_header *) request->data)->xid; >> + int error; >> + >> + *replyp = NULL; >> + error = vconn_send_block(vconn, request); >> + if (error) { >> + ofpbuf_delete(request); >> + } >> + return error ? error : vconn_recv_xid__(vconn, send_xid, replyp, >> + error_reporter); >> +} >> + >> /* Sends 'request' to 'vconn' and blocks until it receives a reply with a >> * matching transaction ID. Returns 0 if successful, in which case the reply >> * is stored in '*replyp' for the caller to examine and free. Otherwise >> @@ -790,15 +822,7 @@ int >> vconn_transact(struct vconn *vconn, struct ofpbuf *request, >> struct ofpbuf **replyp) >> { >> - ovs_be32 send_xid = ((struct ofp_header *) request->data)->xid; >> - int error; >> - >> - *replyp = NULL; >> - error = vconn_send_block(vconn, request); >> - if (error) { >> - ofpbuf_delete(request); >> - } >> - return error ? error : vconn_recv_xid(vconn, send_xid, replyp); >> + return vconn_transact__(vconn, request, replyp, NULL); >> } >> >> /* Sends 'request' followed by a barrier request to 'vconn', then blocks >> until >> @@ -897,6 +921,174 @@ vconn_transact_multiple_noreply(struct vconn *vconn, >> struct ovs_list *requests, >> return 0; >> } >> >> +static enum ofperr >> +vconn_bundle_reply_validate(struct ofpbuf *reply, enum ofp_version version, >> + struct ofputil_bundle_ctrl_msg *request, >> + void (*error_reporter)(const struct ofp_header *)) >> +{ >> + 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) { >> + error_reporter(oh); >> + error = ofperr_decode_msg(oh, NULL); >> + return error; >> + } >> + if (type != OFPTYPE_BUNDLE_CONTROL) { >> + return OFPERR_OFPBRC_BAD_TYPE; >> + } >> + >> + error = ofputil_decode_bundle_ctrl(oh, &rbc); >> + if (error) { >> + return error; >> + } >> + >> + if (rbc.bundle_id != request->bundle_id) { >> + return OFPERR_OFPBFC_BAD_ID; >> + } >> + >> + if (rbc.type != request->type + 1) { >> + return OFPERR_OFPBFC_BAD_TYPE; >> + } >> + >> + return 0; >> +} >> + >> +static int >> +vconn_bundle_control_transact(struct vconn *vconn, >> + struct ofputil_bundle_ctrl_msg *bc, >> + uint16_t type, >> + void (*error_reporter)(const struct ofp_header *)) >> +{ >> + struct ofpbuf *request, *reply; >> + int error; >> + enum ofperr ofperr; >> + >> + bc->type = type; >> + request = ofputil_encode_bundle_ctrl_request(vconn->version, bc); >> + ofpmsg_update_length(request); >> + error = vconn_transact__(vconn, request, &reply, error_reporter); >> + if (error) { >> + return error; >> + } >> + >> + ofperr = vconn_bundle_reply_validate(reply, vconn->version, bc, >> + error_reporter); >> + if (ofperr) { >> + VLOG_WARN_RL(&bad_ofmsg_rl, "Bundle %s failed (%s).", >> + type == OFPBCT_OPEN_REQUEST ? "open" >> + : type == OFPBCT_CLOSE_REQUEST ? "close" >> + : type == OFPBCT_COMMIT_REQUEST ? "commit" >> + : type == OFPBCT_DISCARD_REQUEST ? "discard" >> + : "control message", >> + ofperr_to_string(ofperr)); >> + } >> + ofpbuf_delete(reply); >> + >> + return ofperr ? EPROTO : 0; >> +} >> + >> +/* Checks if error responses can be received on >> + * 'vconn'. */ >> +static void >> +vconn_recv_error(struct vconn *vconn, >> + void (*error_reporter)(const struct ofp_header *)) >> +{ >> + struct ofpbuf *reply; >> + int error; >> + >> + error = vconn_recv(vconn, &reply); >> + if (!error) { >> + const struct ofp_header *oh; >> + enum ofptype type; >> + enum ofperr ofperr; >> + >> + oh = reply->data; >> + ofperr = ofptype_decode(&type, oh); >> + if (!ofperr && type == OFPTYPE_ERROR) { >> + error_reporter(oh); >> + } else { >> + VLOG_DBG_RL(&bad_ofmsg_rl, >> + "%s: received unexpected reply with xid %08"PRIx32, >> + vconn->name, ntohl(oh->xid)); >> + } >> + ofpbuf_delete(reply); >> + } >> +} >> + >> +static int >> +vconn_bundle_add_msg(struct vconn *vconn, struct ofputil_bundle_ctrl_msg >> *bc, >> + struct ofpbuf *msg, >> + void (*error_reporter)(const struct ofp_header *)) >> +{ >> + struct ofputil_bundle_add_msg bam; >> + struct ofpbuf *request; >> + int error; >> + >> + bam.bundle_id = bc->bundle_id; >> + bam.flags = bc->flags; >> + bam.msg = msg->data; >> + >> + request = ofputil_encode_bundle_add(vconn->version, &bam); >> + ofpmsg_update_length(request); >> + >> + error = vconn_send_block(vconn, request); >> + if (!error) { >> + /* Check for an error return, so that the socket buffer does not become >> + * full of errors. */ >> + vconn_recv_error(vconn, error_reporter); >> + } >> + return error; >> +} >> + >> +int >> +vconn_bundle_transact(struct vconn *vconn, struct ovs_list *requests, >> + uint16_t flags, >> + void (*error_reporter)(const struct ofp_header *)) >> +{ >> + struct ofputil_bundle_ctrl_msg bc; >> + struct ofpbuf *request; >> + int error; >> + >> + memset(&bc, 0, sizeof bc); >> + bc.flags = flags; >> + error = vconn_bundle_control_transact(vconn, &bc, OFPBCT_OPEN_REQUEST, >> + error_reporter); >> + if (error) { >> + return error; >> + } >> + >> + LIST_FOR_EACH (request, list_node, requests) { >> + error = vconn_bundle_add_msg(vconn, &bc, request, error_reporter); >> + if (error) { >> + break; >> + } >> + } >> + >> + if (!error) { >> + error = vconn_bundle_control_transact(vconn, &bc, >> + OFPBCT_COMMIT_REQUEST, >> + error_reporter); >> + } else { >> + error = vconn_bundle_control_transact(vconn, &bc, >> + OFPBCT_DISCARD_REQUEST, >> + error_reporter); >> + } >> + return error; >> +} >> + >> void >> vconn_wait(struct vconn *vconn, enum vconn_wait_type wait) >> { >> diff --git a/tests/ofproto-macros.at <http://ofproto-macros.at/> >> b/tests/ofproto-macros.at <http://ofproto-macros.at/> >> index fd915ef..65bfe40 100644 >> --- a/tests/ofproto-macros.at <http://ofproto-macros.at/> >> +++ b/tests/ofproto-macros.at <http://ofproto-macros.at/> >> @@ -15,6 +15,16 @@ s/ hard_age=[0-9]*,// >> ' >> } >> >> +# Filter (multiline) vconn debug messages from ovs-vswitchd.log. >> +# Use with ofctl_strip() >> +print_vconn_debug () { awk -F\| < ovs-vswitchd.log ' >> +BEGIN { prt=0 } >> +/\|vconn\|DBG\|/ { sub(/[ \t]*$/, ""); print $3 "|" $4 "|" $5; prt=1; next } >> +$4 != "" { prt=0; next } >> +prt==1 { sub(/[ \t]*$/, ""); print $0 } >> +' >> +} >> + >> # parse_listening_port [SERVER] >> # >> # Parses the TCP or SSL port on which a server is listening from the >> diff --git a/tests/ofproto.at <http://ofproto.at/> b/tests/ofproto.at >> <http://ofproto.at/> >> index 9729a7c..e6ede1a 100644 >> --- a/tests/ofproto.at <http://ofproto.at/> >> +++ b/tests/ofproto.at <http://ofproto.at/> >> @@ -3450,3 +3450,247 @@ 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-appctl vlog/set vconn:dbg]) >> + >> +AT_CHECK([ovs-ofctl 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 bundle br0 flows.txt]) >> + >> +AT_CHECK([ovs-ofctl 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 >> +NXST_FLOW reply: >> +]) >> + >> +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 bundle br0 flows.txt]) >> + >> +AT_CHECK([ovs-ofctl 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 >> +NXST_FLOW reply: >> +]) >> + >> +# 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 bundle br0 flows.txt]) >> + >> +AT_CHECK([ovs-ofctl 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 >> +NXST_FLOW reply: >> +]) >> + >> +dnl Check logs for OpenFlow trace >> +# Prevent race. >> +OVS_WAIT_UNTIL([test `grep -- "|vconn|DBG|unix: sent (Success): NXST_FLOW >> reply" ovs-vswitchd.log | wc -l` -ge 3]) >> +AT_CHECK([print_vconn_debug | ofctl_strip], [0], [dnl >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO: >> + version bitmap: 0x01 >> +vconn|DBG|unix: negotiated OpenFlow version 0x01 (we support version 0x06 >> and earlier, peer supports version 0x01) >> +vconn|DBG|unix: received: OFPT_FLOW_MOD: DEL actions=drop >> +vconn|DBG|unix: received: OFPT_BARRIER_REQUEST: >> +vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> + version bitmap: 0x01, 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 >> and earlier, peer supports versions 0x01, 0x05) >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REPLY flags=0 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa idle:50 >> actions=output:1 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:77:88:99:aa:bb idle:60 >> actions=output:2 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:88:99:aa:bb:cc idle:70 >> actions=output:3 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa idle:50 >> actions=output:4 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): DEL table:255 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa idle:50 >> actions=output:5 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:77:88:99:aa:bb idle:60 >> actions=output:6 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:88:99:aa:bb:cc idle:70 >> actions=output:7 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): DEL table:255 in_port=2,dl_src=00:88:99:aa:bb:cc >> actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REPLY flags=0 >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO: >> + version bitmap: 0x01 >> +vconn|DBG|unix: negotiated OpenFlow version 0x01 (we support version 0x06 >> and earlier, peer supports version 0x01) >> +vconn|DBG|unix: received: NXT_SET_FLOW_FORMAT: format=nxm >> +vconn|DBG|unix: received: OFPT_BARRIER_REQUEST: >> +vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: >> +vconn|DBG|unix: received: NXST_FLOW request: >> +vconn|DBG|unix: sent (Success): NXST_FLOW reply: >> + 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 >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> + version bitmap: 0x01, 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 >> and earlier, peer supports versions 0x01, 0x05) >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REPLY flags=0 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): MOD actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): MOD_STRICT in_port=2,dl_src=00:77:88:99:aa:bb >> actions=output:7 >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REPLY flags=0 >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO: >> + version bitmap: 0x01 >> +vconn|DBG|unix: negotiated OpenFlow version 0x01 (we support version 0x06 >> and earlier, peer supports version 0x01) >> +vconn|DBG|unix: received: NXT_SET_FLOW_FORMAT: format=nxm >> +vconn|DBG|unix: received: OFPT_BARRIER_REQUEST: >> +vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: >> +vconn|DBG|unix: received: NXST_FLOW request: >> +vconn|DBG|unix: sent (Success): NXST_FLOW reply: >> + 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 >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> + version bitmap: 0x01, 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 >> and earlier, peer supports versions 0x01, 0x05) >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REPLY flags=0 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:77:88:99:aa:bb idle:60 >> actions=output:8 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 >> in_port=2,dl_src=00:66:77:88:99:aa actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD in_port=2,dl_src=00:66:77:88:99:aa actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REPLY flags=0 >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO: >> + version bitmap: 0x01 >> +vconn|DBG|unix: negotiated OpenFlow version 0x01 (we support version 0x06 >> and earlier, peer supports version 0x01) >> +vconn|DBG|unix: received: NXT_SET_FLOW_FORMAT: format=nxm >> +vconn|DBG|unix: received: OFPT_BARRIER_REQUEST: >> +vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: >> +vconn|DBG|unix: received: NXST_FLOW request: >> +vconn|DBG|unix: sent (Success): NXST_FLOW reply: >> + 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 >> +]) >> + >> +OVS_VSWITCHD_STOP >> +AT_CLEANUP >> + >> + >> +AT_SETUP([ofproto - failing bundle commit (OpenFlow 1.4)]) >> +AT_KEYWORDS([monitor]) >> +OVS_VSWITCHD_START >> + >> +AT_CHECK([ovs-ofctl 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 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 >> +NXST_FLOW reply: >> +]) >> + >> +# 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 bundle br0 flows.txt 2>&1 | sed '/|WARN|/d >> +s/unix:.*br0\.mgmt/unix:br0.mgmt/'], [0], [dnl >> +OFPT_ERROR (OF1.4) (xid=0xb): OFPBRC_EPERM >> +OFPT_FLOW_MOD (OF1.4) (xid=0xb): ADD table:254 actions=drop >> +OFPT_ERROR (OF1.4) (xid=0xd): OFPBFC_MSG_FAILED >> +OFPT_BUNDLE_CONTROL (OF1.4) (xid=0xd): >> + bundle_id=0 type=COMMIT_REQUEST flags=ordered >> +ovs-ofctl: talking to unix:br0.mgmt (Protocol error) >> +]) >> + >> +AT_CHECK([ovs-ofctl 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 >> +NXST_FLOW reply: >> +]) >> + >> +OVS_VSWITCHD_STOP >> +AT_CLEANUP >> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at >> index 42be8f0..d2164bb 100644 >> --- a/tests/ovs-ofctl.at >> +++ b/tests/ovs-ofctl.at >> @@ -2813,3 +2813,110 @@ AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | >> ofctl_strip | sed '/OFPST_FLO >> >> OVS_VSWITCHD_STOP >> AT_CLEANUP >> + >> +AT_SETUP([ovs-ofctl replace-flows with --bundle]) >> +OVS_VSWITCHD_START >> + >> +AT_CHECK([ovs-appctl vlog/set vconn:dbg]) >> + >> +dnl Add flows to br0 with importance via OF1.4+, using an OF1.4+ bundle. >> For more details refer "ovs-ofctl rule with importance" test case. >> +for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; >> done > add-flows.txt >> +AT_CHECK([ovs-ofctl --bundle add-flows br0 add-flows.txt]) >> + >> +dnl Replace some flows in the bridge. >> +for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + >> 10`,actions=drop"; done > replace-flows.txt >> +AT_CHECK([ovs-ofctl --bundle replace-flows br0 replace-flows.txt]) >> + >> +dnl Dump them and compare the dump flows output against the expected output. >> +for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then >> importance=`expr $i + 10`; else importance=$i; fi; echo " >> importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout >> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed >> '/OFPST_FLOW/d' | sort], >> + [0], [expout]) >> + >> +dnl Check logs for OpenFlow trace >> +# Prevent race. >> +OVS_WAIT_UNTIL([test `grep -- "|vconn|DBG|unix: sent (Success): OFPST_FLOW >> reply" ovs-vswitchd.log | wc -l` -ge 2]) >> +# AT_CHECK([sed -n "s/^.*\(|vconn|DBG|.*xid=.*:\).*$/\1/p" >> ovs-vswitchd.log], [0], [dnl >> +AT_CHECK([print_vconn_debug | ofctl_strip], [0], [dnl >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> + version bitmap: 0x01, 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 >> and earlier, peer supports versions 0x01, 0x05) >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REPLY flags=0 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:1 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=2 importance:2 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:3 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=4 importance:4 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:5 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=6 importance:6 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:7 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=8 importance:8 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REPLY flags=0 >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> + version bitmap: 0x01, 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 >> and earlier, peer supports versions 0x01, 0x05) >> +vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): >> +vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REPLY flags=0 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:11 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:13 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:15 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:17 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REPLY flags=0 >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> + version bitmap: 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 >> and earlier, peer supports version 0x05) >> +vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): >> +vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): >> + importance=11, dl_vlan=1 actions=drop >> + importance=2, dl_vlan=2 actions=drop >> + importance=13, dl_vlan=3 actions=drop >> + importance=4, dl_vlan=4 actions=drop >> + importance=15, dl_vlan=5 actions=drop >> + importance=6, dl_vlan=6 actions=drop >> + importance=17, dl_vlan=7 actions=drop >> + importance=8, dl_vlan=8 actions=drop >> +]) >> + >> +OVS_VSWITCHD_STOP >> +AT_CLEANUP >> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in >> index c667aa4..1805a64 100644 >> --- a/utilities/ovs-ofctl.8.in >> +++ b/utilities/ovs-ofctl.8.in >> @@ -296,29 +296,31 @@ Print meter features. >> . >> These commands manage the flow table in an OpenFlow switch. In each >> case, \fIflow\fR specifies a flow entry in the format described in >> -\fBFlow Syntax\fR, below, and \fIfile\fR is a text file that contains >> -zero or more flows in the same syntax, one per line. >> -. >> -.IP "\fBadd\-flow \fIswitch flow\fR" >> -.IQ "\fBadd\-flow \fIswitch \fB\- < \fIfile\fR" >> -.IQ "\fBadd\-flows \fIswitch file\fR" >> +\fBFlow Syntax\fR, below, \fIfile\fR is a text file that contains zero >> +or more flows in the same syntax, one per line, and the optional >> +\fB\-\-bundle\fR option operates the command as a single transation, >> +see command \fBbundle\fR, below. >> +. >> +.IP "[\fB\-\-bundle\fR] \fBadd\-flow \fIswitch flow\fR" >> +.IQ "[\fB\-\-bundle\fR] \fBadd\-flow \fIswitch \fB\- < \fIfile\fR" >> +.IQ "[\fB\-\-bundle\fR] \fBadd\-flows \fIswitch file\fR" >> Add each flow entry to \fIswitch\fR's tables. >> . >> -.IP "[\fB\-\-strict\fR] \fBmod\-flows \fIswitch flow\fR" >> -.IQ "[\fB\-\-strict\fR] \fBmod\-flows \fIswitch \fB\- < \fIfile\fR" >> +.IP "[\fB\-\-bundle\fR] [\fB\-\-strict\fR] \fBmod\-flows \fIswitch flow\fR" >> +.IQ "[\fB\-\-bundle\fR] [\fB\-\-strict\fR] \fBmod\-flows \fIswitch \fB\- < >> \fIfile\fR" >> Modify the actions in entries from \fIswitch\fR's tables that match >> the specified flows. With \fB\-\-strict\fR, wildcards are not treated >> as active for matching purposes. >> . >> -.IP "\fBdel\-flows \fIswitch\fR" >> -.IQ "[\fB\-\-strict\fR] \fBdel\-flows \fIswitch \fR[\fIflow\fR]" >> -.IQ "[\fB\-\-strict\fR] \fBdel\-flows \fIswitch \fB\- < \fIfile\fR" >> +.IP "[\fB\-\-bundle\fR] \fBdel\-flows \fIswitch\fR" >> +.IQ "[\fB\-\-bundle\fR] [\fB\-\-strict\fR] \fBdel\-flows \fIswitch >> \fR[\fIflow\fR]" >> +.IQ "[\fB\-\-bundle\fR] [\fB\-\-strict\fR] \fBdel\-flows \fIswitch \fB\- < >> \fIfile\fR" >> Deletes entries from \fIswitch\fR's flow table. With only a >> \fIswitch\fR argument, deletes all flows. Otherwise, deletes flow >> entries that match the specified flows. With \fB\-\-strict\fR, >> wildcards are not treated as active for matching purposes. >> . >> -.IP "[\fB\-\-readd\fR] \fBreplace\-flows \fIswitch file\fR" >> +.IP "[\fB\-\-bundle\fR] [\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 >> up any differences, adding flows from \fIflow\fR that are missing on >> @@ -332,6 +334,44 @@ With \fB\-\-readd\fR, \fBovs\-ofctl\fR adds all the >> flows from >> timeout in \fIswitch\fR. This resets all the flow packet and byte >> counters to 0, which can be useful for debugging. >> . >> +.IP "\fBbundle \fIswitch \fIfile\fR" >> +.IQ "\fBbundle \fIswitch \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 >> +The following applies to the flow table modification commands with the >> +\fB\-\-bundle\fR option as well: >> +. >> +.RS >> +.IP - >> +Within a bundle, all flow mods are processed in the order they appear >> +and 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. However, the \fIswitch\fR may perform >> +resource management, such as evicting flows, that may not be unrolled >> +even when the transaction fails. >> +. >> +.IP - >> +The beginning and the end of the flow table modification commands in a >> +bundle are delimited with OpenFlow 1.4 bundle control messages, which >> +makes it possible to stream the included commands without explicit >> +OpenFlow barriers, which are otherwise used after each flow table >> +modification command. This may make large modifications execute >> +faster as a bundle. >> +. >> +.IP - >> +Bundles require OpenFlow 1.4 or higher. An explicit \fB-O >> +OpenFlow14\fR option is not needed, but you may need to enable >> +OpenFlow 1.4 support for OVS by setting the OVSDB \fIprotocols\fR >> +column in the \fIbridge\fR table. > > Maybe add a paragraph about the types of bundle that are supported? > E.g. document that all bundles are ordered, but not atomic? > >> +.RE >> .IP "\fBdiff\-flows \fIsource1 source2\fR" >> Reads flow entries from \fIsource1\fR and \fIsource2\fR and prints the >> differences. A flow that is in \fIsource1\fR but not in \fIsource2\fR >> @@ -2386,6 +2426,9 @@ depending on its configuration. >> \fB\-\-strict\fR >> Uses strict matching when running flow modification commands. >> . >> +.IP "\fB\-\-bundle\fR" >> +Execute flow mods as an OpenFlow 1.4 bundle transaction. >> +. >> .so lib/ofp-version.man >> . >> .IP "\fB\-F \fIformat\fR[\fB,\fIformat\fR...]" >> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c >> index 54a5bb8..0b5f002 100644 >> --- a/utilities/ovs-ofctl.c >> +++ b/utilities/ovs-ofctl.c >> @@ -67,6 +67,12 @@ >> >> VLOG_DEFINE_THIS_MODULE(ofctl); >> >> +/* --bundle: Use OpenFlow 1.4 bundle for making the flow table change >> atomic. >> + * NOTE: Also the flow mod will use OpenFlow 1.4, so the semantics may be >> + * different (see the comment in parse_options() for details). >> + */ >> +static bool bundle = false; >> + >> /* --strict: Use strict matching for flow mod commands? Additionally governs >> * use of nx_pull_match() instead of nx_pull_match_loose() in parse-nx-match. >> */ >> @@ -159,6 +165,7 @@ parse_options(int argc, char *argv[]) >> OPT_SORT, >> OPT_RSORT, >> OPT_UNIXCTL, >> + OPT_BUNDLE, >> DAEMON_OPTION_ENUMS, >> OFP_VERSION_OPTION_ENUMS, >> VLOG_OPTION_ENUMS >> @@ -176,6 +183,7 @@ parse_options(int argc, char *argv[]) >> {"unixctl", required_argument, NULL, OPT_UNIXCTL}, >> {"help", no_argument, NULL, 'h'}, >> {"option", no_argument, NULL, 'o'}, >> + {"bundle", no_argument, NULL, OPT_BUNDLE}, >> DAEMON_LONG_OPTIONS, >> OFP_VERSION_LONG_OPTIONS, >> VLOG_LONG_OPTIONS, >> @@ -249,6 +257,10 @@ parse_options(int argc, char *argv[]) >> ovs_cmdl_print_options(long_options); >> exit(EXIT_SUCCESS); >> >> + case OPT_BUNDLE: >> + bundle = true; >> + break; >> + >> case OPT_STRICT: >> strict = true; >> break; >> @@ -293,6 +305,12 @@ parse_options(int argc, char *argv[]) >> >> free(short_options); >> >> + /* Implicit OpenFlow 1.4 with the '--bundle' option. */ >> + if (bundle) { >> + /* Add implicit allowance for OpenFlow 1.4. */ >> + add_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( >> + OFPUTIL_P_OF14_OXM)); >> + } >> versions = get_allowed_ofp_versions(); >> version_protocols = ofputil_protocols_from_version_bitmap(versions); >> if (!(allowed_protocols & version_protocols)) { >> @@ -602,6 +620,26 @@ transact_multiple_noreply(struct vconn *vconn, struct >> ovs_list *requests) >> ofpbuf_delete(reply); >> } >> >> +static void >> +bundle_error_reporter(const struct ofp_header *oh) >> +{ >> + ofp_print(stderr, oh, ntohs(oh->length), verbosity + 1); >> + fflush(stderr); >> +} >> + >> +static void >> +bundle_transact(struct vconn *vconn, struct ovs_list *requests, uint16_t >> flags) >> +{ >> + struct ofpbuf *request; >> + >> + LIST_FOR_EACH (request, list_node, requests) { >> + ofpmsg_update_length(request); >> + } >> + >> + run(vconn_bundle_transact(vconn, requests, flags, bundle_error_reporter), >> + "talking to %s", vconn_get_name(vconn)); >> +} >> + >> /* Sends 'request', which should be a request that only has a reply if an >> error >> * occurs, and waits for it to succeed or fail. If an error does occur, prints >> * it and exits with an error. >> @@ -1194,6 +1232,10 @@ ofctl_flow_mod__(const char *remote, struct >> ofputil_flow_mod *fms, >> } >> >> static void >> +bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms, >> + size_t n_fms, enum ofputil_protocol); >> + >> +static void >> ofctl_flow_mod_file(int argc OVS_UNUSED, char *argv[], uint16_t command) >> { >> enum ofputil_protocol usable_protocols; >> @@ -1206,7 +1248,11 @@ ofctl_flow_mod_file(int argc OVS_UNUSED, char >> *argv[], uint16_t command) >> if (error) { >> ovs_fatal(0, "%s", error); >> } >> - ofctl_flow_mod__(argv[1], fms, n_fms, usable_protocols); >> + if (bundle) { >> + bundle_flow_mod__(argv[1], fms, n_fms, usable_protocols); >> + } else { >> + ofctl_flow_mod__(argv[1], fms, n_fms, usable_protocols); >> + } >> free(fms); >> } >> >> @@ -1225,7 +1271,11 @@ ofctl_flow_mod(int argc, char *argv[], uint16_t >> command) >> if (error) { >> ovs_fatal(0, "%s", error); >> } >> - ofctl_flow_mod__(argv[1], &fm, 1, usable_protocols); >> + if (bundle) { >> + bundle_flow_mod__(argv[1], &fm, 1, usable_protocols); >> + } else { >> + ofctl_flow_mod__(argv[1], &fm, 1, usable_protocols); >> + } >> } >> } >> >> @@ -2262,6 +2312,55 @@ ofctl_dump_group_features(struct ovs_cmdl_context >> *ctx) >> } >> >> static void >> +bundle_flow_mod__(const char *remote, struct ofputil_flow_mod *fms, >> + size_t n_fms, enum ofputil_protocol usable_protocols) >> +{ >> + enum ofputil_protocol protocol; >> + struct vconn *vconn; >> + struct ovs_list requests; >> + size_t i; >> + >> + list_init(&requests); >> + >> + /* Add implicit allowance for OpenFlow 1.4. */ >> + allowed_protocols |= OFPUTIL_P_OF14_OXM; >> + add_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( >> + OFPUTIL_P_OF14_OXM)); >> + usable_protocols &= OFPUTIL_P_OF14_UP; >> + protocol = open_vconn_for_flow_mod(remote, &vconn, usable_protocols); >> + >> + for (i = 0; i < n_fms; i++) { >> + struct ofputil_flow_mod *fm = &fms[i]; >> + struct ofpbuf *request = ofputil_encode_flow_mod(fm, protocol); >> + >> + list_push_back(&requests, &request->list_node); >> + free(CONST_CAST(struct ofpact *, fm->ofpacts)); >> + } >> + >> + bundle_transact(vconn, &requests, OFPBF_ORDERED); >> + vconn_close(vconn); >> +} >> + >> +static void >> +ofctl_bundle(struct ovs_cmdl_context *ctx) >> +{ >> + enum ofputil_protocol usable_protocols; >> + struct ofputil_flow_mod *fms = NULL; >> + size_t n_fms = 0; >> + char *error; >> + >> + /* Parse flow mods from the file. */ >> + error = parse_ofp_flow_mod_file(ctx->argv[2], -2, &fms, &n_fms, >> + &usable_protocols); >> + if (error) { >> + ovs_fatal(0, "%s", error); >> + } >> + >> + bundle_flow_mod__(ctx->argv[1], fms, n_fms, usable_protocols); >> + free(fms); >> +} >> + >> +static void >> ofctl_help(struct ovs_cmdl_context *ctx OVS_UNUSED) >> { >> usage(); >> @@ -2636,7 +2735,11 @@ ofctl_replace_flows(struct ovs_cmdl_context *ctx) >> fte_make_flow_mod(fte, FILE_IDX, OFPFC_ADD, protocol, &requests); >> } >> } >> - transact_multiple_noreply(vconn, &requests); >> + if (bundle) { >> + bundle_transact(vconn, &requests, OFPBF_ORDERED); >> + } else { >> + transact_multiple_noreply(vconn, &requests); >> + } >> vconn_close(vconn); >> >> fte_free_all(&cls); >> @@ -3578,6 +3681,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 file", 2, 2, ofctl_bundle }, >> { "help", NULL, 0, INT_MAX, ofctl_help }, >> { "list-commands", NULL, 0, INT_MAX, ofctl_list_commands }, >> >> -- >> 1.7.10.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> http://openvswitch.org/mailman/listinfo/dev >> <http://openvswitch.org/mailman/listinfo/dev> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
