On Tue, Apr 19, 2016 at 1:28 PM, Martin KaFai Lau <ka...@fb.com> wrote: > On Tue, Apr 19, 2016 at 01:32:14AM -0400, Soheil Hassas Yeganeh wrote: >> > + TCP_SKB_CB(skb)->txstamp_ack = >> > + !!(shinfo->tx_flags & SKBTX_ACK_TSTAMP); >> >> Maybe we can skip a conditional jump here (because of !!), by simply >> using the cached bit in next_skb: >> TCP_SKB_CB(skb)->txstamp_ack = TCP_SKB_CB(next_skb)->txstamp_ack; > Recall the tx_flags are merged/combined (and so should be the txstamp_ack).
Oh sure, sorry, I missed an "or": TCP_SKB_CB(skb)->txstamp_ack |= TCP_SKB_CB(next_skb)->txstamp_ack; > Would there be a case that TCP_SKB_CB(skb)->txstamp_ack is 1 and > TCP_SKB_CB(next_skb)->txstamp_ack is 0? > > I can change it like the following which may help in showing the intention: > if (TCP_SKB_CB(next_skb)->txstamp_ack) > TCP_SKB_CB(skb)->txstamp_ack = 1; > > A bit off topic, I feel like the SKBTX_ACK_TSTAMP and txstamp_ack are sort > of redundant but I have not look into the details yet, so not completely > sure. It wwould be a separate cleanup patch if it is the case. As Eric mentioned, this is needed to avoid a cache-line miss in accessing the shared info.
