On 1/3/23 12:33, Eelco Chaudron wrote: > Depending on the driver implementation it can take up to 2 seconds before > offloaded flow statistics are updated. This is causing a problem with > min-revalidate-pps, as old statistic values are used during this period. > > This fix will wait for at least 2 seconds before assuming no packets > where received during this period. > > Signed-off-by: Eelco Chaudron <[email protected]> > --- > > Changes: > - v2: Use existing ukey->offloaded so the revalidate_missed_dp_flow case is > also covered. > > ofproto/ofproto-dpif-upcall.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index ad9635496..c395adbc6 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -2094,10 +2094,11 @@ ukey_delete(struct umap *umap, struct udpif_key *ukey) > } > > static bool > -should_revalidate(const struct udpif *udpif, uint64_t packets, > - long long int used) > +should_revalidate(const struct udpif *udpif, struct udpif_key *ukey, > + uint64_t packets) > { > long long int metric, now, duration; > + long long int used = ukey->stats.used; > > if (!used) { > /* Always revalidate the first time a flow is dumped. */ > @@ -2124,8 +2125,12 @@ should_revalidate(const struct udpif *udpif, uint64_t > packets, > duration = now - used; > metric = duration / packets; > > - if (metric < 1000 / ofproto_min_revalidate_pps) { > - /* The flow is receiving more than min-revalidate-pps, so keep it. */ > + if (metric < 1000 / ofproto_min_revalidate_pps || > + (ukey->offloaded && duration < 2000)) { > + /* The flow is receiving more than min-revalidate-pps, so keep it. > + * Or it's a hardware offloaded flow that might take up to 2 seconds > + * to update its statistics. Until we are sure the statistics had a > + * chance to be updated, also keep it. */
Hmm. If you know that a flow dump is taking a long time on this setup, shouldn't we just bump the max-revalidator value in the database instead of hardcoding yet another magic constant? How long the dump actually takes? And how many flows in it? Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
