From: Paul Menzel <redacted>
Date: Sat, 10 Jan 2026 09:35:58 +0100

Dear Mina,


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

[...]

quoted

@@ -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.

quoted

+}
+
  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