On Sun, Aug 21, 2005 at 11:29:11PM -0700, David S. Miller wrote:
>
> Ok, this is a full redo of the "do not disable TSO on loss" patch
> including all of the refinements and fixes we've discussed so
> far.
Thanks Dave.
> +extern int tcp_fragment(struct sock *, struct sk_buff *, u32, unsigned int);
If we're going to leave this patch in mm for a while then I'd like
to try tso_fragment() instead of tcp_fragment() since that should
always work for TSO-packets. Actually even if it's not a TSO-packet
it'll be OK since tso_fragment() now calls tcp_fragment() for packets
with a head.
> + 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.
In fact, I think we should use this rule whenever we're recalculating
tso_segs for an skb:
* If it hasn't been transmitted at all or is about to be retransmitted
then we use the current MSS.
* If it's on the retransmit queue and it isn't about to be retransmitted
then we use tso_size.
In particular when we do tcp_fragment() on the retransmit path, I'd
like to see the first part calculated using the current MSS while the
second part is still calculated with the original MSS.
The reason for this is that when the current MSS changes it can't
affect the number of packets already on the network. So it makes
no sense to use the current MSS to recalculate tso_segs unless we're
about to retransmit the packet.
It will also make the counters behave as close as possible to the non-TSO
scenario.
> - if (TCP_SKB_CB(skb)->sacked & TCPCB_LOST) {
> - tp->lost_out += tcp_skb_pcount(skb);
> - tp->left_out += tcp_skb_pcount(skb);
> - }
This seems to be unintended. Ditto for the other TCPCB_LOST chunk below.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
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
