netdev - Re: [PATCH net-next v3 1/2] net-timestamp: filter out report when setting SOF_TIMESTAMPING

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <66d78a1e5e6ad_cefcf294f1@willemb.c.googlers.com.notmuch>
Date: Tue, 03 Sep 2024 18:13:50 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, 
 willemb@...gle.com
Cc: Jason Xing <kerneljasonxing@...il.com>, 
 davem@...emloft.net, 
 edumazet@...gle.com, 
 pabeni@...hat.com, 
 dsahern@...nel.org, 
 netdev@...r.kernel.org, 
 Jason Xing <kernelxing@...cent.com>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH net-next v3 1/2] net-timestamp: filter out report when
 setting SOF_TIMESTAMPING_SOFTWARE

Jakub Kicinski wrote:
> On Fri, 30 Aug 2024 23:37:50 +0800 Jason Xing wrote:
> > +	if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
> > +	    val & SOF_TIMESTAMPING_OPT_RX_SOFTWARE_FILTER)
> > +		return -EINVAL;
> 
> 
> > -		if (READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_SOFTWARE)
> > +		if (tsflags & SOF_TIMESTAMPING_SOFTWARE &&
> > +		    (tsflags & SOF_TIMESTAMPING_RX_SOFTWARE ||
> > +		     !(tsflags & SOF_TIMESTAMPING_OPT_RX_SOFTWARE_FILTER)))
> >  			has_timestamping = true;
> >  		else
> >  			tss->ts[0] = (struct timespec64) {0};
> >  	}
> 
> >  	memset(&tss, 0, sizeof(tss));
> >  	tsflags = READ_ONCE(sk->sk_tsflags);
> > -	if ((tsflags & SOF_TIMESTAMPING_SOFTWARE) &&
> > +	if ((tsflags & SOF_TIMESTAMPING_SOFTWARE &&
> > +	     (tsflags & SOF_TIMESTAMPING_RX_SOFTWARE ||
> > +	     skb_is_err_queue(skb) ||
> > +	     !(tsflags & SOF_TIMESTAMPING_OPT_RX_SOFTWARE_FILTER))) &&
> 
> Willem, do you prefer to keep the:
> 
> 	tsflags & SOF_TIMESTAMPING_RX_SOFTWARE ||
> 	!(tsflags & SOF_TIMESTAMPING_OPT_RX_SOFTWARE_FILTER)
> 
> conditions?IIUC we prevent both from being set at once. So 
> 
> 	!(tsflags & SOF_TIMESTAMPING_OPT_RX_SOFTWARE_FILTER)
> 
> is sufficient (and, subjectively, more intuitive).

Good point. Yes, let's definitely simplify.

> Question #2 -- why are we only doing this for SW stamps?
> HW stamps for TCP are also all or nothing.

Fair. Else we'll inevitably add a
SOF_TIMESTAMPING_OPT_RX_HARDWARE_FILTER at some point.

There probably is no real use to filter one, but not the other.

So SOF_TIMESTAMPING_OPT_RX_FILTER then, and also apply
to the branch below:

        if (shhwtstamps &&
            (tsflags & SOF_TIMESTAMPING_RAW_HARDWARE) &&
            !skb_is_swtx_tstamp(skb, false_tstamp)) {

and same for tcp_recv_timestamp.