Message ID | 20210901104732.10956-20-magnus.karlsson@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | BPF |
Headers | show |
Series | selftests: xsk: facilitate adding tests | expand |
On Wed, Sep 01, 2021 at 12:47:31PM +0200, Magnus Karlsson wrote: > From: Magnus Karlsson <magnus.karlsson@intel.com> > > Add tests for invalid xsk descriptors in the Tx ring. A number of > handcrafted nasty invalid descriptors are created and submitted to the > tx ring to check that they are validated correctly. Corener case valid Corner > ones are also sent. The tests are run for both aligned and unaligned > mode. > > pkt_stream_set() is introduced to be able to create a hand-crafted > packet stream where every single packet is specified in detail. > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> > --- > tools/testing/selftests/bpf/xdpxceiver.c | 143 ++++++++++++++++++++--- > tools/testing/selftests/bpf/xdpxceiver.h | 7 +- > 2 files changed, 131 insertions(+), 19 deletions(-) > > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c > index d085033afd53..a4f6ce3a6b14 100644 > --- a/tools/testing/selftests/bpf/xdpxceiver.c > +++ b/tools/testing/selftests/bpf/xdpxceiver.c > @@ -46,6 +46,8 @@ > * then remove xsk sockets from queue 0 on both veth interfaces and > * finally run a traffic on queues ids 1 > * g. unaligned mode > + * h. tests for invalid and corner case Tx descriptors so that the correct ones > + * are discarded and let through, respectively. > * > * Total tests: 12 > * > @@ -394,7 +396,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx, > for (j = 0; j < MAX_SOCKETS; j++) { > memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j])); > memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j])); > - ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4; > + ifobj->umem_arr[j].num_frames = DEFAULT_UMEM_BUFFERS; > ifobj->umem_arr[j].frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE; > ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS; > } > @@ -450,6 +452,16 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb) > return &pkt_stream->pkts[pkt_nb]; > } > > +static struct pkt *pkt_stream_get_next_rx_pkt(struct pkt_stream *pkt_stream) > +{ > + while (pkt_stream->rx_pkt_nb < pkt_stream->nb_pkts) { > + if (pkt_stream->pkts[pkt_stream->rx_pkt_nb].valid) > + return &pkt_stream->pkts[pkt_stream->rx_pkt_nb++]; > + pkt_stream->rx_pkt_nb++; > + } > + return NULL; > +} > + > static void pkt_stream_delete(struct pkt_stream *pkt_stream) > { > free(pkt_stream->pkts); > @@ -465,17 +477,31 @@ static void pkt_stream_restore_default(struct test_spec *test) > test->ifobj_rx->pkt_stream = test->pkt_stream_default; > } > > -static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len) > +static struct pkt_stream *__pkt_stream_alloc(u32 nb_pkts) > { > struct pkt_stream *pkt_stream; > - u32 i; > > - pkt_stream = malloc(sizeof(*pkt_stream)); > + pkt_stream = calloc(1, sizeof(*pkt_stream)); So with that probably my previous comment about use_addr_for_fill being not set is not relevant. > if (!pkt_stream) > - exit_with_error(ENOMEM); > + return NULL; > > pkt_stream->pkts = calloc(nb_pkts, sizeof(*pkt_stream->pkts)); > - if (!pkt_stream->pkts) > + if (!pkt_stream->pkts) { > + free(pkt_stream); > + return NULL; > + } > + > + pkt_stream->nb_pkts = nb_pkts; > + return pkt_stream; > +} > + > +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len) > +{ > + struct pkt_stream *pkt_stream; > + u32 i; > + > + pkt_stream = __pkt_stream_alloc(nb_pkts); > + if (!pkt_stream) > exit_with_error(ENOMEM); > > pkt_stream->nb_pkts = nb_pkts; > @@ -525,6 +551,26 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 off > test->ifobj_rx->pkt_stream = pkt_stream; > } > > +static void pkt_stream_set(struct test_spec *test, struct pkt *pkts, u32 nb_pkts) This is still a generation of pkt stream, can we name this is as: pkt_stream_generate_custom or is this too long? WDYT? > +{ > + struct pkt_stream *pkt_stream; > + u32 i; > + > + pkt_stream = __pkt_stream_alloc(nb_pkts); > + if (!pkt_stream) > + exit_with_error(ENOMEM); > + > + test->ifobj_tx->pkt_stream = pkt_stream; > + test->ifobj_rx->pkt_stream = pkt_stream; > + > + for (i = 0; i < nb_pkts; i++) { > + pkt_stream->pkts[i].addr = pkts[i].addr; > + pkt_stream->pkts[i].len = pkts[i].len; > + pkt_stream->pkts[i].payload = i; > + pkt_stream->pkts[i].valid = pkts[i].valid; > + } > +} > + > static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb) > { > struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb); > @@ -535,6 +581,8 @@ static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb) > > if (!pkt) > return NULL; > + if (!pkt->valid || pkt->len < PKT_SIZE) > + return pkt; > > data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr); > udp_hdr = (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr)); > @@ -596,19 +644,24 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len) > return false; > } > > + if (len < PKT_SIZE) { > + /*Do not try to verify packets that are smaller than minimun size. */ minimum > + return true; > + } > + > + if (pkt->len != len) { > + ksft_test_result_fail > + ("ERROR: [%s] expected length [%d], got length [%d]\n", > + __func__, pkt->len, len); > + return false; > + } > + > if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) { > u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE))); > > if (opt_pkt_dump) > pkt_dump(data, PKT_SIZE); > > - if (pkt->len != len) { > - ksft_test_result_fail > - ("ERROR: [%s] expected length [%d], got length [%d]\n", > - __func__, pkt->len, len); > - return false; > - } > - > if (pkt->payload != seqnum) { > ksft_test_result_fail > ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n", > @@ -645,6 +698,15 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size) > > rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx); > if (rcvd) { > + if (rcvd > xsk->outstanding_tx) { > + u64 addr = *xsk_ring_cons__comp_addr(&xsk->umem->cq, idx + rcvd - 1); > + > + ksft_test_result_fail("ERROR: [%s] Too many packets completed\n", > + __func__); > + ksft_print_msg("Last completion address: %llx\n", addr); > + return; > + } > + > xsk_ring_cons__release(&xsk->umem->cq, rcvd); > xsk->outstanding_tx -= rcvd; > } > @@ -653,11 +715,10 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size) > static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *xsk, > struct pollfd *fds) > { > - u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0; > - struct pkt *pkt; > + struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream); > + u32 idx_rx = 0, idx_fq = 0, rcvd, i; > int ret; > > - pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++); > while (pkt) { > rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx); > if (!rcvd) { > @@ -685,13 +746,21 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info * > const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++); > u64 addr = desc->addr, orig; > > + if (!pkt) { > + ksft_test_result_fail("ERROR: [%s] Received too many packets.\n", > + __func__); > + ksft_print_msg("Last packet has addr: %llx len: %u\n", > + addr, desc->len); > + return; > + } > + > orig = xsk_umem__extract_addr(addr); > addr = xsk_umem__add_offset_to_addr(addr); > if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len)) > return; > > *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig; > - pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++); > + pkt = pkt_stream_get_next_rx_pkt(pkt_stream); > } > > xsk_ring_prod__submit(&xsk->umem->fq, rcvd); > @@ -875,6 +944,7 @@ static void testapp_cleanup_xsk_res(struct ifobject *ifobj) > { > print_verbose("Destroying socket\n"); > xsk_socket__delete(ifobj->xsk->xsk); > + munmap(ifobj->umem->buffer, ifobj->umem->num_frames * ifobj->umem->frame_size); > xsk_umem__delete(ifobj->umem->umem); > } > > @@ -1118,6 +1188,33 @@ static bool testapp_unaligned(struct test_spec *test) > return true; > } > > +static void testapp_inv_desc(struct test_spec *test) I'd say that speaking out whole 'invalid' word wouldn't hurt us here. > +{ > + struct pkt pkts[] = {{0, 0, 0, true}, /* Zero packet length and zero address allowed */ Above looks a bit weird, maybe this should be formatted same as the lines below: struct pkt pkts[] = { /* Zero packet length and zero address allowed */ {0, 0, 0, true}, > + /* Zero packet length allowed */ > + {0x1000, 0, 0, true}, > + /* Straddling the start of umem */ > + {-2, PKT_SIZE, 0, false}, > + /* Packet too large */ > + {0x2000, XSK_UMEM__INVALID_FRAME_SIZE, 0, false}, > + /* After umem ends */ > + {UMEM_SIZE, PKT_SIZE, 0, false}, > + /* Straddle the end of umem */ > + {UMEM_SIZE - PKT_SIZE / 2, PKT_SIZE, 0, false}, > + /* Straddle a page boundrary */ > + {0x3000 - PKT_SIZE / 2, PKT_SIZE, 0, false}, > + /* Valid packet for synch so that something is received */ > + {0x4000, PKT_SIZE, 0, true}}; > + > + if (test->ifobj_tx->umem->unaligned_mode) { > + /* Crossing a page boundrary allowed */ > + pkts[6].valid = true; > + } > + pkt_stream_set(test, pkts, ARRAY_SIZE(pkts)); > + testapp_validate_traffic(test); > + pkt_stream_restore_default(test); > +} > + > static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac, > const char *dst_ip, const char *src_ip, const u16 dst_port, > const u16 src_port, thread_func_t func_ptr) > @@ -1159,7 +1256,7 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_ > case TEST_TYPE_BPF_RES: > testapp_bpf_res(test); > break; > - case TEST_TYPE_NOPOLL: > + case TEST_TYPE_RUN_TO_COMPLETION: I think that you were updating some comment around it on previous patches and it probably belongs to this one. > test_spec_set_name(test, "RUN_TO_COMPLETION"); > testapp_validate_traffic(test); > break; > @@ -1169,6 +1266,16 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_ > test_spec_set_name(test, "POLL"); > testapp_validate_traffic(test); > break; > + case TEST_TYPE_ALIGNED_INV_DESC: > + test_spec_set_name(test, "ALIGNED_INV_DESC"); > + testapp_inv_desc(test); > + break; > + case TEST_TYPE_UNALIGNED_INV_DESC: > + test_spec_set_name(test, "UNALIGNED_INV_DESC"); > + test->ifobj_tx->umem->unaligned_mode = true; > + test->ifobj_rx->umem->unaligned_mode = true; > + testapp_inv_desc(test); > + break; > case TEST_TYPE_UNALIGNED: > if (!testapp_unaligned(test)) > return; > diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h > index 129801eb013c..2d9efb89ea28 100644 > --- a/tools/testing/selftests/bpf/xdpxceiver.h > +++ b/tools/testing/selftests/bpf/xdpxceiver.h > @@ -38,6 +38,8 @@ > #define BATCH_SIZE 8 > #define POLL_TMOUT 1000 > #define DEFAULT_PKT_CNT (4 * 1024) > +#define DEFAULT_UMEM_BUFFERS (DEFAULT_PKT_CNT / 4) > +#define UMEM_SIZE (DEFAULT_UMEM_BUFFERS * XSK_UMEM__DEFAULT_FRAME_SIZE) > #define RX_FULL_RXQSIZE 32 > #define DEFAULT_OFFSET 256 > #define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1) > @@ -51,9 +53,11 @@ enum test_mode { > }; > > enum test_type { > - TEST_TYPE_NOPOLL, > + TEST_TYPE_RUN_TO_COMPLETION, > TEST_TYPE_POLL, > TEST_TYPE_UNALIGNED, > + TEST_TYPE_ALIGNED_INV_DESC, > + TEST_TYPE_UNALIGNED_INV_DESC, > TEST_TYPE_TEARDOWN, > TEST_TYPE_BIDI, > TEST_TYPE_STATS, > @@ -104,6 +108,7 @@ struct pkt { > > struct pkt_stream { > u32 nb_pkts; > + u32 rx_pkt_nb; > struct pkt *pkts; > bool use_addr_for_fill; > }; > -- > 2.29.0 >
On Fri, Sep 3, 2021 at 6:57 PM Maciej Fijalkowski <maciej.fijalkowski@intel.com> wrote: > > On Wed, Sep 01, 2021 at 12:47:31PM +0200, Magnus Karlsson wrote: > > From: Magnus Karlsson <magnus.karlsson@intel.com> > > > > Add tests for invalid xsk descriptors in the Tx ring. A number of > > handcrafted nasty invalid descriptors are created and submitted to the > > tx ring to check that they are validated correctly. Corener case valid > > Corner Will fix. > > ones are also sent. The tests are run for both aligned and unaligned > > mode. > > > > pkt_stream_set() is introduced to be able to create a hand-crafted > > packet stream where every single packet is specified in detail. > > > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> > > --- > > tools/testing/selftests/bpf/xdpxceiver.c | 143 ++++++++++++++++++++--- > > tools/testing/selftests/bpf/xdpxceiver.h | 7 +- > > 2 files changed, 131 insertions(+), 19 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c > > index d085033afd53..a4f6ce3a6b14 100644 > > --- a/tools/testing/selftests/bpf/xdpxceiver.c > > +++ b/tools/testing/selftests/bpf/xdpxceiver.c > > @@ -46,6 +46,8 @@ > > * then remove xsk sockets from queue 0 on both veth interfaces and > > * finally run a traffic on queues ids 1 > > * g. unaligned mode > > + * h. tests for invalid and corner case Tx descriptors so that the correct ones > > + * are discarded and let through, respectively. > > * > > * Total tests: 12 > > * > > @@ -394,7 +396,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx, > > for (j = 0; j < MAX_SOCKETS; j++) { > > memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j])); > > memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j])); > > - ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4; > > + ifobj->umem_arr[j].num_frames = DEFAULT_UMEM_BUFFERS; > > ifobj->umem_arr[j].frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE; > > ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS; > > } > > @@ -450,6 +452,16 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb) > > return &pkt_stream->pkts[pkt_nb]; > > } > > > > +static struct pkt *pkt_stream_get_next_rx_pkt(struct pkt_stream *pkt_stream) > > +{ > > + while (pkt_stream->rx_pkt_nb < pkt_stream->nb_pkts) { > > + if (pkt_stream->pkts[pkt_stream->rx_pkt_nb].valid) > > + return &pkt_stream->pkts[pkt_stream->rx_pkt_nb++]; > > + pkt_stream->rx_pkt_nb++; > > + } > > + return NULL; > > +} > > + > > static void pkt_stream_delete(struct pkt_stream *pkt_stream) > > { > > free(pkt_stream->pkts); > > @@ -465,17 +477,31 @@ static void pkt_stream_restore_default(struct test_spec *test) > > test->ifobj_rx->pkt_stream = test->pkt_stream_default; > > } > > > > -static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len) > > +static struct pkt_stream *__pkt_stream_alloc(u32 nb_pkts) > > { > > struct pkt_stream *pkt_stream; > > - u32 i; > > > > - pkt_stream = malloc(sizeof(*pkt_stream)); > > + pkt_stream = calloc(1, sizeof(*pkt_stream)); > > So with that probably my previous comment about use_addr_for_fill being > not set is not relevant. Will fix this earlier. > > if (!pkt_stream) > > - exit_with_error(ENOMEM); > > + return NULL; > > > > pkt_stream->pkts = calloc(nb_pkts, sizeof(*pkt_stream->pkts)); > > - if (!pkt_stream->pkts) > > + if (!pkt_stream->pkts) { > > + free(pkt_stream); > > + return NULL; > > + } > > + > > + pkt_stream->nb_pkts = nb_pkts; > > + return pkt_stream; > > +} > > + > > +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len) > > +{ > > + struct pkt_stream *pkt_stream; > > + u32 i; > > + > > + pkt_stream = __pkt_stream_alloc(nb_pkts); > > + if (!pkt_stream) > > exit_with_error(ENOMEM); > > > > pkt_stream->nb_pkts = nb_pkts; > > @@ -525,6 +551,26 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 off > > test->ifobj_rx->pkt_stream = pkt_stream; > > } > > > > +static void pkt_stream_set(struct test_spec *test, struct pkt *pkts, u32 nb_pkts) > > This is still a generation of pkt stream, can we name this is as: > pkt_stream_generate_custom or is this too long? WDYT? Sounds good. > > +{ > > + struct pkt_stream *pkt_stream; > > + u32 i; > > + > > + pkt_stream = __pkt_stream_alloc(nb_pkts); > > + if (!pkt_stream) > > + exit_with_error(ENOMEM); > > + > > + test->ifobj_tx->pkt_stream = pkt_stream; > > + test->ifobj_rx->pkt_stream = pkt_stream; > > + > > + for (i = 0; i < nb_pkts; i++) { > > + pkt_stream->pkts[i].addr = pkts[i].addr; > > + pkt_stream->pkts[i].len = pkts[i].len; > > + pkt_stream->pkts[i].payload = i; > > + pkt_stream->pkts[i].valid = pkts[i].valid; > > + } > > +} > > + > > static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb) > > { > > struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb); > > @@ -535,6 +581,8 @@ static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb) > > > > if (!pkt) > > return NULL; > > + if (!pkt->valid || pkt->len < PKT_SIZE) > > + return pkt; > > > > data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr); > > udp_hdr = (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr)); > > @@ -596,19 +644,24 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len) > > return false; > > } > > > > + if (len < PKT_SIZE) { > > + /*Do not try to verify packets that are smaller than minimun size. */ > > minimum > > > + return true; > > + } > > + > > + if (pkt->len != len) { > > + ksft_test_result_fail > > + ("ERROR: [%s] expected length [%d], got length [%d]\n", > > + __func__, pkt->len, len); > > + return false; > > + } > > + > > if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) { > > u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE))); > > > > if (opt_pkt_dump) > > pkt_dump(data, PKT_SIZE); > > > > - if (pkt->len != len) { > > - ksft_test_result_fail > > - ("ERROR: [%s] expected length [%d], got length [%d]\n", > > - __func__, pkt->len, len); > > - return false; > > - } > > - > > if (pkt->payload != seqnum) { > > ksft_test_result_fail > > ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n", > > @@ -645,6 +698,15 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size) > > > > rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx); > > if (rcvd) { > > + if (rcvd > xsk->outstanding_tx) { > > + u64 addr = *xsk_ring_cons__comp_addr(&xsk->umem->cq, idx + rcvd - 1); > > + > > + ksft_test_result_fail("ERROR: [%s] Too many packets completed\n", > > + __func__); > > + ksft_print_msg("Last completion address: %llx\n", addr); > > + return; > > + } > > + > > xsk_ring_cons__release(&xsk->umem->cq, rcvd); > > xsk->outstanding_tx -= rcvd; > > } > > @@ -653,11 +715,10 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size) > > static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *xsk, > > struct pollfd *fds) > > { > > - u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0; > > - struct pkt *pkt; > > + struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream); > > + u32 idx_rx = 0, idx_fq = 0, rcvd, i; > > int ret; > > > > - pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++); > > while (pkt) { > > rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx); > > if (!rcvd) { > > @@ -685,13 +746,21 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info * > > const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++); > > u64 addr = desc->addr, orig; > > > > + if (!pkt) { > > + ksft_test_result_fail("ERROR: [%s] Received too many packets.\n", > > + __func__); > > + ksft_print_msg("Last packet has addr: %llx len: %u\n", > > + addr, desc->len); > > + return; > > + } > > + > > orig = xsk_umem__extract_addr(addr); > > addr = xsk_umem__add_offset_to_addr(addr); > > if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len)) > > return; > > > > *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig; > > - pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++); > > + pkt = pkt_stream_get_next_rx_pkt(pkt_stream); > > } > > > > xsk_ring_prod__submit(&xsk->umem->fq, rcvd); > > @@ -875,6 +944,7 @@ static void testapp_cleanup_xsk_res(struct ifobject *ifobj) > > { > > print_verbose("Destroying socket\n"); > > xsk_socket__delete(ifobj->xsk->xsk); > > + munmap(ifobj->umem->buffer, ifobj->umem->num_frames * ifobj->umem->frame_size); > > xsk_umem__delete(ifobj->umem->umem); > > } > > > > @@ -1118,6 +1188,33 @@ static bool testapp_unaligned(struct test_spec *test) > > return true; > > } > > > > +static void testapp_inv_desc(struct test_spec *test) > > I'd say that speaking out whole 'invalid' word wouldn't hurt us here. > > > +{ > > + struct pkt pkts[] = {{0, 0, 0, true}, /* Zero packet length and zero address allowed */ > > Above looks a bit weird, maybe this should be formatted same as the lines > below: > > struct pkt pkts[] = { > /* Zero packet length and zero address allowed */ > {0, 0, 0, true}, > > > + /* Zero packet length allowed */ > > + {0x1000, 0, 0, true}, > > + /* Straddling the start of umem */ > > + {-2, PKT_SIZE, 0, false}, > > + /* Packet too large */ > > + {0x2000, XSK_UMEM__INVALID_FRAME_SIZE, 0, false}, > > + /* After umem ends */ > > + {UMEM_SIZE, PKT_SIZE, 0, false}, > > + /* Straddle the end of umem */ > > + {UMEM_SIZE - PKT_SIZE / 2, PKT_SIZE, 0, false}, > > + /* Straddle a page boundrary */ > > + {0x3000 - PKT_SIZE / 2, PKT_SIZE, 0, false}, > > + /* Valid packet for synch so that something is received */ > > + {0x4000, PKT_SIZE, 0, true}}; > > + > > + if (test->ifobj_tx->umem->unaligned_mode) { > > + /* Crossing a page boundrary allowed */ > > + pkts[6].valid = true; > > + } > > + pkt_stream_set(test, pkts, ARRAY_SIZE(pkts)); > > + testapp_validate_traffic(test); > > + pkt_stream_restore_default(test); > > +} > > + > > static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac, > > const char *dst_ip, const char *src_ip, const u16 dst_port, > > const u16 src_port, thread_func_t func_ptr) > > @@ -1159,7 +1256,7 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_ > > case TEST_TYPE_BPF_RES: > > testapp_bpf_res(test); > > break; > > - case TEST_TYPE_NOPOLL: > > + case TEST_TYPE_RUN_TO_COMPLETION: > > I think that you were updating some comment around it on previous patches > and it probably belongs to this one. Will fix plus all the other suggestions above. Thank you Maciej! /Magnus > > test_spec_set_name(test, "RUN_TO_COMPLETION"); > > testapp_validate_traffic(test); > > break; > > @@ -1169,6 +1266,16 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_ > > test_spec_set_name(test, "POLL"); > > testapp_validate_traffic(test); > > break; > > + case TEST_TYPE_ALIGNED_INV_DESC: > > + test_spec_set_name(test, "ALIGNED_INV_DESC"); > > + testapp_inv_desc(test); > > + break; > > + case TEST_TYPE_UNALIGNED_INV_DESC: > > + test_spec_set_name(test, "UNALIGNED_INV_DESC"); > > + test->ifobj_tx->umem->unaligned_mode = true; > > + test->ifobj_rx->umem->unaligned_mode = true; > > + testapp_inv_desc(test); > > + break; > > case TEST_TYPE_UNALIGNED: > > if (!testapp_unaligned(test)) > > return; > > diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h > > index 129801eb013c..2d9efb89ea28 100644 > > --- a/tools/testing/selftests/bpf/xdpxceiver.h > > +++ b/tools/testing/selftests/bpf/xdpxceiver.h > > @@ -38,6 +38,8 @@ > > #define BATCH_SIZE 8 > > #define POLL_TMOUT 1000 > > #define DEFAULT_PKT_CNT (4 * 1024) > > +#define DEFAULT_UMEM_BUFFERS (DEFAULT_PKT_CNT / 4) > > +#define UMEM_SIZE (DEFAULT_UMEM_BUFFERS * XSK_UMEM__DEFAULT_FRAME_SIZE) > > #define RX_FULL_RXQSIZE 32 > > #define DEFAULT_OFFSET 256 > > #define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1) > > @@ -51,9 +53,11 @@ enum test_mode { > > }; > > > > enum test_type { > > - TEST_TYPE_NOPOLL, > > + TEST_TYPE_RUN_TO_COMPLETION, > > TEST_TYPE_POLL, > > TEST_TYPE_UNALIGNED, > > + TEST_TYPE_ALIGNED_INV_DESC, > > + TEST_TYPE_UNALIGNED_INV_DESC, > > TEST_TYPE_TEARDOWN, > > TEST_TYPE_BIDI, > > TEST_TYPE_STATS, > > @@ -104,6 +108,7 @@ struct pkt { > > > > struct pkt_stream { > > u32 nb_pkts; > > + u32 rx_pkt_nb; > > struct pkt *pkts; > > bool use_addr_for_fill; > > }; > > -- > > 2.29.0 > >
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c index d085033afd53..a4f6ce3a6b14 100644 --- a/tools/testing/selftests/bpf/xdpxceiver.c +++ b/tools/testing/selftests/bpf/xdpxceiver.c @@ -46,6 +46,8 @@ * then remove xsk sockets from queue 0 on both veth interfaces and * finally run a traffic on queues ids 1 * g. unaligned mode + * h. tests for invalid and corner case Tx descriptors so that the correct ones + * are discarded and let through, respectively. * * Total tests: 12 * @@ -394,7 +396,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx, for (j = 0; j < MAX_SOCKETS; j++) { memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j])); memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j])); - ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4; + ifobj->umem_arr[j].num_frames = DEFAULT_UMEM_BUFFERS; ifobj->umem_arr[j].frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE; ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS; } @@ -450,6 +452,16 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb) return &pkt_stream->pkts[pkt_nb]; } +static struct pkt *pkt_stream_get_next_rx_pkt(struct pkt_stream *pkt_stream) +{ + while (pkt_stream->rx_pkt_nb < pkt_stream->nb_pkts) { + if (pkt_stream->pkts[pkt_stream->rx_pkt_nb].valid) + return &pkt_stream->pkts[pkt_stream->rx_pkt_nb++]; + pkt_stream->rx_pkt_nb++; + } + return NULL; +} + static void pkt_stream_delete(struct pkt_stream *pkt_stream) { free(pkt_stream->pkts); @@ -465,17 +477,31 @@ static void pkt_stream_restore_default(struct test_spec *test) test->ifobj_rx->pkt_stream = test->pkt_stream_default; } -static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len) +static struct pkt_stream *__pkt_stream_alloc(u32 nb_pkts) { struct pkt_stream *pkt_stream; - u32 i; - pkt_stream = malloc(sizeof(*pkt_stream)); + pkt_stream = calloc(1, sizeof(*pkt_stream)); if (!pkt_stream) - exit_with_error(ENOMEM); + return NULL; pkt_stream->pkts = calloc(nb_pkts, sizeof(*pkt_stream->pkts)); - if (!pkt_stream->pkts) + if (!pkt_stream->pkts) { + free(pkt_stream); + return NULL; + } + + pkt_stream->nb_pkts = nb_pkts; + return pkt_stream; +} + +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len) +{ + struct pkt_stream *pkt_stream; + u32 i; + + pkt_stream = __pkt_stream_alloc(nb_pkts); + if (!pkt_stream) exit_with_error(ENOMEM); pkt_stream->nb_pkts = nb_pkts; @@ -525,6 +551,26 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 off test->ifobj_rx->pkt_stream = pkt_stream; } +static void pkt_stream_set(struct test_spec *test, struct pkt *pkts, u32 nb_pkts) +{ + struct pkt_stream *pkt_stream; + u32 i; + + pkt_stream = __pkt_stream_alloc(nb_pkts); + if (!pkt_stream) + exit_with_error(ENOMEM); + + test->ifobj_tx->pkt_stream = pkt_stream; + test->ifobj_rx->pkt_stream = pkt_stream; + + for (i = 0; i < nb_pkts; i++) { + pkt_stream->pkts[i].addr = pkts[i].addr; + pkt_stream->pkts[i].len = pkts[i].len; + pkt_stream->pkts[i].payload = i; + pkt_stream->pkts[i].valid = pkts[i].valid; + } +} + static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb) { struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb); @@ -535,6 +581,8 @@ static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb) if (!pkt) return NULL; + if (!pkt->valid || pkt->len < PKT_SIZE) + return pkt; data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr); udp_hdr = (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr)); @@ -596,19 +644,24 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len) return false; } + if (len < PKT_SIZE) { + /*Do not try to verify packets that are smaller than minimun size. */ + return true; + } + + if (pkt->len != len) { + ksft_test_result_fail + ("ERROR: [%s] expected length [%d], got length [%d]\n", + __func__, pkt->len, len); + return false; + } + if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) { u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE))); if (opt_pkt_dump) pkt_dump(data, PKT_SIZE); - if (pkt->len != len) { - ksft_test_result_fail - ("ERROR: [%s] expected length [%d], got length [%d]\n", - __func__, pkt->len, len); - return false; - } - if (pkt->payload != seqnum) { ksft_test_result_fail ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n", @@ -645,6 +698,15 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size) rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx); if (rcvd) { + if (rcvd > xsk->outstanding_tx) { + u64 addr = *xsk_ring_cons__comp_addr(&xsk->umem->cq, idx + rcvd - 1); + + ksft_test_result_fail("ERROR: [%s] Too many packets completed\n", + __func__); + ksft_print_msg("Last completion address: %llx\n", addr); + return; + } + xsk_ring_cons__release(&xsk->umem->cq, rcvd); xsk->outstanding_tx -= rcvd; } @@ -653,11 +715,10 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size) static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *xsk, struct pollfd *fds) { - u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0; - struct pkt *pkt; + struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream); + u32 idx_rx = 0, idx_fq = 0, rcvd, i; int ret; - pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++); while (pkt) { rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx); if (!rcvd) { @@ -685,13 +746,21 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info * const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++); u64 addr = desc->addr, orig; + if (!pkt) { + ksft_test_result_fail("ERROR: [%s] Received too many packets.\n", + __func__); + ksft_print_msg("Last packet has addr: %llx len: %u\n", + addr, desc->len); + return; + } + orig = xsk_umem__extract_addr(addr); addr = xsk_umem__add_offset_to_addr(addr); if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len)) return; *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig; - pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++); + pkt = pkt_stream_get_next_rx_pkt(pkt_stream); } xsk_ring_prod__submit(&xsk->umem->fq, rcvd); @@ -875,6 +944,7 @@ static void testapp_cleanup_xsk_res(struct ifobject *ifobj) { print_verbose("Destroying socket\n"); xsk_socket__delete(ifobj->xsk->xsk); + munmap(ifobj->umem->buffer, ifobj->umem->num_frames * ifobj->umem->frame_size); xsk_umem__delete(ifobj->umem->umem); } @@ -1118,6 +1188,33 @@ static bool testapp_unaligned(struct test_spec *test) return true; } +static void testapp_inv_desc(struct test_spec *test) +{ + struct pkt pkts[] = {{0, 0, 0, true}, /* Zero packet length and zero address allowed */ + /* Zero packet length allowed */ + {0x1000, 0, 0, true}, + /* Straddling the start of umem */ + {-2, PKT_SIZE, 0, false}, + /* Packet too large */ + {0x2000, XSK_UMEM__INVALID_FRAME_SIZE, 0, false}, + /* After umem ends */ + {UMEM_SIZE, PKT_SIZE, 0, false}, + /* Straddle the end of umem */ + {UMEM_SIZE - PKT_SIZE / 2, PKT_SIZE, 0, false}, + /* Straddle a page boundrary */ + {0x3000 - PKT_SIZE / 2, PKT_SIZE, 0, false}, + /* Valid packet for synch so that something is received */ + {0x4000, PKT_SIZE, 0, true}}; + + if (test->ifobj_tx->umem->unaligned_mode) { + /* Crossing a page boundrary allowed */ + pkts[6].valid = true; + } + pkt_stream_set(test, pkts, ARRAY_SIZE(pkts)); + testapp_validate_traffic(test); + pkt_stream_restore_default(test); +} + static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac, const char *dst_ip, const char *src_ip, const u16 dst_port, const u16 src_port, thread_func_t func_ptr) @@ -1159,7 +1256,7 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_ case TEST_TYPE_BPF_RES: testapp_bpf_res(test); break; - case TEST_TYPE_NOPOLL: + case TEST_TYPE_RUN_TO_COMPLETION: test_spec_set_name(test, "RUN_TO_COMPLETION"); testapp_validate_traffic(test); break; @@ -1169,6 +1266,16 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_ test_spec_set_name(test, "POLL"); testapp_validate_traffic(test); break; + case TEST_TYPE_ALIGNED_INV_DESC: + test_spec_set_name(test, "ALIGNED_INV_DESC"); + testapp_inv_desc(test); + break; + case TEST_TYPE_UNALIGNED_INV_DESC: + test_spec_set_name(test, "UNALIGNED_INV_DESC"); + test->ifobj_tx->umem->unaligned_mode = true; + test->ifobj_rx->umem->unaligned_mode = true; + testapp_inv_desc(test); + break; case TEST_TYPE_UNALIGNED: if (!testapp_unaligned(test)) return; diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h index 129801eb013c..2d9efb89ea28 100644 --- a/tools/testing/selftests/bpf/xdpxceiver.h +++ b/tools/testing/selftests/bpf/xdpxceiver.h @@ -38,6 +38,8 @@ #define BATCH_SIZE 8 #define POLL_TMOUT 1000 #define DEFAULT_PKT_CNT (4 * 1024) +#define DEFAULT_UMEM_BUFFERS (DEFAULT_PKT_CNT / 4) +#define UMEM_SIZE (DEFAULT_UMEM_BUFFERS * XSK_UMEM__DEFAULT_FRAME_SIZE) #define RX_FULL_RXQSIZE 32 #define DEFAULT_OFFSET 256 #define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1) @@ -51,9 +53,11 @@ enum test_mode { }; enum test_type { - TEST_TYPE_NOPOLL, + TEST_TYPE_RUN_TO_COMPLETION, TEST_TYPE_POLL, TEST_TYPE_UNALIGNED, + TEST_TYPE_ALIGNED_INV_DESC, + TEST_TYPE_UNALIGNED_INV_DESC, TEST_TYPE_TEARDOWN, TEST_TYPE_BIDI, TEST_TYPE_STATS, @@ -104,6 +108,7 @@ struct pkt { struct pkt_stream { u32 nb_pkts; + u32 rx_pkt_nb; struct pkt *pkts; bool use_addr_for_fill; };