Re: [PATCH v2 bpf-next 5/5] selftests/bpf: Test for sk helpers in cgroup skb

[Andrey Ignatov <rdna@fb.com>]

Yonghong Song <yhs@fb.com> [Thu, 2020-05-14 09:07 -0700]:
> On 5/13/20 2:38 PM, Andrey Ignatov wrote:

> > @@ -0,0 +1,99 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2020 Facebook
> > +
> > +#include <test_progs.h>
> > +
> > +#include "network_helpers.h"
> > +#include "cgroup_skb_sk_lookup_kern.skel.h"
> > +
> > +static void run_lookup_test(int map_fd, int out_sk)
> > +{
> > +	int serv_sk = -1, in_sk = -1, serv_in_sk = -1, err;
> > +	__u32 serv_port_key = 0, duration = 0;
> > +	struct sockaddr_in6 addr = {};
> > +	socklen_t addr_len = sizeof(addr);
> > +
> > +	serv_sk = start_server(AF_INET6, SOCK_STREAM);
> > +	if (CHECK(serv_sk < 0, "start_server", "failed to start server\n"))
> > +		return;
> > +
> > +	err = getsockname(serv_sk, (struct sockaddr *)&addr, &addr_len);
> > +	if (CHECK(err, "getsockname", "errno %d\n", errno))
> > +		goto cleanup;
> > +
> > +	err = bpf_map_update_elem(map_fd, &serv_port_key, &addr.sin6_port, 0);
> > +	if (CHECK(err < 0, "map_update", "errno %d", errno))
> > +		goto cleanup;
> > +
> > +	/* Client outside of test cgroup should fail to connect by timeout. */
> > +	err = connect_fd_to_fd(out_sk, serv_sk);
> > +	if (CHECK(!err || errno != EINPROGRESS, "connect_fd_to_fd",
> > +		  "unexpected result err %d errno %d\n", err, errno))
> > +		goto cleanup;
> > +
> > +	err = connect_wait(out_sk);
> > +	if (CHECK(err, "connect_wait", "unexpected result %d\n", err))
> > +		goto cleanup;
> > +
> > +	/* Client inside test cgroup should connect just fine. */
> > +	in_sk = connect_to_fd(AF_INET6, SOCK_STREAM, serv_sk);
> > +	if (CHECK(in_sk < 0, "connect_to_fd", "errno %d\n", errno))
> > +		goto cleanup;
> > +
> > +	serv_in_sk = accept(serv_sk, NULL, NULL);
> > +	if (CHECK(serv_in_sk < 0, "accept", "errno %d\n", errno))
> > +		goto cleanup;
> > +
> > +cleanup:
> > +	close(serv_in_sk);
> > +	close(in_sk);
> > +	close(serv_sk);
> > +}
> > +
> > +static void run_cgroup_bpf_test(const char *cg_path, const char *bpf_file,
> 
> You are using skeleton, bpf_file parameter is not needed any more.

Oops, forgot to remove it while switching to skeleton. Will fix. Thanks.

> > +struct {
> > +	__uint(type, BPF_MAP_TYPE_ARRAY);
> > +	__uint(max_entries, 1);
> > +	__type(key, __u32);
> > +	__type(value, __u16);
> > +} serv_port SEC(".maps");
> 
> This can be simplified as a global variable, e.g.,
> 
> __u16 serv_port = 0;
> 
> and use skeleton to access this variable in user space
> and in kernel you can directly use this variable.

No strong preference here. Ok, will try global var.

-- 
Andrey Ignatov