Re: [PATCH 3/3] netfilter: conntrack: unify established states for SCTP paths

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, Jan 16, 2023 at 10:35:56AM +0100, Sriram Yagnaraman wrote:
> An SCTP endpoint can start an association through a path and tear it
> down over another one. That means the initial path will not see the
> shutdown sequence, and the conntrack entry will remain in ESTABLISHED
> state for 5 days.
> 
> By merging the HEARTBEAT_ACKED and ESTABLISHED states into one
> ESTABLISHED state, there remains no difference between a primary or
> secondary path. The timeout for the merged ESTABLISHED state is set to
> 210 seconds (hb_interval * max_path_retrans + rto_max). So, even if a
> path doesn't see the shutdown sequence, it will expire in a reasonable
> amount of time.
> 
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
>  .../uapi/linux/netfilter/nf_conntrack_sctp.h  |  4 +-
>  .../linux/netfilter/nfnetlink_cttimeout.h     |  4 +-
>  net/netfilter/nf_conntrack_proto_sctp.c       | 90 ++++++++-----------
>  net/netfilter/nf_conntrack_standalone.c       | 16 ----
>  4 files changed, 42 insertions(+), 72 deletions(-)
> 
> diff --git a/include/uapi/linux/netfilter/nf_conntrack_sctp.h b/include/uapi/linux/netfilter/nf_conntrack_sctp.h
> index c742469afe21..150fc3c056ea 100644
> --- a/include/uapi/linux/netfilter/nf_conntrack_sctp.h
> +++ b/include/uapi/linux/netfilter/nf_conntrack_sctp.h
> @@ -15,8 +15,8 @@ enum sctp_conntrack {
>  	SCTP_CONNTRACK_SHUTDOWN_RECD,
>  	SCTP_CONNTRACK_SHUTDOWN_ACK_SENT,
>  	SCTP_CONNTRACK_HEARTBEAT_SENT,
> -	SCTP_CONNTRACK_HEARTBEAT_ACKED,
> -	SCTP_CONNTRACK_DATA_SENT,
> +	SCTP_CONNTRACK_HEARTBEAT_ACKED,	/* no longer used */
> +	SCTP_CONNTRACK_DATA_SENT,	/* no longer used */

_DATA_SENT was added in the previous development cycle, to my
knowledged it has been present in 6.1-rc only. Then I think you can
post a patch to revert this explaining why there is no need for
_DATA_SENT anymore. You can revert it before this patch (with my
suggestion, your series will contain with 4 patches).

One question of mine: Did you extract the new established timeout from
RFC, where this formula came from?

210 seconds = hb_interval * max_path_retrans + rto_max

And thanks, if this works for you, I prefer this incremental approach
by improving the existing SCTP tracker.