Re: [PATCH net v3] sctp: carry peer capabilities across an INIT collision

[Warren Briggs]

> > The testing notes describe ASCONF and RE-CONFIG appearing on the wire, which
> > exercises the outbound counters (addip_serial and strreset_outseq, both set
> > locally from asoc->c.initial_tsn).  Was the inbound direction tested too?
> >
> > Should the merge also carry strreset_inseq and peer.addip_serial, or
> > re-derive them from new->peer.i.initial_tsn the way sctp_process_init() does?
>
> Makes sense, please follow the suggestion, and add something like:
>
> +       /* Re-derive sequence counters from the peer's Initial TSN */
> +       asoc->peer.addip_serial = asoc->peer.i.initial_tsn - 1;
> +       asoc->strreset_inseq = asoc->peer.i.initial_tsn;

Will do, placed after the peer.i copy so they use the new Initial TSN.

The inbound direction was not tested on v3. I will test inbound
ASCONF and RE-CONFIG on v4 before sending it.

> >
> > > +     asoc->peer.intl_capable = new->peer.intl_capable;
> >
> > Would calling sctp_stream_interleave_init() and sctp_assoc_update_frag_point()
> > on the surviving association after the copy address both directions?
>
> Outqueue may already have data at the time, we can't just call
> sctp_stream_interleave_init(). Let's drop the intl_capable update from
> this patch and address it separately in another patch.
>
> You may add a note for this in the commit message, so that the AI review
> won't report intl_capable update missing.

Agreed. intl_capable is dropped from v4, and the commit message says
why and that it will follow as a separate patch.

> Please address this one as well by adding:
>
> +       asoc->peer.adaptation_ind = new->peer.adaptation_ind;
>
> either in this patch or a separate one.

I'll include it in v4.

v4 to follow.

Thanks,
Warren

pw-bot: cr