[External] [PATCH bpf-next v2 0/3] bpf: Add support to set and get

[External] [PATCH bpf-next v2 0/3] bpf: Add support to set and get

[fankaixi.li]

From: "kaixi.fan" <fankaixi.li@bytedance.com>

Now bpf code could not set tunnel source ip address of ip tunnel. So it
could not support flow based tunnel mode completely. Because flow based
tunnel mode could set tunnel source, destination ip address and tunnel 
key simultaneously.

Flow based tunnel is useful for overlay networks. And by configuring tunnel
source ip address, user could make their networks more elastic.
For example, tunnel source ip could be used to select different egress
nic interface for different flows with same tunnel destination ip. Another
example, user could choose one of multiple ip address of the egress nic
interface as the packet's tunnel source ip.

v1 -> v2:
v1: https://lore.kernel.org/bpf/20220319130538.55741-1-fankaixi.li@bytedance.com

- Add secondary ip and set tunnel remote ip in "add_vxlan_tunnel" and
"add_ip6vxlan_tunnel"

kaixi.fan (3):
  bpf: Add source ip in "struct bpf_tunnel_key"
  selftests/bpf: add ipv4 vxlan tunnel source testcase
  selftests/bpf: add ipv6 vxlan tunnel source testcase

 include/uapi/linux/bpf.h                      |   4 +
 net/core/filter.c                             |   9 ++
 tools/include/uapi/linux/bpf.h                |   4 +
 .../selftests/bpf/progs/test_tunnel_kern.c    | 115 ++++++++++++++++++
 tools/testing/selftests/bpf/test_tunnel.sh    |  80 +++++++++++-
 5 files changed, 206 insertions(+), 6 deletions(-)

-- 
2.24.3 (Apple Git-128)

[External] [PATCH bpf-next v2 1/3] bpf: Add source ip in "struct bpf_tunnel_key"

[fankaixi.li]

From: "kaixi.fan" <fankaixi.li@bytedance.com>

Add tunnel source ip field in "struct bpf_tunnel_key".
Add code in "bpf_skb_set_tunnel_key" and "bpf_skb_get_tunnel_key" to set
and get this field based on the tunnel key from "struct ip_tunnel_info".

Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
---
 include/uapi/linux/bpf.h       | 4 ++++
 net/core/filter.c              | 9 +++++++++
 tools/include/uapi/linux/bpf.h | 4 ++++
 3 files changed, 17 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4eebea830613..3007d3bc1f7a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5562,6 +5562,10 @@ struct bpf_tunnel_key {
 	__u8 tunnel_ttl;
 	__u16 tunnel_ext;	/* Padding, future use. */
 	__u32 tunnel_label;
+	union {
+		__u32 local_ipv4;
+		__u32 local_ipv6[4];
+	};
 };
 
 /* user accessible mirror of in-kernel xfrm_state.
diff --git a/net/core/filter.c b/net/core/filter.c
index 88767f7da150..cbd8471f4db4 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4498,6 +4498,7 @@ BPF_CALL_4(bpf_skb_get_tunnel_key, struct sk_buff *, skb, struct bpf_tunnel_key
 	if (unlikely(size != sizeof(struct bpf_tunnel_key))) {
 		err = -EINVAL;
 		switch (size) {
+		case offsetof(struct bpf_tunnel_key, local_ipv6[0]):
 		case offsetof(struct bpf_tunnel_key, tunnel_label):
 		case offsetof(struct bpf_tunnel_key, tunnel_ext):
 			goto set_compat;
@@ -4523,10 +4524,14 @@ BPF_CALL_4(bpf_skb_get_tunnel_key, struct sk_buff *, skb, struct bpf_tunnel_key
 	if (flags & BPF_F_TUNINFO_IPV6) {
 		memcpy(to->remote_ipv6, &info->key.u.ipv6.src,
 		       sizeof(to->remote_ipv6));
+		memcpy(to->local_ipv6, &info->key.u.ipv6.dst,
+		       sizeof(to->local_ipv6));
 		to->tunnel_label = be32_to_cpu(info->key.label);
 	} else {
 		to->remote_ipv4 = be32_to_cpu(info->key.u.ipv4.src);
 		memset(&to->remote_ipv6[1], 0, sizeof(__u32) * 3);
+		to->local_ipv4 = be32_to_cpu(info->key.u.ipv4.dst);
+		memset(&to->local_ipv6[1], 0, sizeof(__u32) * 3);
 		to->tunnel_label = 0;
 	}
 
@@ -4597,6 +4602,7 @@ BPF_CALL_4(bpf_skb_set_tunnel_key, struct sk_buff *, skb,
 		return -EINVAL;
 	if (unlikely(size != sizeof(struct bpf_tunnel_key))) {
 		switch (size) {
+		case offsetof(struct bpf_tunnel_key, local_ipv6[0]):
 		case offsetof(struct bpf_tunnel_key, tunnel_label):
 		case offsetof(struct bpf_tunnel_key, tunnel_ext):
 		case offsetof(struct bpf_tunnel_key, remote_ipv6[1]):
@@ -4639,10 +4645,13 @@ BPF_CALL_4(bpf_skb_set_tunnel_key, struct sk_buff *, skb,
 		info->mode |= IP_TUNNEL_INFO_IPV6;
 		memcpy(&info->key.u.ipv6.dst, from->remote_ipv6,
 		       sizeof(from->remote_ipv6));
+		memcpy(&info->key.u.ipv6.src, from->local_ipv6,
+		       sizeof(from->local_ipv6));
 		info->key.label = cpu_to_be32(from->tunnel_label) &
 				  IPV6_FLOWLABEL_MASK;
 	} else {
 		info->key.u.ipv4.dst = cpu_to_be32(from->remote_ipv4);
+		info->key.u.ipv4.src = cpu_to_be32(from->local_ipv4);
 	}
 
 	return 0;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4eebea830613..3007d3bc1f7a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5562,6 +5562,10 @@ struct bpf_tunnel_key {
 	__u8 tunnel_ttl;
 	__u16 tunnel_ext;	/* Padding, future use. */
 	__u32 tunnel_label;
+	union {
+		__u32 local_ipv4;
+		__u32 local_ipv6[4];
+	};
 };
 
 /* user accessible mirror of in-kernel xfrm_state.
-- 
2.24.3 (Apple Git-128)

Re: [External] [PATCH bpf-next v2 1/3] bpf: Add source ip in "struct bpf_tunnel_key"

[Yonghong Song]

On 3/22/22 8:42 AM, fankaixi.li@bytedance.com wrote:
> From: "kaixi.fan" <fankaixi.li@bytedance.com>
> 
> Add tunnel source ip field in "struct bpf_tunnel_key".
> Add code in "bpf_skb_set_tunnel_key" and "bpf_skb_get_tunnel_key" to set
> and get this field based on the tunnel key from "struct ip_tunnel_info".
> 
> Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>

Could you use proper name, e.g., "Kaixi Fan", in Signed-off-by tag?

> ---
>   include/uapi/linux/bpf.h       | 4 ++++
>   net/core/filter.c              | 9 +++++++++
>   tools/include/uapi/linux/bpf.h | 4 ++++
>   3 files changed, 17 insertions(+)
> 
[...]

[External] [PATCH bpf-next v2 2/3] selftests/bpf: add ipv4 vxlan tunnel source testcase

[fankaixi.li]

From: "kaixi.fan" <fankaixi.li@bytedance.com>

Vxlan tunnel is chosen to test bpf code could configure tunnel
source ipv4 address. It's sufficient to prove that other types
tunnels could also do it.
In the vxlan tunnel testcase, two underlay ipv4 addresses
are configured on veth device in root namespace. Test bpf kernel
code would configure the secondary ipv4 address as the tunnel
source ip.

Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
---
 .../selftests/bpf/progs/test_tunnel_kern.c    | 64 +++++++++++++++++++
 tools/testing/selftests/bpf/test_tunnel.sh    | 37 ++++++++++-
 2 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
index ef0dde83b85a..ab635c55ae9b 100644
--- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
@@ -676,4 +676,68 @@ int _xfrm_get_state(struct __sk_buff *skb)
 	return TC_ACT_OK;
 }
 
+SEC("vxlan_set_tunnel_src")
+int _vxlan_set_tunnel_src(struct __sk_buff *skb)
+{
+	int ret;
+	struct bpf_tunnel_key key;
+	struct vxlan_metadata md;
+
+	__builtin_memset(&key, 0x0, sizeof(key));
+	key.local_ipv4 = 0xac100114; /* 172.16.1.20 */
+	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) {
+		ERROR(ret);
+		return TC_ACT_SHOT;
+	}
+
+	md.gbp = 0x800FF; /* Set VXLAN Group Policy extension */
+	ret = bpf_skb_set_tunnel_opt(skb, &md, sizeof(md));
+	if (ret < 0) {
+		ERROR(ret);
+		return TC_ACT_SHOT;
+	}
+
+	return TC_ACT_OK;
+}
+
+SEC("vxlan_get_tunnel_src")
+int _vxlan_get_tunnel_src(struct __sk_buff *skb)
+{
+	int ret;
+	struct bpf_tunnel_key key;
+	struct vxlan_metadata md;
+	char fmt[] = "key %d remote ip 0x%x source ip 0x%x\n";
+	char fmt2[] = "vxlan gbp 0x%x\n";
+
+	ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key), 0);
+	if (ret < 0) {
+		ERROR(ret);
+		return TC_ACT_SHOT;
+	}
+
+	ret = bpf_skb_get_tunnel_opt(skb, &md, sizeof(md));
+	if (ret < 0) {
+		ERROR(ret);
+		return TC_ACT_SHOT;
+	}
+
+	bpf_trace_printk(fmt, sizeof(fmt),
+			 key.tunnel_id, key.remote_ipv4, key.local_ipv4);
+	bpf_trace_printk(fmt2, sizeof(fmt2),
+			 md.gbp);
+
+	if (key.local_ipv4 != 0xac100114) {
+		ERROR(ret);
+		return TC_ACT_SHOT;
+	}
+	return TC_ACT_OK;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_tunnel.sh b/tools/testing/selftests/bpf/test_tunnel.sh
index ca1372924023..b6923392bf16 100755
--- a/tools/testing/selftests/bpf/test_tunnel.sh
+++ b/tools/testing/selftests/bpf/test_tunnel.sh
@@ -161,10 +161,15 @@ add_vxlan_tunnel()
 	# on L3 packet, as a result not applying to ARP packets,
 	# causing errors at get_tunnel_{key/opt}.
 
+	# add a secondary ip
+	if [ "$2" == "2" ]; then
+		ip addr add dev veth1 172.16.1.20/24
+	fi
+
 	# at_ns0 namespace
 	ip netns exec at_ns0 \
 		ip link add dev $DEV_NS type $TYPE \
-		id 2 dstport 4789 gbp remote 172.16.1.200
+		id 2 dstport 4789 gbp remote $1
 	ip netns exec at_ns0 \
 		ip link set dev $DEV_NS address 52:54:00:d9:01:00 up
 	ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
@@ -412,7 +417,7 @@ test_vxlan()
 
 	check $TYPE
 	config_device
-	add_vxlan_tunnel
+	add_vxlan_tunnel 172.16.1.200 1
 	attach_bpf $DEV vxlan_set_tunnel vxlan_get_tunnel
 	ping $PING_ARG 10.1.1.100
 	check_err $?
@@ -661,6 +666,30 @@ test_xfrm_tunnel()
 	echo -e ${GREEN}"PASS: xfrm tunnel"${NC}
 }
 
+test_vxlan_tunsrc()
+{
+	TYPE=vxlan
+	DEV_NS=vxlan00
+	DEV=vxlan11
+	ret=0
+
+	check $TYPE
+	config_device
+	add_vxlan_tunnel 172.16.1.20 2
+	attach_bpf $DEV vxlan_set_tunnel_src vxlan_get_tunnel_src
+	ping $PING_ARG 10.1.1.100
+	check_err $?
+	ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
+	check_err $?
+	cleanup
+
+	if [ $ret -ne 0 ]; then
+                echo -e ${RED}"FAIL: ${TYPE}_tunsrc"${NC}
+                return 1
+        fi
+        echo -e ${GREEN}"PASS: ${TYPE}_tunsrc"${NC}
+}
+
 attach_bpf()
 {
 	DEV=$1
@@ -782,6 +811,10 @@ bpf_tunnel_test()
 	test_xfrm_tunnel
 	errors=$(( $errors + $? ))
 
+	echo "Testing VXLAN tunnel source..."
+	test_vxlan_tunsrc
+	errors=$(( $errors + $? ))
+
 	return $errors
 }
 
-- 
2.24.3 (Apple Git-128)

Re: [External] [PATCH bpf-next v2 2/3] selftests/bpf: add ipv4 vxlan tunnel source testcase

[Yonghong Song]

On 3/22/22 8:42 AM, fankaixi.li@bytedance.com wrote:
> From: "kaixi.fan" <fankaixi.li@bytedance.com>
> 
> Vxlan tunnel is chosen to test bpf code could configure tunnel
> source ipv4 address.

The added test configures tunnel source ipv4 address.

 >It's sufficient to prove that other types
> tunnels could also do it.

Could you be more specific what other types will also use source ipv4 
address. It is too vague to claim "it's sufficient to prove ...".


> In the vxlan tunnel testcase, two underlay ipv4 addresses
> are configured on veth device in root namespace. Test bpf kernel
> code would configure the secondary ipv4 address as the tunnel
> source ip.
> 
> Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>

Again, please use proper name in Signed-off-by tag.

> ---
>   .../selftests/bpf/progs/test_tunnel_kern.c    | 64 +++++++++++++++++++
>   tools/testing/selftests/bpf/test_tunnel.sh    | 37 ++++++++++-
>   2 files changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> index ef0dde83b85a..ab635c55ae9b 100644
> --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> @@ -676,4 +676,68 @@ int _xfrm_get_state(struct __sk_buff *skb)
>   	return TC_ACT_OK;
>   }
>   
> +SEC("vxlan_set_tunnel_src")
> +int _vxlan_set_tunnel_src(struct __sk_buff *skb)
> +{
> +	int ret;
> +	struct bpf_tunnel_key key;
> +	struct vxlan_metadata md;
> +
> +	__builtin_memset(&key, 0x0, sizeof(key));
> +	key.local_ipv4 = 0xac100114; /* 172.16.1.20 */
> +	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) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	md.gbp = 0x800FF; /* Set VXLAN Group Policy extension */
> +	ret = bpf_skb_set_tunnel_opt(skb, &md, sizeof(md));
> +	if (ret < 0) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	return TC_ACT_OK;
> +}
> +
> +SEC("vxlan_get_tunnel_src")
> +int _vxlan_get_tunnel_src(struct __sk_buff *skb)
> +{
> +	int ret;
> +	struct bpf_tunnel_key key;
> +	struct vxlan_metadata md;
> +	char fmt[] = "key %d remote ip 0x%x source ip 0x%x\n";
> +	char fmt2[] = "vxlan gbp 0x%x\n";
> +
> +	ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key), 0);
> +	if (ret < 0) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	ret = bpf_skb_get_tunnel_opt(skb, &md, sizeof(md));
> +	if (ret < 0) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	bpf_trace_printk(fmt, sizeof(fmt),
> +			 key.tunnel_id, key.remote_ipv4, key.local_ipv4);
> +	bpf_trace_printk(fmt2, sizeof(fmt2),

In bpf_helpers.h, bpf_printk can be used instead of bpf_trace_printk.

> +			 md.gbp);
> +
> +	if (key.local_ipv4 != 0xac100114) {

I would suggest to make 0xac100114 a macro, so both set_* and get_* 
programs can use the same macro, which makes it easier to understand
and check.

> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +	return TC_ACT_OK;
> +}
> +
[...]

Re: [External] [PATCH bpf-next v2 2/3] selftests/bpf: add ipv4 vxlan tunnel source testcase

[范开喜]

Yonghong Song <yhs@fb.com> 于2022年3月26日周六 00:41写道：
>
>
>
> On 3/22/22 8:42 AM, fankaixi.li@bytedance.com wrote:
> > From: "kaixi.fan" <fankaixi.li@bytedance.com>
> >
> > Vxlan tunnel is chosen to test bpf code could configure tunnel
> > source ipv4 address.
>
> The added test configures tunnel source ipv4 address.
>
>  >It's sufficient to prove that other types
> > tunnels could also do it.
>
> Could you be more specific what other types will also use source ipv4
> address. It is too vague to claim "it's sufficient to prove ...".
>

Is it better to add more test cases for other types of ip tunnels ? It would
introduce more duplicate codes.

In the kernel, this is referred to as collect metadata mode as follows:
https://man7.org/linux/man-pages/man8/ip-link.8.html
Kernel use "struct ip_tunnel_info" to save tunnel parameters, and use
it for tunnel encapsulation. The process is similar for vxlan, gre,/gretap,
geneve, ipip and erspan tunnels.
The previous test cases in "test_tunnel.sh" test this mechanism for bpf
program code already.  Based on this mechanism, I just use vxlan tunnel
to test tunnel source ip configuration.

>
> > In the vxlan tunnel testcase, two underlay ipv4 addresses
> > are configured on veth device in root namespace. Test bpf kernel
> > code would configure the secondary ipv4 address as the tunnel
> > source ip.
> >
> > Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
>
> Again, please use proper name in Signed-off-by tag.

Thanks. I will fix it.

>
> > ---
> >   .../selftests/bpf/progs/test_tunnel_kern.c    | 64 +++++++++++++++++++
> >   tools/testing/selftests/bpf/test_tunnel.sh    | 37 ++++++++++-
> >   2 files changed, 99 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > index ef0dde83b85a..ab635c55ae9b 100644
> > --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > @@ -676,4 +676,68 @@ int _xfrm_get_state(struct __sk_buff *skb)
> >       return TC_ACT_OK;
> >   }
> >
> > +SEC("vxlan_set_tunnel_src")
> > +int _vxlan_set_tunnel_src(struct __sk_buff *skb)
> > +{
> > +     int ret;
> > +     struct bpf_tunnel_key key;
> > +     struct vxlan_metadata md;
> > +
> > +     __builtin_memset(&key, 0x0, sizeof(key));
> > +     key.local_ipv4 = 0xac100114; /* 172.16.1.20 */
> > +     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) {
> > +             ERROR(ret);
> > +             return TC_ACT_SHOT;
> > +     }
> > +
> > +     md.gbp = 0x800FF; /* Set VXLAN Group Policy extension */
> > +     ret = bpf_skb_set_tunnel_opt(skb, &md, sizeof(md));
> > +     if (ret < 0) {
> > +             ERROR(ret);
> > +             return TC_ACT_SHOT;
> > +     }
> > +
> > +     return TC_ACT_OK;
> > +}
> > +
> > +SEC("vxlan_get_tunnel_src")
> > +int _vxlan_get_tunnel_src(struct __sk_buff *skb)
> > +{
> > +     int ret;
> > +     struct bpf_tunnel_key key;
> > +     struct vxlan_metadata md;
> > +     char fmt[] = "key %d remote ip 0x%x source ip 0x%x\n";
> > +     char fmt2[] = "vxlan gbp 0x%x\n";
> > +
> > +     ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key), 0);
> > +     if (ret < 0) {
> > +             ERROR(ret);
> > +             return TC_ACT_SHOT;
> > +     }
> > +
> > +     ret = bpf_skb_get_tunnel_opt(skb, &md, sizeof(md));
> > +     if (ret < 0) {
> > +             ERROR(ret);
> > +             return TC_ACT_SHOT;
> > +     }
> > +
> > +     bpf_trace_printk(fmt, sizeof(fmt),
> > +                      key.tunnel_id, key.remote_ipv4, key.local_ipv4);
> > +     bpf_trace_printk(fmt2, sizeof(fmt2),
>
> In bpf_helpers.h, bpf_printk can be used instead of bpf_trace_printk.
>
> > +                      md.gbp);
> > +
> > +     if (key.local_ipv4 != 0xac100114) {
>
> I would suggest to make 0xac100114 a macro, so both set_* and get_*
> programs can use the same macro, which makes it easier to understand
> and check.

OK. I will replace it with a macro.

>
> > +             ERROR(ret);
> > +             return TC_ACT_SHOT;
> > +     }
> > +     return TC_ACT_OK;
> > +}
> > +
> [...]

Re: [External] [PATCH bpf-next v2 2/3] selftests/bpf: add ipv4 vxlan tunnel source testcase

[Yonghong Song]

On 3/26/22 10:04 AM, 范开喜 wrote:
> Yonghong Song <yhs@fb.com> 于2022年3月26日周六 00:41写道：
>>
>>
>>
>> On 3/22/22 8:42 AM, fankaixi.li@bytedance.com wrote:
>>> From: "kaixi.fan" <fankaixi.li@bytedance.com>
>>>
>>> Vxlan tunnel is chosen to test bpf code could configure tunnel
>>> source ipv4 address.
>>
>> The added test configures tunnel source ipv4 address.
>>
>>   >It's sufficient to prove that other types
>>> tunnels could also do it.
>>
>> Could you be more specific what other types will also use source ipv4
>> address. It is too vague to claim "it's sufficient to prove ...".
>>
> 
> Is it better to add more test cases for other types of ip tunnels ? It would
> introduce more duplicate codes.
> 
> In the kernel, this is referred to as collect metadata mode as follows:
> https://man7.org/linux/man-pages/man8/ip-link.8.html
> Kernel use "struct ip_tunnel_info" to save tunnel parameters, and use
> it for tunnel encapsulation. The process is similar for vxlan, gre,/gretap,
> geneve, ipip and erspan tunnels.
> The previous test cases in "test_tunnel.sh" test this mechanism for bpf
> program code already.  Based on this mechanism, I just use vxlan tunnel
> to test tunnel source ip configuration.

You can just mention something like:
   Other type of tunnels, e.g., gre, gretap, geneve, ipip, erspan, etc, 
can also configure tunnel source ip addresses.

> 
>>
>>> In the vxlan tunnel testcase, two underlay ipv4 addresses
>>> are configured on veth device in root namespace. Test bpf kernel
>>> code would configure the secondary ipv4 address as the tunnel
>>> source ip.
>>>
>>> Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
>>
>> Again, please use proper name in Signed-off-by tag.
> 
> Thanks. I will fix it.
> 
[...]

Re: [External] [PATCH bpf-next v2 2/3] selftests/bpf: add ipv4 vxlan tunnel source testcase

[范开喜]

Yonghong Song <yhs@fb.com> 于2022年3月29日周二 07:10写道：
>
>
>
> On 3/26/22 10:04 AM, 范开喜 wrote:
> > Yonghong Song <yhs@fb.com> 于2022年3月26日周六 00:41写道：
> >>
> >>
> >>
> >> On 3/22/22 8:42 AM, fankaixi.li@bytedance.com wrote:
> >>> From: "kaixi.fan" <fankaixi.li@bytedance.com>
> >>>
> >>> Vxlan tunnel is chosen to test bpf code could configure tunnel
> >>> source ipv4 address.
> >>
> >> The added test configures tunnel source ipv4 address.
> >>
> >>   >It's sufficient to prove that other types
> >>> tunnels could also do it.
> >>
> >> Could you be more specific what other types will also use source ipv4
> >> address. It is too vague to claim "it's sufficient to prove ...".
> >>
> >
> > Is it better to add more test cases for other types of ip tunnels ? It would
> > introduce more duplicate codes.
> >
> > In the kernel, this is referred to as collect metadata mode as follows:
> > https://man7.org/linux/man-pages/man8/ip-link.8.html
> > Kernel use "struct ip_tunnel_info" to save tunnel parameters, and use
> > it for tunnel encapsulation. The process is similar for vxlan, gre,/gretap,
> > geneve, ipip and erspan tunnels.
> > The previous test cases in "test_tunnel.sh" test this mechanism for bpf
> > program code already.  Based on this mechanism, I just use vxlan tunnel
> > to test tunnel source ip configuration.
>
> You can just mention something like:
>    Other type of tunnels, e.g., gre, gretap, geneve, ipip, erspan, etc,
> can also configure tunnel source ip addresses.
>

Thanks. It's more clear as you say !

> >
> >>
> >>> In the vxlan tunnel testcase, two underlay ipv4 addresses
> >>> are configured on veth device in root namespace. Test bpf kernel
> >>> code would configure the secondary ipv4 address as the tunnel
> >>> source ip.
> >>>
> >>> Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
> >>
> >> Again, please use proper name in Signed-off-by tag.
> >
> > Thanks. I will fix it.
> >
> [...]

[External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[fankaixi.li]

From: "kaixi.fan" <fankaixi.li@bytedance.com>

Add two ipv6 address on underlay nic interface, and use bpf code to
configure the secondary ipv6 address as the vxlan tunnel source ip.
Then check ping6 result and log contains the correct tunnel source
ip.

Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
---
 .../selftests/bpf/progs/test_tunnel_kern.c    | 51 +++++++++++++++++++
 tools/testing/selftests/bpf/test_tunnel.sh    | 43 ++++++++++++++--
 2 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
index ab635c55ae9b..56e1aee0ba5a 100644
--- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
@@ -740,4 +740,55 @@ int _vxlan_get_tunnel_src(struct __sk_buff *skb)
 	return TC_ACT_OK;
 }
 
+SEC("ip6vxlan_set_tunnel_src")
+int _ip6vxlan_set_tunnel_src(struct __sk_buff *skb)
+{
+	struct bpf_tunnel_key key;
+	int ret;
+
+	__builtin_memset(&key, 0x0, sizeof(key));
+	key.local_ipv6[3] = bpf_htonl(0xbb); /* ::bb */
+	key.remote_ipv6[3] = bpf_htonl(0x11); /* ::11 */
+	key.tunnel_id = 22;
+	key.tunnel_tos = 0;
+	key.tunnel_ttl = 64;
+
+	ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
+				     BPF_F_TUNINFO_IPV6);
+	if (ret < 0) {
+		ERROR(ret);
+		return TC_ACT_SHOT;
+	}
+
+	return TC_ACT_OK;
+}
+
+SEC("ip6vxlan_get_tunnel_src")
+int _ip6vxlan_get_tunnel_src(struct __sk_buff *skb)
+{
+	char fmt[] = "key %d remote ip6 ::%x source ip6 ::%x\n";
+	char fmt2[] = "label %x\n";
+	struct bpf_tunnel_key key;
+	int ret;
+
+	ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key),
+				     BPF_F_TUNINFO_IPV6);
+	if (ret < 0) {
+		ERROR(ret);
+		return TC_ACT_SHOT;
+	}
+
+	bpf_trace_printk(fmt, sizeof(fmt),
+			 key.tunnel_id, key.remote_ipv6[3], key.local_ipv6[3]);
+	bpf_trace_printk(fmt2, sizeof(fmt2),
+			 key.tunnel_label);
+
+	if (bpf_ntohl(key.local_ipv6[3]) != 0xbb) {
+		ERROR(ret);
+		return TC_ACT_SHOT;
+	}
+
+	return TC_ACT_OK;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_tunnel.sh b/tools/testing/selftests/bpf/test_tunnel.sh
index b6923392bf16..4b7bf9c7bbe1 100755
--- a/tools/testing/selftests/bpf/test_tunnel.sh
+++ b/tools/testing/selftests/bpf/test_tunnel.sh
@@ -191,12 +191,15 @@ add_ip6vxlan_tunnel()
 	ip netns exec at_ns0 ip link set dev veth0 up
 	#ip -4 addr del 172.16.1.200 dev veth1
 	ip -6 addr add dev veth1 ::22/96
+	if [ "$2" == "2" ]; then
+		ip -6 addr add dev veth1 ::bb/96
+	fi
 	ip link set dev veth1 up
 
 	# at_ns0 namespace
 	ip netns exec at_ns0 \
 		ip link add dev $DEV_NS type $TYPE id 22 dstport 4789 \
-		local ::11 remote ::22
+		local ::11 remote $1
 	ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
 	ip netns exec at_ns0 ip link set dev $DEV_NS up
 
@@ -231,7 +234,7 @@ add_ip6geneve_tunnel()
 	# at_ns0 namespace
 	ip netns exec at_ns0 \
 		ip link add dev $DEV_NS type $TYPE id 22 \
-		remote ::22     # geneve has no local option
+		remote ::22    # geneve has no local option
 	ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
 	ip netns exec at_ns0 ip link set dev $DEV_NS up
 
@@ -394,7 +397,7 @@ test_ip6erspan()
 
 	check $TYPE
 	config_device
-	add_ip6erspan_tunnel $1
+	add_ip6erspan_tunnel
 	attach_bpf $DEV ip4ip6erspan_set_tunnel ip4ip6erspan_get_tunnel
 	ping6 $PING_ARG ::11
 	ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
@@ -441,7 +444,7 @@ test_ip6vxlan()
 
 	check $TYPE
 	config_device
-	add_ip6vxlan_tunnel
+	add_ip6vxlan_tunnel ::22 1
 	ip link set dev veth1 mtu 1500
 	attach_bpf $DEV ip6vxlan_set_tunnel ip6vxlan_get_tunnel
 	# underlay
@@ -690,6 +693,34 @@ test_vxlan_tunsrc()
         echo -e ${GREEN}"PASS: ${TYPE}_tunsrc"${NC}
 }
 
+test_ip6vxlan_tunsrc()
+{
+	TYPE=vxlan
+	DEV_NS=ip6vxlan00
+	DEV=ip6vxlan11
+	ret=0
+
+	check $TYPE
+	config_device
+	add_ip6vxlan_tunnel ::bb 2
+	ip link set dev veth1 mtu 1500
+	attach_bpf $DEV ip6vxlan_set_tunnel_src ip6vxlan_get_tunnel_src
+	# underlay
+	ping6 $PING_ARG ::11
+	# ip4 over ip6
+	ping $PING_ARG 10.1.1.100
+	check_err $?
+	ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
+	check_err $?
+	cleanup
+
+	if [ $ret -ne 0 ]; then
+                echo -e ${RED}"FAIL: ip6${TYPE}_tunsrc"${NC}
+                return 1
+        fi
+        echo -e ${GREEN}"PASS: ip6${TYPE}_tunsrc"${NC}
+}
+
 attach_bpf()
 {
 	DEV=$1
@@ -815,6 +846,10 @@ bpf_tunnel_test()
 	test_vxlan_tunsrc
 	errors=$(( $errors + $? ))
 
+	echo "Testing IP6VXLAN tunnel source..."
+	test_ip6vxlan_tunsrc
+	errors=$(( $errors + $? ))
+
 	return $errors
 }
 
-- 
2.24.3 (Apple Git-128)

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[Martin KaFai Lau]

On Tue, Mar 22, 2022 at 11:42:31PM +0800, fankaixi.li@bytedance.com wrote:
> From: "kaixi.fan" <fankaixi.li@bytedance.com>
> 
> Add two ipv6 address on underlay nic interface, and use bpf code to
> configure the secondary ipv6 address as the vxlan tunnel source ip.
> Then check ping6 result and log contains the correct tunnel source
> ip.
> 
> Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
> ---
>  .../selftests/bpf/progs/test_tunnel_kern.c    | 51 +++++++++++++++++++
>  tools/testing/selftests/bpf/test_tunnel.sh    | 43 ++++++++++++++--
>  2 files changed, 90 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> index ab635c55ae9b..56e1aee0ba5a 100644
> --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> @@ -740,4 +740,55 @@ int _vxlan_get_tunnel_src(struct __sk_buff *skb)
>  	return TC_ACT_OK;
>  }
>  
> +SEC("ip6vxlan_set_tunnel_src")
> +int _ip6vxlan_set_tunnel_src(struct __sk_buff *skb)
> +{
> +	struct bpf_tunnel_key key;
> +	int ret;
> +
> +	__builtin_memset(&key, 0x0, sizeof(key));
> +	key.local_ipv6[3] = bpf_htonl(0xbb); /* ::bb */
> +	key.remote_ipv6[3] = bpf_htonl(0x11); /* ::11 */
> +	key.tunnel_id = 22;
> +	key.tunnel_tos = 0;
> +	key.tunnel_ttl = 64;
> +
> +	ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
> +				     BPF_F_TUNINFO_IPV6);
> +	if (ret < 0) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	return TC_ACT_OK;
> +}
> +
> +SEC("ip6vxlan_get_tunnel_src")
> +int _ip6vxlan_get_tunnel_src(struct __sk_buff *skb)
> +{
> +	char fmt[] = "key %d remote ip6 ::%x source ip6 ::%x\n";
> +	char fmt2[] = "label %x\n";
> +	struct bpf_tunnel_key key;
> +	int ret;
> +
> +	ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key),
> +				     BPF_F_TUNINFO_IPV6);
> +	if (ret < 0) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	bpf_trace_printk(fmt, sizeof(fmt),
> +			 key.tunnel_id, key.remote_ipv6[3], key.local_ipv6[3]);
> +	bpf_trace_printk(fmt2, sizeof(fmt2),
> +			 key.tunnel_label);
Going back to the same comment on v1.

How is this printk used?
Especially for the most common test PASS case,
afaict, this will be ignored and left in the trace buffer?

Can it only printk when it detects error? like only print
when it fails the != 0xbb test case below?

Also, a nit, try to use bpf_printk() instead.

Some existing tunnel tests have printk but those are older examples
when test_progs.c was not ready.  In the future, we may want
to move these tunnel tests to test_progs.c where most of the tests
don't printk/grep also and use global variables or map to check and
only printF on unexpected values.  Thus, it may as well
have this new test steering into this direction in term
of only print on error.

> +
> +	if (bpf_ntohl(key.local_ipv6[3]) != 0xbb) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	return TC_ACT_OK;
> +}
> +
>  char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/test_tunnel.sh b/tools/testing/selftests/bpf/test_tunnel.sh
> index b6923392bf16..4b7bf9c7bbe1 100755
> --- a/tools/testing/selftests/bpf/test_tunnel.sh
> +++ b/tools/testing/selftests/bpf/test_tunnel.sh
> @@ -191,12 +191,15 @@ add_ip6vxlan_tunnel()
>  	ip netns exec at_ns0 ip link set dev veth0 up
>  	#ip -4 addr del 172.16.1.200 dev veth1
>  	ip -6 addr add dev veth1 ::22/96
> +	if [ "$2" == "2" ]; then
> +		ip -6 addr add dev veth1 ::bb/96
Testing $1 is not "::22" is as good as another $2 arg and
then $2 is not needed?

> +	fi
>  	ip link set dev veth1 up
>  
>  	# at_ns0 namespace
>  	ip netns exec at_ns0 \
>  		ip link add dev $DEV_NS type $TYPE id 22 dstport 4789 \
> -		local ::11 remote ::22
> +		local ::11 remote $1
>  	ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
>  	ip netns exec at_ns0 ip link set dev $DEV_NS up
>  
> @@ -231,7 +234,7 @@ add_ip6geneve_tunnel()
>  	# at_ns0 namespace
>  	ip netns exec at_ns0 \
>  		ip link add dev $DEV_NS type $TYPE id 22 \
> -		remote ::22     # geneve has no local option
> +		remote ::22    # geneve has no local option
Unnecessary space change.  Please try to avoid.  This distracts
the code review.

>  	ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
>  	ip netns exec at_ns0 ip link set dev $DEV_NS up
>  
> @@ -394,7 +397,7 @@ test_ip6erspan()
>  
>  	check $TYPE
>  	config_device
> -	add_ip6erspan_tunnel $1
> +	add_ip6erspan_tunnel
What is this change for?
How is the patch set related to the test_ip6erspan()?

or it is an unrelated clean up? If it is, please use another patch
in its own cleanup patch set.

>  	attach_bpf $DEV ip4ip6erspan_set_tunnel ip4ip6erspan_get_tunnel
>  	ping6 $PING_ARG ::11
>  	ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
> @@ -441,7 +444,7 @@ test_ip6vxlan()
>  
>  	check $TYPE
>  	config_device
> -	add_ip6vxlan_tunnel
> +	add_ip6vxlan_tunnel ::22 1
>  	ip link set dev veth1 mtu 1500
>  	attach_bpf $DEV ip6vxlan_set_tunnel ip6vxlan_get_tunnel
>  	# underlay
> @@ -690,6 +693,34 @@ test_vxlan_tunsrc()
>          echo -e ${GREEN}"PASS: ${TYPE}_tunsrc"${NC}
>  }
>  
> +test_ip6vxlan_tunsrc()
> +{
> +	TYPE=vxlan
> +	DEV_NS=ip6vxlan00
> +	DEV=ip6vxlan11
> +	ret=0
> +
> +	check $TYPE
> +	config_device
> +	add_ip6vxlan_tunnel ::bb 2
> +	ip link set dev veth1 mtu 1500
> +	attach_bpf $DEV ip6vxlan_set_tunnel_src ip6vxlan_get_tunnel_src
> +	# underlay
> +	ping6 $PING_ARG ::11
> +	# ip4 over ip6
> +	ping $PING_ARG 10.1.1.100
> +	check_err $?
> +	ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
> +	check_err $?
> +	cleanup
> +
> +	if [ $ret -ne 0 ]; then
> +                echo -e ${RED}"FAIL: ip6${TYPE}_tunsrc"${NC}
> +                return 1
> +        fi
> +        echo -e ${GREEN}"PASS: ip6${TYPE}_tunsrc"${NC}
> +}
> +
>  attach_bpf()
>  {
>  	DEV=$1
> @@ -815,6 +846,10 @@ bpf_tunnel_test()
>  	test_vxlan_tunsrc
>  	errors=$(( $errors + $? ))
>  
> +	echo "Testing IP6VXLAN tunnel source..."
> +	test_ip6vxlan_tunsrc
> +	errors=$(( $errors + $? ))
> +
>  	return $errors
>  }
>  
> -- 
> 2.24.3 (Apple Git-128)
> 

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[范开喜]

Martin KaFai Lau <kafai@fb.com> 于2022年3月25日周五 03:38写道：
>
> On Tue, Mar 22, 2022 at 11:42:31PM +0800, fankaixi.li@bytedance.com wrote:
> > From: "kaixi.fan" <fankaixi.li@bytedance.com>
> >
> > Add two ipv6 address on underlay nic interface, and use bpf code to
> > configure the secondary ipv6 address as the vxlan tunnel source ip.
> > Then check ping6 result and log contains the correct tunnel source
> > ip.
> >
> > Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
> > ---
> >  .../selftests/bpf/progs/test_tunnel_kern.c    | 51 +++++++++++++++++++
> >  tools/testing/selftests/bpf/test_tunnel.sh    | 43 ++++++++++++++--
> >  2 files changed, 90 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > index ab635c55ae9b..56e1aee0ba5a 100644
> > --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > @@ -740,4 +740,55 @@ int _vxlan_get_tunnel_src(struct __sk_buff *skb)
> >       return TC_ACT_OK;
> >  }
> >
> > +SEC("ip6vxlan_set_tunnel_src")
> > +int _ip6vxlan_set_tunnel_src(struct __sk_buff *skb)
> > +{
> > +     struct bpf_tunnel_key key;
> > +     int ret;
> > +
> > +     __builtin_memset(&key, 0x0, sizeof(key));
> > +     key.local_ipv6[3] = bpf_htonl(0xbb); /* ::bb */
> > +     key.remote_ipv6[3] = bpf_htonl(0x11); /* ::11 */
> > +     key.tunnel_id = 22;
> > +     key.tunnel_tos = 0;
> > +     key.tunnel_ttl = 64;
> > +
> > +     ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
> > +                                  BPF_F_TUNINFO_IPV6);
> > +     if (ret < 0) {
> > +             ERROR(ret);
> > +             return TC_ACT_SHOT;
> > +     }
> > +
> > +     return TC_ACT_OK;
> > +}
> > +
> > +SEC("ip6vxlan_get_tunnel_src")
> > +int _ip6vxlan_get_tunnel_src(struct __sk_buff *skb)
> > +{
> > +     char fmt[] = "key %d remote ip6 ::%x source ip6 ::%x\n";
> > +     char fmt2[] = "label %x\n";
> > +     struct bpf_tunnel_key key;
> > +     int ret;
> > +
> > +     ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key),
> > +                                  BPF_F_TUNINFO_IPV6);
> > +     if (ret < 0) {
> > +             ERROR(ret);
> > +             return TC_ACT_SHOT;
> > +     }
> > +
> > +     bpf_trace_printk(fmt, sizeof(fmt),
> > +                      key.tunnel_id, key.remote_ipv6[3], key.local_ipv6[3]);
> > +     bpf_trace_printk(fmt2, sizeof(fmt2),
> > +                      key.tunnel_label);
> Going back to the same comment on v1.
>
> How is this printk used?
> Especially for the most common test PASS case,
> afaict, this will be ignored and left in the trace buffer?
>
> Can it only printk when it detects error? like only print
> when it fails the != 0xbb test case below?
>
> Also, a nit, try to use bpf_printk() instead.
>
> Some existing tunnel tests have printk but those are older examples
> when test_progs.c was not ready.  In the future, we may want
> to move these tunnel tests to test_progs.c where most of the tests
> don't printk/grep also and use global variables or map to check and
> only printF on unexpected values.  Thus, it may as well
> have this new test steering into this direction in term
> of only print on error.
>

Thanks. I would use bpf_printk().
For tunnel source test cases, printF would only be used for errors. And I
will use macros for tunnel ip addresses based on Yonghong's suggestion.

> > +
> > +     if (bpf_ntohl(key.local_ipv6[3]) != 0xbb) {
> > +             ERROR(ret);
> > +             return TC_ACT_SHOT;
> > +     }
> > +
> > +     return TC_ACT_OK;
> > +}
> > +
> >  char _license[] SEC("license") = "GPL";
> > diff --git a/tools/testing/selftests/bpf/test_tunnel.sh b/tools/testing/selftests/bpf/test_tunnel.sh
> > index b6923392bf16..4b7bf9c7bbe1 100755
> > --- a/tools/testing/selftests/bpf/test_tunnel.sh
> > +++ b/tools/testing/selftests/bpf/test_tunnel.sh
> > @@ -191,12 +191,15 @@ add_ip6vxlan_tunnel()
> >       ip netns exec at_ns0 ip link set dev veth0 up
> >       #ip -4 addr del 172.16.1.200 dev veth1
> >       ip -6 addr add dev veth1 ::22/96
> > +     if [ "$2" == "2" ]; then
> > +             ip -6 addr add dev veth1 ::bb/96
> Testing $1 is not "::22" is as good as another $2 arg and
> then $2 is not needed?

Yes. It's more refined now.

>
> > +     fi
> >       ip link set dev veth1 up
> >
> >       # at_ns0 namespace
> >       ip netns exec at_ns0 \
> >               ip link add dev $DEV_NS type $TYPE id 22 dstport 4789 \
> > -             local ::11 remote ::22
> > +             local ::11 remote $1
> >       ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
> >       ip netns exec at_ns0 ip link set dev $DEV_NS up
> >
> > @@ -231,7 +234,7 @@ add_ip6geneve_tunnel()
> >       # at_ns0 namespace
> >       ip netns exec at_ns0 \
> >               ip link add dev $DEV_NS type $TYPE id 22 \
> > -             remote ::22     # geneve has no local option
> > +             remote ::22    # geneve has no local option
> Unnecessary space change.  Please try to avoid.  This distracts
> the code review.

OK. Sorry for the confusion.

>
> >       ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
> >       ip netns exec at_ns0 ip link set dev $DEV_NS up
> >
> > @@ -394,7 +397,7 @@ test_ip6erspan()
> >
> >       check $TYPE
> >       config_device
> > -     add_ip6erspan_tunnel $1
> > +     add_ip6erspan_tunnel
> What is this change for?
> How is the patch set related to the test_ip6erspan()?
>
> or it is an unrelated clean up? If it is, please use another patch
> in its own cleanup patch set.

Yes and I would separate it into another patch.

>
> >       attach_bpf $DEV ip4ip6erspan_set_tunnel ip4ip6erspan_get_tunnel
> >       ping6 $PING_ARG ::11
> >       ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
> > @@ -441,7 +444,7 @@ test_ip6vxlan()
> >
> >       check $TYPE
> >       config_device
> > -     add_ip6vxlan_tunnel
> > +     add_ip6vxlan_tunnel ::22 1
> >       ip link set dev veth1 mtu 1500
> >       attach_bpf $DEV ip6vxlan_set_tunnel ip6vxlan_get_tunnel
> >       # underlay
> > @@ -690,6 +693,34 @@ test_vxlan_tunsrc()
> >          echo -e ${GREEN}"PASS: ${TYPE}_tunsrc"${NC}
> >  }
> >
> > +test_ip6vxlan_tunsrc()
> > +{
> > +     TYPE=vxlan
> > +     DEV_NS=ip6vxlan00
> > +     DEV=ip6vxlan11
> > +     ret=0
> > +
> > +     check $TYPE
> > +     config_device
> > +     add_ip6vxlan_tunnel ::bb 2
> > +     ip link set dev veth1 mtu 1500
> > +     attach_bpf $DEV ip6vxlan_set_tunnel_src ip6vxlan_get_tunnel_src
> > +     # underlay
> > +     ping6 $PING_ARG ::11
> > +     # ip4 over ip6
> > +     ping $PING_ARG 10.1.1.100
> > +     check_err $?
> > +     ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
> > +     check_err $?
> > +     cleanup
> > +
> > +     if [ $ret -ne 0 ]; then
> > +                echo -e ${RED}"FAIL: ip6${TYPE}_tunsrc"${NC}
> > +                return 1
> > +        fi
> > +        echo -e ${GREEN}"PASS: ip6${TYPE}_tunsrc"${NC}
> > +}
> > +
> >  attach_bpf()
> >  {
> >       DEV=$1
> > @@ -815,6 +846,10 @@ bpf_tunnel_test()
> >       test_vxlan_tunsrc
> >       errors=$(( $errors + $? ))
> >
> > +     echo "Testing IP6VXLAN tunnel source..."
> > +     test_ip6vxlan_tunsrc
> > +     errors=$(( $errors + $? ))
> > +
> >       return $errors
> >  }
> >
> > --
> > 2.24.3 (Apple Git-128)
> >

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[范开喜]

范开喜 <fankaixi.li@bytedance.com> 于2022年3月27日周日 01:24写道：
>
> Martin KaFai Lau <kafai@fb.com> 于2022年3月25日周五 03:38写道：
> >
> > On Tue, Mar 22, 2022 at 11:42:31PM +0800, fankaixi.li@bytedance.com wrote:
> > > From: "kaixi.fan" <fankaixi.li@bytedance.com>
> > >
> > > Add two ipv6 address on underlay nic interface, and use bpf code to
> > > configure the secondary ipv6 address as the vxlan tunnel source ip.
> > > Then check ping6 result and log contains the correct tunnel source
> > > ip.
> > >
> > > Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>
> > > ---
> > >  .../selftests/bpf/progs/test_tunnel_kern.c    | 51 +++++++++++++++++++
> > >  tools/testing/selftests/bpf/test_tunnel.sh    | 43 ++++++++++++++--
> > >  2 files changed, 90 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > > index ab635c55ae9b..56e1aee0ba5a 100644
> > > --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > > +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> > > @@ -740,4 +740,55 @@ int _vxlan_get_tunnel_src(struct __sk_buff *skb)
> > >       return TC_ACT_OK;
> > >  }
> > >
> > > +SEC("ip6vxlan_set_tunnel_src")
> > > +int _ip6vxlan_set_tunnel_src(struct __sk_buff *skb)
> > > +{
> > > +     struct bpf_tunnel_key key;
> > > +     int ret;
> > > +
> > > +     __builtin_memset(&key, 0x0, sizeof(key));
> > > +     key.local_ipv6[3] = bpf_htonl(0xbb); /* ::bb */
> > > +     key.remote_ipv6[3] = bpf_htonl(0x11); /* ::11 */
> > > +     key.tunnel_id = 22;
> > > +     key.tunnel_tos = 0;
> > > +     key.tunnel_ttl = 64;
> > > +
> > > +     ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
> > > +                                  BPF_F_TUNINFO_IPV6);
> > > +     if (ret < 0) {
> > > +             ERROR(ret);
> > > +             return TC_ACT_SHOT;
> > > +     }
> > > +
> > > +     return TC_ACT_OK;
> > > +}
> > > +
> > > +SEC("ip6vxlan_get_tunnel_src")
> > > +int _ip6vxlan_get_tunnel_src(struct __sk_buff *skb)
> > > +{
> > > +     char fmt[] = "key %d remote ip6 ::%x source ip6 ::%x\n";
> > > +     char fmt2[] = "label %x\n";
> > > +     struct bpf_tunnel_key key;
> > > +     int ret;
> > > +
> > > +     ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key),
> > > +                                  BPF_F_TUNINFO_IPV6);
> > > +     if (ret < 0) {
> > > +             ERROR(ret);
> > > +             return TC_ACT_SHOT;
> > > +     }
> > > +
> > > +     bpf_trace_printk(fmt, sizeof(fmt),
> > > +                      key.tunnel_id, key.remote_ipv6[3], key.local_ipv6[3]);
> > > +     bpf_trace_printk(fmt2, sizeof(fmt2),
> > > +                      key.tunnel_label);
> > Going back to the same comment on v1.
> >
> > How is this printk used?
> > Especially for the most common test PASS case,
> > afaict, this will be ignored and left in the trace buffer?
> >
> > Can it only printk when it detects error? like only print
> > when it fails the != 0xbb test case below?
> >
> > Also, a nit, try to use bpf_printk() instead.
> >
> > Some existing tunnel tests have printk but those are older examples
> > when test_progs.c was not ready.  In the future, we may want
> > to move these tunnel tests to test_progs.c where most of the tests
> > don't printk/grep also and use global variables or map to check and
> > only printF on unexpected values.  Thus, it may as well
> > have this new test steering into this direction in term
> > of only print on error.
> >
>
> Thanks. I would use bpf_printk().
> For tunnel source test cases, printF would only be used for errors. And I
> will use macros for tunnel ip addresses based on Yonghong's suggestion.
>

Hi Martin KaFai Lau and Yonghong,

I have prepared v3 patches for tunnel source ip feature. Some obviously
errors have been fixed. But there are three problems left. They makes me
copy tunnel test cases and put tunnel source ip test codes into them.
I put these three problems here:

I have tried to use bpf_printk in bpf kernel code. But the object file could
not be loaded by tc filter command. There are .relxxx section such as
.relgre_set_tunnel in the output of objdump.  The tc filter says it could
not find the dedicated section.
So I still use bpf_trace_printk now.

I have tried to use a bpf map "local_ip_array" to store tunnel source ip
address. Userspace code change tunnel source ip by updating map
"local_ip_array" in the middle of test. Kernel bpf code get the tunnel source
ip by looking up the map. But the object file could not be loaded by tc filter
command also. The verifier says "R1 type=scalar expected=map_ptr" when
calling "bpf_map_lookup_elem" helper function. I check the assembly code
that the r1 register value is 0 when calling "bpf_map_lookup_elem".
I write a tiny bpf loader for this test. But It's too heavy.

I have read test cases in prog_tests directory. They use c code to replace
shell command to create network namespace and ping. Also functions like
"test_tc_peer__load" are used to load bpf code. It's more complicate than
shell commands. And there are many duplicate funtions like create_ns in
some files.
The code in test_progs.c are common functions not test cases.
Maybe we could move tunnel test code to it in the future until the test
framework is complete.

Thanks.

> > > +
> > > +     if (bpf_ntohl(key.local_ipv6[3]) != 0xbb) {
> > > +             ERROR(ret);
> > > +             return TC_ACT_SHOT;
> > > +     }
> > > +
> > > +     return TC_ACT_OK;
> > > +}
> > > +
> > >  char _license[] SEC("license") = "GPL";
> > > diff --git a/tools/testing/selftests/bpf/test_tunnel.sh b/tools/testing/selftests/bpf/test_tunnel.sh
> > > index b6923392bf16..4b7bf9c7bbe1 100755
> > > --- a/tools/testing/selftests/bpf/test_tunnel.sh
> > > +++ b/tools/testing/selftests/bpf/test_tunnel.sh
> > > @@ -191,12 +191,15 @@ add_ip6vxlan_tunnel()
> > >       ip netns exec at_ns0 ip link set dev veth0 up
> > >       #ip -4 addr del 172.16.1.200 dev veth1
> > >       ip -6 addr add dev veth1 ::22/96
> > > +     if [ "$2" == "2" ]; then
> > > +             ip -6 addr add dev veth1 ::bb/96
> > Testing $1 is not "::22" is as good as another $2 arg and
> > then $2 is not needed?
>
> Yes. It's more refined now.
>
> >
> > > +     fi
> > >       ip link set dev veth1 up
> > >
> > >       # at_ns0 namespace
> > >       ip netns exec at_ns0 \
> > >               ip link add dev $DEV_NS type $TYPE id 22 dstport 4789 \
> > > -             local ::11 remote ::22
> > > +             local ::11 remote $1
> > >       ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
> > >       ip netns exec at_ns0 ip link set dev $DEV_NS up
> > >
> > > @@ -231,7 +234,7 @@ add_ip6geneve_tunnel()
> > >       # at_ns0 namespace
> > >       ip netns exec at_ns0 \
> > >               ip link add dev $DEV_NS type $TYPE id 22 \
> > > -             remote ::22     # geneve has no local option
> > > +             remote ::22    # geneve has no local option
> > Unnecessary space change.  Please try to avoid.  This distracts
> > the code review.
>
> OK. Sorry for the confusion.
>
> >
> > >       ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
> > >       ip netns exec at_ns0 ip link set dev $DEV_NS up
> > >
> > > @@ -394,7 +397,7 @@ test_ip6erspan()
> > >
> > >       check $TYPE
> > >       config_device
> > > -     add_ip6erspan_tunnel $1
> > > +     add_ip6erspan_tunnel
> > What is this change for?
> > How is the patch set related to the test_ip6erspan()?
> >
> > or it is an unrelated clean up? If it is, please use another patch
> > in its own cleanup patch set.
>
> Yes and I would separate it into another patch.
>
> >
> > >       attach_bpf $DEV ip4ip6erspan_set_tunnel ip4ip6erspan_get_tunnel
> > >       ping6 $PING_ARG ::11
> > >       ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
> > > @@ -441,7 +444,7 @@ test_ip6vxlan()
> > >
> > >       check $TYPE
> > >       config_device
> > > -     add_ip6vxlan_tunnel
> > > +     add_ip6vxlan_tunnel ::22 1
> > >       ip link set dev veth1 mtu 1500
> > >       attach_bpf $DEV ip6vxlan_set_tunnel ip6vxlan_get_tunnel
> > >       # underlay
> > > @@ -690,6 +693,34 @@ test_vxlan_tunsrc()
> > >          echo -e ${GREEN}"PASS: ${TYPE}_tunsrc"${NC}
> > >  }
> > >
> > > +test_ip6vxlan_tunsrc()
> > > +{
> > > +     TYPE=vxlan
> > > +     DEV_NS=ip6vxlan00
> > > +     DEV=ip6vxlan11
> > > +     ret=0
> > > +
> > > +     check $TYPE
> > > +     config_device
> > > +     add_ip6vxlan_tunnel ::bb 2
> > > +     ip link set dev veth1 mtu 1500
> > > +     attach_bpf $DEV ip6vxlan_set_tunnel_src ip6vxlan_get_tunnel_src
> > > +     # underlay
> > > +     ping6 $PING_ARG ::11
> > > +     # ip4 over ip6
> > > +     ping $PING_ARG 10.1.1.100
> > > +     check_err $?
> > > +     ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
> > > +     check_err $?
> > > +     cleanup
> > > +
> > > +     if [ $ret -ne 0 ]; then
> > > +                echo -e ${RED}"FAIL: ip6${TYPE}_tunsrc"${NC}
> > > +                return 1
> > > +        fi
> > > +        echo -e ${GREEN}"PASS: ip6${TYPE}_tunsrc"${NC}
> > > +}
> > > +
> > >  attach_bpf()
> > >  {
> > >       DEV=$1
> > > @@ -815,6 +846,10 @@ bpf_tunnel_test()
> > >       test_vxlan_tunsrc
> > >       errors=$(( $errors + $? ))
> > >
> > > +     echo "Testing IP6VXLAN tunnel source..."
> > > +     test_ip6vxlan_tunsrc
> > > +     errors=$(( $errors + $? ))
> > > +
> > >       return $errors
> > >  }
> > >
> > > --
> > > 2.24.3 (Apple Git-128)
> > >

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[Martin KaFai Lau]

On Wed, Apr 06, 2022 at 04:03:32PM +0800, 范开喜 wrote:
> Hi Martin KaFai Lau and Yonghong,
> 
> I have prepared v3 patches for tunnel source ip feature. Some obviously
> errors have been fixed. But there are three problems left. They makes me
> copy tunnel test cases and put tunnel source ip test codes into them.
> I put these three problems here:
> 
> I have tried to use bpf_printk in bpf kernel code. But the object file could
> not be loaded by tc filter command. There are .relxxx section such as
> .relgre_set_tunnel in the output of objdump.  The tc filter says it could
> not find the dedicated section.
> So I still use bpf_trace_printk now.
> 
> I have tried to use a bpf map "local_ip_array" to store tunnel source ip
> address. Userspace code change tunnel source ip by updating map
> "local_ip_array" in the middle of test. Kernel bpf code get the tunnel source
> ip by looking up the map. But the object file could not be loaded by tc filter
> command also. The verifier says "R1 type=scalar expected=map_ptr" when
> calling "bpf_map_lookup_elem" helper function. I check the assembly code
> that the r1 register value is 0 when calling "bpf_map_lookup_elem".
> I write a tiny bpf loader for this test. But It's too heavy.
> 
> I have read test cases in prog_tests directory. They use c code to replace
> shell command to create network namespace and ping. Also functions like
> "test_tc_peer__load" are used to load bpf code. It's more complicate than
> shell commands. And there are many duplicate funtions like create_ns in
> some files.
> The code in test_progs.c are common functions not test cases.
> Maybe we could move tunnel test code to it in the future until the test
> framework is complete.
Regarding the "test_tc_peer__load" is more complicated than shell,
it is the skeleton and a better way to load bpf to do test.
It makes the user space test easier to write, e.g. the bpf_printk+grep
for capturing error can go away and replace it with checking some
bpf's global variables instead.  All newly added tests are using it.

There are examples in prog_tests/ (i.e. test_progs) doing similar things as
the test_tunnel.sh, e.g. creating netns, adding veth, tc filter...etc.
For example, take a look at tc_redirect.c and how it avoids the tc bpf
loading issues that you have seen.

The .sh is not run by CI also.  I also only run test_progs regularly.
I was also not sure if test_tunnel.sh should further be extended, so
did not mention it.  However, based on the issues you are seeing from
making common changes to the bpf prog, it is clear that it should be done
in prog_tests/ (i.e. test_progs) instead.  At least start with the
two new tests that you are adding in patch 2 and 3 instead of further
extending the test_tunnel.sh.  That will setup a base to move
all test_tunnel.sh to prog_tests/ eventually.

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[范开喜]

Martin KaFai Lau <kafai@fb.com> 于2022年4月8日周五 01:53写道：
>
> On Wed, Apr 06, 2022 at 04:03:32PM +0800, 范开喜 wrote:
> > Hi Martin KaFai Lau and Yonghong,
> >
> > I have prepared v3 patches for tunnel source ip feature. Some obviously
> > errors have been fixed. But there are three problems left. They makes me
> > copy tunnel test cases and put tunnel source ip test codes into them.
> > I put these three problems here:
> >
> > I have tried to use bpf_printk in bpf kernel code. But the object file could
> > not be loaded by tc filter command. There are .relxxx section such as
> > .relgre_set_tunnel in the output of objdump.  The tc filter says it could
> > not find the dedicated section.
> > So I still use bpf_trace_printk now.
> >
> > I have tried to use a bpf map "local_ip_array" to store tunnel source ip
> > address. Userspace code change tunnel source ip by updating map
> > "local_ip_array" in the middle of test. Kernel bpf code get the tunnel source
> > ip by looking up the map. But the object file could not be loaded by tc filter
> > command also. The verifier says "R1 type=scalar expected=map_ptr" when
> > calling "bpf_map_lookup_elem" helper function. I check the assembly code
> > that the r1 register value is 0 when calling "bpf_map_lookup_elem".
> > I write a tiny bpf loader for this test. But It's too heavy.
> >
> > I have read test cases in prog_tests directory. They use c code to replace
> > shell command to create network namespace and ping. Also functions like
> > "test_tc_peer__load" are used to load bpf code. It's more complicate than
> > shell commands. And there are many duplicate funtions like create_ns in
> > some files.
> > The code in test_progs.c are common functions not test cases.
> > Maybe we could move tunnel test code to it in the future until the test
> > framework is complete.
> Regarding the "test_tc_peer__load" is more complicated than shell,
> it is the skeleton and a better way to load bpf to do test.
> It makes the user space test easier to write, e.g. the bpf_printk+grep
> for capturing error can go away and replace it with checking some
> bpf's global variables instead.  All newly added tests are using it.
>
> There are examples in prog_tests/ (i.e. test_progs) doing similar things as
> the test_tunnel.sh, e.g. creating netns, adding veth, tc filter...etc.
> For example, take a look at tc_redirect.c and how it avoids the tc bpf
> loading issues that you have seen.
>
> The .sh is not run by CI also.  I also only run test_progs regularly.
> I was also not sure if test_tunnel.sh should further be extended, so
> did not mention it.  However, based on the issues you are seeing from
> making common changes to the bpf prog, it is clear that it should be done
> in prog_tests/ (i.e. test_progs) instead.  At least start with the
> two new tests that you are adding in patch 2 and 3 instead of further
> extending the test_tunnel.sh.  That will setup a base to move
> all test_tunnel.sh to prog_tests/ eventually.

OK. Get it. I will move them to test_progs step by step.
Many thanks.

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[Peilin Ye]

Hi Martin,

On Thu, Apr 07, 2022 at 10:53:33AM -0700, Martin KaFai Lau wrote:
> The .sh is not run by CI also.

Just curious: by "CI", did you mean libbpf-ci [1] ?

If so, why doesn't libbpf-ci run these .sh tests?  Recently we triggered
a bug (see [2]) in ip6_gre by running test_tunnel.sh.  I think it
could've been spotted much sooner if test_tunnel.sh was being run.

[1] https://github.com/libbpf/libbpf/actions/workflows/test.yml
[2] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=ab198e1d0dd8

Thanks,
Peilin Ye

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[Andrii Nakryiko]

On Fri, Apr 15, 2022 at 4:12 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
>
> Hi Martin,
>
> On Thu, Apr 07, 2022 at 10:53:33AM -0700, Martin KaFai Lau wrote:
> > The .sh is not run by CI also.
>
> Just curious: by "CI", did you mean libbpf-ci [1] ?
>
> If so, why doesn't libbpf-ci run these .sh tests?  Recently we triggered
> a bug (see [2]) in ip6_gre by running test_tunnel.sh.  I think it
> could've been spotted much sooner if test_tunnel.sh was being run.
>

If you convert test_tunnel.sh into a test inside test_progs, we'll be
running it regularly.

> [1] https://github.com/libbpf/libbpf/actions/workflows/test.yml
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=ab198e1d0dd8
>
> Thanks,
> Peilin Ye
>

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[Peilin Ye]

Hi Andrii,

On Wed, Apr 20, 2022 at 10:09:59AM -0700, Andrii Nakryiko wrote:
> On Fri, Apr 15, 2022 at 4:12 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> > On Thu, Apr 07, 2022 at 10:53:33AM -0700, Martin KaFai Lau wrote:
> > > The .sh is not run by CI also.
> >
> > Just curious: by "CI", did you mean libbpf-ci [1] ?
> >
> > If so, why doesn't libbpf-ci run these .sh tests?  Recently we triggered
> > a bug (see [2]) in ip6_gre by running test_tunnel.sh.  I think it
> > could've been spotted much sooner if test_tunnel.sh was being run.
> 
> If you convert test_tunnel.sh into a test inside test_progs, we'll be
> running it regularly.

Thanks!  I think Kaixi is working on this.

Peilin Ye

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[范开喜]

Peilin Ye <yepeilin.cs@gmail.com> 于2022年4月21日周四 01:54写道：
>
> Hi Andrii,
>
> On Wed, Apr 20, 2022 at 10:09:59AM -0700, Andrii Nakryiko wrote:
> > On Fri, Apr 15, 2022 at 4:12 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> > > On Thu, Apr 07, 2022 at 10:53:33AM -0700, Martin KaFai Lau wrote:
> > > > The .sh is not run by CI also.
> > >
> > > Just curious: by "CI", did you mean libbpf-ci [1] ?
> > >
> > > If so, why doesn't libbpf-ci run these .sh tests?  Recently we triggered
> > > a bug (see [2]) in ip6_gre by running test_tunnel.sh.  I think it
> > > could've been spotted much sooner if test_tunnel.sh was being run.
> >
> > If you convert test_tunnel.sh into a test inside test_progs, we'll be
> > running it regularly.
>
> Thanks!  I think Kaixi is working on this.
>
> Peilin Ye
>

Hi Peilin and Andrii,

Yes. I am working on it.
Thanks.

Re: [External] [PATCH bpf-next v2 3/3] selftests/bpf: add ipv6 vxlan tunnel source testcase

[Yonghong Song]

On 3/22/22 8:42 AM, fankaixi.li@bytedance.com wrote:
> From: "kaixi.fan" <fankaixi.li@bytedance.com>
> 
> Add two ipv6 address on underlay nic interface, and use bpf code to
> configure the secondary ipv6 address as the vxlan tunnel source ip.
> Then check ping6 result and log contains the correct tunnel source
> ip.
> 
> Signed-off-by: kaixi.fan <fankaixi.li@bytedance.com>

Similar here, proper name in Signed-off-by tag.

> ---
>   .../selftests/bpf/progs/test_tunnel_kern.c    | 51 +++++++++++++++++++
>   tools/testing/selftests/bpf/test_tunnel.sh    | 43 ++++++++++++++--
>   2 files changed, 90 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> index ab635c55ae9b..56e1aee0ba5a 100644
> --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> @@ -740,4 +740,55 @@ int _vxlan_get_tunnel_src(struct __sk_buff *skb)
>   	return TC_ACT_OK;
>   }
>   
> +SEC("ip6vxlan_set_tunnel_src")
> +int _ip6vxlan_set_tunnel_src(struct __sk_buff *skb)
> +{
> +	struct bpf_tunnel_key key;
> +	int ret;
> +
> +	__builtin_memset(&key, 0x0, sizeof(key));
> +	key.local_ipv6[3] = bpf_htonl(0xbb); /* ::bb */
> +	key.remote_ipv6[3] = bpf_htonl(0x11); /* ::11 */
> +	key.tunnel_id = 22;
> +	key.tunnel_tos = 0;
> +	key.tunnel_ttl = 64;
> +
> +	ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
> +				     BPF_F_TUNINFO_IPV6);
> +	if (ret < 0) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	return TC_ACT_OK;
> +}
> +
> +SEC("ip6vxlan_get_tunnel_src")
> +int _ip6vxlan_get_tunnel_src(struct __sk_buff *skb)
> +{
> +	char fmt[] = "key %d remote ip6 ::%x source ip6 ::%x\n";
> +	char fmt2[] = "label %x\n";
> +	struct bpf_tunnel_key key;
> +	int ret;
> +
> +	ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key),
> +				     BPF_F_TUNINFO_IPV6);
> +	if (ret < 0) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}
> +
> +	bpf_trace_printk(fmt, sizeof(fmt),
> +			 key.tunnel_id, key.remote_ipv6[3], key.local_ipv6[3]);
> +	bpf_trace_printk(fmt2, sizeof(fmt2),
> +			 key.tunnel_label);

using bpf_printk().

> +
> +	if (bpf_ntohl(key.local_ipv6[3]) != 0xbb) {
> +		ERROR(ret);
> +		return TC_ACT_SHOT;
> +	}

abstract 0xbb as a macro?

> +
> +	return TC_ACT_OK;
> +}
> +
[...]

Re: [External] [PATCH bpf-next v2 0/3] bpf: Add support to set and get

[Yonghong Song]

On 3/22/22 8:42 AM, fankaixi.li@bytedance.com wrote:
> From: "kaixi.fan" <fankaixi.li@bytedance.com>
> 
> Now bpf code could not set tunnel source ip address of ip tunnel. So it
> could not support flow based tunnel mode completely. Because flow based
> tunnel mode could set tunnel source, destination ip address and tunnel
> key simultaneously.
> 
> Flow based tunnel is useful for overlay networks. And by configuring tunnel
> source ip address, user could make their networks more elastic.
> For example, tunnel source ip could be used to select different egress
> nic interface for different flows with same tunnel destination ip. Another
> example, user could choose one of multiple ip address of the egress nic
> interface as the packet's tunnel source ip.
> 
> v1 -> v2:
> v1: https://lore.kernel.org/bpf/20220319130538.55741-1-fankaixi.li@bytedance.com
> 
> - Add secondary ip and set tunnel remote ip in "add_vxlan_tunnel" and
> "add_ip6vxlan_tunnel"
> 
> kaixi.fan (3):
>    bpf: Add source ip in "struct bpf_tunnel_key"
>    selftests/bpf: add ipv4 vxlan tunnel source testcase
>    selftests/bpf: add ipv6 vxlan tunnel source testcase
> 
>   include/uapi/linux/bpf.h                      |   4 +
>   net/core/filter.c                             |   9 ++
>   tools/include/uapi/linux/bpf.h                |   4 +
>   .../selftests/bpf/progs/test_tunnel_kern.c    | 115 ++++++++++++++++++
>   tools/testing/selftests/bpf/test_tunnel.sh    |  80 +++++++++++-
>   5 files changed, 206 insertions(+), 6 deletions(-)

The subject "bpf: Add support to set and get" is incomplete.

Re: [External] [PATCH bpf-next v2 0/3] bpf: Add support to set and get

[范开喜]

Yonghong Song <yhs@fb.com> 于2022年3月26日周六 00:46写道：
>
>
>
> On 3/22/22 8:42 AM, fankaixi.li@bytedance.com wrote:
> > From: "kaixi.fan" <fankaixi.li@bytedance.com>
> >
> > Now bpf code could not set tunnel source ip address of ip tunnel. So it
> > could not support flow based tunnel mode completely. Because flow based
> > tunnel mode could set tunnel source, destination ip address and tunnel
> > key simultaneously.
> >
> > Flow based tunnel is useful for overlay networks. And by configuring tunnel
> > source ip address, user could make their networks more elastic.
> > For example, tunnel source ip could be used to select different egress
> > nic interface for different flows with same tunnel destination ip. Another
> > example, user could choose one of multiple ip address of the egress nic
> > interface as the packet's tunnel source ip.
> >
> > v1 -> v2:
> > v1: https://lore.kernel.org/bpf/20220319130538.55741-1-fankaixi.li@bytedance.com
> >
> > - Add secondary ip and set tunnel remote ip in "add_vxlan_tunnel" and
> > "add_ip6vxlan_tunnel"
> >
> > kaixi.fan (3):
> >    bpf: Add source ip in "struct bpf_tunnel_key"
> >    selftests/bpf: add ipv4 vxlan tunnel source testcase
> >    selftests/bpf: add ipv6 vxlan tunnel source testcase
> >
> >   include/uapi/linux/bpf.h                      |   4 +
> >   net/core/filter.c                             |   9 ++
> >   tools/include/uapi/linux/bpf.h                |   4 +
> >   .../selftests/bpf/progs/test_tunnel_kern.c    | 115 ++++++++++++++++++
> >   tools/testing/selftests/bpf/test_tunnel.sh    |  80 +++++++++++-
> >   5 files changed, 206 insertions(+), 6 deletions(-)
>
> The subject "bpf: Add support to set and get" is incomplete.

Thanks. Sorry for the confusion. I will fix it.