DPDK patches and discussions
 help / color / Atom feed
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
To: "Wang, Yipeng1" <yipeng1.wang@intel.com>,
	Dharmik Thakkar <Dharmik.Thakkar@arm.com>,
	"Gobriel, Sameh" <sameh.gobriel@intel.com>,
	"Richardson, Bruce" <bruce.richardson@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, nd <nd@arm.com>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH v5 4/4] test/hash: add tests for integrated	RCU QSBR
Date: Wed, 21 Oct 2020 22:34:48 +0000
Message-ID: <DBAPR08MB5814C94EE0CFB93FCD061BCE981C0@DBAPR08MB5814.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <BYAPR11MB34944501F088E24F23F28316C31C0@BYAPR11MB3494.namprd11.prod.outlook.com>

<snip>

> >
> > Add functional and performance tests for the integrated RCU QSBR.
> >
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >  app/test/test_hash.c                   | 390 ++++++++++++++++++++++++-
> >  app/test/test_hash_readwrite_lf_perf.c | 170 ++++++++++-
> >  2 files changed, 556 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test/test_hash.c b/app/test/test_hash.c index
> > 990a1815f893..22b47b3e7728 100644
> > --- a/app/test/test_hash.c
> > +++ b/app/test/test_hash.c
> > @@ -52,7 +52,7 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5,
> > 6, 7, 8, 10, 11, 15, 16, 21,
> >  	}								\
> >  } while(0)
> >
> > -#define RETURN_IF_ERROR_FBK(cond, str, ...) do {
> > 	\
> > +#define RETURN_IF_ERROR_FBK(cond, str, ...) do {			\
> >  	if (cond) {							\
> >  		printf("ERROR line %d: " str "\n", __LINE__, ##__VA_ARGS__); \
> >  		if (handle) rte_fbk_hash_free(handle);			\
> > @@ -60,6 +60,20 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5,
> > 6, 7, 8, 10, 11, 15, 16, 21,
> >  	}								\
> >  } while(0)
> >
> > +#define RETURN_IF_ERROR_RCU_QSBR(cond, str, ...) do {
> > 	\
> > +	if (cond) {							\
> > +		printf("ERROR line %d: " str "\n", __LINE__, ##__VA_ARGS__);
> > \
> > +		if (rcu_cfg.mode == RTE_HASH_QSBR_MODE_SYNC) {
> > 	\
> > +			writer_done = 1;				\
> > +			/* Wait until reader exited. */			\
> > +			rte_eal_mp_wait_lcore();			\
> > +		}							\
> > +		if (g_handle) rte_hash_free(g_handle);			\
> > +		if (g_qsv) rte_free(g_qsv);				\
> > +		return -1;						\
> > +	}								\
> > +} while(0)
> > +
> >  /* 5-tuple key type */
> >  struct flow_key {
> >  	uint32_t ip_src;
> > @@ -1801,6 +1815,365 @@ test_hash_add_delete_jhash_3word(void)
> >  	return ret;
> >  }
> >
> > +static struct rte_hash *g_handle;
> > +static struct rte_rcu_qsbr *g_qsv;
> > +static volatile uint8_t writer_done;
> > +struct flow_key g_rand_keys[9];
> > +/*
> > + * rte_hash_rcu_qsbr_add positive and negative tests.
> > + *  - Add RCU QSBR variable to Hash
> > + *  - Add another RCU QSBR variable to Hash
> > + *  - Check returns
> > + */
> > +static int
> > +test_hash_rcu_qsbr_add(void)
> > +{
> > +	size_t sz;
> > +	struct rte_rcu_qsbr *qsv2 = NULL;
> > +	int32_t status;
> > +	struct rte_hash_rcu_config rcu_cfg = {0};
> > +
> > +	struct rte_hash_parameters params;
> > +
> > +	printf("\n# Running RCU QSBR add tests\n");
> > +	memcpy(&params, &ut_params, sizeof(params));
> > +	params.name = "test_hash_rcu_qsbr_add";
> > +	params.extra_flag = RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF
> > |
> > +
> > 	RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > +	g_handle = rte_hash_create(&params);
> > +	RETURN_IF_ERROR_RCU_QSBR(g_handle == NULL, "Hash creation
> > failed");
> > +
> > +	/* Create RCU QSBR variable */
> > +	sz = rte_rcu_qsbr_get_memsize(RTE_MAX_LCORE);
> > +	g_qsv = (struct rte_rcu_qsbr *)rte_zmalloc_socket(NULL, sz,
> > +					RTE_CACHE_LINE_SIZE,
> > SOCKET_ID_ANY);
> > +	RETURN_IF_ERROR_RCU_QSBR(g_qsv == NULL,
> > +				 "RCU QSBR variable creation failed");
> > +
> > +	status = rte_rcu_qsbr_init(g_qsv, RTE_MAX_LCORE);
> [Wang, Yipeng] It reminds me that could we hide this function in the
> rte_cuckoo_hash.c as well?
> I saw most of the rcu related functions are hidden in the hash implementation, it
> would be less confusing if we hide this one as well.
The functionality required for resource reclamation is split into various parts [1]. The functionality of creating the RCU variable, initializing it etc is left to the application. The application has better knowledge of the data plane threads, which ones are using the (hash) library etc. Hence it is better to keep this in the application.

[1] http://doc.dpdk.org/guides/prog_guide/rcu_lib.html#resource-reclamation-framework-for-dpdk

> 
> > +	RETURN_IF_ERROR_RCU_QSBR(status != 0,
> > +				 "RCU QSBR variable initialization failed");
> > +
> > +	rcu_cfg.v = g_qsv;
> > +	/* Invalid QSBR mode */
> > +	rcu_cfg.mode = 2;
> [Wang, Yipeng] Any other way rather than hardcode 2 here? Maybe just a large
> number like 0xff?
> 
> > +	status = rte_hash_rcu_qsbr_add(g_handle, &rcu_cfg);
> > +	RETURN_IF_ERROR_RCU_QSBR(status == 0, "Invalid QSBR mode test
> > +failed");
> > +
> > +	rcu_cfg.mode = RTE_HASH_QSBR_MODE_DQ;
> [Wang, Yipeng] This reminds me that if there is an explanation on the difference
> of the two modes for users to easy to choose?
> 
> > +	/* Attach RCU QSBR to hash table */
> > +	status = rte_hash_rcu_qsbr_add(g_handle, &rcu_cfg);
> > +	RETURN_IF_ERROR_RCU_QSBR(status != 0,
> > +				 "Attach RCU QSBR to hash table failed");
> > +
> > +	/* Create and attach another RCU QSBR to hash table */
> > +	qsv2 = (struct rte_rcu_qsbr *)rte_zmalloc_socket(NULL, sz,
> > +					RTE_CACHE_LINE_SIZE,
> > SOCKET_ID_ANY);
> > +	RETURN_IF_ERROR_RCU_QSBR(qsv2 == NULL,
> > +				 "RCU QSBR variable creation failed");
> > +
> > +	rcu_cfg.v = qsv2;
> > +	rcu_cfg.mode = RTE_HASH_QSBR_MODE_SYNC;
> > +	status = rte_hash_rcu_qsbr_add(g_handle, &rcu_cfg);
> > +	rte_free(qsv2);
> > +	RETURN_IF_ERROR_RCU_QSBR(status == 0,
> > +			"Attach RCU QSBR to hash table succeeded where
> > failure"
> > +			" is expected");
> > +
> > +	rte_hash_free(g_handle);
> > +	rte_free(g_qsv);
> > +
> > +	return 0;
> > +}
> <...>

  parent reply index

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-01  6:58 [dpdk-dev] [RFC 0/3] RCU integration with hash library Dharmik Thakkar
2019-09-01  6:58 ` [dpdk-dev] [RFC 1/3] net/ixgbe: avoid multpile definitions of 'bool' Dharmik Thakkar
2019-09-05 15:26   ` Stephen Hemminger
2019-09-05 20:10     ` Dharmik Thakkar
2019-09-05 20:23       ` Stephen Hemminger
2019-09-01  6:58 ` [dpdk-dev] [RFC 2/3] lib/hash: integrate RCU QSBR Dharmik Thakkar
2019-09-01  6:58 ` [dpdk-dev] [RFC 3/3] test/hash: add tests for integrated " Dharmik Thakkar
2019-09-05 21:31 ` [dpdk-dev] [RFC 0/3] RCU integration with hash library Wang, Yipeng1
2020-08-19  4:05 ` [dpdk-dev] [RFC v2] lib/hash: integrate RCU QSBR Dharmik Thakkar
2020-08-25 19:59   ` Dharmik Thakkar
2020-08-31 20:47   ` Wang, Yipeng1
2020-09-01 22:01     ` Dharmik Thakkar
2020-10-16 13:53       ` David Marchand
2020-10-16 15:04         ` Dharmik Thakkar
2020-10-16 17:38   ` [dpdk-dev] [PATCH 0/3] hash: " Dharmik Thakkar
2020-10-16 17:38     ` [dpdk-dev] [PATCH v3 1/3] lib/hash: " Dharmik Thakkar
2020-10-19  9:43       ` Kinsella, Ray
2020-10-16 17:38     ` [dpdk-dev] [PATCH v3 2/3] test/hash: replace rte atomic with C11 atomic APIs Dharmik Thakkar
2020-10-16 17:38     ` [dpdk-dev] [PATCH v3 3/3] test/hash: add tests for integrated RCU QSBR Dharmik Thakkar
2020-10-19 14:48     ` [dpdk-dev] [PATCH 0/3] hash: integrate " David Marchand
2020-10-19 16:35     ` [dpdk-dev] [PATCH v4 " Dharmik Thakkar
2020-10-19 16:35       ` [dpdk-dev] [PATCH v4 1/3] lib/hash: " Dharmik Thakkar
2020-10-19 16:35       ` [dpdk-dev] [PATCH v4 2/3] test/hash: replace rte atomic with C11 atomic APIs Dharmik Thakkar
2020-10-19 16:35       ` [dpdk-dev] [PATCH v4 3/3] test/hash: add tests for integrated RCU QSBR Dharmik Thakkar
2020-10-19 21:05       ` [dpdk-dev] [PATCH v4 0/3] hash: integrate " David Marchand
2020-10-20  4:08         ` Dharmik Thakkar
2020-10-20 16:12       ` [dpdk-dev] [PATCH v5 0/4] " Dharmik Thakkar
2020-10-20 16:12         ` [dpdk-dev] [PATCH v5 1/4] rcu: build on Windows Dharmik Thakkar
2020-10-20 16:12         ` [dpdk-dev] [PATCH v5 2/4] lib/hash: integrate RCU QSBR Dharmik Thakkar
2020-10-21  2:42           ` Wang, Yipeng1
2020-10-21  4:52             ` Dharmik Thakkar
2020-10-20 16:13         ` [dpdk-dev] [PATCH v5 3/4] test/hash: replace rte atomic with C11 atomic APIs Dharmik Thakkar
2020-10-21  2:52           ` Wang, Yipeng1
2020-10-20 16:13         ` [dpdk-dev] [PATCH v5 4/4] test/hash: add tests for integrated RCU QSBR Dharmik Thakkar
2020-10-21  3:54           ` Wang, Yipeng1
2020-10-21  4:55             ` Dharmik Thakkar
2020-10-21 22:34             ` Honnappa Nagarahalli [this message]
2020-10-21 22:50         ` [dpdk-dev] [PATCH v6 0/4] hash: integrate " Dharmik Thakkar
2020-10-21 22:50           ` [dpdk-dev] [PATCH v6 1/4] rcu: build on Windows Dharmik Thakkar
2020-10-22  9:05             ` David Marchand
2020-10-22 18:35             ` Kadam, Pallavi
2020-10-21 22:50           ` [dpdk-dev] [PATCH v6 2/4] lib/hash: integrate RCU QSBR Dharmik Thakkar
2020-10-23 21:46             ` Dharmik Thakkar
2020-10-23 22:08             ` Wang, Yipeng1
2020-10-21 22:50           ` [dpdk-dev] [PATCH v6 3/4] test/hash: replace rte atomic with C11 atomic APIs Dharmik Thakkar
2020-10-21 22:50           ` [dpdk-dev] [PATCH v6 4/4] test/hash: add tests for integrated RCU QSBR Dharmik Thakkar
2020-10-23 22:11             ` Wang, Yipeng1
2020-10-24  9:09           ` [dpdk-dev] [PATCH v6 0/4] hash: integrate " David Marchand
2020-10-26 13:56             ` Dharmik Thakkar

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DBAPR08MB5814C94EE0CFB93FCD061BCE981C0@DBAPR08MB5814.eurprd08.prod.outlook.com \
    --to=honnappa.nagarahalli@arm.com \
    --cc=Dharmik.Thakkar@arm.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=nd@arm.com \
    --cc=sameh.gobriel@intel.com \
    --cc=yipeng1.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox