Re: [Intel-wired-lan] [PATCH iwl-next v4] idpf: export RX hardware timestamping information to XDP

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Paul Menzel <pmenzel@molgen.mpg.de>
Date: Sat, 10 Jan 2026 09:35:58 +0100

> Dear Mina,
> 
> 
> Thank you for your patch. Some minor comments, should you resend.

[...]

>> @@ -166,6 +173,19 @@ idpf_xdp_get_qw2(struct idpf_xdp_rx_desc *desc,
>>   #endif
>>   }
>>   +static inline void
>> +idpf_xdp_get_qw3(struct idpf_xdp_rx_desc *desc,
>> +         const struct virtchnl2_rx_flex_desc_adv_nic_3 *rxd)
>> +{
>> +#ifdef __LIBETH_WORD_ACCESS
>> +    desc->qw3 = ((const typeof(desc))rxd)->qw3;
>> +#else
>> +    desc->qw3 = ((u64)le32_to_cpu(rxd->ts_high) << 32) |
>> +            ((u64)le16_to_cpu(rxd->fmd6) << 16) |
>> +            le16_to_cpu(rxd->l2tag1);
>> +#endif
> 
> It’s done elsewhere in the file, but I wonder why use the preprocessor
> and not plain C code, and let the linker(?) remove the unneeded branch?

Do you mean IS_ENABLED()?
In the libeth_xdp code, several paths won't build with IS_ENABLED(),
here in idpf it might work out, but I did it the same way as in
libeth_xdp for consistency.

> 
>> +}
>> +
>>   void idpf_xdp_set_features(const struct idpf_vport *vport);
>>     int idpf_xdp(struct net_device *dev, struct netdev_bpf *xdp);
> 
> 
> Kind regards,
> 
> Paul

Thanks,
Olek