From 270de5dfb80e3e094d072b38c8b25a9242c73958 Mon Sep 17 00:00:00 2001 From: Eli Britstein Date: Mon, 14 Jul 2025 14:29:06 +0300 Subject: [PATCH] ofproto: Move group-modify to mod_start instead of mod_finish. Upon modifying a group, the following steps occur: 1. ofproto_group_mod_start()->modify_group_start(): Find an old group object, create a new one. 2. ofproto_bump_tables_version() 3. ofproto_group_mod_finish(): Modify the new group object with buckets etc. At step #3, the new group object is already in use by revalidators, that may read incorrect data while being modified. Instead, move the group modification of the new object to step #1. Fixes: 0a8f6beb54ab ("ofproto-dpif: Fix dp_hash mapping after select group modification.") Acked-by: Gaetan Rivet Acked-by: Roi Dayan Signed-off-by: Eli Britstein Signed-off-by: Ilya Maximets --- ofproto/ofproto.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index ef615e59c..8fa70e518 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -7820,6 +7820,12 @@ modify_group_start(struct ofproto *ofproto, struct ofproto_group_mod *ogm) ofproto->n_groups[old_group->type]--; ofproto->n_groups[new_group->type]++; } + + if (ofproto->ofproto_class->group_modify) { + /* XXX: OK to lose old group's stats? */ + ofproto->ofproto_class->group_modify(new_group); + } + return 0; out: @@ -7985,15 +7991,6 @@ ofproto_group_mod_finish(struct ofproto *ofproto, struct ofgroup *new_group = ogm->new_group; struct ofgroup *old_group; - if (new_group && group_collection_n(&ogm->old_groups) && - ofproto->ofproto_class->group_modify) { - /* Modify a group. */ - ovs_assert(group_collection_n(&ogm->old_groups) == 1); - - /* XXX: OK to lose old group's stats? */ - ofproto->ofproto_class->group_modify(new_group); - } - /* Delete old groups. */ GROUP_COLLECTION_FOR_EACH(old_group, &ogm->old_groups) { delete_group_finish(ofproto, old_group);