All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin KaFai Lau <martin.lau@linux.dev>
To: Abhishek Chauhan <quic_abchauha@quicinc.com>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: kernel@quicinc.com, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andrew Halaney <ahalaney@redhat.com>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	bpf <bpf@vger.kernel.org>, Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>
Subject: Re: [PATCH net-next v4] net: Re-use and set mono_delivery_time bit for userspace tstamp packets
Date: Tue, 12 Mar 2024 16:52:58 -0700	[thread overview]
Message-ID: <2a4cb416-5d95-459d-8c1c-3fb225240363@linux.dev> (raw)
In-Reply-To: <20240301201348.2815102-1-quic_abchauha@quicinc.com>

On 3/1/24 12:13 PM, Abhishek Chauhan wrote:
> Bridge driver today has no support to forward the userspace timestamp
> packets and ends up resetting the timestamp. ETF qdisc checks the
> packet coming from userspace and encounters to be 0 thereby dropping
> time sensitive packets. These changes will allow userspace timestamps
> packets to be forwarded from the bridge to NIC drivers.
> 
> Setting the same bit (mono_delivery_time) to avoid dropping of
> userspace tstamp packets in the forwarding path.
> 
> Existing functionality of mono_delivery_time remains unaltered here,
> instead just extended with userspace tstamp support for bridge
> forwarding path.

The patch currently broke the bpf selftest test_tc_dtime: 
https://github.com/kernel-patches/bpf/actions/runs/8242487344/job/22541746675

In particular, there is a uapi field __sk_buff->tstamp_type which currently has 
BPF_SKB_TSTAMP_DELIVERY_MONO to mean skb->tstamp has the MONO "delivery" time. 
BPF_SKB_TSTAMP_UNSPEC means everything else (this could be a rx timestamp at 
ingress or a delivery time set by user space).

__sk_buff->tstamp_type depends on skb->mono_delivery_time which does not 
necessarily mean mono after this patch. I thought about fixing it on the bpf 
side such that reading __sk_buff->tstamp_type only returns 
BPF_SKB_TSTAMP_DELIVERY_MONO when the skb->mono_delivery_time is set and skb->sk 
is IPPROTO_TCP. However, it won't work because of bpf_skb_set_tstamp().

There is a bpf helper, bpf_skb_set_tstamp(skb, tstamp, 
BPF_SKB_TSTAMP_DELIVERY_MONO). This helper changes both the skb->tstamp and the 
skb->mono_delivery_time. The expectation is this could change skb->tstamp in the 
ingress skb and redirect to egress sch_fq. It could also set a mono time to 
skb->tstamp where the udp sk->sk_clockid may not be necessary in mono and then 
bpf_redirect to egress sch_fq. When bpf_skb_set_tstamp(skb, tstamp, 
BPF_SKB_TSTAMP_DELIVERY_MONO) succeeds, reading __sk_buff->tstamp_type expects 
BPF_SKB_TSTAMP_DELIVERY_MONO also.

I ran out of idea to solve this uapi breakage.

I am afraid it may need to go back to v1 idea and use another bit 
(user_delivery_time) in the skb.


  parent reply	other threads:[~2024-03-12 23:53 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-01 20:13 [PATCH net-next v4] net: Re-use and set mono_delivery_time bit for userspace tstamp packets Abhishek Chauhan
2024-03-01 22:21 ` Willem de Bruijn
2024-03-05 13:00 ` patchwork-bot+netdevbpf
2024-03-12 23:52 ` Martin KaFai Lau [this message]
2024-03-13  4:34   ` Abhishek Chauhan (ABC)
2024-03-13  5:32     ` Abhishek Chauhan (ABC)
2024-03-13  8:52   ` Willem de Bruijn
2024-03-13 18:42     ` Martin KaFai Lau
2024-03-13 19:36       ` Willem de Bruijn
2024-03-13 20:59         ` Abhishek Chauhan (ABC)
2024-03-13 21:19           ` Martin KaFai Lau
2024-03-13 21:41             ` Daniel Borkmann
2024-03-13 21:01         ` Martin KaFai Lau
2024-03-13 21:26           ` Abhishek Chauhan (ABC)
2024-03-13 21:40             ` Willem de Bruijn
2024-03-13 22:08               ` Martin KaFai Lau
2024-03-14  9:49                 ` Willem de Bruijn
2024-03-14 19:21                   ` Martin KaFai Lau
2024-03-14 20:28                     ` Willem de Bruijn
2024-03-14 20:53                       ` Abhishek Chauhan (ABC)
2024-03-14 21:48                         ` Martin KaFai Lau
2024-03-14 21:54                           ` Martin KaFai Lau
2024-03-14 22:29                           ` Abhishek Chauhan (ABC)
2024-03-18 19:02                             ` Abhishek Chauhan (ABC)
2024-03-19 19:46                               ` Martin KaFai Lau
2024-03-19 20:12                                 ` Abhishek Chauhan (ABC)
2024-03-20  6:22                               ` Abhishek Chauhan (ABC)
2024-03-20 20:30                                 ` Martin KaFai Lau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2a4cb416-5d95-459d-8c1c-3fb225240363@linux.dev \
    --to=martin.lau@linux.dev \
    --cc=ahalaney@redhat.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kernel@quicinc.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=quic_abchauha@quicinc.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.