| 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
| ||
|
Message-ID: <IA3PR11MB89869D66D321FA1C1E5CA23FE5B5A@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Tue, 23 Dec 2025 07:03:06 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: Mina Almasry <almasrymina@...gle.com>, "Lobakin, Aleksander"
<aleksander.lobakin@...el.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "bpf@...r.kernel.org"
<bpf@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, YiFei Zhu <zhuyifei@...gle.com>, "Alexei
Starovoitov" <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, "David
S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, "Jesper
Dangaard Brouer" <hawk@...nel.org>, John Fastabend
<john.fastabend@...il.com>, Stanislav Fomichev <sdf@...ichev.me>, "Nguyen,
Anthony L" <anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "Eric
Dumazet" <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, "Richard
Cochran" <richardcochran@...il.com>, "intel-wired-lan@...ts.osuosl.org"
<intel-wired-lan@...ts.osuosl.org>
Subject: RE: [PATCH net-next v3] idpf: export RX hardware timestamping
information to XDP
> -----Original Message-----
> From: Mina Almasry <almasrymina@...gle.com>
> Sent: Tuesday, December 23, 2025 12:00 AM
> To: Lobakin, Aleksander <aleksander.lobakin@...el.com>
> Cc: netdev@...r.kernel.org; bpf@...r.kernel.org; linux-
> kernel@...r.kernel.org; YiFei Zhu <zhuyifei@...gle.com>; Alexei
> Starovoitov <ast@...nel.org>; Daniel Borkmann <daniel@...earbox.net>;
> David S. Miller <davem@...emloft.net>; Jakub Kicinski
> <kuba@...nel.org>; Jesper Dangaard Brouer <hawk@...nel.org>; John
> Fastabend <john.fastabend@...il.com>; Stanislav Fomichev
> <sdf@...ichev.me>; Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>; Andrew Lunn
> <andrew+netdev@...n.ch>; Eric Dumazet <edumazet@...gle.com>; Paolo
> Abeni <pabeni@...hat.com>; Richard Cochran <richardcochran@...il.com>;
> intel-wired-lan@...ts.osuosl.org; Loktionov, Aleksandr
> <aleksandr.loktionov@...el.com>
> Subject: Re: [PATCH net-next v3] idpf: export RX hardware timestamping
> information to XDP
>
> On Mon, Dec 22, 2025 at 4:55 AM Alexander Lobakin
> <aleksander.lobakin@...el.com> wrote:
> >
> > From: Mina Almasry <almasrymina@...gle.com>
> > Date: Fri, 19 Dec 2025 20:29:54 +0000
> >
> > > From: YiFei Zhu <zhuyifei@...gle.com>
> > >
> > > 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: intel-wired-lan@...ts.osuosl.org
> > >
> > > Signed-off-by: YiFei Zhu <zhuyifei@...gle.com>
> > > Signed-off-by: Mina Almasry <almasrymina@...gle.com>
> > > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> > >
> > > ---
> > >
> > > v3:
> > > https://lore.kernel.org/netdev/20251218022948.3288897-1-
> almasrymina@
> > > google.com/
> > > - Do the idpf_queue_has(PTP) check before we read qw1 (lobakin)
> > > - Fix _qw1 not copying over ts_low on on !__LIBETH_WORD_ACCESS
> systems
> > > (AI)
> > >
> > > v2:
> > > https://lore.kernel.org/netdev/20251122140839.3922015-1-
> almasrymina@
> > > google.com/
> > > - Fixed alphabetical ordering
> > > - Use the xdp desc type instead of virtchnl one (required some
> added
> > > helpers)
> > >
> > > ---
> > > drivers/net/ethernet/intel/idpf/xdp.c | 31
> > > +++++++++++++++++++++++++++ drivers/net/ethernet/intel/idpf/xdp.h
> |
> > > 22 ++++++++++++++++++-
> > > 2 files changed, 52 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/idpf/xdp.c
> > > b/drivers/net/ethernet/intel/idpf/xdp.c
> > > index 958d16f87424..0916d201bf98 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"
> > > @@ -391,8 +392,38 @@ static int idpf_xdpmo_rx_hash(const struct
> xdp_md *ctx, u32 *hash,
> > > pt); }
> > >
> > > +static int idpf_xdpmo_rx_timestamp(const struct xdp_md *ctx, u64
> > > +*timestamp) {
> > > + const struct libeth_xdp_buff *xdp = (typeof(xdp))ctx;
> > > + struct idpf_xdp_rx_desc desc __uninitialized;
> > > + const struct idpf_rx_queue *rxq;
> > > + u64 cached_time, ts_ns;
> > > + u32 ts_high;
> > > +
> > > + rxq = libeth_xdp_buff_to_rq(xdp, typeof(*rxq), xdp_rxq);
> > > +
> > > + if (!idpf_queue_has(PTP, rxq))
> > > + return -ENODATA;
> > > +
> > > + idpf_xdp_get_qw1(&desc, xdp->desc);
> > > +
> > > + if (!(idpf_xdp_rx_ts_low(&desc) &
> VIRTCHNL2_RX_FLEX_TSTAMP_VALID))
> > > + return -ENODATA;
> > > +
> > > + cached_time = READ_ONCE(rxq->cached_phc_time);
> > > +
> > > + idpf_xdp_get_qw3(&desc, xdp->desc);
> > > +
> > > + ts_high = idpf_xdp_rx_ts_high(&desc);
> > > + ts_ns = idpf_ptp_tstamp_extend_32b_to_64b(cached_time,
> > > + ts_high);
> > > +
> > > + *timestamp = ts_ns;
> > > + return 0;
> > > +}
> > > +
> > > static const struct xdp_metadata_ops idpf_xdpmo = {
> > > .xmo_rx_hash = idpf_xdpmo_rx_hash,
> > > + .xmo_rx_timestamp = idpf_xdpmo_rx_timestamp,
> > > };
> > >
> > > void idpf_xdp_set_features(const struct idpf_vport *vport) diff
> > > --git a/drivers/net/ethernet/intel/idpf/xdp.h
> > > b/drivers/net/ethernet/intel/idpf/xdp.h
> > > index 479f5ef3c604..9daae445bde4 100644
> > > --- a/drivers/net/ethernet/intel/idpf/xdp.h
> > > +++ b/drivers/net/ethernet/intel/idpf/xdp.h
> > > @@ -112,11 +112,13 @@ struct idpf_xdp_rx_desc {
> > > aligned_u64 qw1;
> > > #define IDPF_XDP_RX_BUF GENMASK_ULL(47, 32)
> > > #define IDPF_XDP_RX_EOP BIT_ULL(1)
> > > +#define IDPF_XDP_RX_TS_LOW GENMASK_ULL(31, 24)
> > >
> > > aligned_u64 qw2;
> > > #define IDPF_XDP_RX_HASH GENMASK_ULL(31, 0)
> > >
> > > aligned_u64 qw3;
> > > +#define IDPF_XDP_RX_TS_HIGH GENMASK_ULL(63, 32)
> > > } __aligned(4 * sizeof(u64));
> > > static_assert(sizeof(struct idpf_xdp_rx_desc) ==
> > > sizeof(struct virtchnl2_rx_flex_desc_adv_nic_3));
> > > @@ -128,6 +130,8 @@ static_assert(sizeof(struct idpf_xdp_rx_desc)
> ==
> > > #define idpf_xdp_rx_buf(desc) FIELD_GET(IDPF_XDP_RX_BUF,
> (desc)->qw1)
> > > #define idpf_xdp_rx_eop(desc) !!((desc)->qw1 &
> IDPF_XDP_RX_EOP)
> > > #define idpf_xdp_rx_hash(desc) FIELD_GET(IDPF_XDP_RX_HASH,
> (desc)->qw2)
> > > +#define idpf_xdp_rx_ts_low(desc)
> FIELD_GET(IDPF_XDP_RX_TS_LOW, (desc)->qw1)
> > > +#define idpf_xdp_rx_ts_high(desc)
> FIELD_GET(IDPF_XDP_RX_TS_HIGH, (desc)->qw3)
> > >
> > > static inline void
> > > idpf_xdp_get_qw0(struct idpf_xdp_rx_desc *desc, @@ -149,7 +153,10
> > > @@ idpf_xdp_get_qw1(struct idpf_xdp_rx_desc *desc,
> > > desc->qw1 = ((const typeof(desc))rxd)->qw1; #else
> > > desc->qw1 = ((u64)le16_to_cpu(rxd->buf_id) << 32) |
> > > - rxd->status_err0_qw1;
> > > + ((u64)rxd->ts_low << 24) |
> > > + ((u64)rxd->fflags1 << 16) |
> > > + ((u64)rxd->status_err1 << 8) |
> >
> > I'm not sure you need casts to u64 here. Pls rebuild without them
> and
> > check the objdiff / compiler warnings.
> > It's required for buf_id as we shift by 32.
> >
>
> The compiler does not warn if I drop the u64 casts, but are you sure
> you want them dropped? You're already doing u64 casts in all the
> entries that you bit-shift in qw0 and qw2. It makes the code clearer
> imo. But up to you.
>
> > > + rxd->status_err0_qw1;
IMHO FIELD_PREP() looks better, just in case:
desc->qw1 =
FIELD_PREP(RXD_QW1_BUF_ID, le16_to_cpu(rxd->buf_id)) |
FIELD_PREP(RXD_QW1_TS_LOW, rxd->ts_low) |
FIELD_PREP(RXD_QW1_FFLAGS1, rxd->fflags1) |
FIELD_PREP(RXD_QW1_STATUS_ERR1, rxd->status_err1) |
FIELD_PREP(RXD_QW1_STATUS_ERR0, rxd->status_err0);
> >
> > Why did you replace the proper indentation with two tabs in all 4
> > lines above?
> >
>
> Sure, will fix.
>
> --
> Thanks,
> Mina
Powered by blists - more mailing lists