netdev - Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

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-next>] [day] [month] [year] [list]

Message-Id: <448285BC-C58E-475C-BAA2-001501503D6C@timebeat.app>
Date:   Sat, 10 Sep 2022 16:07:48 +0100
From:   Lasse Johnsen <lasse@...ebeat.app>
To:     Richard Cochran <richardcochran@...il.com>
Cc:     netdev@...r.kernel.org, Tony Nguyen <anthony.l.nguyen@...el.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        "Stanton, Kevin B" <kevin.b.stanton@...el.com>,
        Jonathan Lemon <bsd@...com>
Subject: Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc
 driver

Hi Richard,

I understand your point and your concern.

Would you be amenable to an addition to the API so we can take advantage of 
hardware that offers only a subset of the options?

We could for example extend granularity of the HWTSTAMP TX API to make requests 
for different features visible to the user space applications directly. So the TX side 
would become much more granular as is already the case with the RX side. We could 
add HWTSTAMP_TX_ONESTEP_SYNC_L2_V2, HWTSTAMP_TX_ONESTEP_SYNC_L4_V2 etc. 

My worry is that if we do not do this, then the ONESTEP option will continue 
to not see much use because so many permutations (L2, UDPv4, UDPv6, V1, V2, VLAN etc.)
currently have to be supported by the hardware.

I like Intel’s cards. I want to support the features provided by the hardware… :-)

If you agree with this approach, then I will submit instead two patches: One patch that 
extends the API and another modified version of the current igc patch which will 
use the new more granular option. For the former, I will try and persuade (...beg... I will beg...) 
JL to supervise the API work so it does not go off the rails :-)

In parallel, I have reached out to Kevin and asked if the 1-step logic will ever be able to
update the UDP checksum on-the-fly in which case I shall certainly include this functionality
as well.

Let me know what you think.

All the best,

Lasse

> On 9 Sep 2022, at 15:21, Richard Cochran <richardcochran@...il.com> wrote:
> 
> On Fri, Sep 09, 2022 at 09:51:23AM +0100, Lasse Johnsen wrote:
> 
>> So where the driver received an ioctl with tx config option HWTSTAMP_TX_ONESTEP_SYNC it will process 
>> skbs not matching the above criteria (including  PTP_CLASS_IPV4) as it would have had the tx config option 
>> been HWTSTAMP_TX_ON. This patch does not change the behaviour of the latter in any way.
>> 
>> Therefore a user space application which has used the HWTSTAMP_TX_ONESTEP_SYNC tx config option
>> and is sending UDP messages will as usual receive those messages back in the error queue with 
>> hardware timestamps in the normal fashion. (provided of course in the case of this specific driver that
>> another tx timestamping operation is not in progress.)
> 
> Okay, then I must NAK this patch.  If driver offers one-step and user
> enables it, then it should work.
> 
> The option, HWTSTAMP_TX_ONESTEP_SYNC, means to apply one-step to any
> and all Sync frames, not just layer 2.  The API does not offer a way
> to advertise or configure one-step selectively based on network layer.
> 
> Thanks,
> Richard