diff mbox series

[SRU,J:linux-bluefield] UBUNTU: SAUCE: netfilter: flowtable: additional checks for outdated flows

Message ID 20230803154916.1447090-1-bodong@nvidia.com
State New
Headers show
Series [SRU,J:linux-bluefield] UBUNTU: SAUCE: netfilter: flowtable: additional checks for outdated flows | expand

Commit Message

Bodong Wang Aug. 3, 2023, 3:49 p.m. UTC 
From: Vlad Buslov <vladbu@nvidia.com>

BugLink: https://bugs.launchpad.net/bugs/2029497

Current nf_flow_is_outdated() implementation considers any flow table flow
which state diverged from its underlying CT connection status for teardown
which can be problematic in the following cases:

- Flow has never been offloaded to hardware in the first place either
because flow table has hardware offload disabled (flag
NF_FLOWTABLE_HW_OFFLOAD is not set) or because it is still pending on 'add'
workqueue to be offloaded for the first time. The former is incorrect, the
later generates excessive deletions and additions of flows.

- Flow is already pending to be updated on the workqueue. Tearing down such
flows will also generate excessive removals from the flow table, especially
on highly loaded system where the latency to re-offload a flow via 'add'
workqueue can be quite high.

When considering a flow for teardown as outdated verify that it is both
offloaded to hardware and doesn't have any pending updates.

Fixes: 41f2c7c342d3 ("net/sched: act_ct: Fix promotion of offloaded unreplied tuple")
Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
Signed-off-by: Bodong Wang <bodong@nvidia.com>
---
 net/netfilter/nf_flow_table_core.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Tim Gardner Aug. 3, 2023, 6:43 p.m. UTC | #1 
On 8/3/23 9:49 AM, Bodong Wang wrote:
> From: Vlad Buslov <vladbu@nvidia.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/2029497
> 
> Current nf_flow_is_outdated() implementation considers any flow table flow
> which state diverged from its underlying CT connection status for teardown
> which can be problematic in the following cases:
> 
> - Flow has never been offloaded to hardware in the first place either
> because flow table has hardware offload disabled (flag
> NF_FLOWTABLE_HW_OFFLOAD is not set) or because it is still pending on 'add'
> workqueue to be offloaded for the first time. The former is incorrect, the
> later generates excessive deletions and additions of flows.
> 
> - Flow is already pending to be updated on the workqueue. Tearing down such
> flows will also generate excessive removals from the flow table, especially
> on highly loaded system where the latency to re-offload a flow via 'add'
> workqueue can be quite high.
> 
> When considering a flow for teardown as outdated verify that it is both
> offloaded to hardware and doesn't have any pending updates.
> 
> Fixes: 41f2c7c342d3 ("net/sched: act_ct: Fix promotion of offloaded unreplied tuple")
> Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
> Signed-off-by: Bodong Wang <bodong@nvidia.com>
> ---
>   net/netfilter/nf_flow_table_core.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index d35f3f8d9841..d8f0f924b574 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -342,6 +342,8 @@ EXPORT_SYMBOL_GPL(flow_offload_refresh);
>   static bool nf_flow_is_outdated(const struct flow_offload *flow)
>   {
>   	return test_bit(IPS_SEEN_REPLY_BIT, &flow->ct->status) &&
> +		test_bit(IPS_HW_OFFLOAD_BIT, &flow->ct->status) &&
> +		!test_bit(NF_FLOW_HW_PENDING, &flow->flags) &&
>   		!test_bit(NF_FLOW_HW_ESTABLISHED, &flow->flags);
>   }
>   
Acked-by: Tim Gardner <tim.gardner@canonical.com>
Bartlomiej Zolnierkiewicz Aug. 4, 2023, 4:18 p.m. UTC | #2 
Acked-by: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>

On Thu, Aug 3, 2023 at 5:50 PM Bodong Wang <bodong@nvidia.com> wrote:
>
> From: Vlad Buslov <vladbu@nvidia.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2029497
>
> Current nf_flow_is_outdated() implementation considers any flow table flow
> which state diverged from its underlying CT connection status for teardown
> which can be problematic in the following cases:
>
> - Flow has never been offloaded to hardware in the first place either
> because flow table has hardware offload disabled (flag
> NF_FLOWTABLE_HW_OFFLOAD is not set) or because it is still pending on 'add'
> workqueue to be offloaded for the first time. The former is incorrect, the
> later generates excessive deletions and additions of flows.
>
> - Flow is already pending to be updated on the workqueue. Tearing down such
> flows will also generate excessive removals from the flow table, especially
> on highly loaded system where the latency to re-offload a flow via 'add'
> workqueue can be quite high.
>
> When considering a flow for teardown as outdated verify that it is both
> offloaded to hardware and doesn't have any pending updates.
>
> Fixes: 41f2c7c342d3 ("net/sched: act_ct: Fix promotion of offloaded unreplied tuple")
> Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
> Signed-off-by: Bodong Wang <bodong@nvidia.com>
> ---
>  net/netfilter/nf_flow_table_core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index d35f3f8d9841..d8f0f924b574 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -342,6 +342,8 @@ EXPORT_SYMBOL_GPL(flow_offload_refresh);
>  static bool nf_flow_is_outdated(const struct flow_offload *flow)
>  {
>         return test_bit(IPS_SEEN_REPLY_BIT, &flow->ct->status) &&
> +               test_bit(IPS_HW_OFFLOAD_BIT, &flow->ct->status) &&
> +               !test_bit(NF_FLOW_HW_PENDING, &flow->flags) &&
>                 !test_bit(NF_FLOW_HW_ESTABLISHED, &flow->flags);
>  }
>
Bartlomiej Zolnierkiewicz Aug. 4, 2023, 5:05 p.m. UTC | #3 
Applied to jammy:linux-bluefield/master-next. Thanks.

--
Best regards,
Bartlomiej

On Thu, Aug 3, 2023 at 5:50 PM Bodong Wang <bodong@nvidia.com> wrote:
>
> From: Vlad Buslov <vladbu@nvidia.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2029497
>
> Current nf_flow_is_outdated() implementation considers any flow table flow
> which state diverged from its underlying CT connection status for teardown
> which can be problematic in the following cases:
>
> - Flow has never been offloaded to hardware in the first place either
> because flow table has hardware offload disabled (flag
> NF_FLOWTABLE_HW_OFFLOAD is not set) or because it is still pending on 'add'
> workqueue to be offloaded for the first time. The former is incorrect, the
> later generates excessive deletions and additions of flows.
>
> - Flow is already pending to be updated on the workqueue. Tearing down such
> flows will also generate excessive removals from the flow table, especially
> on highly loaded system where the latency to re-offload a flow via 'add'
> workqueue can be quite high.
>
> When considering a flow for teardown as outdated verify that it is both
> offloaded to hardware and doesn't have any pending updates.
>
> Fixes: 41f2c7c342d3 ("net/sched: act_ct: Fix promotion of offloaded unreplied tuple")
> Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
> Signed-off-by: Bodong Wang <bodong@nvidia.com>
> ---
>  net/netfilter/nf_flow_table_core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index d35f3f8d9841..d8f0f924b574 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -342,6 +342,8 @@ EXPORT_SYMBOL_GPL(flow_offload_refresh);
>  static bool nf_flow_is_outdated(const struct flow_offload *flow)
>  {
>         return test_bit(IPS_SEEN_REPLY_BIT, &flow->ct->status) &&
> +               test_bit(IPS_HW_OFFLOAD_BIT, &flow->ct->status) &&
> +               !test_bit(NF_FLOW_HW_PENDING, &flow->flags) &&
>                 !test_bit(NF_FLOW_HW_ESTABLISHED, &flow->flags);
>  }
>
diff mbox series

Patch

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index d35f3f8d9841..d8f0f924b574 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -342,6 +342,8 @@  EXPORT_SYMBOL_GPL(flow_offload_refresh);
 static bool nf_flow_is_outdated(const struct flow_offload *flow)
 {
 	return test_bit(IPS_SEEN_REPLY_BIT, &flow->ct->status) &&
+		test_bit(IPS_HW_OFFLOAD_BIT, &flow->ct->status) &&
+		!test_bit(NF_FLOW_HW_PENDING, &flow->flags) &&
 		!test_bit(NF_FLOW_HW_ESTABLISHED, &flow->flags);
 }