From: Han Zhou <hz...@ovn.org> Move the group changes to the same bundle as the OF flow changes. The steps in ofctrl_put were: - add groups - add meters - bundle - change flows - delete groups - delete meters
Now it becomes: - add meters - bundle - add groups - change flows - delete groups - delete meters It is required for a future change that needs all operations in a bundle. Ideally we should add meters to the bundle as well but it is not supported yet by OVS. Signed-off-by: Han Zhou <hz...@ovn.org> --- controller/ofctrl.c | 110 ++++++++++++++++++++++++-------------------- 1 file changed, 61 insertions(+), 49 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index ecabbcc9b..ff5679d00 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -1737,21 +1737,24 @@ encode_flow_mod(struct ofputil_flow_mod *fm) return ofputil_encode_flow_mod(fm, OFPUTIL_P_OF15_OXM); } -static void -add_flow_mod(struct ofputil_flow_mod *fm, - struct ofputil_bundle_ctrl_msg *bc, - struct ovs_list *msgs) +static struct ofpbuf * +encode_bundle_add(struct ofpbuf *msg, struct ofputil_bundle_ctrl_msg *bc) { - struct ofpbuf *msg = encode_flow_mod(fm); struct ofputil_bundle_add_msg bam = { .bundle_id = bc->bundle_id, .flags = bc->flags, .msg = msg->data, }; - struct ofpbuf *bundle_msg; - - bundle_msg = ofputil_encode_bundle_add(OFP15_VERSION, &bam); + return ofputil_encode_bundle_add(OFP15_VERSION, &bam); +} +static void +add_flow_mod(struct ofputil_flow_mod *fm, + struct ofputil_bundle_ctrl_msg *bc, + struct ovs_list *msgs) +{ + struct ofpbuf *msg = encode_flow_mod(fm); + struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc); ofpbuf_delete(msg); ovs_list_push_back(msgs, &bundle_msg->list_node); } @@ -1765,13 +1768,18 @@ encode_group_mod(const struct ofputil_group_mod *gm) } static void -add_group_mod(struct ofputil_group_mod *gm, struct ovs_list *msgs) +add_group_mod(struct ofputil_group_mod *gm, + struct ofputil_bundle_ctrl_msg *bc, + struct ovs_list *msgs) { struct ofpbuf *msg = encode_group_mod(gm); - if (msg->size <= UINT16_MAX) { - ovs_list_push_back(msgs, &msg->list_node); + if ((msg->size + sizeof(struct ofp14_bundle_ctrl_msg)) <= UINT16_MAX) { + struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc); + ofpbuf_delete(msg); + ovs_list_push_back(msgs, &bundle_msg->list_node); return; } + /* This group mod request is too large to fit in a single OF message * since the header can only specify a 16-bit size. We need to break * this into multiple group_mod requests. @@ -1793,7 +1801,9 @@ add_group_mod(struct ofputil_group_mod *gm, struct ovs_list *msgs) * the size of the buckets, we will not put too many in our new group_mod * message. */ - size_t max_buckets = ((UINT16_MAX - sizeof *ogm) / bucket_size) / 2; + size_t max_buckets = ((UINT16_MAX - sizeof *ogm - + sizeof(struct ofp14_bundle_ctrl_msg)) / bucket_size) + / 2; ovs_assert(max_buckets < ovs_list_size(&gm->buckets)); @@ -1822,14 +1832,16 @@ add_group_mod(struct ofputil_group_mod *gm, struct ovs_list *msgs) ovs_list_splice(&split.buckets, &bucket->list_node, &gm->buckets); struct ofpbuf *orig = encode_group_mod(gm); - ovs_list_push_back(msgs, &orig->list_node); + struct ofpbuf *bundle_msg = encode_bundle_add(orig, bc); + ofpbuf_delete(orig); + ovs_list_push_back(msgs, &bundle_msg->list_node); /* We call this recursively just in case our new * INSERT_BUCKET/REMOVE_BUCKET group_mod is still too * large for an OF message. This will allow for it to * be broken into pieces, too. */ - add_group_mod(&split, msgs); + add_group_mod(&split, bc, msgs); ofputil_uninit_group_mod(&split); } @@ -2438,29 +2450,6 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table, } } - /* Iterate through all the desired groups. If there are new ones, - * add them to the switch. */ - struct ovn_extend_table_info *desired; - EXTEND_TABLE_FOR_EACH_UNINSTALLED (desired, groups) { - /* Create and install new group. */ - struct ofputil_group_mod gm; - enum ofputil_protocol usable_protocols; - char *group_string = xasprintf("group_id=%"PRIu32",%s", - desired->table_id, - desired->name); - char *error = parse_ofp_group_mod_str(&gm, OFPGC15_ADD, group_string, - NULL, NULL, &usable_protocols); - if (!error) { - add_group_mod(&gm, &msgs); - } else { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_ERR_RL(&rl, "new group %s %s", error, group_string); - free(error); - } - free(group_string); - ofputil_uninit_group_mod(&gm); - } - /* Iterate through all the desired meters. If there are new ones, * add them to the switch. */ struct ovn_extend_table_info *m_desired; @@ -2493,6 +2482,29 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table, bundle_open = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc); ovs_list_push_back(&msgs, &bundle_open->list_node); + /* Iterate through all the desired groups. If there are new ones, + * add them to the switch. */ + struct ovn_extend_table_info *desired; + EXTEND_TABLE_FOR_EACH_UNINSTALLED (desired, groups) { + /* Create and install new group. */ + struct ofputil_group_mod gm; + enum ofputil_protocol usable_protocols; + char *group_string = xasprintf("group_id=%"PRIu32",%s", + desired->table_id, + desired->name); + char *error = parse_ofp_group_mod_str(&gm, OFPGC15_ADD, group_string, + NULL, NULL, &usable_protocols); + if (!error) { + add_group_mod(&gm, &bc, &msgs); + } else { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_ERR_RL(&rl, "new group %s %s", error, group_string); + free(error); + } + free(group_string); + ofputil_uninit_group_mod(&gm); + } + /* If skipped last time, then process the flow table * (tracked) flows even if lflows_changed is not set. * Same for pflows_changed. */ @@ -2522,17 +2534,6 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table, skipped_last_time = false; - if (ovs_list_back(&msgs) == &bundle_open->list_node) { - /* No flow updates. Removing the bundle open request. */ - ovs_list_pop_back(&msgs); - ofpbuf_delete(bundle_open); - } else { - /* Committing the bundle. */ - bc.type = OFPBCT_COMMIT_REQUEST; - bundle_commit = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc); - ovs_list_push_back(&msgs, &bundle_commit->list_node); - } - /* Iterate through the installed groups from previous runs. If they * are not needed delete them. */ struct ovn_extend_table_info *installed, *next_group; @@ -2546,7 +2547,7 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table, group_string, NULL, NULL, &usable_protocols); if (!error) { - add_group_mod(&gm, &msgs); + add_group_mod(&gm, &bc, &msgs); } else { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); VLOG_ERR_RL(&rl, "Error deleting group %d: %s", @@ -2558,6 +2559,17 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table, ovn_extend_table_remove_existing(groups, installed); } + if (ovs_list_back(&msgs) == &bundle_open->list_node) { + /* No flow updates. Removing the bundle open request. */ + ovs_list_pop_back(&msgs); + ofpbuf_delete(bundle_open); + } else { + /* Committing the bundle. */ + bc.type = OFPBCT_COMMIT_REQUEST; + bundle_commit = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc); + ovs_list_push_back(&msgs, &bundle_commit->list_node); + } + /* Sync the contents of groups->desired to groups->existing. */ ovn_extend_table_sync(groups); -- 2.37.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev