On Tue, Apr 19, 2016 at 2:24 PM, Soheil Hassas Yeganeh <soh...@google.com> wrote: > On Tue, Apr 19, 2016 at 2:18 PM, Martin KaFai Lau <ka...@fb.com> wrote: >> On Tue, Apr 19, 2016 at 10:35:52AM -0700, Eric Dumazet wrote: >>> On Tue, Apr 19, 2016 at 10:28 AM, Martin KaFai Lau <ka...@fb.com> wrote: >>> >>> > 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.
Yes, with the introduction of txstamp_ack, SKBTX_ACK_TSTAMP is completely redundant. >>> >>> Please read 6b084928baac562ed61866f540a96120e9c9ddb7 changelog ;) >>> >>> A cache line miss avoidance is critical >> I looked at the patch but I probably am missing something :( >> Is checking txstamp_ack alone enough and SKBTX_ACK_TSTAMP is not needed >> since they are always set together? > > That's right, the check on "(shinfo->tx_flags & SKBTX_ACK_TSTAMP)" in > tcp_ack_tstamp() is redundant and I had a patch prepared to remove it. You can even remove the flag completely and - tcb->txstamp_ack = !!(shinfo->tx_flags & SKBTX_ACK_TSTAMP); + if (tsflags & SOF_TIMESTAMPING_TX_ACK) + tcb->txstamp_ack = 1; > But I thought it's better to wait for > https://patchwork.ozlabs.org/patch/611938/ to be merged first. > > Feel free to remove it in your patches, if you'd prefer that.
