| Message ID | 20230117030129.1447363-1-hzhou@ovn.org |
|---|---|
| State | Accepted |
| Commit | e5b3cb9995445cd83ab1d2811bfd79ca03dd46d4 |
| Headers | show |
| Series | [ovs-dev,v2] Revalidator: Allow min-revalidator-pps to be 0. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 17 Jan 2023, at 4:01, Han Zhou wrote: > Today the minimum value for this setting is 1. This patch allows it to > be 0, meaning not checking pps at all, and always do revalidation. > > This is particularly useful for environments where some of the > applications with long-lived connections may have very low traffic for > certain period but have high rate of burst periodically. It is desirable > to keep the datapath flows instead of periodically deleting them to > avoid burst of packet miss to userspace. > > When setting to 0, there may be more datapath flows to be revalidated, > resulting in higher CPU cost of revalidator threads. This is the > downside but in certain cases this is still more desirable than packet > misses to user space. > > Signed-off-by: Han Zhou <hzhou@ovn.org> Thanks for addressing the documentation Han! Acked-by: Eelco Chaudron <echaudro@redhat.com>
On 1/17/23 10:25, Eelco Chaudron wrote: > > > On 17 Jan 2023, at 4:01, Han Zhou wrote: > >> Today the minimum value for this setting is 1. This patch allows it to >> be 0, meaning not checking pps at all, and always do revalidation. >> >> This is particularly useful for environments where some of the >> applications with long-lived connections may have very low traffic for >> certain period but have high rate of burst periodically. It is desirable >> to keep the datapath flows instead of periodically deleting them to >> avoid burst of packet miss to userspace. >> >> When setting to 0, there may be more datapath flows to be revalidated, >> resulting in higher CPU cost of revalidator threads. This is the >> downside but in certain cases this is still more desirable than packet >> misses to user space. >> >> Signed-off-by: Han Zhou <hzhou@ovn.org> > > Thanks for addressing the documentation Han! > > Acked-by: Eelco Chaudron <echaudro@redhat.com> Thanks, Han and Eelco! Applied to master. Best regards, Ilya Maximets.
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index ad96354966f1..442141ccd91d 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -2099,6 +2099,10 @@ should_revalidate(const struct udpif *udpif, uint64_t packets, { long long int metric, now, duration; + if (!ofproto_min_revalidate_pps) { + return true; + } + if (!used) { /* Always revalidate the first time a flow is dumped. */ return true; diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 17f636ed9dc5..e4a1bee769d0 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -724,7 +724,7 @@ ofproto_set_max_revalidator(unsigned max_revalidator) void ofproto_set_min_revalidate_pps(unsigned min_revalidate_pps) { - ofproto_min_revalidate_pps = min_revalidate_pps ? min_revalidate_pps : 1; + ofproto_min_revalidate_pps = min_revalidate_pps; } /* If forward_bpdu is true, the NORMAL action will forward frames with diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 8c4acfb1817f..90174e57fb13 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -205,10 +205,11 @@ </column> <column name="other_config" key="min-revalidate-pps" - type='{"type": "integer", "minInteger": 1}'> + type='{"type": "integer", "minInteger": 0}'> <p> Set minimum pps that flow must have in order to be revalidated when revalidation duration exceeds half of max-revalidator config variable. + Setting to 0 means always revalidate flows regardless of pps. </p> <p> The default is 5.
Today the minimum value for this setting is 1. This patch allows it to be 0, meaning not checking pps at all, and always do revalidation. This is particularly useful for environments where some of the applications with long-lived connections may have very low traffic for certain period but have high rate of burst periodically. It is desirable to keep the datapath flows instead of periodically deleting them to avoid burst of packet miss to userspace. When setting to 0, there may be more datapath flows to be revalidated, resulting in higher CPU cost of revalidator threads. This is the downside but in certain cases this is still more desirable than packet misses to user space. Signed-off-by: Han Zhou <hzhou@ovn.org> --- v2: Addresses Eelco's comment - update document of the option. ofproto/ofproto-dpif-upcall.c | 4 ++++ ofproto/ofproto.c | 2 +- vswitchd/vswitch.xml | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-)