netdev - Re: [Intel-wired-lan] [PATCH iwl-next v7 01/12] virtchnl: add support for enabling PTP on iAVF

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: <222f6e70-b54d-4fe4-8376-1940989b5d03@intel.com>
Date: Mon, 10 Jun 2024 12:18:26 +0200
From: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
To: Simon Horman <horms@...nel.org>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>, Jacob Keller
	<jacob.e.keller@...el.com>, Wojciech Drewek <wojciech.drewek@...el.com>,
	Rahul Rameshbabu <rrameshbabu@...dia.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v7 01/12] virtchnl: add support
 for enabling PTP on iAVF



On 6/8/2024 2:55 PM, Simon Horman wrote:
> On Tue, Jun 04, 2024 at 09:13:49AM -0400, Mateusz Polchlopek wrote:
>> From: Jacob Keller <jacob.e.keller@...el.com>
>>
>> Add support for allowing a VF to enable PTP feature - Rx timestamps
>>
>> The new capability is gated by VIRTCHNL_VF_CAP_PTP, which must be
>> set by the VF to request access to the new operations. In addition, the
>> VIRTCHNL_OP_1588_PTP_CAPS command is used to determine the specific
>> capabilities available to the VF.
>>
>> This support includes the following additional capabilities:
>>
>> * Rx timestamps enabled in the Rx queues (when using flexible advanced
>>    descriptors)
>> * Read access to PHC time over virtchnl using
>>    VIRTCHNL_OP_1588_PTP_GET_TIME
>>
>> Extra space is reserved in most structures to allow for future
>> extension (like set clock, Tx timestamps).  Additional opcode numbers
>> are reserved and space in the virtchnl_ptp_caps structure is
>> specifically set aside for this.
>> Additionally, each structure has some space reserved for future
>> extensions to allow some flexibility.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
>> Reviewed-by: Rahul Rameshbabu <rrameshbabu@...dia.com>
>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
> 
> Hi Mateusz, Jacob, all,
> 
> If you need to respin this for some reason, please consider updating
> the Kernel doc for the following to include a short description.
> Else, please consider doing so as a follow-up
> 
> * struct virtchnl_ptp_caps
> * struct virtchnl_phc_time
> 
> Likewise as a follow-up, as it was not introduced by this patch, for:
> 
> * virtchnl_vc_validate_vf_msg
> 
> Flagged by kernel-doc -none -Wall
> 
> The above not withstanding, this looks good to me.
> 
> Reviewed-by: Simon Horman <horms@...nel.org>
> 
> ...

Hello Simon!

Thanks for Your review - I appreciate it.

I thought about followup series after this being merged but I received
one warning from kernel-bot regarding ARM architecture issue. That being
said I will post (probably tomorrow) v8 with fix for ARM architecture
issue and I will also include fixes for virtchnl_ptp_caps and
virtchnl_phc_time (and exceeded 80 chars issues in commit 6).

As You pointed, the virtchnl_vc_validate_vf_msg function has not been
introduced in this patch so I do not want to mix that. I will create
post-merge followup with documentation changes for mentioned function
(virtchnl_vc_validate_vf_msg) and also for one docs leftover from my
previous series (related to tx scheduler).

Mateusz