Re: [RFC PATCH net-next 2/2] net: Reset forwarded skb->tstamp before delivering to user space

On Thu, Dec 09, 2021 at 01:58:52PM +0100, Daniel Borkmann wrote:

quoted

Daniel, do you have suggestion on where to temporarily store
the forwarded EDT so that the bpf@ingress can access?

Hm, was thinking maybe moving skb->skb_mstamp_ns into the shared info as
in skb_hwtstamps(skb)->skb_mstamp_ns could work. In other words, as a union
with hwtstamp to not bloat it further. And TCP stack as well as everything
else (like sch_fq) could switch to it natively (hwtstamp might only be used
on RX or TX completion from driver side if I'm not mistaken).

But then while this would solve the netns transfer, we would run into the
/same/ issue again when implementing a hairpinning LB where we loop from RX
to TX given this would have to be cleared somewhere again if driver populates
hwtstamp, so not really feasible and bloating shared info with a second
tstamp would bump it by one cacheline. :(

If the edt is set at skb_hwtstamps,
skb->tstamp probably needs to be re-populated for the bpf@tc-egress
but should be minor since there is a skb_at_tc_ingress() test.

It seems fq does not need shinfo now, so that will be an extra cacheline to
bring... hmm

A cleaner BUT still non-generic solution compared to the previous diff I could
think of might be the below. So no change in behavior in general, but if the
bpf@ingress@veth@host needs to access the original tstamp, it could do so
via existing mapping we already have in BPF, and then it could transfer it
for all or certain traffic (up to the prog) via BPF code setting ...

  skb->tstamp = skb->hwtstamp

... and do the redirect from there to the phys dev with BPF_F_KEEP_TSTAMP
flag. Minimal intrusive, but unfortunately only accessible for BPF. Maybe use
of skb_hwtstamps(skb)->nststamp could be extended though (?)

I like the idea of the possibility in temporarily storing a future mono EDT
in skb_shared_hwtstamps.

It may open up some possibilities.  Not sure how that may look like yet
but I will try to develop on this.

I may have to separate the fwd-edt problem from __sk_buff->tstamp accessibility
@ingress to keep it simple first.
will try to make it generic also before scaling back to a bpf-specific solution.

Thanks for the code and the idea !