From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 2A9F72E89 for ; Wed, 5 Nov 2014 16:04:35 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 05 Nov 2014 07:11:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,320,1413270000"; d="scan'208";a="631963276" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga002.jf.intel.com with ESMTP; 05 Nov 2014 07:13:57 -0800 Received: from irsmsx106.ger.corp.intel.com (163.33.3.31) by IRSMSX102.ger.corp.intel.com (163.33.3.155) with Microsoft SMTP Server (TLS) id 14.3.195.1; Wed, 5 Nov 2014 15:13:41 +0000 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.56]) by IRSMSX106.ger.corp.intel.com ([169.254.8.179]) with mapi id 14.03.0195.001; Wed, 5 Nov 2014 15:13:40 +0000 From: "Ananyev, Konstantin" To: "Richardson, Bruce" , Qinglai Xiao Thread-Topic: [dpdk-dev] [PATCH] Add user defined tag calculation callback to librte_distributor. Thread-Index: AQHP+QUSib91niVWOkCh+HICVAhez5xSIwEQ Date: Wed, 5 Nov 2014 15:13:39 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258213A2938@IRSMSX105.ger.corp.intel.com> References: <1415194237-1219-1-git-send-email-jigsaw@gmail.com> <20141105142743.GB9856@bricha3-MOBL3> In-Reply-To: <20141105142743.GB9856@bricha3-MOBL3> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] Add user defined tag calculation callback to librte_distributor. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Nov 2014 15:04:37 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson > Sent: Wednesday, November 05, 2014 2:28 PM > To: Qinglai Xiao > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] Add user defined tag calculation callback= to librte_distributor. >=20 > On Wed, Nov 05, 2014 at 03:30:37PM +0200, Qinglai Xiao wrote: > > User defined tag calculation has access to mbuf. > > Default tag is RSS hash result. > > >=20 > Interesting idea. > Did you investigate was there any performance improvement or regression c= omparing > whether the callback was called per-packet as packets were dequeued for d= istribution > (i.e. how you have things now in your patch), compared to calling > the callback in a loop to extract the tags for all packets initially? I s= uspect > there probably isn't much performance difference either way, but it may b= e worth > checking. > One other point, is that I think the callback to extract the tag should h= ave > additional parameters - at least one, if not two. I would suggest that th= e > distributor pointer be passed in, as well as an arbitrary void * pointer. Just wonder, why do you need a call-back? Why not just make rte_distributor_process() to accept an extra parameter: a= rray of tags? rte_distributor_process(struct rte_distributor *d, struct rte_mbuf **mbufs, uint32_t *mbuf_tags, unsigned num_= mbufs) ? >=20 > Regards, > /Bruce >=20 > > Signed-off-by: Qinglai Xiao > > --- > > app/test/test_distributor.c | 6 +++--- > > app/test/test_distributor_perf.c | 2 +- > > lib/librte_distributor/rte_distributor.c | 12 ++++++++++-- > > lib/librte_distributor/rte_distributor.h | 7 ++++++- > > 4 files changed, 20 insertions(+), 7 deletions(-) > > > > diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c > > index ce06436..6ea4943 100644 > > --- a/app/test/test_distributor.c > > +++ b/app/test/test_distributor.c > > @@ -452,7 +452,7 @@ int test_error_distributor_create_name(void) > > char *name =3D NULL; > > > > d =3D rte_distributor_create(name, rte_socket_id(), > > - rte_lcore_count() - 1); > > + rte_lcore_count() - 1, NULL); > > if (d !=3D NULL || rte_errno !=3D EINVAL) { > > printf("ERROR: No error on create() with NULL name param\n"); > > return -1; > > @@ -467,7 +467,7 @@ int test_error_distributor_create_numworkers(void) > > { > > struct rte_distributor *d =3D NULL; > > d =3D rte_distributor_create("test_numworkers", rte_socket_id(), > > - RTE_MAX_LCORE + 10); > > + RTE_MAX_LCORE + 10, NULL); > > if (d !=3D NULL || rte_errno !=3D EINVAL) { > > printf("ERROR: No error on create() with num_workers > MAX\n"); > > return -1; > > @@ -515,7 +515,7 @@ test_distributor(void) > > > > if (d =3D=3D NULL) { > > d =3D rte_distributor_create("Test_distributor", rte_socket_id(), > > - rte_lcore_count() - 1); > > + rte_lcore_count() - 1, NULL); > > if (d =3D=3D NULL) { > > printf("Error creating distributor\n"); > > return -1; > > diff --git a/app/test/test_distributor_perf.c b/app/test/test_distribut= or_perf.c > > index b04864c..507e446 100644 > > --- a/app/test/test_distributor_perf.c > > +++ b/app/test/test_distributor_perf.c > > @@ -227,7 +227,7 @@ test_distributor_perf(void) > > > > if (d =3D=3D NULL) { > > d =3D rte_distributor_create("Test_perf", rte_socket_id(), > > - rte_lcore_count() - 1); > > + rte_lcore_count() - 1, NULL); > > if (d =3D=3D NULL) { > > printf("Error creating distributor\n"); > > return -1; > > diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_dist= ributor/rte_distributor.c > > index 585ff88..78c92bd 100644 > > --- a/lib/librte_distributor/rte_distributor.c > > +++ b/lib/librte_distributor/rte_distributor.c > > @@ -97,6 +97,7 @@ struct rte_distributor { > > union rte_distributor_buffer bufs[RTE_MAX_LCORE]; > > > > struct rte_distributor_returned_pkts returns; > > + rte_distributor_tag_fn tag_cb; > > }; > > > > TAILQ_HEAD(rte_distributor_list, rte_distributor); > > @@ -267,6 +268,7 @@ rte_distributor_process(struct rte_distributor *d, > > struct rte_mbuf *next_mb =3D NULL; > > int64_t next_value =3D 0; > > uint32_t new_tag =3D 0; > > + rte_distributor_tag_fn tag_cb =3D d->tag_cb; > > unsigned ret_start =3D d->returns.start, > > ret_count =3D d->returns.count; > > > > @@ -282,7 +284,11 @@ rte_distributor_process(struct rte_distributor *d, > > next_mb =3D mbufs[next_idx++]; > > next_value =3D (((int64_t)(uintptr_t)next_mb) > > << RTE_DISTRIB_FLAG_BITS); > > - new_tag =3D (next_mb->hash.rss | 1); > > + if (tag_cb) { > > + new_tag =3D tag_cb(next_mb); > > + } else { > > + new_tag =3D (next_mb->hash.rss | 1); > > + } > > > > uint32_t match =3D 0; > > unsigned i; > > @@ -401,7 +407,8 @@ rte_distributor_clear_returns(struct rte_distributo= r *d) > > struct rte_distributor * > > rte_distributor_create(const char *name, > > unsigned socket_id, > > - unsigned num_workers) > > + unsigned num_workers, > > + rte_distributor_tag_fn tag_cb) > > { > > struct rte_distributor *d; > > struct rte_distributor_list *distributor_list; > > @@ -435,6 +442,7 @@ rte_distributor_create(const char *name, > > d =3D mz->addr; > > snprintf(d->name, sizeof(d->name), "%s", name); > > d->num_workers =3D num_workers; > > + d->tag_cb =3D tag_cb; > > > > rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK); > > TAILQ_INSERT_TAIL(distributor_list, d, next); > > diff --git a/lib/librte_distributor/rte_distributor.h b/lib/librte_dist= ributor/rte_distributor.h > > index ec0d74a..844d325 100644 > > --- a/lib/librte_distributor/rte_distributor.h > > +++ b/lib/librte_distributor/rte_distributor.h > > @@ -52,6 +52,9 @@ extern "C" { > > > > struct rte_distributor; > > > > +typedef uint32_t (*rte_distributor_tag_fn)(struct rte_mbuf *); > > +/**< User defined tag calculation function */ > > + > > /** > > * Function to create a new distributor instance > > * > > @@ -65,12 +68,14 @@ struct rte_distributor; > > * @param num_workers > > * The maximum number of workers that will request packets from this > > * distributor > > + * @param tag_cb > > + * The callback function for calculation of user defined tag. > > * @return > > * The newly created distributor instance > > */ > > struct rte_distributor * > > rte_distributor_create(const char *name, unsigned socket_id, > > - unsigned num_workers); > > + unsigned num_workers, rte_distributor_tag_fn tag_cb); > > > > /* *** APIS to be called on the distributor lcore *** */ > > /* > > -- > > 1.7.1 > >