Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/1207/?format=api
{ "id": 1207, "url": "https://patches.dpdk.org/api/patches/1207/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/CAHVfvh7ggGB_q1Rs1c3-9PRwDr_GKA+etaMXRSeKCfUKoUx8hQ@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": "<CAHVfvh7ggGB_q1Rs1c3-9PRwDr_GKA+etaMXRSeKCfUKoUx8hQ@mail.gmail.com>", "list_archive_url": "https://inbox.dpdk.org/dev/CAHVfvh7ggGB_q1Rs1c3-9PRwDr_GKA+etaMXRSeKCfUKoUx8hQ@mail.gmail.com", "date": "2014-11-07T14:31:18", "name": "[dpdk-dev] 答复: [PATCH] Add user defined tag calculation callback tolibrte_distributor.", "commit_ref": null, "pull_url": null, "state": "not-applicable", "archived": true, "hash": "84594679b44e431573b20448a2811205a10597b3", "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/CAHVfvh7ggGB_q1Rs1c3-9PRwDr_GKA+etaMXRSeKCfUKoUx8hQ@mail.gmail.com/mbox/", "series": [], "comments": "https://patches.dpdk.org/api/patches/1207/comments/", "check": "pending", "checks": "https://patches.dpdk.org/api/patches/1207/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 CAF927F30;\n\tFri, 7 Nov 2014 15:21:48 +0100 (CET)", "from mail-wg0-f43.google.com (mail-wg0-f43.google.com\n\t[74.125.82.43]) by dpdk.org (Postfix) with ESMTP id 6EC422A9\n\tfor <dev@dpdk.org>; Fri, 7 Nov 2014 15:21:46 +0100 (CET)", "by mail-wg0-f43.google.com with SMTP id y10so3907796wgg.16\n\tfor <dev@dpdk.org>; Fri, 07 Nov 2014 06:31:18 -0800 (PST)", "by 10.27.86.144 with HTTP; Fri, 7 Nov 2014 06:31:18 -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=aTHS6x5TR95gg4/lDXltvmX0RpRmOI2gOOjmiLH9Y/M=;\n\tb=o1++sbPXYGfiEH3fJzQm30Nq4Y6ecg5qSpbgkPgwYGdhToXHPbNxPkx9BANAztKG/e\n\tVkMQGoxrjdHTNj/fNfCybpsVf3KJjV0/XIiCShDl+hBT2HsTD6DSJGLK93dN2CyKE3Cn\n\tflxXJWZCy+WgCFhw1Dtp1BliK2vCvJu1f8CeQYeTJ9/9v3RQfNUBmaLe6iH4Oa1DCc7p\n\tqeWinmbHbPfec6NnQ8BoFu7CxNTE8Tg5n5CrzwHHvOAEarJ/nxNj9GM05S1TE5FLzXE1\n\tqGWcPqZmDxYrNVv06vxeWH+DBj81XCoM7P7AHIvRd2l9FOd+bqPu8e5p5NxR4CgrfW16\n\tU1lw==", "MIME-Version": "1.0", "X-Received": "by 10.180.95.201 with SMTP id dm9mr5486712wib.27.1415370678481; \n\tFri, 07 Nov 2014 06:31:18 -0800 (PST)", "In-Reply-To": "<20141107135303.GB12092@bricha3-MOBL3>", "References": "<1415194237-1219-1-git-send-email-jigsaw@gmail.com>\n\t<CAHVfvh4X_sUPUzSJTqBdEnkS94t2Jwj_98Vg0xbUS3MPSeo2ZA@mail.gmail.com>\n\t<20141106092228.GA3056@bricha3-MOBL3> <9190772.1rnKUO3oNV@xps13>\n\t<545b6b74.a96db40a.26af.ffffe7fb@mx.google.com>\n\t<20141106135951.GB7252@bricha3-MOBL3>\n\t<CAHVfvh4U4PZKZSue_kKDQKATC2snb_=10OD08LGmUtieBc_LzA@mail.gmail.com>\n\t<CAHVfvh5SzJ-kpQQ9h=1wmMihiitcJXeR9mcNa1in8x6Gb6tSqQ@mail.gmail.com>\n\t<20141107094521.GB4628@bricha3-MOBL3>\n\t<CAHVfvh6y4f7+bMhzmwOu5c0Y4wzwNaxj4sQPtq8cabGbdHrzXg@mail.gmail.com>\n\t<20141107135303.GB12092@bricha3-MOBL3>", "Date": "Fri, 7 Nov 2014 16:31:18 +0200", "Message-ID": "<CAHVfvh7ggGB_q1Rs1c3-9PRwDr_GKA+etaMXRSeKCfUKoUx8hQ@mail.gmail.com>", "From": "jigsaw <jigsaw@gmail.com>", "To": "Bruce Richardson <bruce.richardson@intel.com>", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "quoted-printable", "X-Content-Filtered-By": "Mailman/MimeDel 2.1.15", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev]\n\t=?utf-8?b?562U5aSNOiAgW1BBVENIXSBBZGQgdXNlciBkZWZpbmVk?=\n\t=?utf-8?q?_tag_calculation_callback_tolibrte=5Fdistributor=2E?=", "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\nPls have a quick look at the diff to see if this is exactly what you mean\nabout the bitmask.\nI just wrote it without even compiling, just to express the idea. So it may\nleave some places unpatched.\nIf this is agreed, I will make a decent test to verify it before sending\nthe patch for RFC.\n\n union rte_distributor_buffer bufs[RTE_MAX_LCORE];\n@@ -188,6 +190,7 @@ static inline void\n handle_worker_shutdown(struct rte_distributor *d, unsigned wkr)\n {\n d->in_flight_tags[wkr] = 0;\n+ d->in_flight_mask &= ~(1 << wkr);\n d->bufs[wkr].bufptr64 = 0;\n if (unlikely(d->backlog[wkr].count != 0)) {\n /* On return of a packet, we need to move the\n@@ -241,6 +244,7 @@ process_returns(struct rte_distributor *d)\n else {\n d->bufs[wkr].bufptr64 = RTE_DISTRIB_GET_BUF;\n d->in_flight_tags[wkr] = 0;\n+ d->in_flight_mask &= ~(1 << wkr);\n }\n oldbuf = data >> RTE_DISTRIB_FLAG_BITS;\n } else if (data & RTE_DISTRIB_RETURN_BUF) {\n@@ -282,12 +286,13 @@ 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+ new_tag = next_mb->hash.rss;\n\n uint32_t match = 0;\n unsigned i;\n for (i = 0; i < d->num_workers; i++)\n- match |= (!(d->in_flight_tags[i] ^ new_tag)\n+ match |= (((!(d->in_flight_tags[i] ^\nnew_tag)) &\n+ (d->in_flight_bitmask >> i))\n << i);\n\n if (match) {\n@@ -309,6 +314,7 @@ rte_distributor_process(struct rte_distributor *d,\n else {\n d->bufs[wkr].bufptr64 = next_value;\n d->in_flight_tags[wkr] = new_tag;\n+ d->in_flight_bitmask |= 1 << wkr;\n next_mb = NULL;\n }\n oldbuf = data >> RTE_DISTRIB_FLAG_BITS;\n\n\n\nOn Fri, Nov 7, 2014 at 3:53 PM, Bruce Richardson <bruce.richardson@intel.com\n> wrote:\n\n> On Fri, Nov 07, 2014 at 02:38:13PM +0200, jigsaw wrote:\n> > Hi Bruce,\n> >\n> > >> If a tag value of zero is ever passed in, then it will start matching\n> > against cores which are not doing any processing.\n> >\n> > Yes, this is true according to current bookkeeping of inflight tags.\n> >\n> > But if the slot in in_flight_tags is not a uint32_t but a struct which\n> has\n> > a filed as indication of \"on/off\", and also with corresponding changes in\n> > looking for a matched tag, then the need for 1 bit mask can be\n> eliminated.\n> > Of course this change requires a little bit more, O(n), memory space and\n> > costs O(n) more branch misses. But the benefit is a more free interface\n> to\n> > user app.\n> >\n> > This is just another trade-off. Since I am in need of such freedom, I am\n> > more interested in the free use of 32bits.\n>\n> If you do implement such a change, I would suggest you simply add a bitmask\n> to the distributor indicating valid workers. Then when we do the check\n> for tag matches, we just need an extra \"and\" instruction to eliminate\n> invalid\n> workers from the match.\n>\n> /Bruce\n>\n> >\n> > thx &\n> > rgds,\n> > -qinglai\n> >\n> >\n> > On Fri, Nov 7, 2014 at 11:45 AM, Bruce Richardson <\n> > bruce.richardson@intel.com> wrote:\n> >\n> > > On Thu, Nov 06, 2014 at 09:52:25PM +0200, jigsaw wrote:\n> > > > Hi Bruce,\n> > > >\n> > > > Actually IMHO it is good to leave the freedom to user to decide how\n> to\n> > > > interpret the tag value, i.e. remove the OR 1 bit.\n> > > > If the tag value is zero, then we assume the programmer know what he\n> is\n> > > > doing. Of course this shall be clearly documented in the\n> comment/doxgen.\n> > > >\n> > > >\n> > > > thx &\n> > > > rgds,\n> > > > -qinglai\n> > >\n> > > I don't believe that will work. If a tag value of zero is ever passed\n> > > in, then it will start matching against cores which are not doing any\n> > > processing. Then it will get queued up to get sent to those cores, and\n> so\n> > > never get processed.\n> > > We need a bit somewhere inside the tag to permanently set - though it\n> can\n> > > be configurable.\n> > >\n> > > /Bruce\n> > >\n> > > >\n> > > > On Thu, Nov 6, 2014 at 8:01 PM, jigsaw <jigsaw@gmail.com> wrote:\n> > > >\n> > > > > Hi Bruce,\n> > > > >\n> > > > > In my use case, unfortunately the tag is not hash. And the tag can\n> be\n> > > on\n> > > > > either low or high bits, depending on configuration.\n> > > > > I wonder if it is possible to let the user to decide which bit to\n> mask,\n> > > > > i.e. to add another param to rte_distributor_create to define the\n> mask.\n> > > > >\n> > > > > thx &\n> > > > > rgds,\n> > > > > -qinglai\n> > > > >\n> > > > > On Thu, Nov 6, 2014 at 3:59 PM, Bruce Richardson <\n> > > > > bruce.richardson@intel.com> wrote:\n> > > > >\n> > > > >> On Thu, Nov 06, 2014 at 02:36:09PM +0200, Qinglai Xiao wrote:\n> > > > >> > Hi Bruce,\n> > > > >> >\n> > > > >> > There is a subtle case in which tag values are 2 and 3,\n> > > respectively.\n> > > > >> Then these two tags cannot be distinguished. There should be a\n> better\n> > > way\n> > > > >> so as to handle this situation.\n> > > > >>\n> > > > >> It's not just in that, case, it's in any case where a pair of tags\n> > > differ\n> > > > >> by\n> > > > >> only a single bit. I've been assuming that the tag is likely to\n> be a\n> > > hash\n> > > > >> value in most cases - given that it's only 32-bit - in which case\n> it\n> > > just\n> > > > >> doesn't\n> > > > >> matter which bit we chose to permanently set to 1, but if there\n> are\n> > > > >> scenarios\n> > > > >> where it's likely that the low bits are used but the high ones not\n> > > so, we\n> > > > >> can\n> > > > >> look to change which bit is set to 1. Either way, the distributor\n> just\n> > > > >> uses a\n> > > > >> 31-bit tag rather than a 32-bit one.\n> > > > >>\n> > > > >> /Bruce\n> > > > >>\n> > > > >> >\n> > > > >> > thx &\n> > > > >> > rgds\n> > > > >> > -qinglai\n> > > > >> >\n> > > > >> > -----原始邮件-----\n> > > > >> > 发件人: \"Thomas Monjalon\" <thomas.monjalon@6wind.com>\n> > > > >> > 发送时间: 2014/11/6 12:36\n> > > > >> > 收件人: \"Bruce Richardson\" <bruce.richardson@intel.com>\n> > > > >> > 抄送: \"dev@dpdk.org\" <dev@dpdk.org>; \"jigsaw\" <jigsaw@gmail.com>\n> > > > >> > 主题: Re: [dpdk-dev] [PATCH] Add user defined tag calculation\n> callback\n> > > > >> tolibrte_distributor.\n> > > > >> >\n> > > > >> > 2014-11-06 09:22, Bruce Richardson:\n> > > > >> > > On Wed, Nov 05, 2014 at 07:24:13PM +0200, jigsaw wrote:\n> > > > >> > > >\n> > > > >>\n> > >\n> http://dpdk.org/browse/dpdk/tree/lib/librte_distributor/rte_distributor.c#n285\n> > > > >> > > >\n> > > > >> > > > new_tag = (next_mb->hash.rss | 1);\n> > > > >> > > >\n> > > > >> > > > Why the logical OR is needed?\n> > > > >> > >\n> > > > >> > > That's needed to ensure that we never track a tag with an\n> actual\n> > > > >> value of zero.\n> > > > >> > > We instead always force the low bit to be 1, so that we can\n> use\n> > > zero\n> > > > >> as an\n> > > > >> > > \"empty\" value.\n> > > > >> >\n> > > > >> > Bruce, could you check how this code may be better commented\n> please?\n> > > > >> > This discussion shows that the distributor library probably\n> needs\n> > > more\n> > > > >> > explanations in the code or doxygen.\n> > > > >> >\n> > > > >> > Thanks\n> > > > >> > --\n> > > > >> > Thomas\n> > > > >>\n> > > > >\n> > > > >\n> > >\n>", "diff": "diff --git a/lib/librte_distributor/rte_distributor.c\nb/lib/librte_distributor/rte_di\nindex 585ff88..d606bcf 100644\n--- a/lib/librte_distributor/rte_distributor.c\n+++ b/lib/librte_distributor/rte_distributor.c\n@@ -92,6 +92,8 @@ struct rte_distributor {\n unsigned num_workers; /**< Number of workers\npolling */\n\n uint32_t in_flight_tags[RTE_MAX_LCORE];\n+ uint32_t in_flight_bitmask;\n+\n struct rte_distributor_backlog backlog[RTE_MAX_LCORE];\n\n", "prefixes": [ "dpdk-dev" ] }