[Intel-wired-lan] [PATCH net-next v6 5/9] igc: Refactor Rx timestamp handling

[Dvora Fuxbrumer <dvorax.fuxbrumer@linux.intel.com>]

On 10/02/2021 23:58, Vedang Patel wrote:
> From: Andre Guedes <andre.guedes@intel.com>
> 
> Refactor the Rx timestamp handling in preparation to land
> XDP support.
> 
> Rx timestamps are put in the Rx buffer by hardware, before the packet
> data. When creating the xdp buffer, we will need to check the Rx
> descriptor to determine if the buffer contains timestamp information
> and consider the offset when setting xdp.data.
> 
> The Rx descriptor check is already done in igc_construct_skb(). To
> avoid code duplication, this patch moves the timestamp handling to
> igc_clean_rx_irq() so both skb and xdp paths can reuse it.
> 
> Signed-off-by: Andre Guedes <andre.guedes@intel.com>
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Vedang Patel <vedang.patel@intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc.h      |  3 +--
>   drivers/net/ethernet/intel/igc/igc_main.c | 30 +++++++++++++++--------
>   drivers/net/ethernet/intel/igc/igc_ptp.c  | 25 ++++++++++---------
>   3 files changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index fbd8c414c3c8..72544b5e747e 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -547,8 +547,7 @@ void igc_ptp_init(struct igc_adapter *adapter);
>   void igc_ptp_reset(struct igc_adapter *adapter);
>   void igc_ptp_suspend(struct igc_adapter *adapter);
>   void igc_ptp_stop(struct igc_adapter *adapter);
> -void igc_ptp_rx_pktstamp(struct igc_q_vector *q_vector, u32 *va,
> -			 struct sk_buff *skb);
> +ktime_t igc_ptp_rx_pktstamp(struct igc_adapter *adapter, u32 *buf);
>   int igc_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr);
>   int igc_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr);
>   void igc_ptp_tx_hang(struct igc_adapter *adapter);
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index d1fc5baadf9b..5f77f1f43a7c 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -1582,10 +1582,11 @@ static struct sk_buff *igc_build_skb(struct igc_ring *rx_ring,
>   
>   static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring,
>   					 struct igc_rx_buffer *rx_buffer,
> -					 union igc_adv_rx_desc *rx_desc,
> -					 unsigned int size)
> +					 unsigned int size, int pkt_offset,
> +					 ktime_t timestamp)
>   {
> -	void *va = page_address(rx_buffer->page) + rx_buffer->page_offset;
> +	void *va = page_address(rx_buffer->page) + rx_buffer->page_offset +
> +		   pkt_offset;
>   	unsigned int truesize = igc_get_rx_frame_truesize(rx_ring, size);
>   	unsigned int headlen;
>   	struct sk_buff *skb;
> @@ -1598,11 +1599,8 @@ static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring,
>   	if (unlikely(!skb))
>   		return NULL;
>   
> -	if (unlikely(igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP))) {
> -		igc_ptp_rx_pktstamp(rx_ring->q_vector, va, skb);
> -		va += IGC_TS_HDR_LEN;
> -		size -= IGC_TS_HDR_LEN;
> -	}
> +	if (timestamp)
> +		skb_hwtstamps(skb)->hwtstamp = timestamp;
>   
>   	/* Determine available headroom for copy */
>   	headlen = size;
> @@ -1896,6 +1894,8 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget)
>   	while (likely(total_packets < budget)) {
>   		union igc_adv_rx_desc *rx_desc;
>   		struct igc_rx_buffer *rx_buffer;
> +		ktime_t timestamp = 0;
> +		int pkt_offset = 0;
>   		unsigned int size;
>   
>   		/* return some buffers to hardware, one at a time is too slow */
> @@ -1917,14 +1917,24 @@ static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int budget)
>   
>   		rx_buffer = igc_get_rx_buffer(rx_ring, size);
>   
> +		if (igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)) {
> +			void *pktbuf = page_address(rx_buffer->page) +
> +				       rx_buffer->page_offset;
> +
> +			timestamp = igc_ptp_rx_pktstamp(q_vector->adapter,
> +							pktbuf);
> +			pkt_offset = IGC_TS_HDR_LEN;
> +			size -= IGC_TS_HDR_LEN;
> +		}
> +
>   		/* retrieve a buffer from the ring */
>   		if (skb)
>   			igc_add_rx_frag(rx_ring, rx_buffer, skb, size);
>   		else if (ring_uses_build_skb(rx_ring))
>   			skb = igc_build_skb(rx_ring, rx_buffer, rx_desc, size);
>   		else
> -			skb = igc_construct_skb(rx_ring, rx_buffer,
> -						rx_desc, size);
> +			skb = igc_construct_skb(rx_ring, rx_buffer, size,
> +						pkt_offset, timestamp);
>   
>   		/* exit if we failed to retrieve a buffer */
>   		if (!skb) {
> diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
> index b6a640bfc991..7301a7b3fadc 100644
> --- a/drivers/net/ethernet/intel/igc/igc_ptp.c
> +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
> @@ -153,20 +153,20 @@ static void igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter,
>   
>   /**
>    * igc_ptp_rx_pktstamp - Retrieve timestamp from Rx packet buffer
> - * @q_vector: Pointer to interrupt specific structure
> - * @va: Pointer to address containing Rx buffer
> - * @skb: Buffer containing timestamp and packet
> + * @adapter: Pointer to adapter the packet buffer belongs to
> + * @buf: Pointer to packet buffer
>    *
>    * This function retrieves the timestamp saved in the beginning of packet
>    * buffer. While two timestamps are available, one in timer0 reference and the
>    * other in timer1 reference, this function considers only the timestamp in
>    * timer0 reference.
> + *
> + * Returns timestamp value.
>    */
> -void igc_ptp_rx_pktstamp(struct igc_q_vector *q_vector, u32 *va,
> -			 struct sk_buff *skb)
> +ktime_t igc_ptp_rx_pktstamp(struct igc_adapter *adapter, u32 *buf)
>   {
> -	struct igc_adapter *adapter = q_vector->adapter;
> -	u64 regval;
> +	ktime_t timestamp;
> +	u32 secs, nsecs;
>   	int adjust;
>   
>   	/* Timestamps are saved in little endian at the beginning of the packet
> @@ -178,9 +178,10 @@ void igc_ptp_rx_pktstamp(struct igc_q_vector *q_vector, u32 *va,
>   	 * SYSTIML holds the nanoseconds part while SYSTIMH holds the seconds
>   	 * part of the timestamp.
>   	 */
> -	regval = le32_to_cpu(va[2]);
> -	regval |= (u64)le32_to_cpu(va[3]) << 32;
> -	igc_ptp_systim_to_hwtstamp(adapter, skb_hwtstamps(skb), regval);
> +	nsecs = le32_to_cpu(buf[2]);
> +	secs = le32_to_cpu(buf[3]);
> +
> +	timestamp = ktime_set(secs, nsecs);
>   
>   	/* Adjust timestamp for the RX latency based on link speed */
>   	switch (adapter->link_speed) {
> @@ -201,8 +202,8 @@ void igc_ptp_rx_pktstamp(struct igc_q_vector *q_vector, u32 *va,
>   		netdev_warn_once(adapter->netdev, "Imprecise timestamp\n");
>   		break;
>   	}
> -	skb_hwtstamps(skb)->hwtstamp =
> -		ktime_sub_ns(skb_hwtstamps(skb)->hwtstamp, adjust);
> +
> +	return ktime_sub_ns(timestamp, adjust);
>   }
>   
>   static void igc_ptp_disable_rx_timestamp(struct igc_adapter *adapter)
> 
Tested-by: Dvora Fuxbrumer <dvorax.fuxbrumer@linux.intel.com>