netdev - RE: [PATCH v4 iwl-next 08/10] idpf: add Tx timestamp flows

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: <MW4PR11MB58899B726FAFEC004F58FAC18E192@MW4PR11MB5889.namprd11.prod.outlook.com>
Date: Wed, 15 Jan 2025 11:56:29 +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>, "Hay, Joshua A" <joshua.a.hay@...el.com>
Subject: RE: [PATCH v4 iwl-next 08/10] idpf: add Tx timestamp flows

On 01/14/2025 5:27PM Simon Horman wrote:

> > Add functions to request Tx timestamp for the PTP packets, read the Tx
> > timestamp when the completion tag for that packet is being received,
> > extend the Tx timestamp value and set the supported timestamping modes.
> > 
> > Tx timestamp is requested for the PTP packets by setting a TSYN bit and
> > index value in the Tx context descriptor. The driver assumption is that
> > the Tx timestamp value is ready to be read when the completion tag is
> > received. Then the driver schedules delayed work and the Tx timestamp
> > value read is requested through virtchnl message. At the end, the Tx
> > timestamp value is extended to 64-bit and provided back to the skb.
> > 
> > Co-developed-by: Josh Hay <joshua.a.hay@...el.com>
> > Signed-off-by: Josh Hay <joshua.a.hay@...el.com>
> > Signed-off-by: Milena Olech <milena.olech@...el.com>
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> 
> ...
> 
> > +/**
> > + * idpf_ptp_request_ts - Request an available Tx timestamp index
> > + * @tx_q: Transmit queue on which the Tx timestamp is requested
> > + * @skb: The SKB to associate with this timestamp request
> > + * @idx: Index of the Tx timestamp latch
> > + *
> > + * Request tx timestamp index negotiated during PTP init that will be set into
> > + * Tx descriptor.
> > + *
> > + * Return: 0 and the index that can be provided to Tx descriptor on success,
> > + * -errno otherwise.
> > + */
> > +int idpf_ptp_request_ts(struct idpf_tx_queue *tx_q, struct sk_buff *skb,
> > +			u32 *idx)
> > +{
> > +	struct idpf_ptp_tx_tstamp *ptp_tx_tstamp;
> > +	struct list_head *head;
> > +
> > +	/* Get the index from the free latches list */
> > +	spin_lock_bh(&tx_q->cached_tstamp_caps->lock_free);
> > +
> > +	head = &tx_q->cached_tstamp_caps->latches_free;
> > +	if (list_empty(head)) {
> > +		spin_unlock_bh(&tx_q->cached_tstamp_caps->lock_in_use);
> 
> Hi Milena and Josh,
> 
> Should the line above be:
> 
> 		spin_unlock_bh(&tx_q->cached_tstamp_caps->lock_free);
> 
> Flagged by Smatch.
> 

Hello Simon,

Yes, definitely - it's a mistake.
I'll change in the next version.

Thanks!
Milena

> > +		return -ENOBUFS;
> > +	}
> > +
> > +	ptp_tx_tstamp = list_first_entry(head, struct idpf_ptp_tx_tstamp,
> > +					 list_member);
> > +	list_del(&ptp_tx_tstamp->list_member);
> > +	spin_unlock_bh(&tx_q->cached_tstamp_caps->lock_free);
> > +
> > +	ptp_tx_tstamp->skb = skb_get(skb);
> > +	skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> > +
> > +	/* Move the element to the used latches list */
> > +	spin_lock_bh(&tx_q->cached_tstamp_caps->lock_in_use);
> > +	list_add(&ptp_tx_tstamp->list_member,
> > +		 &tx_q->cached_tstamp_caps->latches_in_use);
> > +	spin_unlock_bh(&tx_q->cached_tstamp_caps->lock_in_use);
> > +
> > +	*idx = ptp_tx_tstamp->idx;
> > +
> > +	return 0;
> > +}