On Fri, Apr 28, 2017 at 12:23 PM, Miroslav Lichvar <mlich...@redhat.com> wrote: > On Fri, Apr 28, 2017 at 11:50:28AM -0400, Willem de Bruijn wrote: >> On Fri, Apr 28, 2017 at 4:54 AM, Miroslav Lichvar <mlich...@redhat.com> >> wrote: >> >> if (skb_shinfo(skb)->tx_flags & SKBTX_SW_TSTAMP && >> >> - !(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) >> >> + (!(skb_shinfo(orig_skb)->tx_flags & SKBTX_IN_PROGRESS)) || >> >> + (skb->sk && skb->sk->sk_tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) >> > >> > I'm not sure if this can work. sk_buff.h would need to include sock.h >> > in order to get the definition of struct sock. Any suggestions? >> >> A more elegant solution would be to not set SKBTX_IN_PROGRESS >> at all if SOF_TIMESTAMPING_OPT_TX_SWHW is set on the socket. >> But the patch to do so is not elegant, having to update callsites in many >> device drivers. > > Also, it would change the meaning of the flag as it seems some drivers > actually use the SKBTX_IN_PROGRESS flag to check if they expect a > timestamp. > > How about allocating the last bit of tx_flags for SKBTX_SWHW_TSTAMP?
That is such a scarce resource that I really would prefer to avoid using that if we can. >> Otherwise you may indeed have to call skb_tstamp_tx for every packet >> that has SKBTX_SW_TSTAMP set, as you do. We can at least move >> the skb->sk != NULL check into skb_tx_timestamp in skbuff.h. >> >> By the way, if changing this code, I think that it's time to get rid of >> sw_tx_timestamp. It is only called from skb_tx_timestamp. Let's >> just move the condition in there. > > Ok. I assume that should be a separate patch. It's a single statement, I think we can do it as part of this one. Thanks.
