xdp-loader: Make feature flags printing future-proof

In netdev.h, NETDEV_XDP_ACT_MASK is declared as private, so it should
only be used inside the kernel, and according to Jakub Kicinski then
"user space should not care about it"[1].

  linux$ git grep -B1 NETDEV_XDP_ACT_MASK include/
  include/uapi/linux/netdev.h-    /* private: */
  include/uapi/linux/netdev.h:    NETDEV_XDP_ACT_MASK = 127,

In this specific case, then this code is buggy. If a new feature flag
is added to the uapi header, and hence included in NETDEV_XDP_ACT_MASK,
and xdp-tools is not updated, or an older version is re-build in a
distro, with newer linux headers, then NETDEV_XDP_ACT_MASK can contain
a new feature flag, and if the new flag is observed, then it will neither
be checked nor trigger the unknown feature printing.

This patch moves the flags into a constant array, and while looping over
them in iface_print_xdp_features(), it builds a local mask consisting only
of the tested flags. The local mask is used to replace NETDEV_XDP_ACT_MASK.

[1] https://lore.kernel.org/netdev/20230614211715.01940bbd@kernel.org/

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

diff --git a/xdp-loader/xdp-loader.c b/xdp-loader/xdp-loader.c
index d9a6f97024f0b2c2ebbe982248c9f8bc26528aff..48946492e895cb5e97a632ace93e6b8beb6532ec 100644 (file)
--- a/xdp-loader/xdp-loader.c
+++ b/xdp-loader/xdp-loader.c
@@ -51,6 +51,20 @@ struct flag_val load_actions[] = {
        {}
 };
 
+#define XDP_FEATURE(FLAG) {#FLAG, FLAG}
+static const struct flag_val xdp_feature_flags[] = {
+       /* NETDEV_XDP features are defined in <linux/netdev.h> kernel header */
+       XDP_FEATURE(NETDEV_XDP_ACT_BASIC),
+       XDP_FEATURE(NETDEV_XDP_ACT_REDIRECT),
+       XDP_FEATURE(NETDEV_XDP_ACT_NDO_XMIT),
+       XDP_FEATURE(NETDEV_XDP_ACT_XSK_ZEROCOPY),
+       XDP_FEATURE(NETDEV_XDP_ACT_HW_OFFLOAD),
+       XDP_FEATURE(NETDEV_XDP_ACT_RX_SG),
+       XDP_FEATURE(NETDEV_XDP_ACT_NDO_XMIT_SG),
+       {NULL, 0}
+};
+#undef XDP_FEATURE
+
 static struct prog_option load_options[] = {
        DEFINE_OPTION("mode", OPT_ENUM, struct loadopt, mode,
                      .short_opt = 'm',
@@ -391,9 +405,10 @@ static struct prog_option features_options[] = {
        END_OPTIONS
 };
 
-#define CHECK_XDP_FEATURE(f)   (feature_flags & (f) ? "yes" : "no")
 static int iface_print_xdp_features(const struct iface *iface)
 {
+       const struct flag_val *flag;
+       __u64 checked_flags = 0;
        __u64 feature_flags;
        int err;
 
@@ -408,25 +423,16 @@ static int iface_print_xdp_features(const struct iface *iface)
                return err;
        }
 
-       /* NETDEV_XDP features are defined in <linux/netdev.h> kernel header */
-       printf("NETDEV_XDP_ACT_BASIC:\t\t%s\n",
-              CHECK_XDP_FEATURE(NETDEV_XDP_ACT_BASIC));
-       printf("NETDEV_XDP_ACT_REDIRECT:\t%s\n",
-              CHECK_XDP_FEATURE(NETDEV_XDP_ACT_REDIRECT));
-       printf("NETDEV_XDP_ACT_NDO_XMIT:\t%s\n",
-              CHECK_XDP_FEATURE(NETDEV_XDP_ACT_NDO_XMIT));
-       printf("NETDEV_XDP_ACT_XSK_ZEROCOPY:\t%s\n",
-              CHECK_XDP_FEATURE(NETDEV_XDP_ACT_XSK_ZEROCOPY));
-       printf("NETDEV_XDP_ACT_HW_OFFLOAD:\t%s\n",
-              CHECK_XDP_FEATURE(NETDEV_XDP_ACT_HW_OFFLOAD));
-       printf("NETDEV_XDP_ACT_RX_SG:\t\t%s\n",
-              CHECK_XDP_FEATURE(NETDEV_XDP_ACT_RX_SG));
-       printf("NETDEV_XDP_ACT_NDO_XMIT_SG:\t%s\n",
-              CHECK_XDP_FEATURE(NETDEV_XDP_ACT_NDO_XMIT_SG));
-
-       if (feature_flags & ~NETDEV_XDP_ACT_MASK)
+       for (flag = &xdp_feature_flags[0]; flag->flagstring; flag++) {
+               printf("%s:%s%s\n", flag->flagstring,
+                      (strlen(flag->flagstring) < 23 ? "\t\t" : "\t"),
+                      (feature_flags & flag->flagval ? "yes" : "no"));
+               checked_flags |= flag->flagval;
+       }
+
+       if (feature_flags & ~checked_flags)
                pr_debug("unknown reported xdp features: 0x%lx\n",
-                        (unsigned long)feature_flags & ~NETDEV_XDP_ACT_MASK);
+                        (unsigned long)(feature_flags & ~checked_flags));
 
        return 0;
 }