On Fri, Apr 28, 2017 at 04:07:29PM -0400, Willem de Bruijn wrote: > 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: > >> 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.
Ok. I think it won't really matter. We should keep in mind that the reason for adding the OPT_TX_SWHW option was to not break old applications which enabled both SW and HW TX timestamping, even though they could get only one timestamp. I think most applications in future will either enable only SW or HW TX timestamping, or enable both together with the OPT_TX_SWHW option in order to get a SW timestamp when HW timestamp was requested but missing. > >> 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. There are other callers of skb_tx_timestamp() and it's not obvious to me they are all safe (i.e. cannot pass skb will sk==NULL), so I think this should rather be a separate patch if necessary. I'll resend the series with the other changes you have suggested. -- Miroslav Lichvar
