Martin KaFai Lau <kafai@xxxxxx> 于2022年3月25日周五 03:38写道：
>
> On Tue, Mar 22, 2022 at 11:42:31PM +0800, fankaixi.li@xxxxxxxxxxxxx wrote:
> > From: "kaixi.fan" <fankaixi.li@xxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxx>
> > ---
> > .../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)
> >
