Re: [Intel-wired-lan] [PATCH iwl-next v1 1/2] igc: set RX hardware timestamps in igc_build_skb()

["Ruinskiy, Dima" <dima.ruinskiy@intel.com>]

On 10/03/2026 9:43, Loktionov, Aleksandr wrote:
> 
> 
>> -----Original Message-----
>> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
>> Of Kohei Enju
>> Sent: Saturday, March 7, 2026 7:28 PM
>> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
>> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
>> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
>> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
>> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
>> Abeni <pabeni@redhat.com>; kohei.enju@gmail.com; Kohei Enju
>> <kohei@enjuk.jp>
>> Subject: [Intel-wired-lan] [PATCH iwl-next v1 1/2] igc: set RX
>> hardware timestamps in igc_build_skb()
>>
>> igc_construct_skb() sets RX hardware timestamps, but igc_build_skb()
>> does not. This has not been observable so far since igc currently does
>> not enable the build_skb RX path.
>>
>> Set RX hardware timestamps in igc_build_skb() as well so that both skb
>> construction paths provide the same behavior.
Thank you for this patch, it is nice to have consistent behavior in both 
paths. :)

>>
>> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
>> ---
>>   drivers/net/ethernet/intel/igc/igc_main.c | 18 +++++++++++++-----
>>   1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
>> b/drivers/net/ethernet/intel/igc/igc_main.c
>> index ebd831a4ff53..3a4c1ebe4faa 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>> @@ -1964,13 +1964,16 @@ static void igc_add_rx_frag(struct igc_ring
>> *rx_ring,
>>
>>   static struct sk_buff *igc_build_skb(struct igc_ring *rx_ring,
>>   				     struct igc_rx_buffer *rx_buffer,
>> -				     struct xdp_buff *xdp)
>> +				     struct igc_xdp_buff *ctx)
>>   {
>> -	unsigned int size = xdp->data_end - xdp->data;
>> -	unsigned int truesize = igc_get_rx_frame_truesize(rx_ring,
>> size);
>> -	unsigned int metasize = xdp->data - xdp->data_meta;
>> +	unsigned int size, truesize, metasize;
>> +	struct xdp_buff *xdp = &ctx->xdp;
>>   	struct sk_buff *skb;
>>
>> +	size = xdp->data_end - xdp->data;
>> +	truesize = igc_get_rx_frame_truesize(rx_ring, size);
>> +	metasize = xdp->data - xdp->data_meta;
>> +
In the spirit of consistency, would it be possible to restructure the 
variable initialization to match that of igc_construct_skb()? Initialize 
xdp first, and then all the size variable inits can stay as they are. It 
would make the net change smaller.

>>   	/* prefetch first cache line of first page */
>>   	net_prefetch(xdp->data_meta);
>>
>> @@ -1979,6 +1982,11 @@ static struct sk_buff *igc_build_skb(struct
>> igc_ring *rx_ring,
>>   	if (unlikely(!skb))
>>   		return NULL;
>>
>> +	if (ctx->rx_ts) {
>> +		skb_shinfo(skb)->tx_flags |= SKBTX_HW_TSTAMP_NETDEV;
>> +		skb_hwtstamps(skb)->netdev_data = ctx->rx_ts;
>> +	}
>> +
>>   	/* update pointers within the skb to store the data */
>>   	skb_reserve(skb, xdp->data - xdp->data_hard_start);
>>   	__skb_put(skb, size);
>> @@ -2681,7 +2689,7 @@ static int igc_clean_rx_irq(struct igc_q_vector
>> *q_vector, const int budget)
>>   		} else 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,
>> &ctx.xdp);
>> +			skb = igc_build_skb(rx_ring, rx_buffer, &ctx);
>>   		else
>>   			skb = igc_construct_skb(rx_ring, rx_buffer,
>> &ctx);
>>
>> --
>> 2.51.0
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>