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

Message-ID: <Yx9Tv5IYqmcRFuz/@hoboy.vegasvil.org>
Date:   Mon, 12 Sep 2022 08:43:59 -0700
From:   Richard Cochran <richardcochran@...il.com>
To:     Lasse Johnsen <lasse@...ebeat.app>
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

On Sat, Sep 10, 2022 at 04:07:48PM +0100, Lasse Johnsen wrote:
> 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?

I think that is only user friendly option.  However, the question is
whether this would find broad use, or would it remain an isolated hack
for one borken hardware design?

> 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.

Actually IMO the opposite is true.  If the API nickel and dimes every
last combination then:

- that will only encourage even more broken hardware designs
- no user space software will implement the combos

Case in point:

	/* PTP v1, UDP, any kind of event packet */
	HWTSTAMP_FILTER_PTP_V1_L4_EVENT,
	/* PTP v1, UDP, Sync packet */
	HWTSTAMP_FILTER_PTP_V1_L4_SYNC,
	/* PTP v1, UDP, Delay_req packet */
	HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ,
	/* PTP v2, UDP, any kind of event packet */
	HWTSTAMP_FILTER_PTP_V2_L4_EVENT,
	/* PTP v2, UDP, Sync packet */
	HWTSTAMP_FILTER_PTP_V2_L4_SYNC,
	/* PTP v2, UDP, Delay_req packet */
	HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ,

	/* 802.AS1, Ethernet, any kind of event packet */
	HWTSTAMP_FILTER_PTP_V2_L2_EVENT,
	/* 802.AS1, Ethernet, Sync packet */
	HWTSTAMP_FILTER_PTP_V2_L2_SYNC,
	/* 802.AS1, Ethernet, Delay_req packet */
	HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ,

This monstrosity came about because of ONE early, brain dead HW
design.  Seriously, who would ever want to have just "PTP v1, UDP,
Delay_req packet" and not the other event messages?

This horrible API is now written in stone, but never, ever used.

If hardware claims to implement PTP one-step, then it really should do
so in a way that conforms to the published standards.

Thanks,
Richard