xdp loader features #324

LorenzoBianconi · 2023-05-24T10:17:11Z

Add features command to xdp-loader in order to dump XDP features supported by the NIC

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

tohojo

A few comments. Also, CI is failing, PTAL...

xdp-loader/xdp-loader.c

tohojo · 2023-05-30T09:46:07Z

xdp-loader/xdp-loader.c

+static struct prog_option features_options[] = {
+	DEFINE_OPTION("dev", OPT_IFNAME, struct featuresopt, iface,
+		      .positional = true,
+		      .metavar = "[ifname]",


This should be <ifname>, since the parameter is not optional...

tohojo · 2023-05-30T09:46:20Z

xdp-loader/xdp-loader.c

+		      .positional = true,
+		      .metavar = "[ifname]",
+		      .required = true,
+		      .help = "Show XDP features for device [ifname]"),


...also here

xdp-loader/xdp-loader.8

xdp-loader/xdp-loader.c

tohojo

A few more nits, mostly UI things...

tohojo · 2023-06-01T20:39:24Z

xdp-loader/xdp-loader.8

+
+.SS "NETDEV_XDP_ACT_BASIC"
+.PP
+XDP feautues set supported by all drivers (XDP_ABORTED, XDP_DROP, XDP_PASS,


"The networking device has basic support for running XDP programs, and can handle the base set of return codes (XDP_ABORTED, XDP_DROP, XDP_PASS, XDP_TX)."

tohojo · 2023-06-01T20:40:44Z

xdp-loader/xdp-loader.8

+
+.SS "NETDEV_XDP_ACT_REDIRECT"
+.PP
+The netdev supports XDP_REDIRECT.


"The network device supports handling the XDP_REDIRECT return code. This means packets can be redirected from this device by XDP."

tohojo · 2023-06-01T20:43:19Z

xdp-loader/xdp-loader.8

+
+.SS "NETDEV_XDP_ACT_NDO_XMIT"
+.PP
+This feature informs if netdev implements ndo_xdp_xmit callback.


"The networking interfaces implements the ndo_xdp_xmit callback. This means packets can be redirected to this device by XDP."

tohojo · 2023-06-01T20:44:38Z

xdp-loader/xdp-loader.8

+
+.SS "NETDEV_XDP_ACT_XSK_ZEROCOPY"
+.PP
+This feature informs if netdev supports AF_XDP in zero copy mode.


"The networking device supports..." (let's be consistent with the phrasing)

tohojo · 2023-06-01T20:44:47Z

xdp-loader/xdp-loader.8

+
+.SS "NETDEV_XDP_ACT_HW_OFFLOAD"
+.PP
+This feature informs if netdev supports XDP hw offloading.


"The networking device supports..."

tohojo · 2023-06-01T20:47:08Z

xdp-loader/xdp-loader.8

+.SS "NETDEV_XDP_ACT_RX_SG"
+.PP
+This feature informs if netdev implements non-linear XDP buffer support in the
+driver napi callback.


"The networking device supports non-linear XDP frames on the receive side. This means XDP can be used with big MTUs on this device (if the XDP program is compiled with fragments support)."

tohojo · 2023-06-01T20:47:42Z

xdp-loader/xdp-loader.8

+.SS "NETDEV_XDP_ACT_NDO_XMIT_SG"
+.PP
+This feature informs if netdev implements non-linear XDP buffer support in
+ndo_xdp_xmit callback.


"The networking device supports non-linear XDP frames on the transmit side. This means non-linear frames can be redirected to this device."

tohojo · 2023-06-01T20:50:47Z

xdp-loader/xdp-loader.c

+#else
+	__unused const void *i = iface;
+
+	pr_debug("bpf_xdp_query_opts not supported.\n");


pr_debug is not printed by default, so this would still give no output by default. Let's make it a pr_warn, and a more friendly message, like: "Cannot display features, because xdp-loader was compiled against an old version of libbpf without support for querying features."

tohojo · 2023-06-01T20:52:35Z

lib/testing/test_runner.sh

+{
+    if ! grep -q xdp_set_features_flag /proc/kallsyms; then
+        exit "$SKIPPED_TEST"
+    fi


There's a skip_if_missing_kernel_symbol method above, let's reuse that...

tohojo · 2023-06-01T20:55:46Z

xdp-loader/xdp-loader.c

+
+	err = bpf_xdp_query(iface->ifindex, 0, &opts);
+	if (err) {
+		pr_debug("bpf_xdp_query failed (%d)\n", err);


This should be a pr_warn(). And if possible, we should special-case the error code returned by old kernels and print something friendlier (like "The running kernel doesn't support querying XDP features")

tohojo

Code LGTM with a small nit in the test script. However, I totally missed that you were editing the man page file directly. That won't work: the man page is generated by the Makefile from README.org, so the changes have to go into that, and then the man page should be regenerated.

tohojo · 2023-06-07T10:49:10Z

xdp-loader/tests/test-xdp-loader.sh

+{
+    check_run ip link add dev v0 type veth peer name v1
+
+    $XDP_LOADER features "$1" | grep "$2" | grep -q "$3"


Seems a bit odd to have the interface name passed in from the caller, since the device is created with a fixed name in the line above; i.e., any other argument than v0 as $1 will fail, so we may as well not have that argument at all...

@tohojo ack, I will fix it

LorenzoBianconi · 2023-06-07T12:26:19Z

Code LGTM with a small nit in the test script. However, I totally missed that you were editing the man page file directly. That won't work: the man page is generated by the Makefile from README.org, so the changes have to go into that, and then the man page should be regenerated.

@tohojo ack, I will fix it

tohojo · 2023-06-07T14:31:38Z

configure

@@ -283,6 +283,7 @@ check_libbpf_functions()
    check_libbpf_function "bpf_xdp_attach" "(0, 0, 0, NULL)" "" "$LIBBPF_CFLAGS" "$LIBBPF_LDLIBS"
    check_libbpf_function "bpf_map__set_autocreate" "(NULL, false)" "" "$LIBBPF_CFLAGS" "$LIBBPF_LDLIBS"
    check_libbpf_function "bpf_prog_test_run_opts" "(0, &opts)" "DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts, .batch_size = 1)" "$LIBBPF_CFLAGS" "$LIBBPF_LDLIBS"
+    check_libbpf_function "bpf_xdp_query" "(0, &opts)" "DECLARE_LIBBPF_OPTS(bpf_xdp_query_opts, opts, .feature_flags = 1)" "$LIBBPF_CFLAGS" "$LIBBPF_LDLIBS"


The function call here (second argument) is missing the flags argument, so this check always fails...

@tohojo ack, I will fix it

Add features command to xdp-loader in order to dump XDP features supported by the NIC. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

libbpf: update to v1.2.0

b6e3384

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

LorenzoBianconi force-pushed the xdp-loader-features branch 2 times, most recently from 4076c16 to f2dbadb Compare May 24, 2023 16:50

tohojo requested changes May 30, 2023

View reviewed changes

LorenzoBianconi force-pushed the xdp-loader-features branch 2 times, most recently from 85ed5d4 to bb680e2 Compare June 1, 2023 15:07

tohojo requested changes Jun 1, 2023

View reviewed changes

LorenzoBianconi force-pushed the xdp-loader-features branch from bb680e2 to d89591b Compare June 2, 2023 13:32

tohojo requested changes Jun 7, 2023

View reviewed changes

LorenzoBianconi force-pushed the xdp-loader-features branch from d89591b to 977cb03 Compare June 7, 2023 12:51

tohojo approved these changes Jun 7, 2023

View reviewed changes

tohojo requested changes Jun 7, 2023

View reviewed changes

LorenzoBianconi force-pushed the xdp-loader-features branch from 977cb03 to 5e669b5 Compare June 7, 2023 14:39

xdp-loader: introduce features command

0f41b89

Add features command to xdp-loader in order to dump XDP features supported by the NIC. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

LorenzoBianconi force-pushed the xdp-loader-features branch from 5e669b5 to 0f41b89 Compare June 7, 2023 14:41

tohojo approved these changes Jun 7, 2023

View reviewed changes

tohojo merged commit 870db97 into xdp-project:master Jun 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xdp loader features #324

xdp loader features #324

LorenzoBianconi commented May 24, 2023

tohojo left a comment

tohojo May 30, 2023

tohojo May 30, 2023

tohojo left a comment

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo Jun 1, 2023

tohojo left a comment

tohojo Jun 7, 2023

LorenzoBianconi Jun 7, 2023

LorenzoBianconi commented Jun 7, 2023

tohojo Jun 7, 2023

LorenzoBianconi Jun 7, 2023

xdp loader features #324

xdp loader features #324

Conversation

LorenzoBianconi commented May 24, 2023

tohojo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tohojo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tohojo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LorenzoBianconi commented Jun 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment