Re: [ovs-dev] [PATCH 1/2] Revalidator: Allow min-revalidator-pps to be 0 (disabled).

[Eelco Chaudron]

On 17 Jan 2023, at 4:08, Han Zhou wrote:

> On Mon, Jan 16, 2023 at 7:14 AM Eelco Chaudron  wrote:
>>
>>
>>
>> On 18 Oct 2022, at 18:08, Adrian Moreno wrote:
>>
>>> On 10/14/22 19:49, Han Zhou wrote:


 On Thu, Oct 13, 2022 at 11:43 PM Adrian Moreno  > 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 . 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.

>>>
>>> I agree. There are many factors that come into play. My concern is that
> we have so many knobs that tweak the revalidator/handler load distribution
> that depend on so many external factors that it's quite complicated to
> determine when to recommend their use.
>>>
>>> That's why I wanted to have some ballpark numbers to get an intuition
> of the potential side effects.
>>>
>>> Having said that, I think this particular proposal is beneficial as I
> also have seen drops caused by long-lived connections.
>>
>> I do not see anything wrong with adding this feature, however, the
> documentation needs to be clear on what 0 means, i.e. does it result in
> always revalidate or never revalidate?
>
> Thanks Eelco. I thought that min-revalidate-pps=0 naturally means
> revalidating all flows, because pps > 0 would include all flows. However,
> probably the "disable" in my commit title was confusing, and some may think
> it was trying to disable the revalidation (while I meant disable the
> option).
> So, I updated the title and also updated the document to avoid confusion in
> v2. PTAL:
> https://patchwork.ozlabs.org/project/openvswitch/patch/20230117030129.1447363-1-hz...@ovn.org/

Thanks, quickly tested and ACKed your patch.

//Eelco

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] Revalidator: Allow min-revalidator-pps to be 0 (disabled).

[Han Zhou]

On Mon, Jan 16, 2023 at 7:14 AM Eelco Chaudron  wrote:
>
>
>
> On 18 Oct 2022, at 18:08, Adrian Moreno wrote:
>
> > On 10/14/22 19:49, Han Zhou wrote:
> >>
> >>
> >> On Thu, Oct 13, 2022 at 11:43 PM Adrian Moreno mailto:amore...@redhat.com>> 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 . 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.
> >>
> >
> > I agree. There are many factors that come into play. My concern is that
we have so many knobs that tweak the revalidator/handler load distribution
that depend on so many external factors that it's quite complicated to
determine when to recommend their use.
> >
> > That's why I wanted to have some ballpark numbers to get an intuition
of the potential side effects.
> >
> > Having said that, I think this particular proposal is beneficial as I
also have seen drops caused by long-lived connections.
>
> I do not see anything wrong with adding this feature, however, the
documentation needs to be clear on what 0 means, i.e. does it result in
always revalidate or never revalidate?

Thanks Eelco. I thought that min-revalidate-pps=0 naturally means
revalidating all flows, because pps > 0 would include all flows. However,
probably the "disable" in my commit title was confusing, and some may think
it was trying to disable the revalidation (while I meant disable the
option).
So, I updated the title and also updated the document to avoid confusion in
v2. PTAL:
https://patchwork.ozlabs.org/project/openvswitch/patch/20230117030129.1447363-1-hz...@ovn.org/

Thanks,
Han

>
>
> //Eelco
>
>
>  Signed-off-by: Han Zhou mailto:hz...@ovn.org>>
>  ---
>    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..c1296 100644
>  --- a/vswitchd/vswitch.xml
>  +++ b/

Re: [ovs-dev] [PATCH 1/2] Revalidator: Allow min-revalidator-pps to be 0 (disabled).

[Eelco Chaudron]

On 18 Oct 2022, at 18:08, Adrian Moreno wrote:

> On 10/14/22 19:49, Han Zhou wrote:
>>
>>
>> On Thu, Oct 13, 2022 at 11:43 PM Adrian Moreno > > 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 . 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.
>>
>
> I agree. There are many factors that come into play. My concern is that we 
> have so many knobs that tweak the revalidator/handler load distribution that 
> depend on so many external factors that it's quite complicated to determine 
> when to recommend their use.
>
> That's why I wanted to have some ballpark numbers to get an intuition of the 
> potential side effects.
>
> Having said that, I think this particular proposal is beneficial as I also 
> have seen drops caused by long-lived connections.

I do not see anything wrong with adding this feature, however, the 
documentation needs to be clear on what 0 means, i.e. does it result in always 
revalidate or never revalidate?

//Eelco


 Signed-off-by: Han Zhou mailto:hz...@ovn.org>>
 ---
   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..c1296 100644
 --- a/vswitchd/vswitch.xml
 +++ b/vswitchd/vswitch.xml
 @@ -212,7 +212,7 @@
         

         >>> -              type='{"type": "integer", "minInteger": 1}'>
 +              type='{"type": "integer", "minInteger": 0}'>
           
             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
>>>
>
> -- 
> Adrián Moreno
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_

Re: [ovs-dev] [PATCH 1/2] Revalidator: Allow min-revalidator-pps to be 0 (disabled).

[Adrian Moreno]

On 10/14/22 19:49, Han Zhou wrote:



On Thu, Oct 13, 2022 at 11:43 PM Adrian Moreno > 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 . 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.




I agree. There are many factors that come into play. My concern is that we have 
so many knobs that tweak the revalidator/handler load distribution that depend 
on so many external factors that it's quite complicated to determine when to 
recommend their use.


That's why I wanted to have some ballpark numbers to get an intuition of the 
potential side effects.


Having said that, I think this particular proposal is beneficial as I also have 
seen drops caused by long-lived connections.



Thanks,
Han


 > > Signed-off-by: Han Zhou mailto:hz...@ovn.org>>
 > > ---
 > >   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..c1296 100644
 > > --- a/vswitchd/vswitch.xml
 > > +++ b/vswitchd/vswitch.xml
 > > @@ -212,7 +212,7 @@
 > >         
 > >
 > >          > -              type='{"type": "integer", "minInteger": 1}'>
 > > +              type='{"type": "integer", "minInteger": 0}'>
 > >           
 > >             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
 >


--
Adrián Moreno

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] Revalidator: Allow min-revalidator-pps to be 0 (disabled).

[Han Zhou]

On Thu, Oct 13, 2022 at 11:43 PM Adrian Moreno  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 . 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 
> > ---
> >   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..c1296 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -212,7 +212,7 @@
> > 
> >
> >  > -  type='{"type": "integer", "minInteger": 1}'>
> > +  type='{"type": "integer", "minInteger": 0}'>
> >   
> > 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] Revalidator: Allow min-revalidator-pps to be 0 (disabled).

[Adrian Moreno]

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?



Signed-off-by: Han Zhou 
---
  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..c1296 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -212,7 +212,7 @@

  

-  type='{"type": "integer", "minInteger": 1}'>
+  type='{"type": "integer", "minInteger": 0}'>
  
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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/2] Revalidator: Allow min-revalidator-pps to be 0 (disabled).

[Han Zhou]

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 
---
 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..c1296 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -212,7 +212,7 @@
   
 
   
+  type='{"type": "integer", "minInteger": 0}'>
 
   Set minimum pps that flow must have in order to be revalidated when
   revalidation duration exceeds half of max-revalidator config 
variable.
-- 
2.30.2

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev