From: Herbert Xu <[EMAIL PROTECTED]>
Date: Mon, 22 Aug 2005 22:54:50 +1000
> > + if (pcount > 1 &&
> > + (after(start_seq, TCP_SKB_CB(skb)->seq) ||
> > + before(end_seq, TCP_SKB_CB(skb)->end_seq))) {
> > + unsigned int pkt_len;
> > +
> > + if (after(start_seq, TCP_SKB_CB(skb)->seq))
> > + pkt_len = (start_seq -
> > + TCP_SKB_CB(skb)->seq);
> > + else
> > + pkt_len = (end_seq -
> > + TCP_SKB_CB(skb)->seq);
> > + if (tcp_fragment(sk, skb, pkt_len, pkt_len))
> > + break;
>
> Let's say that skb contains 3*MSS and the SACK is for the last MSS.
> This code will set pkt_len to 2*MSS which I think makes no sense.
>
> I'd suggest that you use TCP_SKB_CB(skb)->tso_size instead.
It makes perfect sense. In your example, we want a 2*MSS TSO
frame for the "not-SACKed" area, and the tail 1*MSS frame for
the SACKed area. This provides exactly the necessary granularity
in order to record the received SACK information. That's the
whole point of splitting up the frame here in response to
SACK blocks.
Splitting to tso_size is extremely undesirable. If we have
a SACK block that spans, say, 12 packets, we only need to chop
down to a 12 packet TSO frame to record the SACK information
accurately.
And SACK blocks typically grow, not shrink.
Another issue is that we want to do this splitting as minimally
as possible. But we end up doing enough such that we have
a perfectly ready chopped up SKB for the retransmit by the time
we actually start resending frames to fill up SACK holes. Or,
at least, we'll have less splitting work to actually do by that
time.
Does any of this make sense? :-)
> This seems to be unintended. Ditto for the other TCPCB_LOST chunk below.
Those statements are all equivalent to the "tp->{lost,left}_out -= diff;"
code I added. We were subtracting out the old number, then adding in
the new one, which is the same as just subtracting the difference in
one go.
At least, this simplification looked really nice to me :-)
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
