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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Apr 2022 22:28:10 +0200 From: Gerhard Engleder <gerhard@...leder-embedded.com> To: Vinicius Costa Gomes <vinicius.gomes@...el.com> Cc: Richard Cochran <richardcochran@...il.com>, yangbo.lu@....com, David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, mlichvar@...hat.com, netdev <netdev@...r.kernel.org> Subject: Re: [PATCH net-next v2 4/5] ptp: Support late timestamp determination > >> > > > > @@ -887,18 +885,28 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk, > >> > > > > if (shhwtstamps && > >> > > > > (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE) && > >> > > > > !skb_is_swtx_tstamp(skb, false_tstamp)) { > >> > > > > + rcu_read_lock(); > >> > > > > + orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); > >> > > > > >> > > > __sock_recv_timestamp() is hot path. > >> > > > > >> > > > No need to call dev_get_by_napi_id() for the vast majority of cases > >> > > > using plain old MAC time stamping. > >> > > > >> > > Isn't dev_get_by_napi_id() called most of the time anyway in put_ts_pktinfo()? > >> > > >> > No. Only when SOF_TIMESTAMPING_OPT_PKTINFO is requested. > >> > >> You are right, my fault. > >> > >> > > That's the reason for the removal of a separate flag, which signals the need to > >> > > timestamp determination based on address/cookie. I thought there is no need > >> > > for that flag, as netdev is already available later in the existing code. > >> > > > >> > > > Make this conditional on (sk->sk_tsflags & SOF_TIMESTAMPING_BIND_PHC). > >> > > > >> > > This flag tells netdev_get_tstamp() which timestamp is required. If it > >> > > is not set, then > >> > > netdev_get_tstamp() has to deliver the normal timestamp as always. But > >> > > this normal > >> > > timestamp is only available via address/cookie. So netdev_get_tstamp() must be > >> > > called. > >> > > >> > It should be this: > >> > > >> > - normal, non-vclock: use hwtstamps->hwtstamp directly > >> > - vclock: use slower path with lookup > >> > > >> > I don't see why you can't implement that. > >> > >> I will try to implement it that way. > > > > I'm thinking about why there should be a slow path with lookup. If the > > address/cookie > > points to a defined data structure with two timestamps, then no lookup > > for the phc or > > netdev is necessary. It should be possible for every driver to > > allocate a skbuff with enough > > space for this structure On Tue, Apr 12, 2022 at 11:05 PM Vinicius Costa Gomes <vinicius.gomes@...el.com> wrote: > > Gerhard Engleder <gerhard@...leder-embedded.com> writes: > in front of the received Ethernet frame. The > > structure could be: > > > > struct skb_inline_hwtstamps { > > ktime_t hwtstamp; > > ktime_t hwcstamp; > > }; > > > > Actually my device and igc are storing the timestamps in front of the > > received Ethernet > > frame. In my opinion it is obvious to the store metadata of received > > Ethernet frames in > > front of it, because it eliminates the need for another DMA transfer. > > What is your opinion > > Vinicius? > > If I am understanding this right, the idea is providing both "cycles" > (free running cycles measurement) and PHC timestamp for all packets, for > igc, it will work fine for RX (the HW already writes the timestamps for > two timer registers in the host memory), but for TX it's going be > awkward/slow (I would have to read two extra registers), but I think > it's still possible. > > But it would be best to avoid the overhead, and only providing the > "extra" (the cycles one) measurement if necessary for TX, so > SKBTX_HW_TSTAMP_USE_CYCLES would still be needed. > > So, in short, I am fine with it, as long as I can get away with only > providing the cycles measurement for TX if necessary. Of course for TX only cycles measurement shall be provided and SKBTX_HW_TSTAMP_USE_CYCLES is used for that. Thanks! Gerhard
Powered by blists - more mailing lists