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