Received-SPF: pass (google.com: domain of linux-kernel+bounces-192805-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 28 May 2024 13:24:35 -0400
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: "Abhishek Chauhan (ABC)" <quic_abchauha@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>, 
 Willem de Bruijn <willemdebruijn.kernel@gmail.com>, 
 Martin KaFai Lau <martin.lau@kernel.org>, 
 Martin KaFai Lau <martin.lau@linux.dev>, 
 Daniel Borkmann <daniel@iogearbox.net>, 
 bpf <bpf@vger.kernel.org>
Cc: kernel@quicinc.com, 
 Willem de Bruijn <willemb@google.com>
Message-ID: <665613536e82e_2a1fb929437@willemb.c.googlers.com.notmuch>
In-Reply-To: <6bdba7b6-fd22-4ea5-a356-12268674def1@quicinc.com>
References: <20240509211834.3235191-1-quic_abchauha@quicinc.com>
 <20240509211834.3235191-2-quic_abchauha@quicinc.com>
 <6bdba7b6-fd22-4ea5-a356-12268674def1@quicinc.com>
Subject: Re: [PATCH bpf-next v8 1/3] net: Rename mono_delivery_time to
 tstamp_type for scalabilty
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: 7bit

Abhishek Chauhan (ABC) wrote:

> > +static inline void skb_set_delivery_type_by_clockid(struct sk_buff *skb,
> > +						    ktime_t kt, clockid_t clockid)
> > +{
> > +	u8 tstamp_type = SKB_CLOCK_REALTIME;
> > +
> > +	switch (clockid) {
> > +	case CLOCK_REALTIME:
> > +		break;
> > +	case CLOCK_MONOTONIC:
> > +		tstamp_type = SKB_CLOCK_MONOTONIC;
> > +		break;
> > +	default:
> 
> Willem and Martin, I was thinking we should remove this warn_on_once from below line. Some systems also use panic on warn. 
> So i think this might result in unnecessary crashes. 
> 
> Let me know what you think. 
> 
> Logs which are complaining. 
> https://syzkaller.appspot.com/x/log.txt?x=118c3ae8980000

I received reports too. Agreed that we need to fix these reports.

The alternative is to limit sk_clockid to supported ones, by failing
setsockopt SO_TXTIME on an unsupported clock.

That changes established ABI behavior. But I don't see how another
clock can be used in any realistic way anyway.

Putting it out there as an option. It's riskier, but in the end I
believe a better fix than just allowing this state to continue.

A third option would be to not fail the system call, but silently
fall back to CLOCK_REALTIME. Essentially what happens in the datapath
in skb_set_delivery_type_by_clockid now. That is surprising behavior,
we should not do that.