RE: [PATCH v5 bpf-next 4/4] selftests/bpf: Add geneve with bpf_skb_set_tunnel_opt_dynptr test-case to test_progs

[John Fastabend <john.fastabend@gmail.com>]

Shmulik Ladkani wrote:
> Add geneve test to test_tunnel. The test setup and scheme resembles the
> existing vxlan test.
> 
> The test also checks variable length tunnel option assignment using
> bpf_skb_set_tunnel_opt_dynptr.
> 
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/test_tunnel.c    | 108 ++++++++++++++
>  .../selftests/bpf/progs/test_tunnel_kern.c    | 136 ++++++++++++++++++
>  2 files changed, 244 insertions(+)

Overall lgtm couple missing ret codes.

[...]

> +
> +SEC("tc")
> +int geneve_set_tunnel_src(struct __sk_buff *skb)
> +{
> +	int ret;
> +	struct bpf_tunnel_key key;
> +	__u32 index = 0;
> +	__u32 *local_ip = NULL;
> +
> +	local_ip = bpf_map_lookup_elem(&local_ip_map, &index);
> +	if (!local_ip) {
> +		log_err(ret);

log_err(-1)?

> +		return TC_ACT_SHOT;
> +	}
> +
> +	__builtin_memset(&key, 0x0, sizeof(key));
> +	key.local_ipv4 = *local_ip;
> +	key.remote_ipv4 = 0xac100164; /* 172.16.1.100 */
> +	key.tunnel_id = 2;
> +	key.tunnel_tos = 0;
> +	key.tunnel_ttl = 64;
> +
> +	ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
> +				     BPF_F_ZERO_CSUM_TX);
> +	if (ret < 0) {
> +		log_err(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	return TC_ACT_OK;
> +}
> +
> +SEC("tc")
> +int geneve_get_tunnel_src(struct __sk_buff *skb)
> +{
> +	int ret;
> +	struct bpf_tunnel_key key;
> +	struct tun_opts_raw opts;
> +	int expected_opts_len;
> +	__u32 index = 0;
> +	__u32 *local_ip = NULL;
> +
> +	local_ip = bpf_map_lookup_elem(&local_ip_map, &index);
> +	if (!local_ip) {
> +		log_err(ret);

log_err(-1)

> +		return TC_ACT_SHOT;
> +	}
> +
> +	ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key), 0);
> +	if (ret < 0) {
> +		log_err(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	ret = bpf_skb_get_tunnel_opt(skb, &opts, sizeof(opts));
> +	if (ret < 0) {
> +		log_err(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	expected_opts_len = (skb->len % sizeof(opts)) & ~(sizeof(__u32) - 1);
> +	if (key.local_ipv4 != *local_ip || ret != expected_opts_len) {
> +		bpf_printk("geneve key %d local ip 0x%x remote ip 0x%x opts_len %d\n",
> +			   key.tunnel_id, key.local_ipv4,
> +			   key.remote_ipv4, ret);

In general I don't really like the printks. But ok.

> +		bpf_printk("local_ip 0x%x\n", *local_ip);
> +		log_err(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	return TC_ACT_OK;
> +}
> +
>  SEC("tc")
>  int vxlan_set_tunnel_dst(struct __sk_buff *skb)
>  {
> -- 
> 2.37.2
>