[{"id":1765506,"web_url":"http://patchwork.ozlabs.org/comment/1765506/","msgid":"<20170908164415.GA7356@vergenet.net>","list_archive_url":null,"date":"2017-09-08T16:44:16","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Tue, Sep 05, 2017 at 05:22:54PM +0800, Yuanhan Liu wrote:\n> This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n> \n> It re-uses the flow API (more precisely, the ->flow_put method) to setup\n> the hw flow. The flow_put implementation then is supposed to create a\n> flow with MARK action.\n> \n> Co-authored-by: Finn Christensen \n> Signed-off-by: Yuanhan Liu \n> Signed-off-by: Finn Christensen \n> ---\n> lib/dpif-netdev.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> lib/netdev.h | 6 ++++\n> 2 files changed, 91 insertions(+)\n> \n> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c\n> index 071ec14..f3b7f25 100644\n> --- a/lib/dpif-netdev.c\n> +++ b/lib/dpif-netdev.c\n> @@ -446,6 +446,12 @@ struct dp_netdev_flow {\n> const unsigned pmd_id; /* The 'core_id' of pmd thread owning this */\n> /* flow. */\n> \n> + const struct cmap_node mark_node; /* In owning dp_netdev_pmd_thread's */\n> + /* 'mark_to_flow' */\n> + bool has_mark; /* A flag to tell whether this flow has a\n> + valid mark asscoiated with it. */\n> + uint32_t mark; /* Unique flow mark assiged to a flow */\n> +\n\nI think the fields above could be placed so that the resulting structure\ndoesn't have so many holes due to alignment. In particular 'mark'\ncould be placed next to the existing 'dead' field.\n\n...\n\n> @@ -2235,6 +2249,38 @@ dp_netdev_pmd_find_flow(const struct dp_netdev_pmd_thread *pmd,\n> return NULL;\n> }\n> \n> +static struct dp_netdev_flow *\n> +dp_netdev_pmd_find_flow_by_mark(const struct dp_netdev_pmd_thread *pmd,\n> + const uint32_t mark)\n> +{\n> + struct dp_netdev_flow *netdev_flow;\n> +\n> + CMAP_FOR_EACH_WITH_HASH (netdev_flow, mark_node, mark,\n> + &pmd->mark_to_flow) {\n> + if (netdev_flow->has_mark && netdev_flow->mark == mark) {\n> + return netdev_flow;\n> + }\n> + }\n> +\n> + return NULL;\n> +}\n> +\n> +static bool\n> +dp_netdev_alloc_flow_mark(const struct dp_netdev_pmd_thread *pmd,\n> + uint32_t *mark)\n> +{\n> + uint32_t i;\n> +\n> + for (i = 0; i < UINT32_MAX; i++) {\n> + if (!dp_netdev_pmd_find_flow_by_mark(pmd, i)) {\n> + *mark = i;\n> + return true;\n> + }\n> + }\n> +\n> + return false;\n> +}\n> +\n\nWas consideration given to using id_pool_alloc_id() here?\n\n...","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"tpWan2JF\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xpjpG5zdSz9s76\n\tfor ;\n\tSat, 9 Sep 2017 02:44:22 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 9AAE1B5D;\n\tFri, 8 Sep 2017 16:44:19 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 2C2F18A5\n\tfor ; Fri, 8 Sep 2017 16:44:18 +0000 (UTC)","from mail-wr0-f179.google.com (mail-wr0-f179.google.com\n\t[209.85.128.179])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 07766E0\n\tfor ; Fri, 8 Sep 2017 16:44:16 +0000 (UTC)","by mail-wr0-f179.google.com with SMTP id k20so5622613wre.4\n\tfor ; Fri, 08 Sep 2017 09:44:16 -0700 (PDT)","from vergenet.net (53.red-80-24-208.staticip.rima-tde.net.\n\t[80.24.208.53]) by smtp.gmail.com with ESMTPSA id\n\tg132sm1432799wmd.5.2017.09.08.09.44.14\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tFri, 08 Sep 2017 09:44:14 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=4clZthsIjmDIM1HgZC78eT/QlVsBbtVztJguE3FwZw8=;\n\tb=tpWan2JFoiCO9Z0uL6mUMHoNfeSEKXflrGQKH+a/r1Ersju/vHjd9RZ3FWD2We0YcP\n\txZpn7/QO3tSQ4fDEqn5BjguFzXsawDsUPUnAN/etzdDyMaAh35smIAliq6oErnt5ErA8\n\toeyS16Llj7WeVcIDonD3FlHrviC2xrlevEMjVQUvOaNsANDhaaQWcXyNOSkB9vQormSJ\n\tehy2sW5NSZjQygOp5A58PipstLTBsdKhzVItmk2tcvoFzbFotfXcuHqMtzptEkDZgBXQ\n\tWegPodJotbISGetHBYusmRiN27FJaTZtHoG7a076WQO0UFEEJAowQouCbzbI1rITy7bv\n\tutzQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=4clZthsIjmDIM1HgZC78eT/QlVsBbtVztJguE3FwZw8=;\n\tb=M29DfCiZST96GSQ8gJAo5lWjbBADU7Dt2JRJH9nlhktEdJzRifAWyE+18h7Q2cByjM\n\t4f1qr6XL0v728zOHAsCOIG6luJkOcVh4EvE49/FUcKG0JSOVQl/wtG/TXuV2Bv34ckEx\n\thsjBt37n8IPuyHdo40OIFkvWB4C3q/AuI0bdaFNhQxABS/z8UkCDbGV8GpUjl93UwW8B\n\ttICeoivaiLk073yqXDVP6rIyhvtVSmo5MzkHnKykBfMXT76Z8w6VPebbkHdi32qoGENg\n\tTJYbwkPQBPIKrO3OiWeLBByt1QuL/m1oyl6t93R4UmI1k3dp0ZRsHrgY00qwC09MGaBG\n\t/HVA==","X-Gm-Message-State":"AHPjjUj6WfB3k+Ki14klvyFL0DlPXyayhEN/D6LglB/cIPhej7aND1BT\n\t+KTF2pfe6/Wh4S39WRdAkg==","X-Google-Smtp-Source":"ADKCNb5/J/qtNu2b502T2AGByN/76h1bkz6VbpEsapuC20qZHKyVQ8utQL87Bz9I9j29oJcp/sb2Ug==","X-Received":"by 10.223.170.155 with SMTP id h27mr2419435wrc.95.1504889055548; \n\tFri, 08 Sep 2017 09:44:15 -0700 (PDT)","Date":"Fri, 8 Sep 2017 18:44:16 +0200","From":"Simon Horman ","To":"Yuanhan Liu ","Message-ID":"<20170908164415.GA7356@vergenet.net>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-Spam-Status":"No, score=0.5 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"dev@openvswitch.org","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1765963,"web_url":"http://patchwork.ozlabs.org/comment/1765963/","msgid":"<2EF2F5C0CC56984AA024D0B180335FCB4221DC16@IRSMSX102.ger.corp.intel.com>","list_archive_url":null,"date":"2017-09-10T16:14:01","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":67440,"url":"http://patchwork.ozlabs.org/api/people/67440/","name":"Chandran, Sugesh","email":"sugesh.chandran@intel.com"},"content":"Regards\n_Sugesh\n\n\n> -----Original Message-----\n> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-\n> bounces@openvswitch.org] On Behalf Of Yuanhan Liu\n> Sent: Tuesday, September 5, 2017 10:23 AM\n> To: dev@openvswitch.org\n> Subject: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a mark id\n> \n> This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n> \n> It re-uses the flow API (more precisely, the ->flow_put method) to setup the\n> hw flow. The flow_put implementation then is supposed to create a flow\n> with MARK action.\n> \n> Co-authored-by: Finn Christensen \n> Signed-off-by: Yuanhan Liu \n> Signed-off-by: Finn Christensen \n> ---\n> lib/dpif-netdev.c | 85\n> +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> lib/netdev.h | 6 ++++\n> 2 files changed, 91 insertions(+)\n\n[snip]\n> * Any thread trying to keep a rule from being freed should hold its own\n> @@ -569,6 +575,8 @@ struct dp_netdev_pmd_thread {\n> struct ovs_mutex flow_mutex;\n> struct cmap flow_table OVS_GUARDED; /* Flow table. */\n> \n> + struct cmap mark_to_flow;\n[Sugesh] I would prefer to have this cmap per port than a pmd. The reason being it offers better\nPerformance due to less lookup. Anyway the flow programming is on netdev, hence its good to\nkeep it netdev centric than pmd. What do you think?\n> +\n> /* One classifier per in_port polled by the pmd */\n> struct cmap classifiers;\n> /* Periodically sort subtable vectors according to hit frequencies */ @@ -\n> 1839,6 +1847,8 @@ dp_netdev_pmd_remove_flow(struct\n> dp_netdev_pmd_thread *pmd,\n> OVS_REQUIRES(pmd->flow_mutex)\n> {\n> struct cmap_node *node = CONST_CAST(struct cmap_node *, &flow-\n> >node);\n> + struct cmap_node *mark_node = CONST_CAST(struct cmap_node *,\n> + &flow->mark_node);\n> struct dpcls *cls;\n> odp_port_t in_port = flow->flow.in_port.odp_port;\n> \n> @@ -1846,6 +1856,10 @@ dp_netdev_pmd_remove_flow(struct\n> dp_netdev_pmd_thread *pmd,\n> ovs_assert(cls != NULL);\n> dpcls_remove(cls, &flow->cr);\n> cmap_remove(&pmd->flow_table, node, dp_netdev_flow_hash(&flow-\n> >ufid));\n> + if (flow->has_mark) {\n[Sugesh] Any reason these are not mutex protected? As far as I know the flow remove\nis handled in revalidator thread. Looks like this is a critical section. So the install and delete\nshould be protected.\n> + cmap_remove(&pmd->mark_to_flow, mark_node, flow->mark);\n> + flow->has_mark = false;\n[Sugesh] Where is the hardware flow delete happening?\n> + }\n> flow->dead = true;\n> \n> dp_netdev_flow_unref(flow);\n> @@ -2235,6 +2249,38 @@ dp_netdev_pmd_find_flow(const struct\n> dp_netdev_pmd_thread *pmd,\n> return NULL;\n> }\n> \n> +static struct dp_netdev_flow *\n> +dp_netdev_pmd_find_flow_by_mark(const struct\n> dp_netdev_pmd_thread *pmd,\n> + const uint32_t mark) {\n> + struct dp_netdev_flow *netdev_flow;\n> +\n> + CMAP_FOR_EACH_WITH_HASH (netdev_flow, mark_node, mark,\n> + &pmd->mark_to_flow) {\n> + if (netdev_flow->has_mark && netdev_flow->mark == mark) {\n> + return netdev_flow;\n> + }\n> + }\n> +\n> + return NULL;\n> +}\n> +\n> +static bool\n> +dp_netdev_alloc_flow_mark(const struct dp_netdev_pmd_thread *pmd,\n> + uint32_t *mark) {\n> + uint32_t i;\n> +\n> + for (i = 0; i < UINT32_MAX; i++) {\n> + if (!dp_netdev_pmd_find_flow_by_mark(pmd, i)) {\n> + *mark = i;\n> + return true;\n> + }\n> + }\n> +\n> + return false;\n> +}\n> +\n> static void\n> get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow_,\n> struct dpif_flow_stats *stats) @@ -2434,6 +2480,7 @@\n> dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,\n> struct dp_netdev_flow *flow;\n> struct netdev_flow_key mask;\n> struct dpcls *cls;\n> + struct offload_info info;\n> \n> /* Make sure in_port is exact matched before we read it. */\n> ovs_assert(match->wc.masks.in_port.odp_port == ODPP_NONE); @@ -\n> 2460,6 +2507,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread\n> *pmd,\n> memset(&flow->stats, 0, sizeof flow->stats);\n> flow->dead = false;\n> flow->batch = NULL;\n> + flow->has_mark = false;\n> *CONST_CAST(unsigned *, &flow->pmd_id) = pmd->core_id;\n> *CONST_CAST(struct flow *, &flow->flow) = match->flow;\n> *CONST_CAST(ovs_u128 *, &flow->ufid) = *ufid; @@ -2475,6 +2523,22\n> @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,\n> cmap_insert(&pmd->flow_table, CONST_CAST(struct cmap_node *,\n> &flow->node),\n> dp_netdev_flow_hash(&flow->ufid));\n> \n> + if (netdev_is_flow_api_enabled() &&\n> + dp_netdev_alloc_flow_mark(pmd, &info.flow_mark)) {\n> + struct dp_netdev_port *port;\n> + port = dp_netdev_lookup_port(pmd->dp, in_port);\n> + if (netdev_flow_put(port->netdev, match,\n> + CONST_CAST(struct nlattr *, actions),\n> + actions_len, ufid, &info, NULL) == 0) {\n[Sugesh] Cost of flow install is a concern here. It is vary for device to device.\nHave you done any analysis on the cost of rte_flow translate + hardware flow install?\nIt would be nice to mention it in the cover letter. \n> + flow->has_mark = true;\n> + flow->mark = info.flow_mark;\n> + cmap_insert(&pmd->mark_to_flow,\n> + CONST_CAST(struct cmap_node *, &flow->mark_node),\n> + info.flow_mark);\n> + VLOG_INFO(\"installed flow %p with mark %u\\n\", flow, flow->mark);\n> + }\n> + }\n> +\n> if (OVS_UNLIKELY(!VLOG_DROP_DBG((&upcall_rl)))) {\n> struct ds ds = DS_EMPTY_INITIALIZER;\n> struct ofpbuf key_buf, mask_buf; @@ -2562,6 +2626,25 @@\n> flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,\n> old_actions = dp_netdev_flow_get_actions(netdev_flow);\n> ovsrcu_set(&netdev_flow->actions, new_actions);\n> \n> + if (netdev_flow->has_mark) {\n> + struct offload_info info;\n> + struct dp_netdev_port *port;\n> + odp_port_t in_port;\n> + int ret;\n> +\n> + in_port = netdev_flow->flow.in_port.odp_port;\n> + port = dp_netdev_lookup_port(pmd->dp, in_port);\n> + info.flow_mark = netdev_flow->mark;\n> + ret = netdev_flow_put(port->netdev, match,\n> + CONST_CAST(struct nlattr *,\n> + put->actions),\n> + put->actions_len, ufid, &info,\n> + stats);\n> +\n> + VLOG_INFO(\"%s to modify flow %p associated with mark %u\\n\",\n> + ret == 0 ? \"succeed\" : \"failed\",\n> + netdev_flow, netdev_flow->mark);\n> + }\n> +\n> if (stats) {\n> get_dpif_flow_stats(netdev_flow, stats);\n> }\n> @@ -4504,6 +4587,7 @@ dp_netdev_configure_pmd(struct\n> dp_netdev_pmd_thread *pmd, struct dp_netdev *dp,\n> ovs_mutex_init(&pmd->flow_mutex);\n> ovs_mutex_init(&pmd->port_mutex);\n> cmap_init(&pmd->flow_table);\n> + cmap_init(&pmd->mark_to_flow);\n> cmap_init(&pmd->classifiers);\n> pmd->next_optimization = time_msec() +\n> DPCLS_OPTIMIZATION_INTERVAL;\n> pmd->rxq_interval = time_msec() + PMD_RXQ_INTERVAL_LEN; @@ -\n> 4538,6 +4622,7 @@ dp_netdev_destroy_pmd(struct\n> dp_netdev_pmd_thread *pmd)\n> }\n> cmap_destroy(&pmd->classifiers);\n> cmap_destroy(&pmd->flow_table);\n> + cmap_destroy(&pmd->mark_to_flow);\n> ovs_mutex_destroy(&pmd->flow_mutex);\n> latch_destroy(&pmd->exit_latch);\n> seq_destroy(pmd->reload_seq);\n> diff --git a/lib/netdev.h b/lib/netdev.h index f8482f7..2003165 100644\n> --- a/lib/netdev.h\n> +++ b/lib/netdev.h\n> @@ -188,6 +188,12 @@ void netdev_send_wait(struct netdev *, int qid);\n> struct offload_info {\n> const struct dpif_class *dpif_class;\n> ovs_be16 tp_dst_port; /* Destination port for tunnel in SET action */\n> +\n> + /*\n> + * The flow mark id assigened to the flow. If any pkts hit the flow,\n> + * it will be in the pkt meta data.\n> + */\n> + uint32_t flow_mark;\n> };\n> struct dpif_class;\n> struct netdev_flow_dump;\n> --\n> 2.7.4\n> \n> _______________________________________________\n> dev mailing list\n> dev@openvswitch.org\n> https://mail.openvswitch.org/mailman/listinfo/ovs-dev","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xqx2Z3D1Rz9s7c\n\tfor ;\n\tMon, 11 Sep 2017 02:14:14 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 40DD8A73;\n\tSun, 10 Sep 2017 16:14:07 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 9C5D6A55\n\tfor ; Sun, 10 Sep 2017 16:14:05 +0000 (UTC)","from mga06.intel.com (mga06.intel.com [134.134.136.31])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id EE4D3E5\n\tfor ; Sun, 10 Sep 2017 16:14:04 +0000 (UTC)","from fmsmga001.fm.intel.com ([10.253.24.23])\n\tby orsmga104.jf.intel.com with ESMTP; 10 Sep 2017 09:14:04 -0700","from irsmsx110.ger.corp.intel.com ([163.33.3.25])\n\tby fmsmga001.fm.intel.com with ESMTP; 10 Sep 2017 09:14:02 -0700","from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by\n\tirsmsx110.ger.corp.intel.com ([163.33.3.25]) with mapi id\n\t14.03.0319.002; Sun, 10 Sep 2017 17:14:01 +0100"],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos; i=\"5.42,372,1500966000\"; d=\"scan'208\";\n\ta=\"1193533158\"","From":"\"Chandran, Sugesh\" ","To":"Yuanhan Liu , \"dev@openvswitch.org\"\n\t","Thread-Topic":"[ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","Thread-Index":"AQHTJijQk7sb6sLQE0SsrgYgji86daKuAGEg","Date":"Sun, 10 Sep 2017 16:14:01 +0000","Message-ID":"<2EF2F5C0CC56984AA024D0B180335FCB4221DC16@IRSMSX102.ger.corp.intel.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>","In-Reply-To":"<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","x-titus-metadata-40":"eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjE4MzU0ZWUtMGZhYS00Y2JmLWExNmEtOGVjYmVlZGNiNWQ4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjZvY1Qwb1d6QXRvRkRYOVdFRlVsNWdsS0Y4WTFSM3hoUlhSaEVsYjRZMW89In0=","x-ctpclassification":"CTP_IC","dlp-product":"dlpe-windows","dlp-version":"11.0.0.116","dlp-reaction":"no-action","x-originating-ip":"[163.33.239.180]","MIME-Version":"1.0","X-Spam-Status":"No, score=-2.3 required=5.0 tests=RCVD_IN_DNSWL_MED,\n\tRP_MATCHES_RCVD autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766047,"web_url":"http://patchwork.ozlabs.org/comment/1766047/","msgid":"<20170911053407.GJ9736@yliu-home>","list_archive_url":null,"date":"2017-09-11T05:34:07","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Fri, Sep 08, 2017 at 06:44:16PM +0200, Simon Horman wrote:\n> On Tue, Sep 05, 2017 at 05:22:54PM +0800, Yuanhan Liu wrote:\n> > This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n> > \n> > It re-uses the flow API (more precisely, the ->flow_put method) to setup\n> > the hw flow. The flow_put implementation then is supposed to create a\n> > flow with MARK action.\n> > \n> > Co-authored-by: Finn Christensen \n> > Signed-off-by: Yuanhan Liu \n> > Signed-off-by: Finn Christensen \n> > ---\n> > lib/dpif-netdev.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> > lib/netdev.h | 6 ++++\n> > 2 files changed, 91 insertions(+)\n> > \n> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c\n> > index 071ec14..f3b7f25 100644\n> > --- a/lib/dpif-netdev.c\n> > +++ b/lib/dpif-netdev.c\n> > @@ -446,6 +446,12 @@ struct dp_netdev_flow {\n> > const unsigned pmd_id; /* The 'core_id' of pmd thread owning this */\n> > /* flow. */\n> > \n> > + const struct cmap_node mark_node; /* In owning dp_netdev_pmd_thread's */\n> > + /* 'mark_to_flow' */\n> > + bool has_mark; /* A flag to tell whether this flow has a\n> > + valid mark asscoiated with it. */\n> > + uint32_t mark; /* Unique flow mark assiged to a flow */\n> > +\n> \n> I think the fields above could be placed so that the resulting structure\n> doesn't have so many holes due to alignment. In particular 'mark'\n> could be placed next to the existing 'dead' field.\n\nAgreed.\n\n> > @@ -2235,6 +2249,38 @@ dp_netdev_pmd_find_flow(const struct dp_netdev_pmd_thread *pmd,\n> > return NULL;\n> > }\n> > \n> > +static struct dp_netdev_flow *\n> > +dp_netdev_pmd_find_flow_by_mark(const struct dp_netdev_pmd_thread *pmd,\n> > + const uint32_t mark)\n> > +{\n> > + struct dp_netdev_flow *netdev_flow;\n> > +\n> > + CMAP_FOR_EACH_WITH_HASH (netdev_flow, mark_node, mark,\n> > + &pmd->mark_to_flow) {\n> > + if (netdev_flow->has_mark && netdev_flow->mark == mark) {\n> > + return netdev_flow;\n> > + }\n> > + }\n> > +\n> > + return NULL;\n> > +}\n> > +\n> > +static bool\n> > +dp_netdev_alloc_flow_mark(const struct dp_netdev_pmd_thread *pmd,\n> > + uint32_t *mark)\n> > +{\n> > + uint32_t i;\n> > +\n> > + for (i = 0; i < UINT32_MAX; i++) {\n> > + if (!dp_netdev_pmd_find_flow_by_mark(pmd, i)) {\n> > + *mark = i;\n> > + return true;\n> > + }\n> > + }\n> > +\n> > + return false;\n> > +}\n> > +\n> \n> Was consideration given to using id_pool_alloc_id() here?\n\nThanks. I was actually looking at something like this. Will use it.\n\n\t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=fridaylinux-org.20150623.gappssmtp.com\n\theader.i=@fridaylinux-org.20150623.gappssmtp.com\n\theader.b=\"maF12ydH\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrGnr5qvcz9s81\n\tfor ;\n\tMon, 11 Sep 2017 15:34:22 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id D2416A71;\n\tMon, 11 Sep 2017 05:34:19 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 40EDF92F\n\tfor ; Mon, 11 Sep 2017 05:34:18 +0000 (UTC)","from mail-pf0-f180.google.com (mail-pf0-f180.google.com\n\t[209.85.192.180])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 9FA38D3\n\tfor ; Mon, 11 Sep 2017 05:34:17 +0000 (UTC)","by mail-pf0-f180.google.com with SMTP id y29so11778638pff.0\n\tfor ; Sun, 10 Sep 2017 22:34:17 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\tp71sm14877675pfl.56.2017.09.10.22.34.13\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tSun, 10 Sep 2017 22:34:15 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=fridaylinux-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=5MklBk5zigtHtTgsczsfrJ89Iu8E4IkR+pP/CvCI6wE=;\n\tb=maF12ydH6EruLe07olC8VTkNB5PhsYc2AdSX4jqeYPLTwAgj5qgueWSeCmLDpaIkj0\n\tarrMSzMYXssL3yWUN3NTrko83ObLABnVdu6SbzxJHchUL5tVs5ENnFtbRYRjzOlKvRKc\n\tofbv+YH8sN80ZhIuV8s1ecvwAgk5U2uSSJ6nwqjf42KmBD317Ibo8lgzkHZJFRwjG9wD\n\taqbBRnmv5dq+/gaBeHcQL4jWQpwVUp5z/jBmtFB/Ayxuj1cKnJyA3XQX6kkvqjHwisPd\n\txBbmQ7U0TUazZkJYzxVIk2vokhrwl7OKTrr2+936maRO4m0jzmU/nu3W1vKz1oyz7EaF\n\tgDOg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=5MklBk5zigtHtTgsczsfrJ89Iu8E4IkR+pP/CvCI6wE=;\n\tb=R5Tqds8BZ8niMtMArrToSi5TQaisqlqH3vD5VR50qGOz+P5ZgMQoK0ol/u21/Q9OgM\n\t0Mv2pAYpCSTvyA2UEgbZN9Wwxei5NLESvBVcuVYyFtqIKMQHZ054s0rBe/93AdiedGKL\n\tc3VWEvJDt6cUrnAWSL3QrAqew9KS0fFuA9NzWFRBs5byoELEGwHM3hecEPf0PciX5e55\n\tx47pd1upIvyGPfM9wXI77/4lbhVSDk0dOP4VC1aNgPy8moXZ9f7oh0juh3tCrYY6g1V0\n\tgpjNUG83KesvCwoWaRxiCsOAOHS7EKR4wX2cU5Ksq+8GT7C0UxdJ7hoACdq6TDnzUEXw\n\tyXVQ==","X-Gm-Message-State":"AHPjjUiYogk5AhMf6C2piWtnX7bcYtEEhyw9P+puPnzGJJWRY+BjoL5J\n\t7DdMwzoqy2oKg5ioVqArGw==","X-Google-Smtp-Source":"ADKCNb6hfh/IYsx97F1kfdQk42yQs9EAfrz5hQwEXHoI83uuseGp7sHc3CeUcSyMAizapRpoJQyROg==","X-Received":"by 10.84.177.129 with SMTP id x1mr12763083plb.161.1505108057113; \n\tSun, 10 Sep 2017 22:34:17 -0700 (PDT)","Date":"Mon, 11 Sep 2017 13:34:07 +0800","From":"Yuanhan Liu ","To":"Simon Horman ","Message-ID":"<20170911053407.GJ9736@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>\n\t<20170908164415.GA7356@vergenet.net>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170908164415.GA7356@vergenet.net>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Spam-Status":"No, score=0.5 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"dev@openvswitch.org","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766118,"web_url":"http://patchwork.ozlabs.org/comment/1766118/","msgid":"<20170911080347.GT9736@yliu-home>","list_archive_url":null,"date":"2017-09-11T08:03:47","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Sun, Sep 10, 2017 at 04:14:01PM +0000, Chandran, Sugesh wrote:\n> > This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n> > \n> > It re-uses the flow API (more precisely, the ->flow_put method) to setup the\n> > hw flow. The flow_put implementation then is supposed to create a flow\n> > with MARK action.\n> > \n> > Co-authored-by: Finn Christensen \n> > Signed-off-by: Yuanhan Liu \n> > Signed-off-by: Finn Christensen \n> > ---\n> > lib/dpif-netdev.c | 85\n> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> > lib/netdev.h | 6 ++++\n> > 2 files changed, 91 insertions(+)\n> \n> [snip]\n> > * Any thread trying to keep a rule from being freed should hold its own\n> > @@ -569,6 +575,8 @@ struct dp_netdev_pmd_thread {\n> > struct ovs_mutex flow_mutex;\n> > struct cmap flow_table OVS_GUARDED; /* Flow table. */\n> > \n> > + struct cmap mark_to_flow;\n> [Sugesh] I would prefer to have this cmap per port than a pmd. The reason being it offers better\n> Performance due to less lookup. Anyway the flow programming is on netdev, hence its good to\n> keep it netdev centric than pmd. What do you think?\n\nI think it might be workable.\n\n> > /* One classifier per in_port polled by the pmd */\n> > struct cmap classifiers;\n> > /* Periodically sort subtable vectors according to hit frequencies */ @@ -\n> > 1839,6 +1847,8 @@ dp_netdev_pmd_remove_flow(struct\n> > dp_netdev_pmd_thread *pmd,\n> > OVS_REQUIRES(pmd->flow_mutex)\n> > {\n> > struct cmap_node *node = CONST_CAST(struct cmap_node *, &flow-\n> > >node);\n> > + struct cmap_node *mark_node = CONST_CAST(struct cmap_node *,\n> > + &flow->mark_node);\n> > struct dpcls *cls;\n> > odp_port_t in_port = flow->flow.in_port.odp_port;\n> > \n> > @@ -1846,6 +1856,10 @@ dp_netdev_pmd_remove_flow(struct\n> > dp_netdev_pmd_thread *pmd,\n> > ovs_assert(cls != NULL);\n> > dpcls_remove(cls, &flow->cr);\n> > cmap_remove(&pmd->flow_table, node, dp_netdev_flow_hash(&flow-\n> > >ufid));\n> > + if (flow->has_mark) {\n> [Sugesh] Any reason these are not mutex protected? As far as I know the flow remove\n> is handled in revalidator thread. Looks like this is a critical section. So the install and delete\n> should be protected.\n\nI think you are right.\n\n\n[...]\n> > + if (netdev_is_flow_api_enabled() &&\n> > + dp_netdev_alloc_flow_mark(pmd, &info.flow_mark)) {\n> > + struct dp_netdev_port *port;\n> > + port = dp_netdev_lookup_port(pmd->dp, in_port);\n> > + if (netdev_flow_put(port->netdev, match,\n> > + CONST_CAST(struct nlattr *, actions),\n> > + actions_len, ufid, &info, NULL) == 0) {\n> [Sugesh] Cost of flow install is a concern here. It is vary for device to device.\n> Have you done any analysis on the cost of rte_flow translate + hardware flow install?\n> It would be nice to mention it in the cover letter. \n\nI haven't measured it yet: it's not my current focus. It's not in the\ndatapath after all. Besides, it might not be a trivial task to optimize\nit.\n\n\t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=fridaylinux-org.20150623.gappssmtp.com\n\theader.i=@fridaylinux-org.20150623.gappssmtp.com\n\theader.b=\"vE6XhL6f\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrL6V1hqRz9s81\n\tfor ;\n\tMon, 11 Sep 2017 18:04:02 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id E4CF8B2F;\n\tMon, 11 Sep 2017 08:03:58 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 4A9ADB15\n\tfor ; Mon, 11 Sep 2017 08:03:57 +0000 (UTC)","from mail-pg0-f53.google.com (mail-pg0-f53.google.com\n\t[74.125.83.53])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id EB0D0180\n\tfor ; Mon, 11 Sep 2017 08:03:56 +0000 (UTC)","by mail-pg0-f53.google.com with SMTP id t3so14093910pgt.0\n\tfor ; Mon, 11 Sep 2017 01:03:56 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\t70sm14089076pfh.63.2017.09.11.01.03.52\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tMon, 11 Sep 2017 01:03:55 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=fridaylinux-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=t/dGRRytYCbGooGEyRCbuhLWUiU1EaiLg4VpuuVhWZg=;\n\tb=vE6XhL6ft1/nJMQVECQVx6oXJ9cjXUgueJXdKs3ReNhnyUS7NzDQnwAFYLy8FDsd/V\n\tOC4yseNM5qZ+GZx8/zEcnlv3eiTZjFqrvGuA3HcTb7Y/XcXhvI+Q6vxJD3fLm/e0SYsL\n\tR4GwVLrqr6spVCy4Y0i2htRXiJQINZ3rzq1oULudjUYwlbWNhsnVCXQtD0SJipia7zHH\n\tka6q70Ta9GnXBHjhqjMN7z9CfUh0P7aiHevbNEz+rs0scQc3RSTZuKOmK1kjxvI6pJtV\n\tmA87VYRJQ1k0XisZ9seH6jjHQk5kQmQRZRDsWuNaGRKoqukMr1xA31aNia8F3xYvWexA\n\tyLbQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=t/dGRRytYCbGooGEyRCbuhLWUiU1EaiLg4VpuuVhWZg=;\n\tb=IdI/hjccUiOQX+yGX0b0p/xP6U4B4CBsbzKzwQIO8JRDXv3O3uaVXt6WbiSwRSnfDm\n\tT9MOTkmBxLTZotuy7LaKE3miVG3qlBVdWN5h1m6Apf8dHLj8JD2LtlOU1IKZNoqqHkBy\n\tRAgXITBhRLhXHF+y/KQl4lJLoqa7OQOvaOZxM1jQg6Pb04GbcC+0eG8XoBSPex+3V29G\n\tPEhs+jvF3blpOlEya8DfOil5zQfX6xB33ESehkquKyBIZvjxMdp+ZOyXw8lUe83wDVvx\n\tGIuUL4te4X7zhaYQ31TixgyEcTTHgp6ya5TehGGEGnLSGeftJufI5ql7ToLPA+EZ/f+s\n\tH/EQ==","X-Gm-Message-State":"AHPjjUhjwK6dw/I2ABXiTOOXaVLbdPEOrp+1D/fCuXy15xarp/LfyjAP\n\tHINAnN1JsVVhJLUbzxsHMQ==","X-Google-Smtp-Source":"ADKCNb6hijRgcBokObBwwWKOOwxyjFomitUhQWTBuoKIpxod095VSFW1LtglBcMvpiIMF5Yvw9I7Ww==","X-Received":"by 10.84.131.109 with SMTP id 100mr12811497pld.122.1505117036430;\n\tMon, 11 Sep 2017 01:03:56 -0700 (PDT)","Date":"Mon, 11 Sep 2017 16:03:47 +0800","From":"Yuanhan Liu ","To":"\"Chandran, Sugesh\" ","Message-ID":"<20170911080347.GT9736@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>\n\t<2EF2F5C0CC56984AA024D0B180335FCB4221DC16@IRSMSX102.ger.corp.intel.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<2EF2F5C0CC56984AA024D0B180335FCB4221DC16@IRSMSX102.ger.corp.intel.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Spam-Status":"No, score=0.5 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766144,"web_url":"http://patchwork.ozlabs.org/comment/1766144/","msgid":"<2EF2F5C0CC56984AA024D0B180335FCB4221DFC9@IRSMSX102.ger.corp.intel.com>","list_archive_url":null,"date":"2017-09-11T09:01:34","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":67440,"url":"http://patchwork.ozlabs.org/api/people/67440/","name":"Chandran, Sugesh","email":"sugesh.chandran@intel.com"},"content":"Regards\n_Sugesh\n\n\n> -----Original Message-----\n> From: Yuanhan Liu [mailto:yliu@fridaylinux.org]\n> Sent: Monday, September 11, 2017 9:04 AM\n> To: Chandran, Sugesh \n> Cc: dev@openvswitch.org\n> Subject: Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n> mark id\n> \n> On Sun, Sep 10, 2017 at 04:14:01PM +0000, Chandran, Sugesh wrote:\n> > > This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n> > >\n> > > It re-uses the flow API (more precisely, the ->flow_put method) to\n> > > setup the hw flow. The flow_put implementation then is supposed to\n> > > create a flow with MARK action.\n> > >\n> > > Co-authored-by: Finn Christensen \n> > > Signed-off-by: Yuanhan Liu \n> > > Signed-off-by: Finn Christensen \n> > > ---\n> > > lib/dpif-netdev.c | 85\n> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> > > lib/netdev.h | 6 ++++\n> > > 2 files changed, 91 insertions(+)\n> >\n> > [snip]\n> > > * Any thread trying to keep a rule from being freed should\n> > > hold its own @@ -569,6 +575,8 @@ struct dp_netdev_pmd_thread {\n> > > struct ovs_mutex flow_mutex;\n> > > struct cmap flow_table OVS_GUARDED; /* Flow table. */\n> > >\n> > > + struct cmap mark_to_flow;\n> > [Sugesh] I would prefer to have this cmap per port than a pmd. The\n> > reason being it offers better Performance due to less lookup. Anyway\n> > the flow programming is on netdev, hence its good to keep it netdev\n> centric than pmd. What do you think?\n> \n> I think it might be workable.\n[Sugesh] ok\n> \n> > > /* One classifier per in_port polled by the pmd */\n> > > struct cmap classifiers;\n> > > /* Periodically sort subtable vectors according to hit\n> > > frequencies */ @@ -\n> > > 1839,6 +1847,8 @@ dp_netdev_pmd_remove_flow(struct\n> > > dp_netdev_pmd_thread *pmd,\n> > > OVS_REQUIRES(pmd->flow_mutex)\n> > > {\n> > > struct cmap_node *node = CONST_CAST(struct cmap_node *, &flow-\n> > > >node);\n> > > + struct cmap_node *mark_node = CONST_CAST(struct cmap_node *,\n> > > + &flow->mark_node);\n> > > struct dpcls *cls;\n> > > odp_port_t in_port = flow->flow.in_port.odp_port;\n> > >\n> > > @@ -1846,6 +1856,10 @@ dp_netdev_pmd_remove_flow(struct\n> > > dp_netdev_pmd_thread *pmd,\n> > > ovs_assert(cls != NULL);\n> > > dpcls_remove(cls, &flow->cr);\n> > > cmap_remove(&pmd->flow_table, node,\n> dp_netdev_flow_hash(&flow-\n> > > >ufid));\n> > > + if (flow->has_mark) {\n> > [Sugesh] Any reason these are not mutex protected? As far as I know\n> > the flow remove is handled in revalidator thread. Looks like this is a\n> > critical section. So the install and delete should be protected.\n> \n> I think you are right.\n[Sugesh] ok. Thanks.\n> \n> \n> [...]\n> > > + if (netdev_is_flow_api_enabled() &&\n> > > + dp_netdev_alloc_flow_mark(pmd, &info.flow_mark)) {\n> > > + struct dp_netdev_port *port;\n> > > + port = dp_netdev_lookup_port(pmd->dp, in_port);\n> > > + if (netdev_flow_put(port->netdev, match,\n> > > + CONST_CAST(struct nlattr *, actions),\n> > > + actions_len, ufid, &info, NULL) == 0) {\n> > [Sugesh] Cost of flow install is a concern here. It is vary for device to device.\n> > Have you done any analysis on the cost of rte_flow translate + hardware\n> flow install?\n> > It would be nice to mention it in the cover letter.\n> \n> I haven't measured it yet: it's not my current focus. It's not in the datapath\n> after all. Besides, it might not be a trivial task to optimize it.\n[Sugesh] TBH I feel this is very important aspect to decide if it make any sense\nto install the flow or not. What do you meant by not in datapath? The PMD is\nblocked for all time until the flow install is complete.? For a millions of short lived flow\nuse case, this is very important to know.\nLet me know if I have missed anything.\n\n\n> \n> \t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xrMP074JKz9rxm\n\tfor ;\n\tMon, 11 Sep 2017 19:01:40 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 3B491B66;\n\tMon, 11 Sep 2017 09:01:38 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id C7DE3B30\n\tfor ; Mon, 11 Sep 2017 09:01:37 +0000 (UTC)","from mga09.intel.com (mga09.intel.com [134.134.136.24])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 2616A1D7\n\tfor ; Mon, 11 Sep 2017 09:01:37 +0000 (UTC)","from orsmga003.jf.intel.com ([10.7.209.27])\n\tby orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t11 Sep 2017 02:01:36 -0700","from irsmsx104.ger.corp.intel.com ([163.33.3.159])\n\tby orsmga003.jf.intel.com with ESMTP; 11 Sep 2017 02:01:35 -0700","from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by\n\tIRSMSX104.ger.corp.intel.com ([163.33.3.159]) with mapi id\n\t14.03.0319.002; Mon, 11 Sep 2017 10:01:34 +0100"],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos; i=\"5.42,376,1500966000\"; d=\"scan'208\";\n\ta=\"1013101316\"","From":"\"Chandran, Sugesh\" ","To":"Yuanhan Liu ","Thread-Topic":"[ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","Thread-Index":"AQHTJijQk7sb6sLQE0SsrgYgji86daKuAGEggAFLh4CAAB+g8A==","Date":"Mon, 11 Sep 2017 09:01:34 +0000","Message-ID":"<2EF2F5C0CC56984AA024D0B180335FCB4221DFC9@IRSMSX102.ger.corp.intel.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>\n\t<2EF2F5C0CC56984AA024D0B180335FCB4221DC16@IRSMSX102.ger.corp.intel.com>\n\t<20170911080347.GT9736@yliu-home>","In-Reply-To":"<20170911080347.GT9736@yliu-home>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","x-titus-metadata-40":"eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTYwOTExMjItYjllNS00MzYwLWI1ZmYtZTEzMDg0OWE5MjE1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6InF6R1R0ODBRZVB3YUp4VXpPckV4VFVSWWVcL1wvejczTEZpeUhkalB4ZmFYbz0ifQ==","x-ctpclassification":"CTP_IC","dlp-product":"dlpe-windows","dlp-version":"11.0.0.116","dlp-reaction":"no-action","x-originating-ip":"[163.33.239.181]","MIME-Version":"1.0","X-Spam-Status":"No, score=-5.0 required=5.0 tests=RCVD_IN_DNSWL_HI,\n\tRP_MATCHES_RCVD autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1768866,"web_url":"http://patchwork.ozlabs.org/comment/1768866/","msgid":"<1F0232ED-122F-4945-BCF6-53EF4A97B2DC@vmware.com>","list_archive_url":null,"date":"2017-09-14T23:42:38","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"On 9/5/17, 2:23 AM, \"Yuanhan Liu\" wrote:\n\n This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n \n It re-uses the flow API (more precisely, the ->flow_put method) to setup\n the hw flow. The flow_put implementation then is supposed to create a\n flow with MARK action.\n \n Co-authored-by: Finn Christensen \n Signed-off-by: Yuanhan Liu \n Signed-off-by: Finn Christensen \n ---\n lib/dpif-netdev.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n lib/netdev.h | 6 ++++\n 2 files changed, 91 insertions(+)\n \n diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c\n index 071ec14..f3b7f25 100644\n --- a/lib/dpif-netdev.c\n +++ b/lib/dpif-netdev.c\n @@ -446,6 +446,12 @@ struct dp_netdev_flow {\n const unsigned pmd_id; /* The 'core_id' of pmd thread owning this */\n /* flow. */\n \n + const struct cmap_node mark_node; /* In owning dp_netdev_pmd_thread's */\n + /* 'mark_to_flow' */\n + bool has_mark; /* A flag to tell whether this flow has a\n + valid mark asscoiated with it. */\n + uint32_t mark; /* Unique flow mark assiged to a flow */\n\n\n[Darrell] Since the PMD that receives a flow can change and multiple PMDs can allocate\nthe same mark value at the moment, it seems the mark allocation needs to include the PMD ID or\nbe from a global namespace to enforce uniqueness.\nPls verify.\n\n +\n /* Number of references.\n * The classifier owns one reference.\n * Any thread trying to keep a rule from being freed should hold its own\n @@ -569,6 +575,8 @@ struct dp_netdev_pmd_thread {\n struct ovs_mutex flow_mutex;\n struct cmap flow_table OVS_GUARDED; /* Flow table. */\n \n + struct cmap mark_to_flow;\n +\n /* One classifier per in_port polled by the pmd */\n struct cmap classifiers;\n /* Periodically sort subtable vectors according to hit frequencies */\n @@ -1839,6 +1847,8 @@ dp_netdev_pmd_remove_flow(struct dp_netdev_pmd_thread *pmd,\n OVS_REQUIRES(pmd->flow_mutex)\n {\n struct cmap_node *node = CONST_CAST(struct cmap_node *, &flow->node);\n + struct cmap_node *mark_node = CONST_CAST(struct cmap_node *,\n + &flow->mark_node);\n struct dpcls *cls;\n odp_port_t in_port = flow->flow.in_port.odp_port;\n \n @@ -1846,6 +1856,10 @@ dp_netdev_pmd_remove_flow(struct dp_netdev_pmd_thread *pmd,\n ovs_assert(cls != NULL);\n dpcls_remove(cls, &flow->cr);\n cmap_remove(&pmd->flow_table, node, dp_netdev_flow_hash(&flow->ufid));\n + if (flow->has_mark) {\n + cmap_remove(&pmd->mark_to_flow, mark_node, flow->mark);\n + flow->has_mark = false;\n + }\n flow->dead = true;\n \n dp_netdev_flow_unref(flow);\n @@ -2235,6 +2249,38 @@ dp_netdev_pmd_find_flow(const struct dp_netdev_pmd_thread *pmd,\n return NULL;\n }\n \n +static struct dp_netdev_flow *\n +dp_netdev_pmd_find_flow_by_mark(const struct dp_netdev_pmd_thread *pmd,\n + const uint32_t mark)\n +{\n + struct dp_netdev_flow *netdev_flow;\n +\n + CMAP_FOR_EACH_WITH_HASH (netdev_flow, mark_node, mark,\n + &pmd->mark_to_flow) {\n + if (netdev_flow->has_mark && netdev_flow->mark == mark) {\n + return netdev_flow;\n + }\n + }\n +\n + return NULL;\n +}\n +\n +static bool\n +dp_netdev_alloc_flow_mark(const struct dp_netdev_pmd_thread *pmd,\n + uint32_t *mark)\n +{\n + uint32_t i;\n +\n + for (i = 0; i < UINT32_MAX; i++) {\n + if (!dp_netdev_pmd_find_flow_by_mark(pmd, i)) {\n + *mark = i;\n + return true;\n + }\n + }\n +\n + return false;\n +}\n +\n static void\n get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow_,\n struct dpif_flow_stats *stats)\n @@ -2434,6 +2480,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,\n struct dp_netdev_flow *flow;\n struct netdev_flow_key mask;\n struct dpcls *cls;\n + struct offload_info info;\n \n /* Make sure in_port is exact matched before we read it. */\n ovs_assert(match->wc.masks.in_port.odp_port == ODPP_NONE);\n @@ -2460,6 +2507,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,\n memset(&flow->stats, 0, sizeof flow->stats);\n flow->dead = false;\n flow->batch = NULL;\n + flow->has_mark = false;\n *CONST_CAST(unsigned *, &flow->pmd_id) = pmd->core_id;\n *CONST_CAST(struct flow *, &flow->flow) = match->flow;\n *CONST_CAST(ovs_u128 *, &flow->ufid) = *ufid;\n @@ -2475,6 +2523,22 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,\n cmap_insert(&pmd->flow_table, CONST_CAST(struct cmap_node *, &flow->node),\n dp_netdev_flow_hash(&flow->ufid));\n \n + if (netdev_is_flow_api_enabled() &&\n + dp_netdev_alloc_flow_mark(pmd, &info.flow_mark)) {\n + struct dp_netdev_port *port;\n + port = dp_netdev_lookup_port(pmd->dp, in_port);\n + if (netdev_flow_put(port->netdev, match,\n + CONST_CAST(struct nlattr *, actions),\n + actions_len, ufid, &info, NULL) == 0) {\n + flow->has_mark = true;\n + flow->mark = info.flow_mark;\n + cmap_insert(&pmd->mark_to_flow,\n + CONST_CAST(struct cmap_node *, &flow->mark_node),\n + info.flow_mark);\n + VLOG_INFO(\"installed flow %p with mark %u\\n\", flow, flow->mark);\n + }\n + }\n +\n if (OVS_UNLIKELY(!VLOG_DROP_DBG((&upcall_rl)))) {\n struct ds ds = DS_EMPTY_INITIALIZER;\n struct ofpbuf key_buf, mask_buf;\n @@ -2562,6 +2626,25 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,\n old_actions = dp_netdev_flow_get_actions(netdev_flow);\n ovsrcu_set(&netdev_flow->actions, new_actions);\n \n + if (netdev_flow->has_mark) {\n + struct offload_info info;\n + struct dp_netdev_port *port;\n + odp_port_t in_port;\n + int ret;\n +\n + in_port = netdev_flow->flow.in_port.odp_port;\n + port = dp_netdev_lookup_port(pmd->dp, in_port);\n + info.flow_mark = netdev_flow->mark;\n + ret = netdev_flow_put(port->netdev, match,\n + CONST_CAST(struct nlattr *,\n + put->actions),\n + put->actions_len, ufid, &info, stats);\n +\n + VLOG_INFO(\"%s to modify flow %p associated with mark %u\\n\",\n + ret == 0 ? \"succeed\" : \"failed\",\n + netdev_flow, netdev_flow->mark);\n + }\n +\n if (stats) {\n get_dpif_flow_stats(netdev_flow, stats);\n }\n @@ -4504,6 +4587,7 @@ dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev *dp,\n ovs_mutex_init(&pmd->flow_mutex);\n ovs_mutex_init(&pmd->port_mutex);\n cmap_init(&pmd->flow_table);\n + cmap_init(&pmd->mark_to_flow);\n cmap_init(&pmd->classifiers);\n pmd->next_optimization = time_msec() + DPCLS_OPTIMIZATION_INTERVAL;\n pmd->rxq_interval = time_msec() + PMD_RXQ_INTERVAL_LEN;\n @@ -4538,6 +4622,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd)\n }\n cmap_destroy(&pmd->classifiers);\n cmap_destroy(&pmd->flow_table);\n + cmap_destroy(&pmd->mark_to_flow);\n ovs_mutex_destroy(&pmd->flow_mutex);\n latch_destroy(&pmd->exit_latch);\n seq_destroy(pmd->reload_seq);\n diff --git a/lib/netdev.h b/lib/netdev.h\n index f8482f7..2003165 100644\n --- a/lib/netdev.h\n +++ b/lib/netdev.h\n @@ -188,6 +188,12 @@ void netdev_send_wait(struct netdev *, int qid);\n struct offload_info {\n const struct dpif_class *dpif_class;\n ovs_be16 tp_dst_port; /* Destination port for tunnel in SET action */\n +\n + /*\n + * The flow mark id assigened to the flow. If any pkts hit the flow,\n + * it will be in the pkt meta data.\n + */\n + uint32_t flow_mark;\n };\n struct dpif_class;\n struct netdev_flow_dump;\n -- \n 2.7.4","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"kTNN1kU0\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtZpG3VWmz9s4s\n\tfor ;\n\tFri, 15 Sep 2017 09:42:45 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 503A4AF7;\n\tThu, 14 Sep 2017 23:42:42 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 5B5288EE\n\tfor ; Thu, 14 Sep 2017 23:42:41 +0000 (UTC)","from NAM03-BY2-obe.outbound.protection.outlook.com\n\t(mail-by2nam03on0085.outbound.protection.outlook.com [104.47.42.85])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7C9C4D3\n\tfor ; Thu, 14 Sep 2017 23:42:40 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB787.namprd05.prod.outlook.com (10.141.209.146) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.77.5; Thu, 14 Sep 2017 23:42:38 +0000","from BLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) by\n\tBLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) with mapi id\n\t15.20.0077.005; Thu, 14 Sep 2017 23:42:38 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=j7N1D2HTAAqZxz4SnKX997KWrXepw4HqqL5M7GIsUPw=;\n\tb=kTNN1kU0167awiuLcf+fjQJgmgfcB/tFDdOOjNkRbjtA7/afFwXKc1O/TU26+lhY2zr8VuC4hMpV6iAR2JI7K/FF57c/Su3M/KIoF6UiYvSwJtSLnzmpil3Ver1ING0H3ka82xdtYZZbzGDk4xbj3VOvsrWnDVDw0gip2GjQrzE=","From":"Darrell Ball ","To":"Yuanhan Liu , \"dev@openvswitch.org\"\n\t","Thread-Topic":"[PATCH v2 1/8] dpif-netdev: associate flow with a mark id","Thread-Index":"AQHTJiiszD9lntM1Ck6QL6cMWc01r6K0pKCA","Date":"Thu, 14 Sep 2017 23:42:38 +0000","Message-ID":"<1F0232ED-122F-4945-BCF6-53EF4A97B2DC@vmware.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>","In-Reply-To":"<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.25.0.170815","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"kTNN1kU0\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"x-originating-ip":"[208.91.2.2]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB787;\n\t20:/GeDyZq5dVMeWlYI8NXTLX2o1iwavr8stdfcgyZ+1jOub5OsEauYAfRNMwyL3bhxhYnqeiN/8Z+BpAnDej19dDXaQpbAt4VBlsMuDSZJsVAZ4Za5mB1ogTXu8rZWajH5Aqzrvjtq5ooSRs9oAblzvNcrLtV8F7+4CSD85LCKUNc=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"53bca118-7bcf-413c-411a-08d4fbca48b1","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(300000502095)(300135100095)(22001)(2017030254152)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:BLUPR05MB787; ","x-ms-traffictypediagnostic":"BLUPR05MB787:","x-exchange-antispam-report-test":"UriScan:;","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(100000703101)(100105400095)(93006095)(93001095)(10201501046)(3002001)(6041248)(20161123560025)(20161123564025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123555025)(20161123562025)(20161123558100)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB787; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB787; ","x-forefront-prvs":"0430FA5CB7","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(39860400002)(376002)(346002)(377454003)(199003)(189002)(24454002)(6506006)(36756003)(6512007)(99286003)(53936002)(7736002)(305945005)(14454004)(53546010)(86362001)(6486002)(54906002)(33656002)(77096006)(6246003)(6436002)(229853002)(2900100001)(66066001)(101416001)(3660700001)(25786009)(82746002)(5660300001)(81166006)(54356999)(8936002)(4326008)(2950100002)(81156014)(105586002)(8676002)(50986999)(76176999)(106356001)(2906002)(478600001)(316002)(68736007)(3280700002)(3846002)(83716003)(6116002)(102836003)(2501003)(83506001)(189998001)(97736004)(58126008);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB787;\n\tH:BLUPR05MB611.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; A:1; MX:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"14 Sep 2017 23:42:38.6999\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB787","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1768874,"web_url":"http://patchwork.ozlabs.org/comment/1768874/","msgid":"<2C5CDF54-2930-45E4-82C4-2237A0DF693A@vmware.com>","list_archive_url":null,"date":"2017-09-14T23:59:35","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"On 9/11/17, 1:04 AM, \"ovs-dev-bounces@openvswitch.org on behalf of Yuanhan Liu\" wrote:\n\n On Sun, Sep 10, 2017 at 04:14:01PM +0000, Chandran, Sugesh wrote:\n > > This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n > > \n > > It re-uses the flow API (more precisely, the ->flow_put method) to setup the\n > > hw flow. The flow_put implementation then is supposed to create a flow\n > > with MARK action.\n > > \n > > Co-authored-by: Finn Christensen \n > > Signed-off-by: Yuanhan Liu \n > > Signed-off-by: Finn Christensen \n > > ---\n > > lib/dpif-netdev.c | 85\n > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n > > lib/netdev.h | 6 ++++\n > > 2 files changed, 91 insertions(+)\n > \n > [snip]\n > > * Any thread trying to keep a rule from being freed should hold its own\n > > @@ -569,6 +575,8 @@ struct dp_netdev_pmd_thread {\n > > struct ovs_mutex flow_mutex;\n > > struct cmap flow_table OVS_GUARDED; /* Flow table. */\n > > \n > > + struct cmap mark_to_flow;\n > [Sugesh] I would prefer to have this cmap per port than a pmd. The reason being it offers better\n > Performance due to less lookup. Anyway the flow programming is on netdev, hence its good to\n > keep it netdev centric than pmd. What do you think?\n \n I think it might be workable.\n \n > > /* One classifier per in_port polled by the pmd */\n > > struct cmap classifiers;\n > > /* Periodically sort subtable vectors according to hit frequencies */ @@ -\n > > 1839,6 +1847,8 @@ dp_netdev_pmd_remove_flow(struct\n > > dp_netdev_pmd_thread *pmd,\n > > OVS_REQUIRES(pmd->flow_mutex)\n > > {\n > > struct cmap_node *node = CONST_CAST(struct cmap_node *, &flow-\n > > >node);\n > > + struct cmap_node *mark_node = CONST_CAST(struct cmap_node *,\n > > + &flow->mark_node);\n > > struct dpcls *cls;\n > > odp_port_t in_port = flow->flow.in_port.odp_port;\n > > \n > > @@ -1846,6 +1856,10 @@ dp_netdev_pmd_remove_flow(struct\n > > dp_netdev_pmd_thread *pmd,\n > > ovs_assert(cls != NULL);\n > > dpcls_remove(cls, &flow->cr);\n > > cmap_remove(&pmd->flow_table, node, dp_netdev_flow_hash(&flow-\n > > >ufid));\n > > + if (flow->has_mark) {\n > [Sugesh] Any reason these are not mutex protected? As far as I know the flow remove\n > is handled in revalidator thread. Looks like this is a critical section. So the install and delete\n > should be protected.\n \n I think you are right.\n\n\n[Darrell] Pls verify, but I thought you held the flow_mutex during create/delete.\n\n \n [...]\n > > + if (netdev_is_flow_api_enabled() &&\n > > + dp_netdev_alloc_flow_mark(pmd, &info.flow_mark)) {\n > > + struct dp_netdev_port *port;\n > > + port = dp_netdev_lookup_port(pmd->dp, in_port);\n > > + if (netdev_flow_put(port->netdev, match,\n > > + CONST_CAST(struct nlattr *, actions),\n > > + actions_len, ufid, &info, NULL) == 0) {\n > [Sugesh] Cost of flow install is a concern here. It is vary for device to device.\n > Have you done any analysis on the cost of rte_flow translate + hardware flow install?\n > It would be nice to mention it in the cover letter. \n \n I haven't measured it yet: it's not my current focus. It's not in the\n datapath after all. Besides, it might not be a trivial task to optimize\n it.\n \n \t--yliu\n _______________________________________________\n dev mailing list\n dev@openvswitch.org\n https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=SxSo6-I3zZTRdns-rYk4h9OLFx1BNBJFuJMPg9ALn6w&s=wrMA8ThZTWY-xuZQdO83DhWwloYLkD4FrOu2OQXoh40&e=","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"Dkiur8bu\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtb9q4CCkz9s7h\n\tfor ;\n\tFri, 15 Sep 2017 09:59:43 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 0072AB08;\n\tThu, 14 Sep 2017 23:59:40 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 2C073A7F\n\tfor ; Thu, 14 Sep 2017 23:59:38 +0000 (UTC)","from NAM01-BY2-obe.outbound.protection.outlook.com\n\t(mail-by2nam01on0054.outbound.protection.outlook.com [104.47.34.54])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 77C40D3\n\tfor ; Thu, 14 Sep 2017 23:59:37 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB756.namprd05.prod.outlook.com (10.141.208.151) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.77.5; Thu, 14 Sep 2017 23:59:35 +0000","from BLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) by\n\tBLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) with mapi id\n\t15.20.0077.005; Thu, 14 Sep 2017 23:59:35 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=9VNv5uP/AWPMkePYxxCTtwzRgMPUduq+bfcIhgBxBtk=;\n\tb=Dkiur8buxa4WwXTQIYX+hgfairngzZQcVulIiXPk2Pm8XWNmHJoDOgdfQfGpiE+zXnKUq3f4M+WxlstHEN10gxpoTJdjSEAAAQjS9+I57kI7d921aC7XY3R2B1IukVq+ONGzBLfxwt4J6p7nbjKywR486NuMWGbImOL48nTSt2A=","From":"Darrell Ball ","To":"Yuanhan Liu , \"Chandran, Sugesh\"\n\t","Thread-Topic":"[ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","Thread-Index":"AQHTJiiszD9lntM1Ck6QL6cMWc01r6KuU0+AgAEJXICABUyxAA==","Date":"Thu, 14 Sep 2017 23:59:35 +0000","Message-ID":"<2C5CDF54-2930-45E4-82C4-2237A0DF693A@vmware.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>\n\t<2EF2F5C0CC56984AA024D0B180335FCB4221DC16@IRSMSX102.ger.corp.intel.com>\n\t<20170911080347.GT9736@yliu-home>","In-Reply-To":"<20170911080347.GT9736@yliu-home>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.25.0.170815","x-originating-ip":"[208.91.2.2]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB756;\n\t20:5gvjVx79AoVwjSuvr1wwSmsmcjS+WFhRwEl3oclOxZqLxpPvOS9aR1BxuAFKVf+VHjOqlYX04NXY/WJelVsoa9NPer6fXCBs7FEI8hYn6rbr6Bp8GnGTzED/Vpjx9bkWA0q0yC87mt6QHFk4M7e+vgIq23Y///AOOL++7B4lsJI=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"71a4f889-d5ae-4ccd-1bc3-08d4fbcca6b3","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(300000502095)(300135100095)(22001)(2017030254152)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:BLUPR05MB756; ","x-ms-traffictypediagnostic":"BLUPR05MB756:","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"Dkiur8bu\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"x-exchange-antispam-report-test":"UriScan:(10436049006162)(216315784871565);","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(3002001)(10201501046)(93006095)(93001095)(100000703101)(100105400095)(6041248)(20161123562025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123555025)(20161123564025)(20161123558100)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB756; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB756; ","x-forefront-prvs":"0430FA5CB7","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(376002)(346002)(39860400002)(76104003)(189002)(24454002)(199003)(377454003)(8676002)(229853002)(66066001)(189998001)(6486002)(966005)(81156014)(93886005)(6436002)(33656002)(6506006)(36756003)(25786009)(2950100002)(53546010)(50986999)(54356999)(81166006)(7736002)(14454004)(76176999)(102836003)(101416001)(305945005)(3280700002)(86362001)(68736007)(77096006)(3660700001)(97736004)(106356001)(5660300001)(83506001)(2900100001)(83716003)(6246003)(8936002)(478600001)(4326008)(99286003)(6306002)(2906002)(105586002)(6116002)(82746002)(58126008)(6512007)(53936002)(316002)(3846002);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB756;\n\tH:BLUPR05MB611.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"14 Sep 2017 23:59:35.3957\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB756","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1769071,"web_url":"http://patchwork.ozlabs.org/comment/1769071/","msgid":"<20170915092329.GJ2050@yliu-home>","list_archive_url":null,"date":"2017-09-15T09:23:29","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Thu, Sep 14, 2017 at 11:42:38PM +0000, Darrell Ball wrote:\n> \n> \n> On 9/5/17, 2:23 AM, \"Yuanhan Liu\" wrote:\n> \n> This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n> \n> It re-uses the flow API (more precisely, the ->flow_put method) to setup\n> the hw flow. The flow_put implementation then is supposed to create a\n> flow with MARK action.\n> \n> Co-authored-by: Finn Christensen \n> Signed-off-by: Yuanhan Liu \n> Signed-off-by: Finn Christensen \n> ---\n> lib/dpif-netdev.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> lib/netdev.h | 6 ++++\n> 2 files changed, 91 insertions(+)\n> \n> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c\n> index 071ec14..f3b7f25 100644\n> --- a/lib/dpif-netdev.c\n> +++ b/lib/dpif-netdev.c\n> @@ -446,6 +446,12 @@ struct dp_netdev_flow {\n> const unsigned pmd_id; /* The 'core_id' of pmd thread owning this */\n> /* flow. */\n> \n> + const struct cmap_node mark_node; /* In owning dp_netdev_pmd_thread's */\n> + /* 'mark_to_flow' */\n> + bool has_mark; /* A flag to tell whether this flow has a\n> + valid mark asscoiated with it. */\n> + uint32_t mark; /* Unique flow mark assiged to a flow */\n> \n> \n> [Darrell] Since the PMD that receives a flow can change and multiple PMDs can allocate\n> the same mark value at the moment, it seems the mark allocation needs to include the PMD ID or\n> be from a global namespace to enforce uniqueness.\n> Pls verify.\n\nYes, you are right. Let it be global then. Talking about that, one of my\ncolleague actually suggests to use an array to do the mapping. It's a\n\"number -> flow\" mapping after all: using a hash map looks like an overkill.\nUsing array also would give us the best performance.\n\nThe reason I chose cmap in the beginning is we don't know how big the\narray is. Also, it doesn't make sense to allocate a static array big\nenough to hold the maximum entires (it's uint32_t after all).\n\nSo I'm now thinking:\n\n- allocate a small array first, say with 1024 entries\n- if there is no space, double the max (say, 2048) and re-allocate\n\nI don't know the performance difference yet. I will get back to you\nwhen I have the number. Besides that, what do you think.\n\n\t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=fridaylinux-org.20150623.gappssmtp.com\n\theader.i=@fridaylinux-org.20150623.gappssmtp.com\n\theader.b=\"k9nG99Yf\"; dkim-atps=neutral"],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtqhd4pFVz9ryk\n\tfor ;\n\tFri, 15 Sep 2017 19:23:44 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 0EF9BA6E;\n\tFri, 15 Sep 2017 09:23:40 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 4BD4DA47\n\tfor ; Fri, 15 Sep 2017 09:23:38 +0000 (UTC)","from mail-pg0-f44.google.com (mail-pg0-f44.google.com\n\t[74.125.83.44])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id B74C51E8\n\tfor ; Fri, 15 Sep 2017 09:23:37 +0000 (UTC)","by mail-pg0-f44.google.com with SMTP id 7so1191332pgd.13\n\tfor ; Fri, 15 Sep 2017 02:23:37 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\te13sm1671671pgt.14.2017.09.15.02.23.34\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tFri, 15 Sep 2017 02:23:36 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=fridaylinux-org.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=MGu40SvGB7r0x9jMBxm3W7B4DhmspwFMalbEuULWtmo=;\n\tb=k9nG99Yf2VU6Sk4DAIY55NMJealNdtxxBHl6YlTGcAyxLIleXDD8DS6iipEn9nTuuD\n\tlTwuys5yxLpsuelUYvRaw6vXMvC8KMexDtY3IBf1LuWuWVel7BTbNckUjTjfuxAcEhuU\n\t0RH0nrPSazECLlj+7j+YTZdaue+0+Xd/HAsiyL73hBdPRBajHzSkw1+wuWHFfcCJEjPz\n\tLWkO/ZGb/IT0Kp9U3YPyalqsoZMhFe6gR7lLQ7aNprCJ4uUHYwjEOZGq6aKp46q0F43M\n\tSNpOqQfGHc8Qp+Bdw2CbPXTRHCmcfEYyFR2Ss4njAVXRrBmzPKTvJ7SOjNsC3iLT1dQ5\n\tp80A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=MGu40SvGB7r0x9jMBxm3W7B4DhmspwFMalbEuULWtmo=;\n\tb=FwYCN+ENO6l9iKmOv9AVcs1ZmpITirlKHyLZBqo/kIipWp29R0BQDYrLjbJOedmeb1\n\tUc8G+aACVNiQx98qXzGcRltuTzyfpa7vP0M7k3aInBkiyXmU+Eym3FrlEsm8a6dUbWAe\n\tRi+QtGfccomrXF7UM8YrMdomcBkNP8Fs2z967nzEma7WUWHVsDfn4gUzchcbobTkWE8g\n\tJAFVaZ9xqfsVqScwfQy33bqo/wcRLyHiCMhBL4blqrJfe9wUhzGKVW3myqrDrrwnJYKc\n\ta6EmLT1qWtt0pxAGD2dJ3OuRh5qD9y1Q4Ns65vWga8OsPe+jlIQIodpvT+M7II+778Uv\n\tCxqg==","X-Gm-Message-State":"AHPjjUg16fuf4XSzzJkMGsdIq1/NN2TwVdVyel32S4XaY3lBSnNg5NPN\n\tmKeKA5cb4lQBfAw2","X-Google-Smtp-Source":"ADKCNb5GyMSRFwEyWmwkoZysdjtHnosC2WgWwQG7trovwWlLHXC2FkwFR5w3ZOUfTRf4ORpdtWT1hg==","X-Received":"by 10.84.235.67 with SMTP id g3mr26027393plt.299.1505467417328; \n\tFri, 15 Sep 2017 02:23:37 -0700 (PDT)","Date":"Fri, 15 Sep 2017 17:23:29 +0800","From":"Yuanhan Liu ","To":"Darrell Ball ","Message-ID":"<20170915092329.GJ2050@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>\n\t<1F0232ED-122F-4945-BCF6-53EF4A97B2DC@vmware.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1F0232ED-122F-4945-BCF6-53EF4A97B2DC@vmware.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Spam-Status":"No, score=0.5 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1769435,"web_url":"http://patchwork.ozlabs.org/comment/1769435/","msgid":"<72F5E9BE-F02A-4986-AC88-9269CA7D69B5@vmware.com>","list_archive_url":null,"date":"2017-09-15T20:29:25","subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"On 9/15/17, 2:23 AM, \"Yuanhan Liu\" wrote:\n\n On Thu, Sep 14, 2017 at 11:42:38PM +0000, Darrell Ball wrote:\n > \n > \n > On 9/5/17, 2:23 AM, \"Yuanhan Liu\" wrote:\n > \n > This patch associate a flow with a mark id (a uint32_t number) by CMAP.\n > \n > It re-uses the flow API (more precisely, the ->flow_put method) to setup\n > the hw flow. The flow_put implementation then is supposed to create a\n > flow with MARK action.\n > \n > Co-authored-by: Finn Christensen \n > Signed-off-by: Yuanhan Liu \n > Signed-off-by: Finn Christensen \n > ---\n > lib/dpif-netdev.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n > lib/netdev.h | 6 ++++\n > 2 files changed, 91 insertions(+)\n > \n > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c\n > index 071ec14..f3b7f25 100644\n > --- a/lib/dpif-netdev.c\n > +++ b/lib/dpif-netdev.c\n > @@ -446,6 +446,12 @@ struct dp_netdev_flow {\n > const unsigned pmd_id; /* The 'core_id' of pmd thread owning this */\n > /* flow. */\n > \n > + const struct cmap_node mark_node; /* In owning dp_netdev_pmd_thread's */\n > + /* 'mark_to_flow' */\n > + bool has_mark; /* A flag to tell whether this flow has a\n > + valid mark asscoiated with it. */\n > + uint32_t mark; /* Unique flow mark assiged to a flow */\n > \n > \n > [Darrell] Since the PMD that receives a flow can change and multiple PMDs can allocate\n > the same mark value at the moment, it seems the mark allocation needs to include the PMD ID or\n > be from a global namespace to enforce uniqueness.\n > Pls verify.\n \n Yes, you are right. Let it be global then. \n\n[Darrell] One downside of global vs including PMD ID (maybe 10 bits) is \nmore contention b/w pmds.\n\nTalking about that, one of my\n colleague actually suggests to use an array to do the mapping. It's a\n \"number -> flow\" mapping after all: using a hash map looks like an overkill.\n Using array also would give us the best performance.\n \n The reason I chose cmap in the beginning is we don't know how big the\n array is. Also, it doesn't make sense to allocate a static array big\n enough to hold the maximum entires (it's uint32_t after all).\n \n So I'm now thinking:\n \n - allocate a small array first, say with 1024 entries\n - if there is no space, double the max (say, 2048) and re-allocate\n \n I don't know the performance difference yet. I will get back to you\n when I have the number. Besides that, what do you think.\n\n[Darrell] Seems fine.\ncmap has rcu build-in, but that can be replicated, if you find it is makes a significant difference.\n \n \t--yliu","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"Uhps1s8B\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xv6Sr1Sr7z9s83\n\tfor ;\n\tSat, 16 Sep 2017 06:29:31 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 5D5B1B7C;\n\tFri, 15 Sep 2017 20:29:30 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 3CA3CB5D\n\tfor ; Fri, 15 Sep 2017 20:29:29 +0000 (UTC)","from NAM03-CO1-obe.outbound.protection.outlook.com\n\t(mail-co1nam03on0077.outbound.protection.outlook.com [104.47.40.77])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id A4DF0113\n\tfor ; Fri, 15 Sep 2017 20:29:28 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB963.namprd05.prod.outlook.com (10.141.206.12) with Microsoft\n\tSMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.77.5; Fri, 15 Sep 2017 20:29:26 +0000","from BLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) by\n\tBLUPR05MB611.namprd05.prod.outlook.com ([10.141.204.27]) with mapi id\n\t15.20.0077.006; Fri, 15 Sep 2017 20:29:25 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=30wz/KavLEgNzhtMyjWqGR80QHVqa/whTEZowcFEWd8=;\n\tb=Uhps1s8B315/S6WRmKqWnjRLq5Lvjs160e61BXLwoMzFhR62uU1mkEhELPZjweHhzhNB4pCacdxKdgVpr34/QES8hzF7uI8SvHnvz5ZQmkJYZLpNRJBmOXmP68X1jyh+gE0HQaik2R8ZxrJDc/wyPBivLK+tlEg0XAwNu5JEkaA=","From":"Darrell Ball ","To":"Yuanhan Liu ","Thread-Topic":"[PATCH v2 1/8] dpif-netdev: associate flow with a mark id","Thread-Index":"AQHTJiiszD9lntM1Ck6QL6cMWc01r6K0pKCAgAEXo4CAAES2AA==","Date":"Fri, 15 Sep 2017 20:29:25 +0000","Message-ID":"<72F5E9BE-F02A-4986-AC88-9269CA7D69B5@vmware.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-2-git-send-email-yliu@fridaylinux.org>\n\t<1F0232ED-122F-4945-BCF6-53EF4A97B2DC@vmware.com>\n\t<20170915092329.GJ2050@yliu-home>","In-Reply-To":"<20170915092329.GJ2050@yliu-home>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.25.0.170815","x-originating-ip":"[73.162.236.45]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB963;\n\t20:jBKDPg+admbk9KafCL7MKFYvwUoHWlKoOL9gSJUFx6TdPCOcQ12CqRfljarbZYDfOYmnElP+KnRDJ3zMsfHDgQA3fW/60hoepiYpuO7CzdLnmLygG578sU3O0wSLz6xHIigRdgaTteNGwcPMCFdmaupqbDmaHe/rwnwPxKydljo=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"79d2ee9b-636c-4215-aa7b-08d4fc78753b","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:BLUPR05MB963; ","x-ms-traffictypediagnostic":"BLUPR05MB963:","x-exchange-antispam-report-test":"UriScan:;","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(100000703101)(100105400095)(3002001)(10201501046)(93006095)(93001095)(6041248)(20161123562025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123560025)(20161123564025)(20161123558100)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB963; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB963; ","x-forefront-prvs":"0431F981D8","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(376002)(346002)(377454003)(24454002)(76104003)(189002)(199003)(53936002)(14454004)(189998001)(8676002)(81166006)(53546010)(81156014)(8936002)(106356001)(105586002)(36756003)(33656002)(101416001)(54906002)(4326008)(99286003)(5660300001)(83716003)(2900100001)(6512007)(2950100002)(6246003)(2906002)(3280700002)(3660700001)(93886005)(76176999)(54356999)(50986999)(97736004)(66066001)(110136004)(68736007)(7736002)(305945005)(82746002)(83506001)(25786009)(77096006)(86362001)(6486002)(478600001)(6506006)(6916009)(6436002)(6116002)(102836003)(316002)(58126008)(229853002)(3846002);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB963;\n\tH:BLUPR05MB611.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"Uhps1s8B\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"<2EB5BA569FFF454DA2782B741BC0CAAF@namprd05.prod.outlook.com>","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"15 Sep 2017 20:29:25.6702\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB963","X-Spam-Status":"No, score=0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tRCVD_IN_DNSWL_NONE autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"dev@openvswitch.org\" ","Subject":"Re: [ovs-dev] [PATCH v2 1/8] dpif-netdev: associate flow with a\n\tmark id","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}}]