On Thu, Jan 19, 2023 at 08:27:27AM +0000, Sriram Yagnaraman wrote:
> > -----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 for explaining.
Please add this information to the commit description in the next
version.
