On Wed, Nov 16, 2022 at 12:38 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 11/14/22 7:10 PM, Stanislav Fomichev wrote: > > LWT_XMIT to test L3 case, TC to test L2 case. > > It will be useful to add more details here to explain which test is testing the > skb->len check in __bpf_redirect_no_mac() and __bpf_redirect_common() in patch 1. SG, will try to annotate. > > > > Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx> > > --- > > .../selftests/bpf/prog_tests/empty_skb.c | 140 ++++++++++++++++++ > > tools/testing/selftests/bpf/progs/empty_skb.c | 37 +++++ > > 2 files changed, 177 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/empty_skb.c > > create mode 100644 tools/testing/selftests/bpf/progs/empty_skb.c > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c b/tools/testing/selftests/bpf/prog_tests/empty_skb.c > > new file mode 100644 > > index 000000000000..6e35739babed > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c > > @@ -0,0 +1,140 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include <test_progs.h> > > +#include <network_helpers.h> > > +#include <net/if.h> > > +#include "empty_skb.skel.h" > > + > > +#define SYS(cmd) ({ \ > > + if (!ASSERT_OK(system(cmd), (cmd))) \ > > + goto out; \ > > +}) > > + > > +void test_empty_skb(void) > > +{ > > + LIBBPF_OPTS(bpf_test_run_opts, tattr); > > + struct empty_skb *bpf_obj = NULL; > > + struct nstoken *tok = NULL; > > + struct bpf_program *prog; > > + char eth_hlen_pp[15]; > > + char eth_hlen[14]; > > + int veth_ifindex; > > + int ipip_ifindex; > > + int err; > > + int i; > > + > > + struct { > > + const char *msg; > > + const void *data_in; > > + __u32 data_size_in; > > + int *ifindex; > > + int err; > > + int ret; > > + bool success_on_tc; > > + } tests[] = { > > + /* Empty packets are always rejected. */ > > + > > + { > > + .msg = "veth empty ingress packet", > > + .data_in = NULL, > > + .data_size_in = 0, > > + .ifindex = &veth_ifindex, > > + .err = -EINVAL, > > + }, > > + { > > + .msg = "ipip empty ingress packet", > > + .data_in = NULL, > > + .data_size_in = 0, > > + .ifindex = &ipip_ifindex, > > + .err = -EINVAL, > > + }, > > > > + > > + /* ETH_HLEN-sized packets: > > + * - can not be redirected at LWT_XMIT > > + * - can be redirected at TC > > + */ > > + > > + { > > + .msg = "veth ETH_HLEN packet ingress", > > + .data_in = eth_hlen, > > + .data_size_in = sizeof(eth_hlen), > > + .ifindex = &veth_ifindex, > > + .ret = -ERANGE, > > + .success_on_tc = true, > > + }, > > + { > > + .msg = "ipip ETH_HLEN packet ingress", > > + .data_in = eth_hlen, > > + .data_size_in = sizeof(eth_hlen), > > + .ifindex = &veth_ifindex, > > + .ret = -ERANGE, > > + .success_on_tc = true, > > + }, > > > hmm... these two tests don't look right. They are the same except the ".msg" > part. The latter one should use &ipip_ifindex? Oh, good catch! > > + > > + /* ETH_HLEN+1-sized packet should be redirected. */ > > + > > + { > > + .msg = "veth ETH_HLEN+1 packet ingress", > > + .data_in = eth_hlen_pp, > > + .data_size_in = sizeof(eth_hlen_pp), > > + .ifindex = &veth_ifindex, > > + }, > > + { > > + .msg = "ipip ETH_HLEN+1 packet ingress", > > + .data_in = eth_hlen_pp, > > + .data_size_in = sizeof(eth_hlen_pp), > > + .ifindex = &veth_ifindex, > > + }, > > Same here. >