On 12/12/24 3:25 PM, Martin KaFai Lau wrote:
A more subtle thing for the hwtstamps case is, afaik the bpf prog will not be
called. All drivers are still only testing SKBTX_HW_TSTAMP instead of testing
(SKBTX_HW_TSTAMP | SKBTX_BPF).
There are a lot of drivers to change though. A quick thought is to rename the
existing SKBTX_HW_TSTAMP (e.g. __SKBTX_HW_TSTAMP = 1 << 0) and define
SKBTX_HW_TSTAMP like:
#define SKBTX_HW_TSTAMP (__SKBTX_HW_TSTAMP | SKBTX_BPF)
Then change some of the existing skb_shinfo(skb)->tx_flags "setting" site to use
__SKBTX_HW_TSTAMP instead. e.g. in __sock_tx_timestamp(). Not very pretty but
may be still better than changing many drivers. May be there is a better way...
While talking about where to test the SKBTX_BPF bit, I wonder if the new
skb_tstamp_is_set() is needed. For the non SKBTX_HW_TSTAMP case, the number of
tx_flags testing sites should be limited, so should be easy to add the SKBTX_BPF
bit test. e.g. at the __dev_queue_xmit, test "if (unlikely(skb_shinfo(skb)-
>tx_flags & (SKBTX_SCHED_TSTAMP | SKBTX_BPF)))". Patch 6 has also tested the
bpf specific bit at tcp_ack_tstamp() before calling the __skb_tstamp_tx().
At the beginning of __skb_tstamp_tx(), do something like this:
void __skb_tstamp_tx(struct sk_buff *orig_skb,
const struct sk_buff *ack_skb,
struct skb_shared_hwtstamps *hwtstamps,
struct sock *sk, int tstype)
{
if (cgroup_bpf_enabled(CGROUP_SOCK_OPS) &&
unlikely(skb_shinfo(skb)->tx_flags & SKBTX_BPF))
__skb_tstamp_tx_bpf(sk, orig_skb, hwtstamps, tstype);
if (unlikely(!(skb_shinfo(skb)->tx_flags & ~SKBTX_BPF)))
This is not enough. I was wrong here. The test in skb_tstamp_is_set() is needed
when SKBTX_BPF is not set.
return;
