[{"id":1765508,"web_url":"http://patchwork.ozlabs.org/comment/1765508/","msgid":"<20170908164621.GC7356@vergenet.net>","list_archive_url":null,"date":"2017-09-08T16:46:22","subject":"Re: [ovs-dev] [PATCH v2 3/8] netdev-dpdk: convert ufid to dpdk flow","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:56PM +0800, Yuanhan Liu wrote:\n> Flows offloaded to DPDK are identified by rte_flow pointer while OVS\n> flows are identified by ufid. This patches adds a hmap to convert ufid\n> to dpdk flow (rte_flow).\n> \n> Most of the code are stolen from netdev-tc-offloads.c, with some\n> modificatons.\n> \n> Some functions are marked as \"inline\", which is a trick to workaround\n> the temp \"functiond defined but not used\" warnings.\n\n...\n\n> +/*\n> + * Remove ufid_dpdk_flow_data node associated with @ufid\n> + */\n> +static inline void\n> +del_ufid_dpdk_flow_mapping(const ovs_u128 *ufid)\n> +{\n> + struct ufid_dpdk_flow_data *data;\n> +\n> + data = find_ufid_dpdk_flow_mapping(ufid);\n> + if (data) {\n> + ovs_mutex_lock(&ufid_lock);\n> + hmap_remove(&ufid_dpdk_flow, &data->node);\n> + free(data);\n> + ovs_mutex_unlock(&ufid_lock);\n> + }\n\nI think it would be nicer to exit early:\n\n+ if (!data) {\n+ return;\n+ }\n+\n+ ovs_mutex_lock(&ufid_lock);\n+ hmap_remove(&ufid_dpdk_flow, &data->node);\n+ free(data);\n+ ovs_mutex_unlock(&ufid_lock);\n\n> +}\n> +\n> +/* Add ufid to dpdk_flow mapping */\n> +static inline void\n> +add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow)\n> +{\n> + size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);\n> + struct ufid_dpdk_flow_data *data = xzalloc(sizeof(*data));\n> +\n> + /*\n> + * We should not simply overwrite an existing rte flow.\n> + * We should have deleted it first before re-adding it.\n> + * Thus, if following assert triggers, something is wrong:\n> + * the rte_flow is not destroyed.\n> + */\n> + ovs_assert(find_ufid_dpdk_flow_mapping(ufid) == NULL);\n> +\n> + data->ufid = *ufid;\n> + data->rte_flow = rte_flow;\n> +\n> + ovs_mutex_lock(&ufid_lock);\n> + hmap_insert(&ufid_dpdk_flow, &data->node, hash);\n> + ovs_mutex_unlock(&ufid_lock);\n> +}\n\nI am not sure that the locking in the add and del functions above can't\nrace. f.e. can two deletion requests for the same flow occur in parallel?\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=\"J0F3553m\"; 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 3xpjrd6vtKz9s8J\n\tfor ;\n\tSat, 9 Sep 2017 02:46:25 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id AC1B0B78;\n\tFri, 8 Sep 2017 16:46:23 +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 532B78A5\n\tfor ; Fri, 8 Sep 2017 16:46:23 +0000 (UTC)","from mail-wr0-f174.google.com (mail-wr0-f174.google.com\n\t[209.85.128.174])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id B5272467\n\tfor ; Fri, 8 Sep 2017 16:46:22 +0000 (UTC)","by mail-wr0-f174.google.com with SMTP id v109so5679948wrc.1\n\tfor ; Fri, 08 Sep 2017 09:46:22 -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\tz51sm2298234wrz.80.2017.09.08.09.46.20\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tFri, 08 Sep 2017 09:46:20 -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=xV69MZ1RQmSjZHBF/QEVSQFiEVCif1snXYxBS6PjA3c=;\n\tb=J0F3553mr9XCGLNfUMKKSJr7W4RE9cyWPhv4C2HtQ3BOArcXZUvgeTZ0DuskUD8oUl\n\t37W0NGW+DnmtdiT9rDvLYJFhqpD0E+JFRpSVYDbeAZbyBg1cX2BmtUVY9IOD+G7NfJVY\n\trGlfz3GnLFn8EhmN9mXN5TIKciVfzvVmGwfPMr3EASwNBrgn0LpZ78LPFlZKNtPnJxtU\n\t7NHEHSsBPvm1NXqODh14C3rnDZAF/Cys0Vy3r9mgL5H0SYDs7a6iiodmWKTnWJcoRgz8\n\tifoFknH5IVwS2aVOv32Pc/sa2gLxOi9ySvZM2pRBFYKxnkmXNBCF9CkG27fAyq24nY1D\n\tC+OQ==","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=xV69MZ1RQmSjZHBF/QEVSQFiEVCif1snXYxBS6PjA3c=;\n\tb=SkC3LkCPxDtlgE/UGthwyhq9aE1K+hlEU12jQH6g+ce/9gZQYRDL168o7wdgDrnwYC\n\tFw8njT6/PcAFKZZqvZPYE2yG96S165rYrW5gSnXuuqBNvJbjRxw0SOfLYfgTx+3Wx5+C\n\tFmf8yQWzPPEyl4i5lyjwTVzcBBGpkuAYuLPcF7upjIsw5sYA9Hmw8D3aLCH1yoca70U9\n\tzMRjMIhCR3ydUgbRhBrBZ2HJJk8ijQ2aKELq7VdJxCoy7A9IRqlseE8uG96/WP05FglF\n\tgRdWyhPJBoPU3DgCZWak2tg17eWEAbpYAOijebxk/rae0773iozz7CgWzJYbjLFU0iO1\n\tk3HA==","X-Gm-Message-State":"AHPjjUiW2vnTNPon0WL2ae2WTPcXdmayNSph2QwYnJR0cKPxAsevAXxZ\n\tCjRbDbZjAY+JWWA4","X-Google-Smtp-Source":"ADKCNb7LTzgKtlnT0sjIz6qWx33+sZ+DBWu0g7VDDP0co8flusgrptdqXIOwi5iBkKDeoR4Dq1qVqg==","X-Received":"by 10.223.170.214 with SMTP id i22mr2433288wrc.271.1504889181335;\n\tFri, 08 Sep 2017 09:46:21 -0700 (PDT)","Date":"Fri, 8 Sep 2017 18:46:22 +0200","From":"Simon Horman ","To":"Yuanhan Liu ","Message-ID":"<20170908164621.GC7356@vergenet.net>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-4-git-send-email-yliu@fridaylinux.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1504603381-30071-4-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 3/8] netdev-dpdk: convert ufid to dpdk flow","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":1765550,"web_url":"http://patchwork.ozlabs.org/comment/1765550/","msgid":"<1314483A-1977-434B-BCF3-0522DFEA56C8@vmware.com>","list_archive_url":null,"date":"2017-09-08T17:48:36","subject":"Re: [ovs-dev] [PATCH v2 3/8] netdev-dpdk: convert ufid to dpdk flow","submitter":{"id":68212,"url":"http://patchwork.ozlabs.org/api/people/68212/","name":"Darrell Ball","email":"dball@vmware.com"},"content":"On 9/8/17, 9:46 AM, \"ovs-dev-bounces@openvswitch.org on behalf of Simon Horman\" wrote:\r\n\r\n On Tue, Sep 05, 2017 at 05:22:56PM +0800, Yuanhan Liu wrote:\r\n > Flows offloaded to DPDK are identified by rte_flow pointer while OVS\r\n > flows are identified by ufid. This patches adds a hmap to convert ufid\r\n > to dpdk flow (rte_flow).\r\n > \r\n > Most of the code are stolen from netdev-tc-offloads.c, with some\r\n > modificatons.\r\n > \r\n > Some functions are marked as \"inline\", which is a trick to workaround\r\n > the temp \"functiond defined but not used\" warnings.\r\n \r\n ...\r\n \r\n > +/*\r\n > + * Remove ufid_dpdk_flow_data node associated with @ufid\r\n > + */\r\n > +static inline void\r\n > +del_ufid_dpdk_flow_mapping(const ovs_u128 *ufid)\r\n > +{\r\n > + struct ufid_dpdk_flow_data *data;\r\n > +\r\n > + data = find_ufid_dpdk_flow_mapping(ufid);\r\n > + if (data) {\r\n > + ovs_mutex_lock(&ufid_lock);\r\n > + hmap_remove(&ufid_dpdk_flow, &data->node);\r\n > + free(data);\r\n > + ovs_mutex_unlock(&ufid_lock);\r\n > + }\r\n \r\n I think it would be nicer to exit early:\r\n \r\n + if (!data) {\r\n + return;\r\n + }\r\n +\r\n + ovs_mutex_lock(&ufid_lock);\r\n + hmap_remove(&ufid_dpdk_flow, &data->node);\r\n + free(data);\r\n + ovs_mutex_unlock(&ufid_lock);\r\n\r\n\r\n[Darrell] It is a minor point, but the preference is typically given to the affirmative\r\n condition check. A slight variation of Yuanhan’s version would be:\r\n\r\n + data = find_ufid_dpdk_flow_mapping(ufid);\r\n + if (OVS_LIKELY(data)) {\r\n + ovs_mutex_lock(&ufid_lock);\r\n + hmap_remove(&ufid_dpdk_flow, &data->node);\r\n + free(data);\r\n + ovs_mutex_unlock(&ufid_lock);\r\n + } else {\r\n + VLOG_WARN….. <<<<< since this is probably unexpected\r\n + }\r\n\r\n//////////////////////////\r\n\r\n \r\n > +}\r\n > +\r\n > +/* Add ufid to dpdk_flow mapping */\r\n > +static inline void\r\n > +add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow)\r\n > +{\r\n > + size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);\r\n > + struct ufid_dpdk_flow_data *data = xzalloc(sizeof(*data));\r\n > +\r\n > + /*\r\n > + * We should not simply overwrite an existing rte flow.\r\n > + * We should have deleted it first before re-adding it.\r\n > + * Thus, if following assert triggers, something is wrong:\r\n > + * the rte_flow is not destroyed.\r\n > + */\r\n > + ovs_assert(find_ufid_dpdk_flow_mapping(ufid) == NULL);\r\n > +\r\n > + data->ufid = *ufid;\r\n > + data->rte_flow = rte_flow;\r\n > +\r\n > + ovs_mutex_lock(&ufid_lock);\r\n > + hmap_insert(&ufid_dpdk_flow, &data->node, hash);\r\n > + ovs_mutex_unlock(&ufid_lock);\r\n > +}\r\n \r\n I am not sure that the locking in the add and del functions above can't\r\n race. f.e. can two deletion requests for the same flow occur in parallel?\r\n \r\n ...\r\n _______________________________________________\r\n dev mailing list\r\n dev@openvswitch.org\r\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=EqZ7zhHzkcbfWx0DXX2Yi0ilPUo3gLVNfo5C7ZFlXkc&s=VZc7ZWCSFtFZCpNq-3YBxOJkzVgP-EmVevjn7nwMJTI&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=\"ixG7UkxW\"; \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 3xplDX052kz9s8J\n\tfor ;\n\tSat, 9 Sep 2017 03:48:43 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 0996CB01;\n\tFri, 8 Sep 2017 17:48:41 +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 D6291AE7\n\tfor ; Fri, 8 Sep 2017 17:48:39 +0000 (UTC)","from NAM02-SN1-obe.outbound.protection.outlook.com\n\t(mail-sn1nam02on0069.outbound.protection.outlook.com [104.47.36.69])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 13C07469\n\tfor ; Fri, 8 Sep 2017 17:48:38 +0000 (UTC)","from BLUPR05MB611.namprd05.prod.outlook.com (10.141.204.27) by\n\tBLUPR05MB307.namprd05.prod.outlook.com (10.141.23.149) with Microsoft\n\tSMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.20.56.4; Fri, 8 Sep 2017 17:48:36 +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.0056.003; Fri, 8 Sep 2017 17:48:36 +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=fAlsyjQoWsZHZzNzutYFTf9aqKwvJcA94SitDs6qr5g=;\n\tb=ixG7UkxWlYtUNdPgpVcHip6k32b7TtmYnkullfb/TRVIXEwHhSw9s0wK/tj5nqXTh806ZTWhhZ7s9CrF2PhB+nugbEkbBW0p52nXUczOof2UcMVEZP2UWc+xKBKNocixfxjQT93KsznGigqaPh2s9gjkst0KtYLOomSIgOniL20=","From":"Darrell Ball ","To":"Simon Horman , Yuanhan Liu\n\t","Thread-Topic":"[ovs-dev] [PATCH v2 3/8] netdev-dpdk: convert ufid to dpdk flow","Thread-Index":"AQHTJiiuffYBwNF67kuFXvgu1FPhuqKrN68A//+cCYA=","Date":"Fri, 8 Sep 2017 17:48:36 +0000","Message-ID":"<1314483A-1977-434B-BCF3-0522DFEA56C8@vmware.com>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-4-git-send-email-yliu@fridaylinux.org>\n\t<20170908164621.GC7356@vergenet.net>","In-Reply-To":"<20170908164621.GC7356@vergenet.net>","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Microsoft-MacOutlook/f.23.0.170610","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=\"ixG7UkxW\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=dball@vmware.com; "],"x-originating-ip":"[73.162.236.45]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; BLUPR05MB307;\n\t20:yO1oirlBpKxUUnihjvDEDfzOKcO2mjUHS8G/WLIjEROBpMVmumVjVW/CFymRbknZ2ww62dTAyeMH+ii1Ets9xsogmxxBAqo679XHHXa6EKraLcHsRe5v8lVwHBKBRBMgpulv3yl4cuDXeWVBFk7A6MoIhWMmmMy6ZpM4FKaT4Ak=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"72c89140-25a5-49fe-c574-08d4f6e1d4f8","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:BLUPR05MB307; ","x-ms-traffictypediagnostic":"BLUPR05MB307:","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)(100000703101)(100105400095)(10201501046)(93006095)(93001095)(3002001)(6041248)(20161123555025)(20161123564025)(20161123558100)(20161123562025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:BLUPR05MB307; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:BLUPR05MB307; ","x-forefront-prvs":"04244E0DC5","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(189002)(377454003)(199003)(24454002)(478600001)(97736004)(6436002)(4001350100001)(6512007)(966005)(3846002)(105586002)(6116002)(102836003)(25786009)(66066001)(99286003)(83506001)(14454004)(83716003)(81166006)(575784001)(86362001)(2950100002)(8676002)(229853002)(81156014)(6306002)(8936002)(76176999)(3280700002)(3660700001)(305945005)(189998001)(101416001)(68736007)(50986999)(106356001)(4326008)(7736002)(2900100001)(82746002)(54356999)(6486002)(39060400002)(6246003)(6506006)(53936002)(53546010)(36756003)(33656002)(5660300001)(77096006)(2906002);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR05MB307;\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":"08 Sep 2017 17:48:36.5511\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"BLUPR05MB307","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 3/8] netdev-dpdk: convert ufid to dpdk flow","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1766050,"web_url":"http://patchwork.ozlabs.org/comment/1766050/","msgid":"<20170911054628.GL9736@yliu-home>","list_archive_url":null,"date":"2017-09-11T05:46:28","subject":"Re: [ovs-dev] [PATCH v2 3/8] netdev-dpdk: convert ufid to dpdk flow","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 05:48:36PM +0000, Darrell Ball wrote:\n> \n> > +static inline void\n> > +del_ufid_dpdk_flow_mapping(const ovs_u128 *ufid)\n> > +{\n> > + struct ufid_dpdk_flow_data *data;\n> > +\n> > + data = find_ufid_dpdk_flow_mapping(ufid);\n> > + if (data) {\n> > + ovs_mutex_lock(&ufid_lock);\n> > + hmap_remove(&ufid_dpdk_flow, &data->node);\n> > + free(data);\n> > + ovs_mutex_unlock(&ufid_lock);\n> > + }\n> \n> I think it would be nicer to exit early:\n> \n> + if (!data) {\n> + return;\n> + }\n> +\n> + ovs_mutex_lock(&ufid_lock);\n> + hmap_remove(&ufid_dpdk_flow, &data->node);\n> + free(data);\n> + ovs_mutex_unlock(&ufid_lock);\n> \n> \n> [Darrell] It is a minor point, but the preference is typically given to the affirmative\n> condition check. A slight variation of Yuanhan’s version would be:\n> \n> + data = find_ufid_dpdk_flow_mapping(ufid);\n> + if (OVS_LIKELY(data)) {\n> + ovs_mutex_lock(&ufid_lock);\n> + hmap_remove(&ufid_dpdk_flow, &data->node);\n> + free(data);\n> + ovs_mutex_unlock(&ufid_lock);\n> + } else {\n> + VLOG_WARN….. <<<<< since this is probably unexpected\n> + }\n> \n> //////////////////////////\n\nI could do that.\n\n> > +}\n> > +\n> > +/* Add ufid to dpdk_flow mapping */\n> > +static inline void\n> > +add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow)\n> > +{\n> > + size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);\n> > + struct ufid_dpdk_flow_data *data = xzalloc(sizeof(*data));\n> > +\n> > + /*\n> > + * We should not simply overwrite an existing rte flow.\n> > + * We should have deleted it first before re-adding it.\n> > + * Thus, if following assert triggers, something is wrong:\n> > + * the rte_flow is not destroyed.\n> > + */\n> > + ovs_assert(find_ufid_dpdk_flow_mapping(ufid) == NULL);\n> > +\n> > + data->ufid = *ufid;\n> > + data->rte_flow = rte_flow;\n> > +\n> > + ovs_mutex_lock(&ufid_lock);\n> > + hmap_insert(&ufid_dpdk_flow, &data->node, hash);\n> > + ovs_mutex_unlock(&ufid_lock);\n> > +}\n> \n> I am not sure that the locking in the add and del functions above can't\n> race. f.e. can two deletion requests for the same flow occur in parallel?\n\nSeems no to me, but I'm also note sure about that. If that's concerned,\nI could add a locked version of find: find_ufid_dpdk_flow_mapping_locked.\nGood to you?\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=\"vl0wUIxI\"; 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 3xrH4369Yqz9s7f\n\tfor ;\n\tMon, 11 Sep 2017 15:46:43 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id ABF6CA84;\n\tMon, 11 Sep 2017 05:46: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 F3081A6E\n\tfor ; Mon, 11 Sep 2017 05:46:38 +0000 (UTC)","from mail-pg0-f49.google.com (mail-pg0-f49.google.com\n\t[74.125.83.49])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 10A12A4\n\tfor ; Mon, 11 Sep 2017 05:46:37 +0000 (UTC)","by mail-pg0-f49.google.com with SMTP id i130so6278770pgc.3\n\tfor ; Sun, 10 Sep 2017 22:46:37 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\tq67sm14193237pfg.37.2017.09.10.22.46.34\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tSun, 10 Sep 2017 22:46: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:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=zw1DYyNt+d/9SK3aaZ/3FNoEXL51BLiSqNoLKS3Nlxw=;\n\tb=vl0wUIxIV0Yxd/GI/CLFzVmqVWwHC2fCswIn56jmIKftsVZSY0Qroro8Tcqwda/lot\n\tAjGx/s2A5ZxuL3YRUR5ggJpIG8/JGBweUK5oUftFGcfBqIAxQYTVPaoOQD7oIGkEesQs\n\tb4LB04hQulK5twkGCjpfVnCg95Ep5fDqkWo4B1qcNfdJoxISzmdb41Zr3pMamcJ5QYld\n\tX/wDxPCYCkiIdujK1Px6mrycm1JPHIZyIJ2P0Fzux3ONpoKBWSzF8TSWNVQmlTXRgxzR\n\t2kSQmTMPxrXVkwbqBIIle0yX1N8YwbeNC2Zvh6kovRSQepvv+Z2qLGa3dBPLpRLsm8pe\n\tCKKQ==","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:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=zw1DYyNt+d/9SK3aaZ/3FNoEXL51BLiSqNoLKS3Nlxw=;\n\tb=JofCa3ch6Xj4BoNZAoU4BeHy4Wzx0X7jkdaFfv9jtH9cSH4Og9DyqdIPEbZhhNq7hm\n\twi1I69Kr/BszwB3eYS6ELqOPT5j1C3nJNXP8BGZDhRPK4QDzGUJ2FKtLKL+6s19PeffC\n\t8FGQR7zw/1H+pzAziwPIejPgt5Lvx/5Mtx9pw5tnqWhCQ+T6Z0UfsO0h3Bv9FaRz7S+h\n\tJcG5ixCTOiRoaAEDnAnB0T7qxTlVTUTw0wkV0VdmGcfBnXtzi84bY44f+6PB7nNw0CzE\n\tVPcm/dAm6dS/+BoyFV6URIGoes/eXl9/cOuZs8V91/mV/mPUHXAjoSWf0IuXb7FnGr1g\n\tTOWg==","X-Gm-Message-State":"AHPjjUio8IA1QvYCJ0Ir3e/2PThFfslyvsKLNxzVZJgr+YYNcoJvULGh\n\t0Rn/XIu+9FYJPI1t","X-Google-Smtp-Source":"ADKCNb75Z23h1BXPEoHH5hx7FMZwKScH8Of5J+9QtwiLk5bA7+JzByGonwExwrFS+k+nzwgoEH0abA==","X-Received":"by 10.98.29.74 with SMTP id d71mr11047500pfd.141.1505108797653; \n\tSun, 10 Sep 2017 22:46:37 -0700 (PDT)","Date":"Mon, 11 Sep 2017 13:46:28 +0800","From":"Yuanhan Liu ","To":"Darrell Ball ","Message-ID":"<20170911054628.GL9736@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-4-git-send-email-yliu@fridaylinux.org>\n\t<20170908164621.GC7356@vergenet.net>\n\t<1314483A-1977-434B-BCF3-0522DFEA56C8@vmware.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1314483A-1977-434B-BCF3-0522DFEA56C8@vmware.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","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\" ,\n\tSimon Horman ","Subject":"Re: [ovs-dev] [PATCH v2 3/8] netdev-dpdk: convert ufid to dpdk flow","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1767737,"web_url":"http://patchwork.ozlabs.org/comment/1767737/","msgid":"<20170913095233.GD27555@vergenet.net>","list_archive_url":null,"date":"2017-09-13T09:52:34","subject":"Re: [ovs-dev] [PATCH v2 3/8] netdev-dpdk: convert ufid to dpdk flow","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Mon, Sep 11, 2017 at 01:46:28PM +0800, Yuanhan Liu wrote:\n> On Fri, Sep 08, 2017 at 05:48:36PM +0000, Darrell Ball wrote:\n> > \n> > > +static inline void\n> > > +del_ufid_dpdk_flow_mapping(const ovs_u128 *ufid)\n> > > +{\n> > > + struct ufid_dpdk_flow_data *data;\n> > > +\n> > > + data = find_ufid_dpdk_flow_mapping(ufid);\n> > > + if (data) {\n> > > + ovs_mutex_lock(&ufid_lock);\n> > > + hmap_remove(&ufid_dpdk_flow, &data->node);\n> > > + free(data);\n> > > + ovs_mutex_unlock(&ufid_lock);\n> > > + }\n> > \n> > I think it would be nicer to exit early:\n> > \n> > + if (!data) {\n> > + return;\n> > + }\n> > +\n> > + ovs_mutex_lock(&ufid_lock);\n> > + hmap_remove(&ufid_dpdk_flow, &data->node);\n> > + free(data);\n> > + ovs_mutex_unlock(&ufid_lock);\n> > \n> > \n> > [Darrell] It is a minor point, but the preference is typically given to the affirmative\n> > condition check. A slight variation of Yuanhan’s version would be:\n> > \n> > + data = find_ufid_dpdk_flow_mapping(ufid);\n> > + if (OVS_LIKELY(data)) {\n> > + ovs_mutex_lock(&ufid_lock);\n> > + hmap_remove(&ufid_dpdk_flow, &data->node);\n> > + free(data);\n> > + ovs_mutex_unlock(&ufid_lock);\n> > + } else {\n> > + VLOG_WARN….. <<<<< since this is probably unexpected\n> > + }\n> > \n> > //////////////////////////\n> \n> I could do that.\n> \n> > > +}\n> > > +\n> > > +/* Add ufid to dpdk_flow mapping */\n> > > +static inline void\n> > > +add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow)\n> > > +{\n> > > + size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);\n> > > + struct ufid_dpdk_flow_data *data = xzalloc(sizeof(*data));\n> > > +\n> > > + /*\n> > > + * We should not simply overwrite an existing rte flow.\n> > > + * We should have deleted it first before re-adding it.\n> > > + * Thus, if following assert triggers, something is wrong:\n> > > + * the rte_flow is not destroyed.\n> > > + */\n> > > + ovs_assert(find_ufid_dpdk_flow_mapping(ufid) == NULL);\n> > > +\n> > > + data->ufid = *ufid;\n> > > + data->rte_flow = rte_flow;\n> > > +\n> > > + ovs_mutex_lock(&ufid_lock);\n> > > + hmap_insert(&ufid_dpdk_flow, &data->node, hash);\n> > > + ovs_mutex_unlock(&ufid_lock);\n> > > +}\n> > \n> > I am not sure that the locking in the add and del functions above can't\n> > race. f.e. can two deletion requests for the same flow occur in parallel?\n> \n> Seems no to me, but I'm also note sure about that. If that's concerned,\n> I could add a locked version of find: find_ufid_dpdk_flow_mapping_locked.\n> Good to you?\n\nI see that access to ufid_dpdk_flow is protected by ufid_lock, but I'm\nstill not sure how one protects against concurrent access to an\nelement of the hash.\n\nf.e.:\n\n* Two concurrent deletes of the same element\n* A concurrent get and delete of the same element\n\nIts also not clear to me that adding the same id twice is guarded against.\n\nPerhaps the use of this API makes my concerns moot as callers\nnever manipulate the same ufid concurrently.","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=\"m7NEKQTR\"; 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 3xscQx3Nh7z9s76\n\tfor ;\n\tWed, 13 Sep 2017 19:52:40 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id EC0E5BBD;\n\tWed, 13 Sep 2017 09:52: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 AB5CEB93\n\tfor ; Wed, 13 Sep 2017 09:52:37 +0000 (UTC)","from mail-wm0-f42.google.com (mail-wm0-f42.google.com\n\t[74.125.82.42])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id EF101E0\n\tfor ; Wed, 13 Sep 2017 09:52:36 +0000 (UTC)","by mail-wm0-f42.google.com with SMTP id g206so2346121wme.0\n\tfor ; Wed, 13 Sep 2017 02:52:36 -0700 (PDT)","from vergenet.net (reginn.horms.nl.\n\t[2001:470:7eb3:403:d63d:7eff:fe99:ac9d])\n\tby smtp.gmail.com with ESMTPSA id\n\ti6sm6068656edk.45.2017.09.13.02.52.34\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 13 Sep 2017 02:52:35 -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:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=/LB3ublL/zgfsOrt1E3Ccvu4ougJzYznWVAN1GPNfyo=;\n\tb=m7NEKQTR0MfcS0X5V9JOD/TAcAAcHunlz0BAu6MQxmDCXBqvXuNkMvlb2tddvB1ati\n\tSjY3rt/oUxrcfeqYN4MWTdllLedC159JJlaz15+4WT4cBBv8GP27ZA43oY8XCOqmTgUE\n\tZwoaAhjaAN0N9hWiXSIOVfgQHm3mFVypBF5LPDAiZQn35dDZrSmt4h65nCOQLXutZCFm\n\trW8ZUs5AITX9GkTzpvXvUt9qlUKBY5buDVYX1/XiYlvK4HemHNRAc3eJLyZyjnT5pYc4\n\tYAx3z2PZq+6OGYF+vYCrO7kRESrSQj5cjUre+a6kaU3wpIzpgkGI0RZ7PN924SAJZOx4\n\tApsQ==","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:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=/LB3ublL/zgfsOrt1E3Ccvu4ougJzYznWVAN1GPNfyo=;\n\tb=fsHgr1dNBkR1LH/pC047gYuJEikCQGGfFIa5ULJ700zXSckVlZAMJ8f3Aey90o4jaC\n\tFgkCAxgU1X0pGk4um48+QeNIL+IDhOyd7DDQJo5KTQvjEDLhhaS6eax+ntdyzerxKAgu\n\t9QjBwWNeKh/MSSU4z6vXdbfrLNNUmT7m6klXUNWWLhgCbSFrxtZAStO6cjZLviCVg1S7\n\tDDQAe6I9gCg5cYZXf6uYgHbNVV4gMZdqebDi1ADZnSvhrdSSwaYQbQS5IpDpkSGUWWgR\n\tz61ePEgTNhunbWuNTgggIBGjG+mkwEIYVb94QzWpCLnFgvniIAftFmJMjv15HsE2HzuP\n\tTmVw==","X-Gm-Message-State":"AHPjjUjQckE8q1p+Rap896RT+3we7lgeWl4jliJKzasjYAeokDZPfCZF\n\tB5004kQmEWS02xFW","X-Google-Smtp-Source":"ADKCNb734lilZmmI2H7QhwHSwneXsoobdnudofySzlRRjaRGrhdrxSuTnBHA2uKxcuxcLLqb7UwRiw==","X-Received":"by 10.80.135.157 with SMTP id a29mr7364407eda.112.1505296355581; \n\tWed, 13 Sep 2017 02:52:35 -0700 (PDT)","Date":"Wed, 13 Sep 2017 11:52:34 +0200","From":"Simon Horman ","To":"Yuanhan Liu ","Message-ID":"<20170913095233.GD27555@vergenet.net>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-4-git-send-email-yliu@fridaylinux.org>\n\t<20170908164621.GC7356@vergenet.net>\n\t<1314483A-1977-434B-BCF3-0522DFEA56C8@vmware.com>\n\t<20170911054628.GL9736@yliu-home>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170911054628.GL9736@yliu-home>","User-Agent":"Mutt/1.5.23 (2014-03-12)","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 3/8] netdev-dpdk: convert ufid to dpdk flow","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1769086,"web_url":"http://patchwork.ozlabs.org/comment/1769086/","msgid":"<20170915094436.GM2050@yliu-home>","list_archive_url":null,"date":"2017-09-15T09:44:36","subject":"Re: [ovs-dev] [PATCH v2 3/8] netdev-dpdk: convert ufid to dpdk flow","submitter":{"id":72215,"url":"http://patchwork.ozlabs.org/api/people/72215/","name":"Yuanhan Liu","email":"yliu@fridaylinux.org"},"content":"On Wed, Sep 13, 2017 at 11:52:34AM +0200, Simon Horman wrote:\n> > > > +/* Add ufid to dpdk_flow mapping */\n> > > > +static inline void\n> > > > +add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow)\n> > > > +{\n> > > > + size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);\n> > > > + struct ufid_dpdk_flow_data *data = xzalloc(sizeof(*data));\n> > > > +\n> > > > + /*\n> > > > + * We should not simply overwrite an existing rte flow.\n> > > > + * We should have deleted it first before re-adding it.\n> > > > + * Thus, if following assert triggers, something is wrong:\n> > > > + * the rte_flow is not destroyed.\n> > > > + */\n> > > > + ovs_assert(find_ufid_dpdk_flow_mapping(ufid) == NULL);\n> > > > +\n> > > > + data->ufid = *ufid;\n> > > > + data->rte_flow = rte_flow;\n> > > > +\n> > > > + ovs_mutex_lock(&ufid_lock);\n> > > > + hmap_insert(&ufid_dpdk_flow, &data->node, hash);\n> > > > + ovs_mutex_unlock(&ufid_lock);\n> > > > +}\n> > > \n> > > I am not sure that the locking in the add and del functions above can't\n> > > race. f.e. can two deletion requests for the same flow occur in parallel?\n> > \n> > Seems no to me, but I'm also note sure about that. If that's concerned,\n> > I could add a locked version of find: find_ufid_dpdk_flow_mapping_locked.\n> > Good to you?\n> \n> I see that access to ufid_dpdk_flow is protected by ufid_lock, but I'm\n> still not sure how one protects against concurrent access to an\n> element of the hash.\n> \n> f.e.:\n> \n> * Two concurrent deletes of the same element\n> * A concurrent get and delete of the same element\n> \n> Its also not clear to me that adding the same id twice is guarded against.\n> \n> Perhaps the use of this API makes my concerns moot as callers\n> never manipulate the same ufid concurrently.\n\nAs Darrell pointed out (privately), they are protected by the flow_mutex\nlock, that I thin this will not happen?\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=\"wz8QKWel\"; 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 3xtr8x0jlpz9s7c\n\tfor ;\n\tFri, 15 Sep 2017 19:44:48 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 92E89A67;\n\tFri, 15 Sep 2017 09:44:45 +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 625AFA59\n\tfor ; Fri, 15 Sep 2017 09:44:44 +0000 (UTC)","from mail-pf0-f173.google.com (mail-pf0-f173.google.com\n\t[209.85.192.173])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7133A1E8\n\tfor ; Fri, 15 Sep 2017 09:44:43 +0000 (UTC)","by mail-pf0-f173.google.com with SMTP id e1so1189817pfk.1\n\tfor ; Fri, 15 Sep 2017 02:44:43 -0700 (PDT)","from yliu-home ([45.63.61.64]) by smtp.gmail.com with ESMTPSA id\n\td8sm1669895pfh.159.2017.09.15.02.44.40\n\t(version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tFri, 15 Sep 2017 02:44:42 -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=3t9wirlW490usjc3kHLpFoZWiBdnFkp1cumSwjxjbTI=;\n\tb=wz8QKWelgRednIbm/KOxBY0APmO8l8xsu95he6uZvzP2MpdAbAM0ZDoRBpHT/Kh/sZ\n\tkEe6NiSGKJq7Tizvet6lIldjYniOxpqRt4fOuGPBRWshdUY2djK69tKCtYrmqhHMMQ+i\n\tF28qrsSsZMUvsMIz15R5YcZRmHVhNkF+nXujmlp+iVVJ7gWG579o5zIBgGjydOmZH9Mn\n\thJZjc5+KqzeO+aLPyyroXGEx8MiP1Q7WcYzBMIWLquLk56NLitlwgJIBA4cnevv93de0\n\tr9kkr1csn2sqBhTZvFZZ1NimQuzrGIjIhdmmO7rGb/deomLdNam5V4jdcEF9dCLOCxQG\n\tkHPA==","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=3t9wirlW490usjc3kHLpFoZWiBdnFkp1cumSwjxjbTI=;\n\tb=Sb35F5oyQO3i2c5C2XVq2PNBRlhI7HJMojR7oM1GHQJNfigyiMLygEFr2f52hW3ueH\n\tLgwFkBZ7fXNDRL4/Ga/HwzePnvMftufOj8RmBSUWHclbyzQS9jUUhvlAVB0Zm1oQz8f3\n\tVh86YUNQkKmEzaKl2hd4QvMNuNEN+VAvMJ5t+iZN/QL1n97DvyFJyYfdyYKZRWpuVOli\n\t4L8NUgE/1dQi0e1Cg1vPLwYdLPgqQDL4g19w8rEsa+Z1ripED48pSE6gEzzsGhHPoBgV\n\tV/s0atQg4vmKEJFcwRDJ4wA8pjH/swgggmb50koEf27sOQ+CSEu39vo43XhVfDEl1qRJ\n\tADmg==","X-Gm-Message-State":"AHPjjUjm0qlTxYDxlnuTkaSEEL6PwOWVBKSXUc0DU7OODKZq96e8AYxf\n\tMnZeFCwuDcbXpnyF6i1zDA==","X-Google-Smtp-Source":"ADKCNb63xyq1gCeDZseUXXH3rOazEMOYP198sO7YK09yij52DQ4v4bq4YlLverQHh4jKJPbM+uqP1A==","X-Received":"by 10.84.215.9 with SMTP id k9mr27204614pli.25.1505468683118;\n\tFri, 15 Sep 2017 02:44:43 -0700 (PDT)","Date":"Fri, 15 Sep 2017 17:44:36 +0800","From":"Yuanhan Liu ","To":"Simon Horman ","Message-ID":"<20170915094436.GM2050@yliu-home>","References":"<1504603381-30071-1-git-send-email-yliu@fridaylinux.org>\n\t<1504603381-30071-4-git-send-email-yliu@fridaylinux.org>\n\t<20170908164621.GC7356@vergenet.net>\n\t<1314483A-1977-434B-BCF3-0522DFEA56C8@vmware.com>\n\t<20170911054628.GL9736@yliu-home>\n\t<20170913095233.GD27555@vergenet.net>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170913095233.GD27555@vergenet.net>","User-Agent":"Mutt/1.5.24 (2015-08-30)","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 3/8] netdev-dpdk: convert ufid to dpdk flow","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"}}]