Re: [PATCH bpf-next] net: netfilter: add kfunc helper to update ct timeout

[Kumar Kartikeya Dwivedi <memxor@gmail.com>]

On Tue, May 03, 2022 at 09:59:14PM IST, Lorenzo Bianconi wrote:
> Introduce bpf_ct_refresh_timeout kfunc helper in order to update time
> nf_conn lifetime. Move timeout update logic in nf_ct_refresh_timeout
> utility routine.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---

Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

The sparse error can be ignored, kfunc is meant to be global without a
prototype.

>  include/net/netfilter/nf_conntrack.h |  1 +
>  net/netfilter/nf_conntrack_bpf.c     | 20 ++++++++++++++++++++
>  net/netfilter/nf_conntrack_core.c    | 21 +++++++++++++--------
>  3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
> index 69e6c6a218be..02b7115b92d0 100644
> --- a/include/net/netfilter/nf_conntrack.h
> +++ b/include/net/netfilter/nf_conntrack.h
> @@ -205,6 +205,7 @@ bool nf_ct_get_tuplepr(const struct sk_buff *skb, unsigned int nhoff,
>  		       u_int16_t l3num, struct net *net,
>  		       struct nf_conntrack_tuple *tuple);
>
> +void nf_ct_refresh_timeout(struct nf_conn *ct, u32 extra_jiffies);
>  void __nf_ct_refresh_acct(struct nf_conn *ct, enum ip_conntrack_info ctinfo,
>  			  const struct sk_buff *skb,
>  			  u32 extra_jiffies, bool do_acct);
> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
> index bc4d5cd63a94..d6dcadf0e016 100644
> --- a/net/netfilter/nf_conntrack_bpf.c
> +++ b/net/netfilter/nf_conntrack_bpf.c
> @@ -217,16 +217,36 @@ void bpf_ct_release(struct nf_conn *nfct)
>  	nf_ct_put(nfct);
>  }
>
> +/* bpf_ct_refresh_timeout - Refresh nf_conn object
> + *
> + * Refresh timeout associated to the provided connection tracking entry.
> + * This must be invoked for referenced PTR_TO_BTF_ID.
> + *
> + * Parameters:
> + * @nf_conn      - Pointer to referenced nf_conn object, obtained using
> + *		   bpf_xdp_ct_lookup or bpf_skb_ct_lookup.
> + * @timeout      - delta time in msecs used to increase the ct entry lifetime.
> + */
> +void bpf_ct_refresh_timeout(struct nf_conn *nfct, u32 timeout)
> +{
> +	if (!nfct)
> +		return;
> +
> +	nf_ct_refresh_timeout(nfct, msecs_to_jiffies(timeout));
> +}
> +
>  __diag_pop()
>
>  BTF_SET_START(nf_ct_xdp_check_kfunc_ids)
>  BTF_ID(func, bpf_xdp_ct_lookup)
>  BTF_ID(func, bpf_ct_release)
> +BTF_ID(func, bpf_ct_refresh_timeout);
>  BTF_SET_END(nf_ct_xdp_check_kfunc_ids)
>
>  BTF_SET_START(nf_ct_tc_check_kfunc_ids)
>  BTF_ID(func, bpf_skb_ct_lookup)
>  BTF_ID(func, bpf_ct_release)
> +BTF_ID(func, bpf_ct_refresh_timeout);
>  BTF_SET_END(nf_ct_tc_check_kfunc_ids)
>
>  BTF_SET_START(nf_ct_acquire_kfunc_ids)
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 0164e5f522e8..f43e743728bd 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -2030,16 +2030,11 @@ void nf_conntrack_alter_reply(struct nf_conn *ct,
>  }
>  EXPORT_SYMBOL_GPL(nf_conntrack_alter_reply);
>
> -/* Refresh conntrack for this many jiffies and do accounting if do_acct is 1 */
> -void __nf_ct_refresh_acct(struct nf_conn *ct,
> -			  enum ip_conntrack_info ctinfo,
> -			  const struct sk_buff *skb,
> -			  u32 extra_jiffies,
> -			  bool do_acct)
> +void nf_ct_refresh_timeout(struct nf_conn *ct, u32 extra_jiffies)
>  {
>  	/* Only update if this is not a fixed timeout */
>  	if (test_bit(IPS_FIXED_TIMEOUT_BIT, &ct->status))
> -		goto acct;
> +		return;
>
>  	/* If not in hash table, timer will not be active yet */
>  	if (nf_ct_is_confirmed(ct))
> @@ -2047,7 +2042,17 @@ void __nf_ct_refresh_acct(struct nf_conn *ct,
>
>  	if (READ_ONCE(ct->timeout) != extra_jiffies)
>  		WRITE_ONCE(ct->timeout, extra_jiffies);
> -acct:
> +}
> +
> +/* Refresh conntrack for this many jiffies and do accounting if do_acct is 1 */
> +void __nf_ct_refresh_acct(struct nf_conn *ct,
> +			  enum ip_conntrack_info ctinfo,
> +			  const struct sk_buff *skb,
> +			  u32 extra_jiffies,
> +			  bool do_acct)
> +{
> +	nf_ct_refresh_timeout(ct, extra_jiffies);
> +
>  	if (do_acct)
>  		nf_ct_acct_update(ct, CTINFO2DIR(ctinfo), skb->len);
>  }
> --
> 2.35.1
>

--
Kartikeya