Message ID | 1517327640-182072-1-git-send-email-motih@mellanox.com (mailing list archive) |
---|---|
State | Rejected, archived |
Delegated to: | Shahaf Shuler |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5D7201B663; Tue, 30 Jan 2018 16:54:23 +0100 (CET) Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20072.outbound.protection.outlook.com [40.107.2.72]) by dpdk.org (Postfix) with ESMTP id 66AAB1B656; Tue, 30 Jan 2018 16:54:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=hBq2VfTWsqnY5ZP8UOz0ETJW7pS6XrKpT6JeaOPoNIE=; b=CJ+FTdg0buqLDhiDU5oAcafNb0MtNc8wE3Hbg6OAVkEXcCmLFJKTrJqf0PlUnNsM/evT5Koj0cuSlm/SvFVk77kdro8OsL2qYyBj2oL2pxRPHCAWcZ+CmIQecXqQgnitc9iXuf6fQQBtsHUzd0PN6GVt0Av8JII5dmk5Xj3LSF4= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=motih@mellanox.com; Received: from mellanox.com (37.142.13.130) by HE1PR05MB3211.eurprd05.prod.outlook.com (2603:10a6:7:36::33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.444.14; Tue, 30 Jan 2018 15:54:18 +0000 From: Moti Haimovsky <motih@mellanox.com> To: adrien.mazarguil@6wind.com Cc: dev@dpdk.org, Moti Haimovsky <motih@mellanox.com>, stable@dpdk.org Date: Tue, 30 Jan 2018 17:54:00 +0200 Message-Id: <1517327640-182072-1-git-send-email-motih@mellanox.com> X-Mailer: git-send-email 1.8.3.1 MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [37.142.13.130] X-ClientProxiedBy: HE1PR0102CA0054.eurprd01.prod.exchangelabs.com (2603:10a6:7:7d::31) To HE1PR05MB3211.eurprd05.prod.outlook.com (2603:10a6:7:36::33) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 459b5f05-fddd-41a1-7069-08d567f9b963 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(4534165)(4627221)(201703031133081)(201702281549075)(48565401081)(5600026)(4604075)(2017052603307)(7153060)(7193020); SRVR:HE1PR05MB3211; X-Microsoft-Exchange-Diagnostics: 1; HE1PR05MB3211; 3:UFZxbK3gBDg1xPwglUkivAPck8oxFXFIq2q9Ng/v2QQ/F4+HU686dgKu+M6HJpvKQPPh8mEUJKlTpH6aQXERS5eI6IgAAhHXfomin4oUGUjktlHqQFgTwEt2VZc9rRyaa2GBn4it7j/NXtzZQaQUZRHjMxcDd0BZzDzhNHZzUNzCQ5f175j8l9I83SAoILU6CYZuVu7j40DH7dWB6A+owoRKAu2elTqXzckLkkRFkAfjhhePBU8TMk0l7CIJju51; 25:VjFSeH4plgMgkNA42kge2nc7T+f2iOnsyjNV+JH7EvjkSrXhiKr4ORLGx2w6L8wzTXMO6Fe+uQZa0XH1piEAZLqUTuBH0aQmFKvGEiFL9ZeZxm7ZEPuObGvPkUvWtHeb8vT9j0Q/7t7Z58A2zHcblO2Au0yFeU/jh/862CiHMAUL0J/pYl91nprB2xFjbnWqm/TUKcgX8df14oklefrlrqLhZOiDUcpYUadDNPQfpkJeSTa6xMO+XU0nMzUIgjxAbFYVytyeoseZZvgu1eBnU9Ws2HAh99TDV294XBMZYv3arJLP/vnd1VOISpbPx+R14hz5AUv+Ks6Vwnnl97LTRw==; 31:KsxtiaNVoxwlAJqVCAKBhGbic7nMkWn4bdhBcdecgMI2Gfi2bXLEtoEWX7P0sq2TmHWelMbUGzwWBdqRRc3bXqh/7GS8KqdcLfpSfattbEbemGXasoFuV/75Wy6m2qXGqhj6aIcb8995YwQuW29QqghfnaEdQbb611ZE4sBt5XnPJYxS8+fVmMnAjev5I6LVvRI22CDCCK1fGqigdt95zuuNCN6l9NDO0ce+25zQ8tw= X-MS-TrafficTypeDiagnostic: HE1PR05MB3211: X-LD-Processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr X-Microsoft-Exchange-Diagnostics: 1; HE1PR05MB3211; 20:TKF+PZhTy+fKP7hp1ocyKlhfNDtkk6AQN81kbew13rngFNFy1G2hWzn6GAMQvOUxaRaItHd3xBEx2qkkCxZQRZw0KtEpJr21pnoNcjPtC6cbEueXxouWd7zqTVgozcEop/cneXf7FRQ0vABu5Dl9yf+BVL/hFlKy1vBhb2AzBY1mHVeDx8TzR9/wEDbjoPaO3LXxC3kgDkAa/f6gOcDZ5c3wLIu/w2m65cHEMpQQzLN2YCnIUT7qGn1tdwTrZwl6uVxynGdeYDYNQJIFPLQLPgD6+tjK0qgtgeny+4bCpXJ1pM1PzErBQn0a979loODQrydXEGfCPMX7uwW8Hru/B1qc6Tf2nbcoh5LZOAXTBIU4DYg3i/3x++ptuntk3AVUCXaACvkY3CVCDfcDlM4AVcYy7CPqBebONJF+4e27R36RbGoIb7AFGLExAxLtwrdLchYFGFVx/t4THerhfJSc/Wiv52RuV4x945ouRKxKaGMyiwyfajK7uzMbL5YuXqpP; 4:xq9pZM1Eq5b6X666MU8+vJz3OXWfgG+NjF3IoSDy2FD6yvzkOMoKyLbjxn1N81BqjUU2o/84g1li0i2LHcYeR+ltEkG5kO5q2lfBz8eSDIIvWJKGy3kUwyp9ZkhOLtA1dJQTucdRcoqwD9QOpAg+TRGQGeOXbIHjG66P/H5ZQj/60GNCBEfIvC6yFlPPx0/WkjzELYQHpMjepPe7ifKVzXPL+CPLnp9PUab7QWt105KgS/YkNHckBzx0HSOMHd3nouO4q625EoMo5K3sqIVZ0g== X-Microsoft-Antispam-PRVS: <HE1PR05MB3211A1C1B08744DE2D567909D2E40@HE1PR05MB3211.eurprd05.prod.outlook.com> X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040501)(2401047)(8121501046)(5005006)(10201501046)(3231101)(944501161)(3002001)(93006095)(93001095)(6055026)(6041288)(20161123564045)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(20161123558120)(6072148)(201708071742011); SRVR:HE1PR05MB3211; BCL:0; PCL:0; RULEID:; SRVR:HE1PR05MB3211; X-Forefront-PRVS: 0568F32D91 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(396003)(346002)(376002)(39860400002)(39380400002)(366004)(189003)(199004)(186003)(16526019)(97736004)(2361001)(53936002)(50466002)(386003)(51416003)(48376002)(26005)(33026002)(105586002)(69596002)(52116002)(7696005)(47776003)(55016002)(316002)(66066001)(86362001)(16586007)(81166006)(68736007)(5660300001)(25786009)(4326008)(50226002)(2351001)(478600001)(106356001)(8676002)(21086003)(36756003)(8936002)(6666003)(7736002)(2906002)(4720700003)(3846002)(81156014)(6116002)(305945005)(6916009); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR05MB3211; H:mellanox.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; Received-SPF: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; HE1PR05MB3211; 23:LUs024nlWD9Q5KuPV4fAe4s11petK0kLRnVr9yQU0?= c8wGYIjXpwcgGoUfoo0PyC+6irMwCkg9ctohTIbvuuczD29UJY0t/0ehCtUGHZ4gzIF3BAGWyw4mzSkm8oOS1sqzg7ZpViFCoVhlxO4Iwky0UxdXm187vSLro4FI6aN3MXotbKTuP3yC4EfjhAX7BWNHgJuVX6knn7u+Dk5Y7mPXQ3g4Rb4POEBUXJUnXDeHZ6D6VcGEIKWCmUYvY8omssR+CaGAfAt+4sFXo3cvKBysapJj4W6K+opGUL3W5UbhUsOB7/8ZkZw7ZE/59SxlpMbh+EebwR7LU6xZ5OijuLMKPU7hAbCiquqVimKqG5u6ToFbjgkbLqFTLPvOULKkWNG7gDK5DKH6EWZLNIrvhAXee1HnUy56ZtTF6Yy2qvvlquNZpdyHMkO9q3v+9FyiEK25ozWlttRv+GUpNxU9/ZNrIEQKqnxKkWJBWUyHpJ4uuVcG9UjThdg2iwQvZWz95QjvzufBznXF+A0xv6P3eonfctLuLMl9DP/a3tnaUCXwjUrrYOiVmaDz6eRTN/ucBb0zmdWo0VxD2taiWUor8y3SdAYakylSGf5fLMgZE+k8XAki/BbQA5qkHKNN/OcTWkBuzU8T3jAVVswGiyb2ApBM2ZHDXycDpXVe58QuBWrbTl1IDPqKxEP9LbZqu7IO3sl5dN4mUrG4oeR222K3Y3UQhsVqbtZTKDQ3mILgS2yuAnHrQzVkGvg09Nghue0pGrfS38rghe4uNYj0stRdSpCYOoveXJ8IUus6MFybbvaL3sntddIlwowMyMpepvlPfOYY+IZGu1bJt+mkOuBzLGYua2l0CdlGqiyYp579lPKsF36kQdB1Gz2VR7wlj0dtF8hnic9uq6CUyG7dnLZhXuVDsHXAZjSpU6YQyv712JG8PAr1YO3OQwJHh8uU6VSMpRzjPCq+QJEurskcQgVbF9kxrNgsfUv/ex+ZmD+np40G7dhekweS4CGOPIJldtILO+U4B9BYSLIgNvHoiMe+8QK4QGNKsUoJF0PCa3dVL/D7Lvb1rZpsVvro9imksohpyzvjzPBJc/lfXj1QrhZBpUPHhWkHkvn8WruolJEfTj3yVXIArE9b0QDgJsSnt84NCfM7arg5Du5ifHToWih2sWOuw== X-Microsoft-Exchange-Diagnostics: 1; HE1PR05MB3211; 6:SI9foBWLu8n/8zAuMpaRh8CkxHXKhmaEah6oBwgX0HFpFimWhEejq//cTpzK6R48wssGHTsFO18fX6YY04ULdfjKYCOZr1hpTvUDcIAA/VIFnmpjZBjpBnnYdrLtt+eNio5wBQtWShaXF/Da5Gvh2DF8QoPXigxBfvRBhxKjSTaE7KgY/zzWrQxqeeW40oKZs0tr24Gci92Zhy3fNeqKJtGsoCXM9yMUQqd7XVk7OQ1aOlNoqkSI2z2R1u/2x/kF4tTdlOEHD2DAFrj3/FsDMOH+l+OHMnfWG9EiY1gODoNv102ja80Kw4/GZDQ14WD1zdR6F/w0A1QHayQmWrBNf/7yRjxFUQRhMEBogwlJQGA=; 5:IqQxroSVfluoyE8i87okwqlyucNxGBF2G6nsXUkKGgoAtcG8tWWNm3bQ1uY04shKcAym1BpNQEmQrCgSLGg/rDDW0B7DEGyqp1WoXJcdvrfUvDNv6giZ2Ilvxsy/LtwZU75yJaqZiJUj9qSiX9a2Ml0FnNbjYANIWXyQipFcBGw=; 24:iFun/qtbaiAqGP+qviZHrkGEWhO+Nq60A+mBD0pD+pI29uE+LxIfJhVsf+OEnyP1dlBrZSbcrg23C+UjSded7VkayIfFj9+UjX9OuzQRxac=; 7:63Rm4Dwn9BvEQ9mm6okgdT1I31ah91bRIeQCTQhtXsz1HhdN9tFBqPqA3s0LmUuZWcVy5k/SkIRtf8EE41IpiFfV1eXJpFht7PPIcRPdEIT18oNJL0yxtEPT/oxvZoB21DqtKdVPAad3pXDBE7nm4vAJ3OXtALgaedaXa/SQS6mcTiv7sEpHLpv4355clSeJZiW7cu0xd+eXgOmkETiB3EF+KcOrq+gAqu0778MUO8C1rFxSJwDTc1xavT+pdAQ8 SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2018 15:54:18.5737 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 459b5f05-fddd-41a1-7069-08d567f9b963 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR05MB3211 Subject: [dpdk-dev] [PATCH] net/mlx4: fix drop flow resources not freed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Checks
Context | Check | Description |
---|---|---|
ci/checkpatch | success | coding style OK |
ci/Intel-compilation | success | Compilation OK |
Commit Message
Moti Haimovsky
Jan. 30, 2018, 3:54 p.m. UTC
This patch fixes the drop-flow resources not being freed when the device
is closed.
Issue can be observed when running testpmd and adding the following rule
more than once:
"flow create 0 ingress pattern eth / end actions drop / end"
then either exiting testpmd using the "quit" command or by running the
command: "port stop all"
Fixes: d3a7e09234e4 ("net/mlx4: allocate drop flow resources on demand")
Cc: stable@dpdk.org
Signed-off-by: Moti Haimovsky <motih@mellanox.com>
---
drivers/net/mlx4/mlx4_flow.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
Comments
Hi Moti, On Tue, Jan 30, 2018 at 05:54:00PM +0200, Moti Haimovsky wrote: > This patch fixes the drop-flow resources not being freed when the device > is closed. > Issue can be observed when running testpmd and adding the following rule > more than once: > "flow create 0 ingress pattern eth / end actions drop / end" > then either exiting testpmd using the "quit" command or by running the > command: "port stop all" > > Fixes: d3a7e09234e4 ("net/mlx4: allocate drop flow resources on demand") > Cc: stable@dpdk.org > > Signed-off-by: Moti Haimovsky <motih@mellanox.com> Thanks for investigating this problem, however I do not think the proposed patch uses the right approach to address it, more below. > --- > drivers/net/mlx4/mlx4_flow.c | 33 +++++++++++++++++++++++++++++---- > 1 file changed, 29 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c > index fb84060..9e6d8dc 100644 > --- a/drivers/net/mlx4/mlx4_flow.c > +++ b/drivers/net/mlx4/mlx4_flow.c > @@ -895,6 +895,30 @@ struct mlx4_drop { > } > > /** > + * Return the number of active drop flow rules currently present > + * in the list of flows. > + * Active flow is defined as a flow associated with an ibv_flow. > + * > + * @param priv > + * Pointer to private structure. > + * > + * @return > + * Number of active drop-flows. > + */ > +static int > +drop_refcnt(struct priv *priv) > +{ > + struct rte_flow *flow; > + int count = 0; > + > + LIST_FOREACH(flow, &priv->flows, next) { > + if (flow->drop && flow->ibv_flow) > + count++; > + } > + return count; > +} > + > +/** > * Get a drop flow rule resources instance. > * > * @param priv > @@ -910,9 +934,8 @@ struct mlx4_drop { > struct mlx4_drop *drop = priv->drop; > > if (drop) { > - assert(drop->refcnt); > + assert(drop_refcnt(priv)); > assert(drop->priv == priv); > - ++drop->refcnt; > return drop; > } > drop = rte_malloc(__func__, sizeof(*drop), 0); > @@ -955,8 +978,10 @@ struct mlx4_drop { > static void > mlx4_drop_put(struct mlx4_drop *drop) > { > - assert(drop->refcnt); > - if (--drop->refcnt) > + int refcnt = drop_refcnt(drop->priv); > + > + assert(refcnt >= 0); > + if (refcnt) > return; > drop->priv->drop = NULL; > claim_zero(ibv_destroy_qp(drop->qp)); It looks like brute force to me, as in "if the counter doesn't have the right value at this point, decrement it until it does, then assert() will finally shut up". Getting rid of the refcount altogether would have also worked. We need to find out why we do not end up with a number of mlx5_drop_put() calls matching that of mlx5_drop_get(). One is likely missing somewhere. I'll have a look as well.
On Tue, Jan 30, 2018 at 05:41:07PM +0100, Adrien Mazarguil wrote: > Hi Moti, > > On Tue, Jan 30, 2018 at 05:54:00PM +0200, Moti Haimovsky wrote: > > This patch fixes the drop-flow resources not being freed when the device > > is closed. > > Issue can be observed when running testpmd and adding the following rule > > more than once: > > "flow create 0 ingress pattern eth / end actions drop / end" > > then either exiting testpmd using the "quit" command or by running the > > command: "port stop all" > > > > Fixes: d3a7e09234e4 ("net/mlx4: allocate drop flow resources on demand") > > Cc: stable@dpdk.org > > > > Signed-off-by: Moti Haimovsky <motih@mellanox.com> > > Thanks for investigating this problem, however I do not think the proposed > patch uses the right approach to address it, more below. <snip> > We need to find out why we do not end up with a number of mlx5_drop_put() > calls matching that of mlx5_drop_get(). One is likely missing somewhere. > I'll have a look as well. After investigation, the following change in mlx4_flow_toggle() should do the trick: if (flow->drop) { + if (flow->ibv_flow) + return 0; mlx4_drop_get(priv); Without this, an already-enabled drop flow rule takes another reference when re-enabled, hence the issue. I can send a fix tomorrow.
diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c index fb84060..9e6d8dc 100644 --- a/drivers/net/mlx4/mlx4_flow.c +++ b/drivers/net/mlx4/mlx4_flow.c @@ -895,6 +895,30 @@ struct mlx4_drop { } /** + * Return the number of active drop flow rules currently present + * in the list of flows. + * Active flow is defined as a flow associated with an ibv_flow. + * + * @param priv + * Pointer to private structure. + * + * @return + * Number of active drop-flows. + */ +static int +drop_refcnt(struct priv *priv) +{ + struct rte_flow *flow; + int count = 0; + + LIST_FOREACH(flow, &priv->flows, next) { + if (flow->drop && flow->ibv_flow) + count++; + } + return count; +} + +/** * Get a drop flow rule resources instance. * * @param priv @@ -910,9 +934,8 @@ struct mlx4_drop { struct mlx4_drop *drop = priv->drop; if (drop) { - assert(drop->refcnt); + assert(drop_refcnt(priv)); assert(drop->priv == priv); - ++drop->refcnt; return drop; } drop = rte_malloc(__func__, sizeof(*drop), 0); @@ -955,8 +978,10 @@ struct mlx4_drop { static void mlx4_drop_put(struct mlx4_drop *drop) { - assert(drop->refcnt); - if (--drop->refcnt) + int refcnt = drop_refcnt(drop->priv); + + assert(refcnt >= 0); + if (refcnt) return; drop->priv->drop = NULL; claim_zero(ibv_destroy_qp(drop->qp));