netdev - Re: [Intel-wired-lan] [PATCH iwl-next v5 05/12] iavf: negotiate PTP capabilities

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56859450-f8ae-4a46-b57a-103c7b3210b2@intel.com>
Date: Mon, 22 Apr 2024 11:33:16 +0200
From: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
To: Rahul Rameshbabu <rrameshbabu@...dia.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
	<horms@...nel.org>, <anthony.l.nguyen@...el.com>, Jacob Keller
	<jacob.e.keller@...el.com>, Wojciech Drewek <wojciech.drewek@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v5 05/12] iavf: negotiate PTP
 capabilities



On 4/18/2024 9:16 PM, Rahul Rameshbabu wrote:
> On Thu, 18 Apr, 2024 01:24:53 -0400 Mateusz Polchlopek <mateusz.polchlopek@...el.com> wrote:
>> From: Jacob Keller <jacob.e.keller@...el.com>
>>
>> Add a new extended capabilities negotiation to exchange information from
>> the PF about what PTP capabilities are supported by this VF. This
>> requires sending a VIRTCHNL_OP_1588_PTP_GET_CAPS message, and waiting
>> for the response from the PF. Handle this early on during the VF
>> initialization.
>>
>> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>> Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
>> ---
> <snip>
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> <snip>
>> @@ -2684,6 +2686,64 @@ static void iavf_init_recv_supported_rxdids(struct iavf_adapter *adapter)
>>   	iavf_change_state(adapter, __IAVF_INIT_FAILED);
>>   }
>>   
>> +/**
>> + * iavf_init_send_ptp_caps - part of querying for extended PTP capabilities
>> + * @adapter: board private structure
>> + *
>> + * Function processes send of the request for 1588 PTP capabilities to the PF.
>> + * Must clear IAVF_EXTENDED_CAP_SEND_PTP if the message is not sent, e.g.
>> + * due to the PF not negotiating VIRTCHNL_VF_PTP_CAP
>> + */
>> +static void iavf_init_send_ptp_caps(struct iavf_adapter *adapter)
>> +{
>> +	int ret;
>> +
>> +	WARN_ON(!(adapter->extended_caps & IAVF_EXTENDED_CAP_SEND_PTP));
>> +
>> +	ret = iavf_send_vf_ptp_caps_msg(adapter);
>> +	if (ret && ret == -EOPNOTSUPP) {
> 
> Similar simplification to previous patch might be possible. Simply "ret
> == -EOPNOTSUPP"?
> 

Thanks for pointing that.

>> +		/* PF does not support VIRTCHNL_VF_PTP_CAP. In this case, we
>> +		 * did not send the capability exchange message and do not
>> +		 * expect a response.
>> +		 */
>> +		adapter->extended_caps &= ~IAVF_EXTENDED_CAP_RECV_PTP;
>> +	}
>> +
>> +	/* We sent the message, so move on to the next step */
>> +	adapter->extended_caps &= ~IAVF_EXTENDED_CAP_SEND_PTP;
>> +}
>> +
>> +/**
>> + * iavf_init_recv_ptp_caps - part of querying for supported PTP capabilities
>> + * @adapter: board private structure
>> + *
>> + * Function processes receipt of the PTP capabilities supported on this VF.
>> + **/
>> +static void iavf_init_recv_ptp_caps(struct iavf_adapter *adapter)
>> +{
>> +	int ret;
>> +
>> +	WARN_ON(!(adapter->extended_caps & IAVF_EXTENDED_CAP_RECV_PTP));
>> +
>> +	memset(&adapter->ptp.hw_caps, 0, sizeof(adapter->ptp.hw_caps));
>> +
>> +	ret = iavf_get_vf_ptp_caps(adapter);
>> +	if (ret)
>> +		goto err;
>> +
>> +	/* We've processed the PF response to the VIRTCHNL_OP_1588_PTP_GET_CAPS
>> +	 * message we sent previously.
>> +	 */
>> +	adapter->extended_caps &= ~IAVF_EXTENDED_CAP_RECV_PTP;
>> +	return;
>> +err:
>> +	/* We didn't receive a reply. Make sure we try sending again when
>> +	 * __IAVF_INIT_FAILED attempts to recover.
>> +	 */
>> +	adapter->extended_caps |= IAVF_EXTENDED_CAP_SEND_PTP;
>> +	iavf_change_state(adapter, __IAVF_INIT_FAILED);
>> +}
>> +
>>   /**
>>    * iavf_init_process_extended_caps - Part of driver startup
>>    * @adapter: board private structure
> <snip>
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
>> index 52b8f1721147..5d99adb69d75 100644
>> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
>> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
>> @@ -145,6 +145,7 @@ int iavf_send_vf_config_msg(struct iavf_adapter *adapter)
>>   	       VIRTCHNL_VF_OFFLOAD_CRC |
>>   	       VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM |
>>   	       VIRTCHNL_VF_OFFLOAD_REQ_QUEUES |
>> +	       VIRTCHNL_VF_CAP_PTP |
>>   	       VIRTCHNL_VF_OFFLOAD_ADQ |
>>   	       VIRTCHNL_VF_OFFLOAD_USO |
>>   	       VIRTCHNL_VF_OFFLOAD_FDIR_PF |
>> @@ -187,6 +188,41 @@ int iavf_send_vf_supported_rxdids_msg(struct iavf_adapter *adapter)
>>   				NULL, 0);
>>   }
>>   
>> +/**
>> + * iavf_send_vf_ptp_caps_msg - Send request for PTP capabilities
>> + * @adapter: private adapter structure
>> + *
>> + * Send the VIRTCHNL_OP_1588_PTP_GET_CAPS command to the PF to request the PTP
>> + * capabilities available to this device. This includes the following
>> + * potential access:
>> + *
>> + * * READ_PHC - access to read the PTP hardware clock time
>> + * * RX_TSTAMP - access to request Rx timestamps on all received packets
>> + *
>> + * The PF will reply with the same opcode a filled out copy of the
>> + * virtchnl_ptp_caps structure which defines the specifics of which features
>> + * are accessible to this device.
>> + *
>> + * Return: 0 if success, error code otherwise
>> + */
>> +int iavf_send_vf_ptp_caps_msg(struct iavf_adapter *adapter)
>> +{
>> +	struct virtchnl_ptp_caps hw_caps = {};
> 
> To properly zero-init a struct, shouldn't this be the following?
> 
>    struct virtchnl_ptp_caps hw_caps = {0};
> 
> ISO C forbids empty initializer braces.
> 

I see Your point but I think solution I used in commonly used in
netdev, in just drivers/net directory it is used about ~1500 times.

>> +
>> +	adapter->aq_required &= ~IAVF_FLAG_AQ_GET_PTP_CAPS;
>> +
>> +	if (!PTP_ALLOWED(adapter))
>> +		return -EOPNOTSUPP;
>> +
>> +	hw_caps.caps = (VIRTCHNL_1588_PTP_CAP_READ_PHC |
>> +			VIRTCHNL_1588_PTP_CAP_RX_TSTAMP);
>> +
>> +	adapter->current_op = VIRTCHNL_OP_1588_PTP_GET_CAPS;
>> +
>> +	return iavf_send_pf_msg(adapter, VIRTCHNL_OP_1588_PTP_GET_CAPS,
>> +				(u8 *)&hw_caps, sizeof(hw_caps));
>> +}
>> +
>>   /**
>>    * iavf_validate_num_queues
>>    * @adapter: adapter structure
> 
> --
> Thanks,
> 
> Rahul Rameshbabu
>