Yonghong Song <yhs@xxxxxx> 于2022年3月26日周六 00:41写道： > > > > On 3/22/22 8:42 AM, fankaixi.li@xxxxxxxxxxxxx wrote: > > From: "kaixi.fan" <fankaixi.li@xxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxx> > > 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; > > +} > > + > [...]
