[dpdk-dev] [PATCH v5 13/15] test/distributor: add test with packets marking

David Hunt david.hunt at intel.com
Fri Oct 9 14:50:22 CEST 2020


Hi Lukasz,

On 8/10/2020 6:23 AM, Lukasz Wojciechowski wrote:
> All of the former tests analyzed only statistics
> of packets processed by all workers.
> The new test verifies also if packets are processed
> on workers as expected.
> Every packets processed by the worker is marked
> and analyzed after it is returned to distributor.
>
> This test allows finding issues in matching algorithms.
>
> Signed-off-by: Lukasz Wojciechowski <l.wojciechow at partner.samsung.com>
> ---
>   app/test/test_distributor.c | 141 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 141 insertions(+)
>
> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
> index 1e0a079ff..0404e463a 100644
> --- a/app/test/test_distributor.c
> +++ b/app/test/test_distributor.c
> @@ -542,6 +542,141 @@ test_flush_with_worker_shutdown(struct worker_params *wp,
>   	return 0;
>   }
>   
> +static int
> +handle_and_mark_work(void *arg)
> +{
> +	struct rte_mbuf *buf[8] __rte_cache_aligned;
> +	struct worker_params *wp = arg;
> +	struct rte_distributor *db = wp->dist;
> +	unsigned int num, i;
> +	unsigned int id = __atomic_fetch_add(&worker_idx, 1, __ATOMIC_RELAXED);
> +	num = rte_distributor_get_pkt(db, id, buf, NULL, 0);
> +	while (!quit) {
> +		__atomic_fetch_add(&worker_stats[id].handled_packets, num,
> +				__ATOMIC_ACQ_REL);
> +		for (i = 0; i < num; i++)
> +			buf[i]->udata64 += id + 1;
> +		num = rte_distributor_get_pkt(db, id,
> +				buf, buf, num);
> +	}
> +	__atomic_fetch_add(&worker_stats[id].handled_packets, num,
> +			__ATOMIC_ACQ_REL);
> +	rte_distributor_return_pkt(db, id, buf, num);
> +	return 0;
> +}
> +
> +/* sanity_mark_test sends packets to workers which mark them.
> + * Every packet has also encoded sequence number.
> + * The returned packets are sorted and verified if they were handled
> + * by proper workers.
> + */
> +static int
> +sanity_mark_test(struct worker_params *wp, struct rte_mempool *p)
> +{
> +	const unsigned int buf_count = 24;
> +	const unsigned int burst = 8;
> +	const unsigned int shift = 12;
> +	const unsigned int seq_shift = 10;
> +
> +	struct rte_distributor *db = wp->dist;
> +	struct rte_mbuf *bufs[buf_count];
> +	struct rte_mbuf *returns[buf_count];
> +	unsigned int i, count, id;
> +	unsigned int sorted[buf_count], seq;
> +	unsigned int failed = 0;
> +
> +	printf("=== Marked packets test ===\n");
> +	clear_packet_count();
> +	if (rte_mempool_get_bulk(p, (void *)bufs, buf_count) != 0) {
> +		printf("line %d: Error getting mbufs from pool\n", __LINE__);
> +		return -1;
> +	}
> +
> +/* bufs' hashes will be like these below, but shifted left.
> + * The shifting is for avoiding collisions with backlogs
> + * and in-flight tags left by previous tests.
> + * [1, 1, 1, 1, 1, 1, 1, 1
> + *  1, 1, 1, 1, 2, 2, 2, 2
> + *  2, 2, 2, 2, 1, 1, 1, 1]
> + */

I would suggest indenting the comments to the same indent level as the 
code, this
would make the flow easier to read. Same with additional comments below.


> +	for (i = 0; i < burst; i++) {
> +		bufs[0 * burst + i]->hash.usr = 1 << shift;
> +		bufs[1 * burst + i]->hash.usr = ((i < burst / 2) ? 1 : 2)
> +			<< shift;
> +		bufs[2 * burst + i]->hash.usr = ((i < burst / 2) ? 2 : 1)
> +			<< shift;
> +	}
> +/* Assign a sequence number to each packet. The sequence is shifted,
> + * so that lower bits of the udate64 will hold mark from worker.
> + */
> +	for (i = 0; i < buf_count; i++)
> +		bufs[i]->udata64 = i << seq_shift;
> +
> +	count = 0;
> +	for (i = 0; i < buf_count/burst; i++) {
> +		rte_distributor_process(db, &bufs[i * burst], burst);
> +		count += rte_distributor_returned_pkts(db, &returns[count],
> +			buf_count - count);
> +	}
> +
> +	do {
> +		rte_distributor_flush(db);
> +		count += rte_distributor_returned_pkts(db, &returns[count],
> +			buf_count - count);
> +	} while (count < buf_count);
> +
> +	for (i = 0; i < rte_lcore_count() - 1; i++)
> +		printf("Worker %u handled %u packets\n", i,
> +			__atomic_load_n(&worker_stats[i].handled_packets,
> +					__ATOMIC_ACQUIRE));
> +
> +/* Sort returned packets by sent order (sequence numbers). */
> +	for (i = 0; i < buf_count; i++) {
> +		seq = returns[i]->udata64 >> seq_shift;
> +		id = returns[i]->udata64 - (seq << seq_shift);
> +		sorted[seq] = id;
> +	}
> +
> +/* Verify that packets [0-11] and [20-23] were processed
> + * by the same worker
> + */
> +	for (i = 1; i < 12; i++) {
> +		if (sorted[i] != sorted[0]) {
> +			printf("Packet number %u processed by worker %u,"
> +				" but should be processes by worker %u\n",
> +				i, sorted[i], sorted[0]);
> +			failed = 1;
> +		}
> +	}
> +	for (i = 20; i < 24; i++) {
> +		if (sorted[i] != sorted[0]) {
> +			printf("Packet number %u processed by worker %u,"
> +				" but should be processes by worker %u\n",
> +				i, sorted[i], sorted[0]);
> +			failed = 1;
> +		}
> +	}
> +/* And verify that packets [12-19] were processed
> + * by the another worker
> + */
> +	for (i = 13; i < 20; i++) {
> +		if (sorted[i] != sorted[12]) {
> +			printf("Packet number %u processed by worker %u,"
> +				" but should be processes by worker %u\n",
> +				i, sorted[i], sorted[12]);
> +			failed = 1;
> +		}
> +	}
> +
> +	rte_mempool_put_bulk(p, (void *)bufs, buf_count);
> +
> +	if (failed)
> +		return -1;
> +
> +	printf("Marked packets test passed\n");
> +	return 0;
> +}
> +
>   static
>   int test_error_distributor_create_name(void)
>   {
> @@ -726,6 +861,12 @@ test_distributor(void)
>   				goto err;
>   			quit_workers(&worker_params, p);
>   
> +			rte_eal_mp_remote_launch(handle_and_mark_work,
> +					&worker_params, SKIP_MASTER);
> +			if (sanity_mark_test(&worker_params, p) < 0)
> +				goto err;
> +			quit_workers(&worker_params, p);
> +
>   		} else {
>   			printf("Too few cores to run worker shutdown test\n");
>   		}


Checking the flows go to the correct workers is areally good test to 
have. Thanks for the effort here.

Apart from the comment indentation nit: the rest looks good to me.

Acked-by: David Hunt <david.hunt at intel.com>




More information about the dev mailing list