On Thu, Apr 21, 2016 at 12:56 PM, Martin KaFai Lau <ka...@fb.com> wrote: > On Wed, Apr 20, 2016 at 04:04:54PM -0400, Soheil Hassas Yeganeh wrote: >> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c >> > index a6e4a83..96bdf98 100644 >> > --- a/net/ipv4/tcp_output.c >> > +++ b/net/ipv4/tcp_output.c >> > @@ -2494,6 +2494,7 @@ static void tcp_collapse_retrans(struct sock *sk, >> > struct sk_buff *skb) >> > * packet counting does not break. >> > */ >> > TCP_SKB_CB(skb)->sacked |= TCP_SKB_CB(next_skb)->sacked & >> > TCPCB_EVER_RETRANS; >> > + TCP_SKB_CB(skb)->eor = TCP_SKB_CB(next_skb)->eor; >> > >> > /* changed transmit queue under us so clear hints */ >> > tcp_clear_retrans_hints_partial(tp); >> > @@ -2545,6 +2546,9 @@ static void tcp_retrans_try_collapse(struct sock >> > *sk, struct sk_buff *to, >> > if (!tcp_can_collapse(sk, skb)) >> > break; >> > >> > + if (TCP_SKB_CB(to)->eor) >> > + break; >> > + >> >> nit: Perhaps a better place to check for eor is right after entering >> the loop? to skip a few instructions and tcp_can_collapse, in an >> unlikely case eor is set. > hmm... Not sure I understand it. > You meant moving the unlikely case before (or after?) the more likely > cases which may have a better chance to break the loop sooner?
Well I don't have strong preference here. So, feel free to ignore. Though I'm not sure how "likely" are the checks in tcp_can_collapse. On another note, do you think putting this is a self-documenting helper function, say tcp_can_collapse_to(), would help readability? Thanks.
