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

2025-12-17 Thread Alexander Lobakin
From: Alexander Lobakin 
Date: Wed, 17 Dec 2025 15:33:40 +0100

> From: Mina Almasry 
> Date: Sat, 22 Nov 2025 14:08:36 +
> 
>> From: YiFei Zhu 
>>
>> The logic is similar to idpf_rx_hwtstamp, but the data is exported
>> as a BPF kfunc instead of appended to an skb.
>>
>> A idpf_queue_has(PTP, rxq) condition is added to check the queue
>> supports PTP similar to idpf_rx_process_skb_fields.
>>
>> Cc: [email protected]
>>
>> Signed-off-by: YiFei Zhu 
>> Signed-off-by: Mina Almasry 
>> ---
>>  drivers/net/ethernet/intel/idpf/xdp.c | 27 +++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c 
>> b/drivers/net/ethernet/intel/idpf/xdp.c
>> index 21ce25b0567f..850389ca66b6 100644
>> --- a/drivers/net/ethernet/intel/idpf/xdp.c
>> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
>> @@ -2,6 +2,7 @@
>>  /* Copyright (C) 2025 Intel Corporation */
>>  
>>  #include "idpf.h"
>> +#include "idpf_ptp.h"
>>  #include "idpf_virtchnl.h"
>>  #include "xdp.h"
>>  #include "xsk.h"
>> @@ -369,6 +370,31 @@ int idpf_xdp_xmit(struct net_device *dev, int n, struct 
>> xdp_frame **frames,
>> idpf_xdp_tx_finalize);
>>  }
>>  
>> +static int idpf_xdpmo_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)

Also please put this function *after* rx_hash() to...

>> +{
>> +const struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc;
> 
> Sorry I know it's a late comment...
> 
> Could you please you the optimized descriptor structure from idpf/xdp.h
> instead of the regular one? To be consistent with the Rx hash timestamp
> function and give more room for optimization.
> 
>> +const struct libeth_xdp_buff *xdp = (typeof(xdp))ctx;
>> +const struct idpf_rx_queue *rxq;
>> +u64 cached_time, ts_ns;
>> +u32 ts_high;
>> +
>> +rx_desc = xdp->desc;
>> +rxq = libeth_xdp_buff_to_rq(xdp, typeof(*rxq), xdp_rxq);
>> +
>> +if (!idpf_queue_has(PTP, rxq))
>> +return -ENODATA;
>> +if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
>> +return -ENODATA;
>> +
>> +cached_time = READ_ONCE(rxq->cached_phc_time);
>> +
>> +ts_high = le32_to_cpu(rx_desc->ts_high);
>> +ts_ns = idpf_ptp_tstamp_extend_32b_to_64b(cached_time, ts_high);
>> +
>> +*timestamp = ts_ns;
>> +return 0;
>> +}
>> +
>>  static int idpf_xdpmo_rx_hash(const struct xdp_md *ctx, u32 *hash,
>>enum xdp_rss_hash_type *rss_type)
>>  {
>> @@ -392,6 +418,7 @@ static int idpf_xdpmo_rx_hash(const struct xdp_md *ctx, 
>> u32 *hash,
>>  }
>>  
>>  static const struct xdp_metadata_ops idpf_xdpmo = {
>> +.xmo_rx_timestamp   = idpf_xdpmo_rx_timestamp,
>>  .xmo_rx_hash= idpf_xdpmo_rx_hash,

...keep the alphabetic sorting here.

>>  };
>>  
>>
>> base-commit: e05021a829b834fecbd42b173e55382416571b2c

Thanks,
Olek


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

2025-12-17 Thread Alexander Lobakin
From: Mina Almasry 
Date: Sat, 22 Nov 2025 14:08:36 +

> From: YiFei Zhu 
> 
> The logic is similar to idpf_rx_hwtstamp, but the data is exported
> as a BPF kfunc instead of appended to an skb.
> 
> A idpf_queue_has(PTP, rxq) condition is added to check the queue
> supports PTP similar to idpf_rx_process_skb_fields.
> 
> Cc: [email protected]
> 
> Signed-off-by: YiFei Zhu 
> Signed-off-by: Mina Almasry 
> ---
>  drivers/net/ethernet/intel/idpf/xdp.c | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c 
> b/drivers/net/ethernet/intel/idpf/xdp.c
> index 21ce25b0567f..850389ca66b6 100644
> --- a/drivers/net/ethernet/intel/idpf/xdp.c
> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
> @@ -2,6 +2,7 @@
>  /* Copyright (C) 2025 Intel Corporation */
>  
>  #include "idpf.h"
> +#include "idpf_ptp.h"
>  #include "idpf_virtchnl.h"
>  #include "xdp.h"
>  #include "xsk.h"
> @@ -369,6 +370,31 @@ int idpf_xdp_xmit(struct net_device *dev, int n, struct 
> xdp_frame **frames,
>  idpf_xdp_tx_finalize);
>  }
>  
> +static int idpf_xdpmo_rx_timestamp(const struct xdp_md *ctx, u64 *timestamp)
> +{
> + const struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc;

Sorry I know it's a late comment...

Could you please you the optimized descriptor structure from idpf/xdp.h
instead of the regular one? To be consistent with the Rx hash timestamp
function and give more room for optimization.

> + const struct libeth_xdp_buff *xdp = (typeof(xdp))ctx;
> + const struct idpf_rx_queue *rxq;
> + u64 cached_time, ts_ns;
> + u32 ts_high;
> +
> + rx_desc = xdp->desc;
> + rxq = libeth_xdp_buff_to_rq(xdp, typeof(*rxq), xdp_rxq);
> +
> + if (!idpf_queue_has(PTP, rxq))
> + return -ENODATA;
> + if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
> + return -ENODATA;
> +
> + cached_time = READ_ONCE(rxq->cached_phc_time);
> +
> + ts_high = le32_to_cpu(rx_desc->ts_high);
> + ts_ns = idpf_ptp_tstamp_extend_32b_to_64b(cached_time, ts_high);
> +
> + *timestamp = ts_ns;
> + return 0;
> +}
> +
>  static int idpf_xdpmo_rx_hash(const struct xdp_md *ctx, u32 *hash,
> enum xdp_rss_hash_type *rss_type)
>  {
> @@ -392,6 +418,7 @@ static int idpf_xdpmo_rx_hash(const struct xdp_md *ctx, 
> u32 *hash,
>  }
>  
>  static const struct xdp_metadata_ops idpf_xdpmo = {
> + .xmo_rx_timestamp   = idpf_xdpmo_rx_timestamp,
>   .xmo_rx_hash= idpf_xdpmo_rx_hash,
>  };
>  
> 
> base-commit: e05021a829b834fecbd42b173e55382416571b2c

Thanks,
Olek


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

2025-11-30 Thread Loktionov, Aleksandr


> -Original Message-
> From: Mina Almasry 
> Sent: Monday, December 1, 2025 2:27 AM
> To: Loktionov, Aleksandr 
> Cc: [email protected]; [email protected]; linux-
> [email protected]; YiFei Zhu ; Alexei
> Starovoitov ; Daniel Borkmann ;
> David S. Miller ; Jakub Kicinski
> ; Jesper Dangaard Brouer ; John
> Fastabend ; Stanislav Fomichev
> ; Nguyen, Anthony L ;
> Kitszel, Przemyslaw ; Andrew Lunn
> ; Eric Dumazet ; Paolo
> Abeni ; Lobakin, Aleksander
> ; Richard Cochran
> ; [email protected]
> Subject: Re: [Intel-wired-lan] [PATCH net-next v1] idpf: export RX
> hardware timestamping information to XDP
> 
> On Mon, Nov 24, 2025 at 2:33 AM Loktionov, Aleksandr
>  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Intel-wired-lan  On
> Behalf
> > > Of Mina Almasry
> > > Sent: Saturday, November 22, 2025 3:09 PM
> > > To: [email protected]; [email protected]; linux-
> > > [email protected]
> > > Cc: YiFei Zhu ; Alexei Starovoitov
> > > ; Daniel Borkmann ; David S.
> > > Miller ; Jakub Kicinski ;
> > > Jesper Dangaard Brouer ; John Fastabend
> > > ; Stanislav Fomichev ;
> > > Nguyen, Anthony L ; Kitszel,
> Przemyslaw
> > > ; Andrew Lunn
> ;
> > > Eric Dumazet ; Paolo Abeni
> ;
> > > Lobakin, Aleksander ; Richard
> Cochran
> > > ; [email protected]; Mina
> > > Almasry 
> > > Subject: [Intel-wired-lan] [PATCH net-next v1] idpf: export RX
> > > hardware timestamping information to XDP
> > >
> > > From: YiFei Zhu 
> > >
> > > The logic is similar to idpf_rx_hwtstamp, but the data is exported
> > > as a BPF kfunc instead of appended to an skb.
> > >
> > > A idpf_queue_has(PTP, rxq) condition is added to check the queue
> > > supports PTP similar to idpf_rx_process_skb_fields.
> > >
> > > Cc: [email protected]
> > >
> > > Signed-off-by: YiFei Zhu 
> > > Signed-off-by: Mina Almasry 
> > > ---
> > >  drivers/net/ethernet/intel/idpf/xdp.c | 27
> > > +++
> > >  1 file changed, 27 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/intel/idpf/xdp.c
> > > b/drivers/net/ethernet/intel/idpf/xdp.c
> > > index 21ce25b0567f..850389ca66b6 100644
> > > --- a/drivers/net/ethernet/intel/idpf/xdp.c
> > > +++ b/drivers/net/ethernet/intel/idpf/xdp.c
> > > @@ -2,6 +2,7 @@
> > >  /* Copyright (C) 2025 Intel Corporation */
> > >
> > >  #include "idpf.h"
> > > +#include "idpf_ptp.h"
> > >  #include "idpf_virtchnl.h"
> > >  #include "xdp.h"
> > >  #include "xsk.h"
> > > @@ -369,6 +370,31 @@ int idpf_xdp_xmit(struct net_device *dev, int
> > > n, struct xdp_frame **frames,
> > >  idpf_xdp_tx_finalize);  }
> > >
> > > +static int idpf_xdpmo_rx_timestamp(const struct xdp_md *ctx, u64
> > > +*timestamp) {
> > > + const struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc;
> > > + const struct libeth_xdp_buff *xdp = (typeof(xdp))ctx;
> > > + const struct idpf_rx_queue *rxq;
> > > + u64 cached_time, ts_ns;
> > > + u32 ts_high;
> > > +
> > > + rx_desc = xdp->desc;
> > > + rxq = libeth_xdp_buff_to_rq(xdp, typeof(*rxq), xdp_rxq);
> > > +
> > > + if (!idpf_queue_has(PTP, rxq))
> > > + return -ENODATA;
> > > + if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
> > > + return -ENODATA;
> > RX flex desc fields are little‑endian.
> > You already convert ts_high with le32_to_cpu(), but test ts_low
> directly against the mask.
> > On big‑endian this can misdetect the bit and spuriously return -
> ENODATA.
> > Please convert ts_low to host order before the bit test.
> > See existing IDPF/ICE patterns where descriptor words are
> leXX_to_cpu()‑converted prior to FIELD_GET() / bit checks.
> > Also, per the XDP RX metadata kfunc docs, -ENODATA must reflect true
> absence of per‑packet metadata; endianness‑correct testing is required
> to uphold the semantic.
> >
> 
> Hey, sorry for the late reply. Initially when I read the reply, I
> thought: "why not, lets add a leXX_to_cpu".
> 
> But now that I look closer to implement the change and submit v2, it
> looks correct as written. ts_low is defined as a u8:
> 
> ```
> struct virtchnl2_rx_flex_desc_adv_nic_3 { ...
> u8 ts_low;
> ```
> 
> So it should not be fed into any leXX_to_cpu() functions, no?
> 
> I also looked at other u8 members in this struct like `u8
> status_err0_qw0` and `u8 status_err0_qw1`, and both are used in
> existing code without a conversion. So it seems correct as written.
> Can you reconsdirer?
> 
Oh, yes. You are right. For u8 byte endianness has no sense. Sorry.

Reviewed-by: Aleksandr Loktionov 





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

2025-11-30 Thread Mina Almasry
On Mon, Nov 24, 2025 at 2:33 AM Loktionov, Aleksandr
 wrote:
>
>
>
> > -Original Message-
> > From: Intel-wired-lan  On Behalf
> > Of Mina Almasry
> > Sent: Saturday, November 22, 2025 3:09 PM
> > To: [email protected]; [email protected]; linux-
> > [email protected]
> > Cc: YiFei Zhu ; Alexei Starovoitov
> > ; Daniel Borkmann ; David S.
> > Miller ; Jakub Kicinski ; Jesper
> > Dangaard Brouer ; John Fastabend
> > ; Stanislav Fomichev ;
> > Nguyen, Anthony L ; Kitszel, Przemyslaw
> > ; Andrew Lunn ;
> > Eric Dumazet ; Paolo Abeni ;
> > Lobakin, Aleksander ; Richard Cochran
> > ; [email protected]; Mina
> > Almasry 
> > Subject: [Intel-wired-lan] [PATCH net-next v1] idpf: export RX
> > hardware timestamping information to XDP
> >
> > From: YiFei Zhu 
> >
> > The logic is similar to idpf_rx_hwtstamp, but the data is exported as
> > a BPF kfunc instead of appended to an skb.
> >
> > A idpf_queue_has(PTP, rxq) condition is added to check the queue
> > supports PTP similar to idpf_rx_process_skb_fields.
> >
> > Cc: [email protected]
> >
> > Signed-off-by: YiFei Zhu 
> > Signed-off-by: Mina Almasry 
> > ---
> >  drivers/net/ethernet/intel/idpf/xdp.c | 27
> > +++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/idpf/xdp.c
> > b/drivers/net/ethernet/intel/idpf/xdp.c
> > index 21ce25b0567f..850389ca66b6 100644
> > --- a/drivers/net/ethernet/intel/idpf/xdp.c
> > +++ b/drivers/net/ethernet/intel/idpf/xdp.c
> > @@ -2,6 +2,7 @@
> >  /* Copyright (C) 2025 Intel Corporation */
> >
> >  #include "idpf.h"
> > +#include "idpf_ptp.h"
> >  #include "idpf_virtchnl.h"
> >  #include "xdp.h"
> >  #include "xsk.h"
> > @@ -369,6 +370,31 @@ int idpf_xdp_xmit(struct net_device *dev, int n,
> > struct xdp_frame **frames,
> >  idpf_xdp_tx_finalize);
> >  }
> >
> > +static int idpf_xdpmo_rx_timestamp(const struct xdp_md *ctx, u64
> > +*timestamp) {
> > + const struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc;
> > + const struct libeth_xdp_buff *xdp = (typeof(xdp))ctx;
> > + const struct idpf_rx_queue *rxq;
> > + u64 cached_time, ts_ns;
> > + u32 ts_high;
> > +
> > + rx_desc = xdp->desc;
> > + rxq = libeth_xdp_buff_to_rq(xdp, typeof(*rxq), xdp_rxq);
> > +
> > + if (!idpf_queue_has(PTP, rxq))
> > + return -ENODATA;
> > + if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
> > + return -ENODATA;
> RX flex desc fields are little‑endian.
> You already convert ts_high with le32_to_cpu(), but test ts_low directly 
> against the mask.
> On big‑endian this can misdetect the bit and spuriously return -ENODATA.
> Please convert ts_low to host order before the bit test.
> See existing IDPF/ICE patterns where descriptor words are 
> leXX_to_cpu()‑converted prior to FIELD_GET() / bit checks.
> Also, per the XDP RX metadata kfunc docs, -ENODATA must reflect true absence 
> of per‑packet metadata; endianness‑correct testing is required to uphold the 
> semantic.
>

Hey, sorry for the late reply. Initially when I read the reply, I
thought: "why not, lets add a leXX_to_cpu".

But now that I look closer to implement the change and submit v2, it
looks correct as written. ts_low is defined as a u8:

```
struct virtchnl2_rx_flex_desc_adv_nic_3 {
...
u8 ts_low;
```

So it should not be fed into any leXX_to_cpu() functions, no?

I also looked at other u8 members in this struct like `u8
status_err0_qw0` and `u8 status_err0_qw1`, and both are used in
existing code without a conversion. So it seems correct as written.
Can you reconsdirer?

If you insist some change is required, can you elaborate more on what
needs to be changed? There is no le8_to_cpu, unless a trivial one that
does nothing (one byte struct cannot be little or big endian).


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

2025-11-24 Thread Loktionov, Aleksandr


> -Original Message-
> From: YiFei Zhu 
> Sent: Monday, November 24, 2025 11:52 AM
> To: Loktionov, Aleksandr 
> Cc: Mina Almasry ; [email protected];
> [email protected]; [email protected]; Alexei Starovoitov
> ; Daniel Borkmann ; David S.
> Miller ; Jakub Kicinski ; Jesper
> Dangaard Brouer ; John Fastabend
> ; Stanislav Fomichev ;
> Nguyen, Anthony L ; Kitszel, Przemyslaw
> ; Andrew Lunn ;
> Eric Dumazet ; Paolo Abeni ;
> Lobakin, Aleksander ; Richard Cochran
> ; [email protected]
> Subject: Re: [Intel-wired-lan] [PATCH net-next v1] idpf: export RX
> hardware timestamping information to XDP
> 
> On Mon, Nov 24, 2025 at 12:33 AM Loktionov, Aleksandr
>  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Intel-wired-lan  On
> Behalf
> > > Of Mina Almasry
> > > Sent: Saturday, November 22, 2025 3:09 PM
> > > To: [email protected]; [email protected]; linux-
> > > [email protected]
> > > Cc: YiFei Zhu ; Alexei Starovoitov
> > > ; Daniel Borkmann ; David S.
> > > Miller ; Jakub Kicinski ;
> > > Jesper Dangaard Brouer ; John Fastabend
> > > ; Stanislav Fomichev ;
> > > Nguyen, Anthony L ; Kitszel,
> Przemyslaw
> > > ; Andrew Lunn
> ;
> > > Eric Dumazet ; Paolo Abeni
> ;
> > > Lobakin, Aleksander ; Richard
> Cochran
> > > ; [email protected]; Mina
> > > Almasry 
> > > Subject: [Intel-wired-lan] [PATCH net-next v1] idpf: export RX
> > > hardware timestamping information to XDP
> > >
> > > From: YiFei Zhu 
> > >
> > > The logic is similar to idpf_rx_hwtstamp, but the data is exported
> > > as a BPF kfunc instead of appended to an skb.
> > >
> > > A idpf_queue_has(PTP, rxq) condition is added to check the queue
> > > supports PTP similar to idpf_rx_process_skb_fields.
> > >
> > > Cc: [email protected]
> > >
> > > Signed-off-by: YiFei Zhu 
> > > Signed-off-by: Mina Almasry 
> > > ---
> > >  drivers/net/ethernet/intel/idpf/xdp.c | 27
> > > +++
> > >  1 file changed, 27 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/intel/idpf/xdp.c
> > > b/drivers/net/ethernet/intel/idpf/xdp.c
> > > index 21ce25b0567f..850389ca66b6 100644
> > > --- a/drivers/net/ethernet/intel/idpf/xdp.c
> > > +++ b/drivers/net/ethernet/intel/idpf/xdp.c
> > > @@ -2,6 +2,7 @@
> > >  /* Copyright (C) 2025 Intel Corporation */
> > >
> > >  #include "idpf.h"
> > > +#include "idpf_ptp.h"
> > >  #include "idpf_virtchnl.h"
> > >  #include "xdp.h"
> > >  #include "xsk.h"
> > > @@ -369,6 +370,31 @@ int idpf_xdp_xmit(struct net_device *dev, int
> > > n, struct xdp_frame **frames,
> > >  idpf_xdp_tx_finalize);  }
> > >
> > > +static int idpf_xdpmo_rx_timestamp(const struct xdp_md *ctx, u64
> > > +*timestamp) {
> > > + const struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc;
> > > + const struct libeth_xdp_buff *xdp = (typeof(xdp))ctx;
> > > + const struct idpf_rx_queue *rxq;
> > > + u64 cached_time, ts_ns;
> > > + u32 ts_high;
> > > +
> > > + rx_desc = xdp->desc;
> > > + rxq = libeth_xdp_buff_to_rq(xdp, typeof(*rxq), xdp_rxq);
> > > +
> > > + if (!idpf_queue_has(PTP, rxq))
> > > + return -ENODATA;
> > > + if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
> > > + return -ENODATA;
> > RX flex desc fields are little‑endian.
> > You already convert ts_high with le32_to_cpu(), but test ts_low
> directly against the mask.
> > On big‑endian this can misdetect the bit and spuriously return -
> ENODATA.
> > Please convert ts_low to host order before the bit test.
> > See existing IDPF/ICE patterns where descriptor words are
> leXX_to_cpu()‑converted prior to FIELD_GET() / bit checks.
> > Also, per the XDP RX metadata kfunc docs, -ENODATA must reflect true
> absence of per‑packet metadata; endianness‑correct testing is required
> to uphold the semantic.
> 
> The logic is copied as verbatim from idpf_rx_hwtstamp:
> 
> static void
> idpf_rx_hwtstamp(const struct idpf_rx_queue *rxq,
>  const struct virtchnl2_rx_flex_desc_adv_nic_3
> *rx_desc,
>  struct sk_buff *skb)
> {
> u64 cached_time, ts_ns;
> u32 ts_high;
> 
> if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
> return;
> 
> cached_time = READ_ONCE(rxq->cached_phc_time);
> 
> ts_high = le32_to_cpu(rx_desc->ts_high);
> ts_ns = idpf_ptp_tstamp_extend_32b_to_64b(cached_time,
> ts_high); [...]
> 
> I assume that is incorrect and would need to be fixed too?
> 
I think must be fixed.

With the best regards
Alex

> YiFei Zhu


...


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

2025-11-24 Thread YiFei Zhu
On Mon, Nov 24, 2025 at 12:33 AM Loktionov, Aleksandr
 wrote:
>
>
>
> > -Original Message-
> > From: Intel-wired-lan  On Behalf
> > Of Mina Almasry
> > Sent: Saturday, November 22, 2025 3:09 PM
> > To: [email protected]; [email protected]; linux-
> > [email protected]
> > Cc: YiFei Zhu ; Alexei Starovoitov
> > ; Daniel Borkmann ; David S.
> > Miller ; Jakub Kicinski ; Jesper
> > Dangaard Brouer ; John Fastabend
> > ; Stanislav Fomichev ;
> > Nguyen, Anthony L ; Kitszel, Przemyslaw
> > ; Andrew Lunn ;
> > Eric Dumazet ; Paolo Abeni ;
> > Lobakin, Aleksander ; Richard Cochran
> > ; [email protected]; Mina
> > Almasry 
> > Subject: [Intel-wired-lan] [PATCH net-next v1] idpf: export RX
> > hardware timestamping information to XDP
> >
> > From: YiFei Zhu 
> >
> > The logic is similar to idpf_rx_hwtstamp, but the data is exported as
> > a BPF kfunc instead of appended to an skb.
> >
> > A idpf_queue_has(PTP, rxq) condition is added to check the queue
> > supports PTP similar to idpf_rx_process_skb_fields.
> >
> > Cc: [email protected]
> >
> > Signed-off-by: YiFei Zhu 
> > Signed-off-by: Mina Almasry 
> > ---
> >  drivers/net/ethernet/intel/idpf/xdp.c | 27
> > +++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/idpf/xdp.c
> > b/drivers/net/ethernet/intel/idpf/xdp.c
> > index 21ce25b0567f..850389ca66b6 100644
> > --- a/drivers/net/ethernet/intel/idpf/xdp.c
> > +++ b/drivers/net/ethernet/intel/idpf/xdp.c
> > @@ -2,6 +2,7 @@
> >  /* Copyright (C) 2025 Intel Corporation */
> >
> >  #include "idpf.h"
> > +#include "idpf_ptp.h"
> >  #include "idpf_virtchnl.h"
> >  #include "xdp.h"
> >  #include "xsk.h"
> > @@ -369,6 +370,31 @@ int idpf_xdp_xmit(struct net_device *dev, int n,
> > struct xdp_frame **frames,
> >  idpf_xdp_tx_finalize);
> >  }
> >
> > +static int idpf_xdpmo_rx_timestamp(const struct xdp_md *ctx, u64
> > +*timestamp) {
> > + const struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc;
> > + const struct libeth_xdp_buff *xdp = (typeof(xdp))ctx;
> > + const struct idpf_rx_queue *rxq;
> > + u64 cached_time, ts_ns;
> > + u32 ts_high;
> > +
> > + rx_desc = xdp->desc;
> > + rxq = libeth_xdp_buff_to_rq(xdp, typeof(*rxq), xdp_rxq);
> > +
> > + if (!idpf_queue_has(PTP, rxq))
> > + return -ENODATA;
> > + if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
> > + return -ENODATA;
> RX flex desc fields are little‑endian.
> You already convert ts_high with le32_to_cpu(), but test ts_low directly 
> against the mask.
> On big‑endian this can misdetect the bit and spuriously return -ENODATA.
> Please convert ts_low to host order before the bit test.
> See existing IDPF/ICE patterns where descriptor words are 
> leXX_to_cpu()‑converted prior to FIELD_GET() / bit checks.
> Also, per the XDP RX metadata kfunc docs, -ENODATA must reflect true absence 
> of per‑packet metadata; endianness‑correct testing is required to uphold the 
> semantic.

The logic is copied as verbatim from idpf_rx_hwtstamp:

static void
idpf_rx_hwtstamp(const struct idpf_rx_queue *rxq,
 const struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc,
 struct sk_buff *skb)
{
u64 cached_time, ts_ns;
u32 ts_high;

if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
return;

cached_time = READ_ONCE(rxq->cached_phc_time);

ts_high = le32_to_cpu(rx_desc->ts_high);
ts_ns = idpf_ptp_tstamp_extend_32b_to_64b(cached_time, ts_high);
[...]

I assume that is incorrect and would need to be fixed too?

YiFei Zhu

> > +
> > + cached_time = READ_ONCE(rxq->cached_phc_time);
> > +
> > + ts_high = le32_to_cpu(rx_desc->ts_high);
> > + ts_ns = idpf_ptp_tstamp_extend_32b_to_64b(cached_time,
> > ts_high);
> > +
> > + *timestamp = ts_ns;
> > + return 0;
> > +}
> > +
> >  static int idpf_xdpmo_rx_hash(const struct xdp_md *ctx, u32 *hash,
> > enum xdp_rss_hash_type *rss_type)  { @@ -
> > 392,6 +418,7 @@ static int idpf_xdpmo_rx_hash(const struct xdp_md
> > *ctx, u32 *hash,  }
> >
> >  static const struct xdp_metadata_ops idpf_xdpmo = {
> > + .xmo_rx_timestamp   = idpf_xdpmo_rx_timestamp,
> >   .xmo_rx_hash= idpf_xdpmo_rx_hash,
> >  };
> >
> >
> > base-commit: e05021a829b834fecbd42b173e55382416571b2c
> > --
> > 2.52.0.rc2.455.g230fcf2819-goog
>


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

2025-11-24 Thread Loktionov, Aleksandr


> -Original Message-
> From: Intel-wired-lan  On Behalf
> Of Mina Almasry
> Sent: Saturday, November 22, 2025 3:09 PM
> To: [email protected]; [email protected]; linux-
> [email protected]
> Cc: YiFei Zhu ; Alexei Starovoitov
> ; Daniel Borkmann ; David S.
> Miller ; Jakub Kicinski ; Jesper
> Dangaard Brouer ; John Fastabend
> ; Stanislav Fomichev ;
> Nguyen, Anthony L ; Kitszel, Przemyslaw
> ; Andrew Lunn ;
> Eric Dumazet ; Paolo Abeni ;
> Lobakin, Aleksander ; Richard Cochran
> ; [email protected]; Mina
> Almasry 
> Subject: [Intel-wired-lan] [PATCH net-next v1] idpf: export RX
> hardware timestamping information to XDP
> 
> From: YiFei Zhu 
> 
> The logic is similar to idpf_rx_hwtstamp, but the data is exported as
> a BPF kfunc instead of appended to an skb.
> 
> A idpf_queue_has(PTP, rxq) condition is added to check the queue
> supports PTP similar to idpf_rx_process_skb_fields.
> 
> Cc: [email protected]
> 
> Signed-off-by: YiFei Zhu 
> Signed-off-by: Mina Almasry 
> ---
>  drivers/net/ethernet/intel/idpf/xdp.c | 27
> +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c
> b/drivers/net/ethernet/intel/idpf/xdp.c
> index 21ce25b0567f..850389ca66b6 100644
> --- a/drivers/net/ethernet/intel/idpf/xdp.c
> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
> @@ -2,6 +2,7 @@
>  /* Copyright (C) 2025 Intel Corporation */
> 
>  #include "idpf.h"
> +#include "idpf_ptp.h"
>  #include "idpf_virtchnl.h"
>  #include "xdp.h"
>  #include "xsk.h"
> @@ -369,6 +370,31 @@ int idpf_xdp_xmit(struct net_device *dev, int n,
> struct xdp_frame **frames,
>  idpf_xdp_tx_finalize);
>  }
> 
> +static int idpf_xdpmo_rx_timestamp(const struct xdp_md *ctx, u64
> +*timestamp) {
> + const struct virtchnl2_rx_flex_desc_adv_nic_3 *rx_desc;
> + const struct libeth_xdp_buff *xdp = (typeof(xdp))ctx;
> + const struct idpf_rx_queue *rxq;
> + u64 cached_time, ts_ns;
> + u32 ts_high;
> +
> + rx_desc = xdp->desc;
> + rxq = libeth_xdp_buff_to_rq(xdp, typeof(*rxq), xdp_rxq);
> +
> + if (!idpf_queue_has(PTP, rxq))
> + return -ENODATA;
> + if (!(rx_desc->ts_low & VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
> + return -ENODATA;
RX flex desc fields are little‑endian.
You already convert ts_high with le32_to_cpu(), but test ts_low directly 
against the mask.
On big‑endian this can misdetect the bit and spuriously return -ENODATA.
Please convert ts_low to host order before the bit test.
See existing IDPF/ICE patterns where descriptor words are 
leXX_to_cpu()‑converted prior to FIELD_GET() / bit checks.
Also, per the XDP RX metadata kfunc docs, -ENODATA must reflect true absence of 
per‑packet metadata; endianness‑correct testing is required to uphold the 
semantic.

> +
> + cached_time = READ_ONCE(rxq->cached_phc_time);
> +
> + ts_high = le32_to_cpu(rx_desc->ts_high);
> + ts_ns = idpf_ptp_tstamp_extend_32b_to_64b(cached_time,
> ts_high);
> +
> + *timestamp = ts_ns;
> + return 0;
> +}
> +
>  static int idpf_xdpmo_rx_hash(const struct xdp_md *ctx, u32 *hash,
> enum xdp_rss_hash_type *rss_type)  { @@ -
> 392,6 +418,7 @@ static int idpf_xdpmo_rx_hash(const struct xdp_md
> *ctx, u32 *hash,  }
> 
>  static const struct xdp_metadata_ops idpf_xdpmo = {
> + .xmo_rx_timestamp   = idpf_xdpmo_rx_timestamp,
>   .xmo_rx_hash= idpf_xdpmo_rx_hash,
>  };
> 
> 
> base-commit: e05021a829b834fecbd42b173e55382416571b2c
> --
> 2.52.0.rc2.455.g230fcf2819-goog