NACK: ACK/Cmnt: [SRU][F:linux-bluefield][PATCH 0/5] Control nf flow table timeouts

[Tim Gardner]

v2 is on the list

On 7/7/21 9:15 AM, Bodong Wang wrote:
> On 7/7/2021 2:08 AM, Stefan Bader wrote:
>> On 06.07.21 19:21, Bodong Wang wrote:
>>> On 7/6/2021 2:33 AM, Stefan Bader wrote:
>>>> On 01.07.21 22:38, Bodong Wang wrote:
>>>>> TCP and UDP connections may be offloaded from nf conntrack to nf 
>>>>> flow table.
>>>>> Offloaded connections are aged after 30 seconds of inactivity.
>>>>> Once aged, ownership is returned to conntrack with a hard coded 
>>>>> tcp/udp
>>>>> pickup time of 120/30 seconds, after which the connection may be 
>>>>> deleted.
>>>>>
>>>>> The current hard-coded pickup intervals may introduce a very 
>>>>> aggressive
>>>>> aging policy. For example, offloaded tcp connections in established 
>>>>> state
>>>>> will timeout from nf conntrack after just 150 seconds of inactivity,
>>>>> instead of 5 days. In addition, the hard-coded 30 second offload 
>>>>> timeout
>>>>> period can significantly increase the hardware insertion rate 
>>>>> requirements
>>>>> in some use cases.
>>>>>
>>>>> This patchset provides the user with the ability to configure protocol
>>>>> specific offload timeout and pickup intervals via sysctl.
>>>>>
>>>>> The first and second patches revert the existing non-upstream 
>>>>> solution.
>>>>> The next two patches introduce the sysctl configuration for tcp and 
>>>>> udp
>>>>> protocols.
>>>>> The last patch modifies nf flow table aging mechanisms to use the 
>>>>> configured
>>>>> time intervals.
>>>>>
>>>>> Oz Shlomo (5):
>>>>>    Revert "UBUNTU: SAUCE: net/sched: Add module parameter to set CT 
>>>>> age
>>>>>      out time"
>>>>>    Revert "UBUNTU: SAUCE: netfilter: flowtable: Control flow timeout
>>>>>      interval"
>>>>>    (upstream) netfilter: conntrack: Introduce tcp offload timeout
>>>>>      configuration
>>>>>    (upstream) netfilter: conntrack: Introduce udp offload timeout
>>>>>      configuration
>>>>>    (upstream) netfilter: flowtable: Set offload timeouts according to
>>>>>      proto values
>>>>>
>>>>>   include/net/netfilter/nf_flow_table.h   | 10 ++-----
>>>>>   include/net/netns/conntrack.h           |  8 +++++
>>>>>   net/netfilter/nf_conntrack_proto_tcp.c  |  5 ++++
>>>>>   net/netfilter/nf_conntrack_proto_udp.c  |  5 ++++
>>>>>   net/netfilter/nf_conntrack_standalone.c | 46 
>>>>> ++++++++++++++++++++++++++++
>>>>>   net/netfilter/nf_flow_table_core.c      | 53 
>>>>> +++++++++++++++++++++++----------
>>>>>   net/netfilter/nf_flow_table_offload.c   |  5 ++--
>>>>>   net/sched/act_ct.c                      |  5 ----
>>>>>   8 files changed, 106 insertions(+), 31 deletions(-)
>>>>>
>>>> The 3 new patches are upstream picks, in that case the commit 
>>>> message should be exactly like upstream (no "(upstream)" 
>>>> annotation). This can be fixed while applying, though.
>>>>
>>>> Acked-by: Stefan Bader <stefan.bader at canonical.com>
>>>>
>>> Those 3 patches are only merged to maintainers tree. According to 
>>> Kernel/Dev/StablePatchFormat - Ubuntu Wiki 
>>> <https://wiki.ubuntu.com/Kernel/Dev/StablePatchFormat>, it's needed. 
>>> Do we need to follow it?
>>>
>>> (upstream)
>>
>> If those are not upstream, then the cherry pick line is wrong as that, 
>> without pointing to the repo it was taken from, is meaningless.
>>
>> -Stefan
>>
> Sent V2 with the cherry-pick repo in place. Please NACK this version.
> 
> 
> 

-- 
-----------
Tim Gardner
Canonical, Inc