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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



范开喜 <fankaixi.li@xxxxxxxxxxxxx> 于2022年3月27日周日 01:24写道:
>
> 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.
>

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)
> > >




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux