Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/1143/?format=api
{ "id": 1143, "url": "https://patches.dpdk.org/api/patches/1143/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/CAHVfvh4yAdF-aDnV+sh+PgJpEUW18+aF0=JoZDwJtGHhh=5ZzQ@mail.gmail.com/", "project": { "id": 1, "url": "https://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<CAHVfvh4yAdF-aDnV+sh+PgJpEUW18+aF0=JoZDwJtGHhh=5ZzQ@mail.gmail.com>", "list_archive_url": "https://inbox.dpdk.org/dev/CAHVfvh4yAdF-aDnV+sh+PgJpEUW18+aF0=JoZDwJtGHhh=5ZzQ@mail.gmail.com", "date": "2014-11-05T15:11:51", "name": "[dpdk-dev] Add user defined tag calculation callback to librte_distributor.", "commit_ref": null, "pull_url": null, "state": "rejected", "archived": true, "hash": "cd3852960026d70e0017ed040fe9d1a106c484b0", "submitter": { "id": 105, "url": "https://patches.dpdk.org/api/people/105/?format=api", "name": "Qinglai Xiao", "email": "jigsaw@gmail.com" }, "delegate": null, "mbox": "https://patches.dpdk.org/project/dpdk/patch/CAHVfvh4yAdF-aDnV+sh+PgJpEUW18+aF0=JoZDwJtGHhh=5ZzQ@mail.gmail.com/mbox/", "series": [], "comments": "https://patches.dpdk.org/api/patches/1143/comments/", "check": "pending", "checks": "https://patches.dpdk.org/api/patches/1143/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id B350E7EC4;\n\tWed, 5 Nov 2014 16:02:30 +0100 (CET)", "from mail-wg0-f50.google.com (mail-wg0-f50.google.com\n\t[74.125.82.50]) by dpdk.org (Postfix) with ESMTP id 563B57E80\n\tfor <dev@dpdk.org>; Wed, 5 Nov 2014 16:02:28 +0100 (CET)", "by mail-wg0-f50.google.com with SMTP id z12so1143893wgg.37\n\tfor <dev@dpdk.org>; Wed, 05 Nov 2014 07:11:51 -0800 (PST)", "by 10.27.86.144 with HTTP; Wed, 5 Nov 2014 07:11:51 -0800 (PST)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\n\th=mime-version:in-reply-to:references:date:message-id:subject:from:to\n\t:cc:content-type;\n\tbh=5YuhUET8IzG28oZrcDqB2v1oFMnQiKGvThfOAjnqaP8=;\n\tb=hfbFCmGdpJcdtvscNCEFsd6I0UxlhmZX7Ec6M1VGrl/EgwibjLRxX2bpXsbO1vDUzj\n\tg5s8wbuZDuRWLOd4GdcVGiZ7Ry2+rTnFNYvwrvWw0M7OtYDmBCyVHYY7Xhs+/CQxIci1\n\tJRD2kmJm3RlqFCPmRYSSiOPZkywxFcHuB7gXpooYxcxjJE4ztbXsXsCCYPPsFPauaYZ0\n\t+cHuQ0FfXQPe4hW/Bn++kctIqR/9hm9hwqxn98qyXr1jnIYkR8t5Mh+/Ys2rIcOVjhzG\n\tHjHs5HIOBbH9JmyVmXERU89KRPzPVk/zR70HKlCR8g6agjwijYEP3CgrRSbcMZJMi3p0\n\tlXwQ==", "MIME-Version": "1.0", "X-Received": "by 10.194.175.67 with SMTP id by3mr64822870wjc.32.1415200311710; \n\tWed, 05 Nov 2014 07:11:51 -0800 (PST)", "In-Reply-To": "<20141105142743.GB9856@bricha3-MOBL3>", "References": "<1415194237-1219-1-git-send-email-jigsaw@gmail.com>\n\t<20141105142743.GB9856@bricha3-MOBL3>", "Date": "Wed, 5 Nov 2014 17:11:51 +0200", "Message-ID": "<CAHVfvh4yAdF-aDnV+sh+PgJpEUW18+aF0=JoZDwJtGHhh=5ZzQ@mail.gmail.com>", "From": "jigsaw <jigsaw@gmail.com>", "To": "Bruce Richardson <bruce.richardson@intel.com>", "Content-Type": "text/plain; charset=UTF-8", "X-Content-Filtered-By": "Mailman/MimeDel 2.1.15", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH] Add user defined tag calculation callback to\n\tlibrte_distributor.", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Hi Bruce,\n\nThanks for reply.\nThe idea is triggered by real life use case, where the flow id is buried in\nL3 payload. Deep packet inspection is one of the scenarios, tunneled pkts\nis another.\nHowever, only functionality is verified. Performance impact has not been\nchecked yet.\n\nTo add distributor and another void * as params is nice.\n\nYour advice of extract tags in a row inspired me another solution, which is\nto change the union hash inside rte_mbuf:\n\n\nThe new union field user is actually for documentation purpose only, coz\nuser application can set hash.rss value and have the same result.\nTherefore, the user application is free to calculate the tag in burst mode\nbefore calling rte_distributor_process.\n\nThen rte_distributor_process needs to read next_mb->hash.user.\nDoes it sounds better?\n\nI have another question: why the logical OR 1 is added to new_tag?\n\nthx &\nrgds,\n-qinglai\n\n\n\n\n\n\n\nOn Wed, Nov 5, 2014 at 4:27 PM, Bruce Richardson <bruce.richardson@intel.com\n> wrote:\n\n> On Wed, Nov 05, 2014 at 03:30:37PM +0200, Qinglai Xiao wrote:\n> > User defined tag calculation has access to mbuf.\n> > Default tag is RSS hash result.\n> >\n>\n> Interesting idea.\n> Did you investigate was there any performance improvement or regression\n> comparing\n> whether the callback was called per-packet as packets were dequeued for\n> distribution\n> (i.e. how you have things now in your patch), compared to calling\n> the callback in a loop to extract the tags for all packets initially? I\n> suspect\n> there probably isn't much performance difference either way, but it may be\n> worth\n> checking.\n> One other point, is that I think the callback to extract the tag should\n> have\n> additional parameters - at least one, if not two. I would suggest that the\n> distributor pointer be passed in, as well as an arbitrary void * pointer.\n>\n> Regards,\n> /Bruce\n>\n> > Signed-off-by: Qinglai Xiao <jigsaw@gmail.com>\n> > ---\n> > app/test/test_distributor.c | 6 +++---\n> > app/test/test_distributor_perf.c | 2 +-\n> > lib/librte_distributor/rte_distributor.c | 12 ++++++++++--\n> > lib/librte_distributor/rte_distributor.h | 7 ++++++-\n> > 4 files changed, 20 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c\n> > index ce06436..6ea4943 100644\n> > --- a/app/test/test_distributor.c\n> > +++ b/app/test/test_distributor.c\n> > @@ -452,7 +452,7 @@ int test_error_distributor_create_name(void)\n> > char *name = NULL;\n> >\n> > d = rte_distributor_create(name, rte_socket_id(),\n> > - rte_lcore_count() - 1);\n> > + rte_lcore_count() - 1, NULL);\n> > if (d != NULL || rte_errno != EINVAL) {\n> > printf(\"ERROR: No error on create() with NULL name\n> param\\n\");\n> > return -1;\n> > @@ -467,7 +467,7 @@ int test_error_distributor_create_numworkers(void)\n> > {\n> > struct rte_distributor *d = NULL;\n> > d = rte_distributor_create(\"test_numworkers\", rte_socket_id(),\n> > - RTE_MAX_LCORE + 10);\n> > + RTE_MAX_LCORE + 10, NULL);\n> > if (d != NULL || rte_errno != EINVAL) {\n> > printf(\"ERROR: No error on create() with num_workers >\n> MAX\\n\");\n> > return -1;\n> > @@ -515,7 +515,7 @@ test_distributor(void)\n> >\n> > if (d == NULL) {\n> > d = rte_distributor_create(\"Test_distributor\",\n> rte_socket_id(),\n> > - rte_lcore_count() - 1);\n> > + rte_lcore_count() - 1, NULL);\n> > if (d == NULL) {\n> > printf(\"Error creating distributor\\n\");\n> > return -1;\n> > diff --git a/app/test/test_distributor_perf.c\n> b/app/test/test_distributor_perf.c\n> > index b04864c..507e446 100644\n> > --- a/app/test/test_distributor_perf.c\n> > +++ b/app/test/test_distributor_perf.c\n> > @@ -227,7 +227,7 @@ test_distributor_perf(void)\n> >\n> > if (d == NULL) {\n> > d = rte_distributor_create(\"Test_perf\", rte_socket_id(),\n> > - rte_lcore_count() - 1);\n> > + rte_lcore_count() - 1, NULL);\n> > if (d == NULL) {\n> > printf(\"Error creating distributor\\n\");\n> > return -1;\n> > diff --git a/lib/librte_distributor/rte_distributor.c\n> b/lib/librte_distributor/rte_distributor.c\n> > index 585ff88..78c92bd 100644\n> > --- a/lib/librte_distributor/rte_distributor.c\n> > +++ b/lib/librte_distributor/rte_distributor.c\n> > @@ -97,6 +97,7 @@ struct rte_distributor {\n> > union rte_distributor_buffer bufs[RTE_MAX_LCORE];\n> >\n> > struct rte_distributor_returned_pkts returns;\n> > + rte_distributor_tag_fn tag_cb;\n> > };\n> >\n> > TAILQ_HEAD(rte_distributor_list, rte_distributor);\n> > @@ -267,6 +268,7 @@ rte_distributor_process(struct rte_distributor *d,\n> > struct rte_mbuf *next_mb = NULL;\n> > int64_t next_value = 0;\n> > uint32_t new_tag = 0;\n> > + rte_distributor_tag_fn tag_cb = d->tag_cb;\n> > unsigned ret_start = d->returns.start,\n> > ret_count = d->returns.count;\n> >\n> > @@ -282,7 +284,11 @@ rte_distributor_process(struct rte_distributor *d,\n> > next_mb = mbufs[next_idx++];\n> > next_value = (((int64_t)(uintptr_t)next_mb)\n> > << RTE_DISTRIB_FLAG_BITS);\n> > - new_tag = (next_mb->hash.rss | 1);\n> > + if (tag_cb) {\n> > + new_tag = tag_cb(next_mb);\n> > + } else {\n> > + new_tag = (next_mb->hash.rss | 1);\n> > + }\n> >\n> > uint32_t match = 0;\n> > unsigned i;\n> > @@ -401,7 +407,8 @@ rte_distributor_clear_returns(struct rte_distributor\n> *d)\n> > struct rte_distributor *\n> > rte_distributor_create(const char *name,\n> > unsigned socket_id,\n> > - unsigned num_workers)\n> > + unsigned num_workers,\n> > + rte_distributor_tag_fn tag_cb)\n> > {\n> > struct rte_distributor *d;\n> > struct rte_distributor_list *distributor_list;\n> > @@ -435,6 +442,7 @@ rte_distributor_create(const char *name,\n> > d = mz->addr;\n> > snprintf(d->name, sizeof(d->name), \"%s\", name);\n> > d->num_workers = num_workers;\n> > + d->tag_cb = tag_cb;\n> >\n> > rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);\n> > TAILQ_INSERT_TAIL(distributor_list, d, next);\n> > diff --git a/lib/librte_distributor/rte_distributor.h\n> b/lib/librte_distributor/rte_distributor.h\n> > index ec0d74a..844d325 100644\n> > --- a/lib/librte_distributor/rte_distributor.h\n> > +++ b/lib/librte_distributor/rte_distributor.h\n> > @@ -52,6 +52,9 @@ extern \"C\" {\n> >\n> > struct rte_distributor;\n> >\n> > +typedef uint32_t (*rte_distributor_tag_fn)(struct rte_mbuf *);\n> > +/**< User defined tag calculation function */\n> > +\n> > /**\n> > * Function to create a new distributor instance\n> > *\n> > @@ -65,12 +68,14 @@ struct rte_distributor;\n> > * @param num_workers\n> > * The maximum number of workers that will request packets from this\n> > * distributor\n> > + * @param tag_cb\n> > + * The callback function for calculation of user defined tag.\n> > * @return\n> > * The newly created distributor instance\n> > */\n> > struct rte_distributor *\n> > rte_distributor_create(const char *name, unsigned socket_id,\n> > - unsigned num_workers);\n> > + unsigned num_workers, rte_distributor_tag_fn tag_cb);\n> >\n> > /* *** APIS to be called on the distributor lcore *** */\n> > /*\n> > --\n> > 1.7.1\n> >\n>", "diff": "diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h\nindex e8f9bfc..5b13c0b 100644\n--- a/lib/librte_mbuf/rte_mbuf.h\n+++ b/lib/librte_mbuf/rte_mbuf.h\n@@ -185,6 +185,7 @@ struct rte_mbuf {\n uint16_t id;\n } fdir; /**< Filter identifier if FDIR enabled */\n uint32_t sched; /**< Hierarchical scheduler */\n+ uint32_t user; /**< User defined hash tag */\n } hash; /**< hash information */\n\n /* second cache line - fields only used in slow path or on TX */\n", "prefixes": [ "dpdk-dev" ] }