Ilya Maximets <[email protected]> writes:

> On 2/27/23 13:30, Paolo Valerio wrote:
>> Since 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists
>> with rculists.") the sweep interval changed as well as the constraints
>> related to the sweeper.
>> Being able to change the default reschedule time may be convenient in
>> some conditions, like debugging.
>> This patch introduces new commands allowing to get and set the sweep
>> next run in ms.
>> 
>> Signed-off-by: Paolo Valerio <[email protected]>
>> ---
>> v2:
>> - resolved conflict in NEWS
>> - added missing comment
>> - added missing '\' in dpctl.man
>> ---
>>  NEWS                    |    4 +++
>>  lib/conntrack-private.h |    1 +
>>  lib/conntrack.c         |   18 +++++++++++++-
>>  lib/conntrack.h         |    2 ++
>>  lib/ct-dpif.c           |   14 +++++++++++
>>  lib/ct-dpif.h           |    1 +
>>  lib/dpctl.c             |   61 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  lib/dpctl.man           |    8 ++++++
>>  lib/dpif-netdev.c       |   17 +++++++++++++
>>  lib/dpif-netlink.c      |    2 ++
>>  lib/dpif-provider.h     |    4 +++
>>  11 files changed, 131 insertions(+), 1 deletion(-)
>> 
>> diff --git a/NEWS b/NEWS
>> index 85b349621..4c4ef4b2b 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -10,6 +10,10 @@ Post-v3.1.0
>>         in order to create OVSDB sockets with access mode of 0770.
>>     - QoS:
>>       * Added new configuration option 'jitter' for a linux-netem QoS type.
>> +   - ovs-appctl:
>> +     * New commands "dpctl/{ct-get-sweep-next-run,ct-set-sweep-next-run}" 
>> that
>> +       allow to get and set, for the userspace datapath, the next run 
>> interval
>> +       for the conntrack garbage collector.
>
> Hi, Paolo.  Thanks for the patch!
>
> It looks good to me in general, but the command name seems a bit
> strange.  It sounds like it is a one-shot configuration that only
> applies to the next run and will be dropped to default afterwards.
> But that doesn't seem to be the case in the code.  It's a permanent
> configuration for the sweep interval.  So, maybe we should call it
> ct-[gs]et-sweep-interval, or something like that ?
>
> What do you think?
>

Agreed, ct-[gs]et-sweep-interval seems a better name.

> Also, some small unit test, even a basic set+get check, would be
> nice to have.  We have some similar tests in tests/ofproto-dpif.at.
>

sure, I'll add it.

Thank you.

> Best regards, Ilya Maximets.

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to