> -----Original Message-----
> From: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> Sent: Wednesday, 18 January 2023 16:29
> To: Sriram Yagnaraman <sriram.yagnaraman@xxxxxxxx>
> Cc: netfilter-devel@xxxxxxxxxxxxxxx; Florian Westphal <fw@xxxxxxxxx>;
> Marcelo Ricardo Leitner <mleitner@xxxxxxxxxx>; Long Xin
> <lxin@xxxxxxxxxx>; Claudio Porfiri <claudio.porfiri@xxxxxxxxxxxx>
> Subject: Re: [PATCH v3 4/4] netfilter: conntrack: unify established states for
> SCTP paths
>
> On Wed, Jan 18, 2023 at 12:38:53PM +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.
>
> Thanks for new patchset version. One question below.
>
> > @@ -523,8 +512,7 @@ int nf_conntrack_sctp_packet(struct nf_conn *ct,
> >
> > nf_ct_refresh_acct(ct, ctinfo, skb, timeouts[new_state]);
> >
> > - if (old_state == SCTP_CONNTRACK_COOKIE_ECHOED &&
> > - dir == IP_CT_DIR_REPLY &&
> > + if (dir == IP_CT_DIR_REPLY &&
> > new_state == SCTP_CONNTRACK_ESTABLISHED) {
> > pr_debug("Setting assured bit\n");
> > set_bit(IPS_ASSURED_BIT, &ct->status);
>
> Why old_state == SCTP_CONNTRACK_COOKIE_ECHOED was removed to set
> on the assured bit?
>
There is more than one state from which we can transition to ESTABLISHED now, COOKIE_ECHOED and HEARTBEAT_SENT. I will add a "old_state != new_state" check instead, so we don't set ASSURED every time there is a packet in the REPLY direction. I will wait for other review comments, before pushing another patchset version.
> Thanks.
