On 11/27/22 08:28, Peng He wrote: > The userspace datapath mananges all the magaflows by a cmap. The cmap > data structrue will grow/shrink during the datapath processing and it > will re-position megaflows. This might result in two revalidator threads > might process a same megaflow during one dump stage. > > Consider a situation that, revalidator 1 processes a megaflow A, and > decides to delete it from the datapath, at the mean time, this megaflow > A is also queued in the process batch of revalidator 2. Normally it's ok > for revalidators to process the same megaflow multiple times, as the > dump_seq shows it's already dumped and the stats will not be contributed > twice. > > Assume that right after A is deleted, a PMD thread generates again > a new megaflow B which has the same match and action of A. The ukey > of megaflow B will replace the one of megaflow A. Now the ukey B is > new to the revalidator system and its dump seq is 0. > > Now since the dump seq of ukey B is 0, when processing megaflow A, > the revalidator 2 will not identify this megaflow A has already been > dumped by revalidator 1 and will contribute the old megaflow A's stats > again, this results in an inconsistent stats between ukeys and megaflows. > > To fix this, the newly generated the ukey B should take the dump_seq > of the replaced ukey A to avoid a same megaflow being revalidated > twice in one dump stage. > > We observe in the production environment, the OpenFlow rules' stats > sometimes are amplified compared to the actual value. > > Signed-off-by: Peng He <[email protected]> > Acked-by: Eelco Chaudron <[email protected]> > --- > ofproto/ofproto-dpif-upcall.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index c2cefbeb8..8b5db6103 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -1877,6 +1877,7 @@ try_ukey_replace(struct umap *umap, struct udpif_key > *old_ukey, > ovs_mutex_lock(&new_ukey->mutex); > cmap_replace(&umap->cmap, &old_ukey->cmap_node, > &new_ukey->cmap_node, new_ukey->hash); > + new_ukey->dump_seq = old_ukey->dump_seq; > ovsrcu_postpone(ukey_delete__, old_ukey); > transition_ukey(old_ukey, UKEY_DELETED); > transition_ukey(new_ukey, UKEY_VISIBLE);
While there is still some discussion on other patches from the set, I applied this one patch. Backported down to 2.17. Thanks! Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
