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? > 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. -- Miroslav Lichvar
