On 2 Jan 2023, at 14:57, Roi Dayan wrote:
> On 21/12/2022 11:34, 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]> >> --- >> ofproto/ofproto-dpif-upcall.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c >> index ad9635496..464c304a8 100644 >> --- a/ofproto/ofproto-dpif-upcall.c >> +++ b/ofproto/ofproto-dpif-upcall.c >> @@ -2095,7 +2095,7 @@ ukey_delete(struct umap *umap, struct udpif_key *ukey) >> >> static bool >> should_revalidate(const struct udpif *udpif, uint64_t packets, >> - long long int used) >> + long long int used, bool offloaded) >> { >> long long int metric, now, duration; >> >> @@ -2124,8 +2124,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 || >> + (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. */ >> return true; >> } >> return false; >> @@ -2342,7 +2346,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key >> *ukey, >> : 0); >> >> if (need_revalidate) { >> - if (should_revalidate(udpif, push.n_packets, ukey->stats.used)) { >> + if (should_revalidate(udpif, push.n_packets, ukey->stats.used, >> + offloaded)) { >> if (!ukey->xcache) { >> ukey->xcache = xlate_cache_new(); >> } else { >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > looks good. thanks Thanks for reviewing Roi, however, there was one more corner case that showed up. So I’ll send a v2 soon, can you please take another look? Thanks, Eelco > Acked-by: Roi Dayan <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
