On Mon, Sep 19, 2016 at 01:02:55PM -0300, 'Marcelo Ricardo Leitner' wrote:
> On Mon, Sep 19, 2016 at 10:41:34AM +0000, David Laight wrote:
> > From: 'Marcelo Ricardo Leitner'
> > > Sent: 16 September 2016 18:22
> > ...
> > > frags = sack->variable;
> > > - gap = tsn - ctsn;
> > > - for (i = 0; i < ntohs(sack->num_gap_ack_blocks); ++i) {
> > > - if (TSN_lte(ntohs(frags[i].gab.start), gap) &&
> > > - TSN_lte(gap, ntohs(frags[i].gab.end)))
> > > + blocks = ntohs(sack->num_gap_ack_blocks);
> > > + for (i = 0; i < blocks; ++i) {
> > > + if (TSN_lte(ctsn + ntohs(frags[i].gab.start), tsn) &&
> > > + TSN_lte(tsn, ctsn + ntohs(frags[i].gab.end)))
> > > goto pass;
> >
> > I think I now understand what all the values are!
> > The correct test is just:
> > if (gap > ntohs(frags[i].gab.start) &&
> > gap <= ntohs(frags[i].gab.end))
> > (I think the ends are right).
> > gab.start/end are 16bit unsigned offsets from ctsn.
> > All the module arithmetic is sorted out when 'tsn' is converted to an
> > offset from ctsn. 'gap' is a very bad name for the tsn_offset.
>
> Yep! I like this approach, thanks.
> And in case tsn is actually smaller than ctsn in this code, than that's
> another bug actually, so I think your suggestion covers all the cases.
Confirming, this situation is verified earlier in the same function, all
good.
>
> Though the ends are both inclusive.
> Handy https://tools.ietf.org/html/rfc4960#section-3.3.4 if you want.
>
> >
> > Looks like gap.start/end should be saved in host order though.
>
> I lost you here. gab.start/end you mean? Saved?
>
> Marcelo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
