| 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
| ||
|
Date: Fri, 27 Jan 2023 09:41:05 -0800
From: Stanislav Fomichev <sdf@...gle.com>
To: Lorenzo Bianconi <lorenzo@...nel.org>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...nel.org,
daniel@...earbox.net, andrii@...nel.org, davem@...emloft.net,
kuba@...nel.org, hawk@...nel.org, pabeni@...hat.com,
edumazet@...gle.com, toke@...hat.com, memxor@...il.com,
alardam@...il.com, saeedm@...dia.com, anthony.l.nguyen@...el.com,
gospo@...adcom.com, vladimir.oltean@....com, nbd@....name,
john@...ozen.org, leon@...nel.org, simon.horman@...igine.com,
aelior@...vell.com, christophe.jaillet@...adoo.fr,
ecree.xilinx@...il.com, mst@...hat.com, bjorn@...nel.org,
magnus.karlsson@...el.com, maciej.fijalkowski@...el.com,
intel-wired-lan@...ts.osuosl.org, lorenzo.bianconi@...hat.com,
martin.lau@...ux.dev
Subject: Re: [PATCH v3 bpf-next 8/8] selftests/bpf: introduce XDP compliance
test tool
On Fri, Jan 27, 2023 at 9:26 AM Lorenzo Bianconi <lorenzo@...nel.org> wrote:
>
> > On 01/26, Lorenzo Bianconi wrote:
>
> [...]
>
> >
> > Why do we need the namespaces? Why not have two veth peers in the
> > current namespace?
>
> I think we can use just a veth pair here, we do not need two, I will fix it.
>
> >
> > (not sure it matters, just wondering)
> >
> > > +ret=1
> > > +
> > > +setup() {
> > > + {
> > > + ip netns add ${NS0}
> > > + ip netns add ${NS1}
> > > +
> > > + ip link add v01 index 111 type veth peer name v00 netns ${NS0}
> > > + ip link add v10 index 222 type veth peer name v11 netns ${NS1}
> > > +
> > > + ip link set v01 up
> > > + ip addr add 10.10.0.1/24 dev v01
> > > + ip link set v01 address 00:11:22:33:44:55
> > > + ip -n ${NS0} link set dev v00 up
> > > + ip -n ${NS0} addr add 10.10.0.11/24 dev v00
> > > + ip -n ${NS0} route add default via 10.10.0.1
> > > + ip -n ${NS0} link set v00 address 00:12:22:33:44:55
> > > +
> > > + ip link set v10 up
> > > + ip addr add 10.10.1.1/24 dev v10
> > > + ip link set v10 address 00:13:22:33:44:55
> > > + ip -n ${NS1} link set dev v11 up
> > > + ip -n ${NS1} addr add 10.10.1.11/24 dev v11
> > > + ip -n ${NS1} route add default via 10.10.1.1
> > > + ip -n ${NS1} link set v11 address 00:14:22:33:44:55
> > > +
> > > + sysctl -w net.ipv4.ip_forward=1
> > > + # Enable XDP mode
> > > + ethtool -K v01 gro on
> > > + ethtool -K v01 tx-checksumming off
> > > + ip netns exec ${NS0} ethtool -K v00 gro on
> > > + ip netns exec ${NS0} ethtool -K v00 tx-checksumming off
> > > + ethtool -K v10 gro on
> > > + ethtool -K v10 tx-checksumming off
> > > + ip netns exec ${NS1} ethtool -K v11 gro on
> > > + ip netns exec ${NS1} ethtool -K v11 tx-checksumming off
> > > + } > /dev/null 2>&1
> > > +}
>
> [...]
> >
> > IIRC, Martin mentioned IPv6 support in the previous version. Should we
> > also make the userspace v6 aware by at least using AF_INET6 dualstack
> > sockets? I feel like listening on inaddr_any with AF_INET6 should
> > get us there without too much pain..
>
> ack, I will fix it.
>
> >
> > > +
> > > + /* start echo channel */
> > > + *echo_sockfd = sockfd;
> > > + err = pthread_create(t, NULL, dut_echo_thread, echo_sockfd);
> > > + if (err) {
> > > + fprintf(stderr, "Failed creating dut_echo thread: %s\n",
> > > + strerror(-err));
> > > + close(sockfd);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int dut_attach_xdp_prog(struct xdp_features *skel, int feature,
> > > + int flags)
> > > +{
> > > + struct bpf_program *prog;
> > > + unsigned int key = 0;
> > > + int err, fd = 0;
> > > +
> > > + switch (feature) {
> > > + case XDP_FEATURE_TX:
> > > + prog = skel->progs.xdp_do_tx;
> > > + break;
> > > + case XDP_FEATURE_DROP:
> > > + case XDP_FEATURE_ABORTED:
> > > + prog = skel->progs.xdp_do_drop;
> > > + break;
> > > + case XDP_FEATURE_PASS:
> > > + prog = skel->progs.xdp_do_pass;
> > > + break;
> > > + case XDP_FEATURE_NDO_XMIT: {
> > > + struct bpf_devmap_val entry = {
> > > + .ifindex = env.ifindex,
> > > + };
> > > +
> > > + err = bpf_map__update_elem(skel->maps.dev_map,
> > > + &key, sizeof(key),
> > > + &entry, sizeof(entry), 0);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + fd = bpf_program__fd(skel->progs.xdp_do_redirect_cpumap);
> > > + }
> > > + case XDP_FEATURE_REDIRECT: {
> > > + struct bpf_cpumap_val entry = {
> > > + .qsize = 2048,
> > > + .bpf_prog.fd = fd,
> > > + };
> > > +
> > > + err = bpf_map__update_elem(skel->maps.cpu_map,
> > > + &key, sizeof(key),
> > > + &entry, sizeof(entry), 0);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + prog = skel->progs.xdp_do_redirect;
> > > + break;
> > > + }
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + err = bpf_xdp_attach(env.ifindex, bpf_program__fd(prog), flags, NULL);
> > > + if (err)
> > > + fprintf(stderr,
> > > + "Failed to attach XDP program to ifindex %d\n",
> > > + env.ifindex);
> > > + return err;
> > > +}
> > > +
> > > +static int __recv_msg(int sockfd, void *buf, size_t bufsize,
> > > + unsigned int *val, unsigned int val_size)
> > > +{
> > > + struct tlv_hdr *tlv = (struct tlv_hdr *)buf;
> > > + int len, n = sizeof(*tlv), i = 0;
> > > +
> > > + len = recv(sockfd, buf, bufsize, 0);
> > > + if (len != ntohs(tlv->len))
> > > + return -EINVAL;
> > > +
> > > + while (n < len && i < val_size) {
> > > + val[i] = ntohl(tlv->data[i]);
> > > + n += sizeof(tlv->data[0]);
> > > + i++;
> > > + }
> > > +
> > > + return i;
> > > +}
> > > +
> > > +static int recv_msg(int sockfd, void *buf, size_t bufsize)
> > > +{
> > > + return __recv_msg(sockfd, buf, bufsize, NULL, 0);
> > > +}
> > > +
> > > +static int dut_run(struct xdp_features *skel)
> > > +{
> > > + int flags = XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_DRV_MODE;
> > > + int state, err, sockfd, ctrl_sockfd, echo_sockfd, optval = 1;
> > > + struct sockaddr_in ctrl_addr, addr = {
> > > + .sin_family = AF_INET,
> > > + .sin_addr.s_addr = htonl(INADDR_ANY),
> > > + .sin_port = htons(DUT_CTRL_PORT),
> > > + };
> > > + unsigned int len = sizeof(ctrl_addr);
> > > + pthread_t dut_thread;
> > > +
> >
> > [..]
> >
> > > + sockfd = socket(AF_INET, SOCK_STREAM, 0);
> > > + if (sockfd < 0) {
> > > + fprintf(stderr, "Failed to create DUT socket\n");
> > > + return -errno;
> > > + }
> > > +
> > > + err = setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &optval,
> > > + sizeof(optval));
> > > + if (err < 0) {
> > > + fprintf(stderr, "Failed sockopt on DUT socket\n");
> > > + return -errno;
> > > + }
> > > +
> > > + err = bind(sockfd, (struct sockaddr *)&addr, sizeof(addr));
> > > + if (err < 0) {
> > > + fprintf(stderr, "Failed to bind DUT socket\n");
> > > + return -errno;
> > > + }
> > > +
> > > + err = listen(sockfd, 5);
> > > + if (err) {
> > > + fprintf(stderr, "Failed to listen DUT socket\n");
> > > + return -errno;
> > > + }
> >
> > Should we use start_server from network_helpers.h here?
>
> ack, I will use it.
>
> >
> > > +
> > > + ctrl_sockfd = accept(sockfd, (struct sockaddr *)&ctrl_addr, &len);
> > > + if (ctrl_sockfd < 0) {
> > > + fprintf(stderr, "Failed to accept connection on DUT socket\n");
> > > + close(sockfd);
> > > + return -errno;
> > > + }
> > > +
>
> [...]
>
> >
> > There is also connect_to_fd, maybe we can use that? It should take
> > care of the timeouts.. (requires plumbing server_fd, not sure whether
> > it's a problem or not)
>
> please correct me if I am wrong, but in order to have server_fd it is mandatory
> both tester and DUT are running on the same process, right? Here, I guess 99% of
> the times DUT and tester will run on two separated devices. Agree?
Yes, it's targeting more the case where you have a server fd and a
bunch of clients in the same process. But I think it's still usable in
your case, you're not using fork() anywhere afaict, so even if these
are separate devices, connect_to_fd should still work. (unless I'm
missing something, haven't looked too closely)
> Regards,
> Lorenzo
>
> >
> > > +
> > > + if (i == 10) {
> > > + fprintf(stderr, "Failed to connect to the DUT\n");
> > > + return -ETIMEDOUT;
> > > + }
> > > +
> > > + err = __send_and_recv_msg(sockfd, CMD_GET_XDP_CAP, val,
> > > ARRAY_SIZE(val));
> > > + if (err < 0) {
> > > + close(sockfd);
> > > + return err;
> > > + }
> > > +
> > > + advertised_cap = tester_collect_advertised_cap(val[0]);
> > > +
> > > + err = bpf_xdp_attach(env.ifindex,
> > > + bpf_program__fd(skel->progs.xdp_tester),
> > > + flags, NULL);
> > > + if (err) {
> > > + fprintf(stderr, "Failed to attach XDP program to ifindex %d\n",
> > > + env.ifindex);
> > > + goto out;
> > > + }
> > > +
> > > + err = send_and_recv_msg(sockfd, CMD_START);
> > > + if (err)
> > > + goto out;
> > > +
> > > + for (i = 0; i < 10 && !exiting; i++) {
> > > + err = send_echo_msg();
> > > + if (err < 0)
> > > + goto out;
> > > +
> > > + sleep(1);
> > > + }
> > > +
> > > + err = __send_and_recv_msg(sockfd, CMD_GET_STATS, val, ARRAY_SIZE(val));
> > > + if (err)
> > > + goto out;
> > > +
> > > + /* stop the test */
> > > + err = send_and_recv_msg(sockfd, CMD_STOP);
> > > + /* send a new echo message to wake echo thread of the dut */
> > > + send_echo_msg();
> > > +
> > > + detected_cap = tester_collect_detected_cap(skel, val[0]);
> > > +
> > > + fprintf(stdout, "Feature %s: [%s][%s]\n",
> > > get_xdp_feature_str(env.feature),
> > > + detected_cap ? GREEN("DETECTED") : RED("NOT DETECTED"),
> > > + advertised_cap ? GREEN("ADVERTISED") : RED("NOT ADVERTISED"));
> > > +out:
> > > + bpf_xdp_detach(env.ifindex, flags, NULL);
> > > + close(sockfd);
> > > + return err < 0 ? err : 0;
> > > +}
> > > +
> > > +int main(int argc, char **argv)
> > > +{
> > > + struct xdp_features *skel;
> > > + int err;
> > > +
> > > + libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
> > > + libbpf_set_print(libbpf_print_fn);
> > > +
> > > + signal(SIGINT, sig_handler);
> > > + signal(SIGTERM, sig_handler);
> > > +
> > > + set_env_defaul();
> > > +
> > > + /* Parse command line arguments */
> > > + err = argp_parse(&argp, argc, argv, 0, NULL, NULL);
> > > + if (err)
> > > + return err;
> > > +
> > > + if (env.ifindex < 0) {
> > > + fprintf(stderr, "Invalid ifindex\n");
> > > + return -ENODEV;
> > > + }
> > > +
> > > + /* Load and verify BPF application */
> > > + skel = xdp_features__open();
> > > + if (!skel) {
> > > + fprintf(stderr, "Failed to open and load BPF skeleton\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + skel->rodata->expected_feature = env.feature;
> > > + skel->rodata->dut_ip = env.dut_ip;
> > > + skel->rodata->tester_ip = env.tester_ip;
> > > +
> > > + /* Load & verify BPF programs */
> > > + err = xdp_features__load(skel);
> > > + if (err) {
> > > + fprintf(stderr, "Failed to load and verify BPF skeleton\n");
> > > + goto cleanup;
> > > + }
> > > +
> > > + err = xdp_features__attach(skel);
> > > + if (err) {
> > > + fprintf(stderr, "Failed to attach BPF skeleton\n");
> > > + goto cleanup;
> > > + }
> > > +
> > > + if (env.tester) {
> > > + /* Tester */
> > > + fprintf(stdout, "Starting tester on device %d\n", env.ifindex);
> > > + err = tester_run(skel);
> > > + } else {
> > > + /* DUT */
> > > + fprintf(stdout, "Starting DUT on device %d\n", env.ifindex);
> > > + err = dut_run(skel);
> > > + }
> > > +
> > > +cleanup:
> > > + xdp_features__destroy(skel);
> > > +
> > > + return err < 0 ? -err : 0;
> > > +}
> > > diff --git a/tools/testing/selftests/bpf/xdp_features.h
> > > b/tools/testing/selftests/bpf/xdp_features.h
> > > new file mode 100644
> > > index 000000000000..28d7614c4f02
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/xdp_features.h
> > > @@ -0,0 +1,33 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +/* test commands */
> > > +enum test_commands {
> > > + CMD_STOP, /* CMD */
> > > + CMD_START, /* CMD + xdp feature */
> > > + CMD_ECHO, /* CMD */
> > > + CMD_ACK, /* CMD + data */
> > > + CMD_GET_XDP_CAP, /* CMD */
> > > + CMD_GET_STATS, /* CMD */
> > > +};
> > > +
> > > +#define DUT_CTRL_PORT 12345
> > > +#define DUT_ECHO_PORT 12346
> > > +
> > > +struct tlv_hdr {
> > > + __be16 type;
> > > + __be16 len;
> > > + __be32 data[];
> > > +};
> > > +
> > > +enum {
> > > + XDP_FEATURE_ABORTED,
> > > + XDP_FEATURE_DROP,
> > > + XDP_FEATURE_PASS,
> > > + XDP_FEATURE_TX,
> > > + XDP_FEATURE_REDIRECT,
> > > + XDP_FEATURE_NDO_XMIT,
> > > + XDP_FEATURE_XSK_ZEROCOPY,
> > > + XDP_FEATURE_HW_OFFLOAD,
> > > + XDP_FEATURE_RX_SG,
> > > + XDP_FEATURE_NDO_XMIT_SG,
> > > +};
> > > --
> > > 2.39.1
> >
Powered by blists - more mailing lists