On Thu, Oct 13, 2022 at 11:43 PM Adrian Moreno <[email protected]> wrote: > > Hi Han. > > On 8/9/22 03:40, 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. > > > I am trying to quantify this CPU cost. Do you have any numbers so we understand > the effects of disabling pps-based evictions? > Hi Adrian,
Thanks for reviewing. First of all, the CPU cost is added to revalidator threads only, but saving cost for the handler threads which is more critical to the packet processing. Now regarding the actual CPU cost of revalidator threads, the extra cost really depends on the traffic pattern - how many long lived DP flows are there with pps smaller than <min-revalidator-pps>. The more such DP flows, the more revalidtor CPU, of course. We don't see a problem when there are 10k DP flows when setting min-revalidator-pps to 0, and this is on a DPU with small number of less powerful ARM cores. It also depends on whether it is a dedicated network node/DPU where it is ok for OVS to take a significant portion of the CPU or it is a compute node where more cores are supposed to be reserved for applications. In any case, this is just an option and totally depends on user settings according to their use case, as explained in the commit message. Thanks, Han > > Signed-off-by: Han Zhou <[email protected]> > > --- > > ofproto/ofproto-dpif-upcall.c | 4 ++++ > > ofproto/ofproto.c | 2 +- > > vswitchd/vswitch.xml | 2 +- > > 3 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > > index 57f94df54..23b52ef40 100644 > > --- a/ofproto/ofproto-dpif-upcall.c > > +++ b/ofproto/ofproto-dpif-upcall.c > > @@ -2079,6 +2079,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 3a527683c..259fad4b5 100644 > > --- a/ofproto/ofproto.c > > +++ b/ofproto/ofproto.c > > @@ -723,7 +723,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 4a9284f6b..2222c1296 100644 > > --- a/vswitchd/vswitch.xml > > +++ b/vswitchd/vswitch.xml > > @@ -212,7 +212,7 @@ > > </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. > > -- > Adrián Moreno > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
