netdev - RE: [PATCH v2 iwl-next 07/10] idpf: add Tx timestamp capabilities negotiation

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: <PH7PR11MB588506F083523298ADF85EF48E052@PH7PR11MB5885.namprd11.prod.outlook.com>
Date: Wed, 18 Dec 2024 15:08:02 +0000
From: "Olech, Milena" <milena.olech@...el.com>
To: Simon Horman <horms@...nel.org>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, "Lobakin, Aleksander"
	<aleksander.lobakin@...el.com>, "Tantilov, Emil S"
	<emil.s.tantilov@...el.com>, "Linga, Pavan Kumar"
	<pavan.kumar.linga@...el.com>
Subject: RE: [PATCH v2 iwl-next 07/10] idpf: add Tx timestamp capabilities
 negotiation

On 12/03/2024 3:56PM Simon Horman wrote:

>> Tx timestamp capabilities are negotiated for the uplink Vport.
>> Driver receives information about the number of available Tx timestamp
>> latches, the size of Tx timestamp value and the set of indexes used
>> for Tx timestamping.
>> 
>> Add function to get the Tx timestamp capabilities and parse the uplink
>> vport flag.
>> 
>> Reviewed-by: Alexander Lobakin <aleksander.lobakin@...el.com>
>> Co-developed-by: Emil Tantilov <emil.s.tantilov@...el.com>
>> Signed-off-by: Emil Tantilov <emil.s.tantilov@...el.com>
>> Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
>> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
>> Signed-off-by: Milena Olech <milena.olech@...el.com>
>> ---
>> v1 -> v2: change the idpf_for_each_vport macro
>
>Hi Milena,
>
>Some minor nits from my side.
>
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> index e7ccdcbdbd47..057d1c546417 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> @@ -83,6 +83,70 @@ struct idpf_ptp_secondary_mbx {
>>  	bool valid:1;
>>  };
>>  
>> +/**
>> + * enum idpf_ptp_tx_tstamp_state - Tx timestamp states
>> + * @IDPF_PTP_FREE: Tx timestamp index free to use
>> + * @IDPF_PTP_REQUEST: Tx timestamp index set to the Tx descriptor
>> + * @IDPF_PTP_READ_VALUE: Tx timestamp value ready to be read
>> + */
>> +enum idpf_ptp_tx_tstamp_state {
>> +	IDPF_PTP_FREE,
>> +	IDPF_PTP_REQUEST,
>> +	IDPF_PTP_READ_VALUE,
>> +};
>> +
>> +/**
>> + * struct idpf_ptp_tx_tstamp_status - Parameters to track Tx timestamp
>> + * @skb: the pointer to the SKB that received the completion tag
>> + * @state: the state of the Tx timestamp
>> + */
>> +struct idpf_ptp_tx_tstamp_status {
>> +	struct sk_buff *skb;
>> +	enum idpf_ptp_tx_tstamp_state state;
>> +};
>> +
>> +/**
>> + * struct idpf_ptp_tx_tstamp - Parameters for Tx timestamping
>> + * @list_member: the list member strutcure
>
>nit: structure
>
>     Flagged by checkpatch.pl --codespell
>
>> + * @tx_latch_reg_offset_l: Tx tstamp latch low register offset
>> + * @tx_latch_reg_offset_h: Tx tstamp latch high register offset
>> + * @skb: the pointer to the SKB for this timestamp request
>> + * @tstamp: the Tx tstamp value
>> + * @idx: the index of the Tx tstamp
>> + */
>> +struct idpf_ptp_tx_tstamp {
>> +	struct list_head list_member;
>> +	u32 tx_latch_reg_offset_l;
>> +	u32 tx_latch_reg_offset_h;
>> +	struct sk_buff *skb;
>> +	u64 tstamp;
>> +	u32 idx;
>> +};
>
>...
>
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
>
>...
>
>> @@ -3154,6 +3157,14 @@ void idpf_vport_init(struct idpf_vport *vport, struct idpf_vport_max_q *max_q)
>>  	idpf_vport_alloc_vec_indexes(vport);
>>  
>>  	vport->crc_enable = adapter->crc_enable;
>> +
>> +	if (!(vport_msg->vport_flags &
>> +	      le16_to_cpu(VIRTCHNL2_VPORT_UPLINK_PORT)))
>
>I think this should be cpu_to_le16.
>
>Flagged by Sparse.

Definitely! Thanks

>
>> +		return;
>> +
>> +	err = idpf_ptp_get_vport_tstamps_caps(vport);
>> +	if (err)
>> +		pci_dbg(vport->adapter->pdev, "Tx timestamping not supported\n");
>>  }
>>  
>>  /**
>
>...
>
>> diff --git a/drivers/net/ethernet/intel/idpf/virtchnl2.h b/drivers/net/ethernet/intel/idpf/virtchnl2.h
>> index 44a5ee84ed60..fdeebc621bdb 100644
>> --- a/drivers/net/ethernet/intel/idpf/virtchnl2.h
>> +++ b/drivers/net/ethernet/intel/idpf/virtchnl2.h
>> @@ -569,6 +569,14 @@ struct virtchnl2_queue_reg_chunks {
>>  };
>>  VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_queue_reg_chunks);
>>  
>> +/**
>> + * enum virtchnl2_vport_flags - Vport flags that indicate vport capabilities.
>> + * @VIRTCHNL2_VPORT_UPLINK_PORT: Representatives of underlying physical ports
>> + */
>> +enum virtchnl2_vport_flags {
>> +	VIRTCHNL2_VPORT_UPLINK_PORT	= BIT(0),
>> +};
>> +
>>  /**
>>   * struct virtchnl2_create_vport - Create vport config info.
>>   * @vport_type: See enum virtchnl2_vport_type.
>> @@ -620,7 +628,7 @@ struct virtchnl2_create_vport {
>>  	__le16 max_mtu;
>>  	__le32 vport_id;
>>  	u8 default_mac_addr[ETH_ALEN];
>> -	__le16 pad;
>> +	__le16 vport_flags;
>
>The kernel doc for this structure, which is immediately above the
>structure, should also be updated.

Correct, will fix in v3.

>
>Flagged by ./scripts/kernel-doc -none
>
>>  	__le64 rx_desc_ids;
>>  	__le64 tx_desc_ids;
>>  	u8 pad1[72];
>> -- 
>> 2.31.1
>>

Thanks,
Milena