From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f67.google.com (mail-wm0-f67.google.com [74.125.82.67]) by dpdk.org (Postfix) with ESMTP id 2900F1B65A for ; Tue, 30 Jan 2018 17:41:21 +0100 (CET) Received: by mail-wm0-f67.google.com with SMTP id v71so2440995wmv.2 for ; Tue, 30 Jan 2018 08:41:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=+/qFnvcRUFLj7n/8X8tRv3JKgg2g1tEKGmHZpG84e5w=; b=udBj+XEE6BkfEaJUUgAvhKJ4MzGVvUDdZJmqtw/tGmn7rrdOOMotbpb/UMwdGb94Rl d5zs9zhFPrsiaatTiBHAQDiwSAwtGb2i6xQGSuE7Mkc+V4wcPiW3Xqf/IM1w7DxDDmxv RgguB6bgpZjqI1yUG9et6He3hn4zmnyrO3fi1MGtZ5Q5Cd1RPBvCZ0luuv0MnPxFLbyi 6rccotTY4kZvvPB/t2Js5A54HPi0Ie2dnB5d3djS2hNz9qf2kjdOTOyJnR6TuKTyJ2mr gP0edTm7L0/h1GliRDcexPP2wPHKDOTstCXq3EaGLX7ApFIWgFomzoYdUZvjYjGx5KBX aspQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=+/qFnvcRUFLj7n/8X8tRv3JKgg2g1tEKGmHZpG84e5w=; b=tmdA90eoLRqTP/mhgHLrmDpbmS5iFOkwCBzzVdH+5S2ywEEaTgkM+aUq2g8odb9QrZ DG2ffEfKxj4OnGuk2nIEVrw7OyGiYcUD9hbD5nxCZ15TvalaeRybRZi0GxCSVHzozeqF zvfEYbHvqtQmrCViArLlwRJckUp66DXYSWgZ0DK7bCRgwnangR/4yxF/QYHnIc5sssAh ICeKuAia3VeWU9WnmMRkJKPzahjOdYLbgGpQSY1vmr6Hw4ZkCWTQa9wi1i/OMDs2lQy+ SRw5NGdZYHdOimwNXrFuZHPfz2+Sh6jUPAJg509lG4AxlrcslYC/6hFSyemP65OllXqp Ks+A== X-Gm-Message-State: AKwxytd2iBmTJJKIHyyDSJlZ5R8ppkEq9hst59ZEUNfB8ATCCK/7WVA7 iKAbPL36DYIV0ZWaUthImXTizg== X-Google-Smtp-Source: AH8x224SWjHKGUxNZNT/xIAp2wSqZotr3OZOYh9J88fU0/1XT7CMgOyDwC8y3xs8vv37jaD4A1szfQ== X-Received: by 10.28.14.129 with SMTP id 123mr15243260wmo.111.1517330480833; Tue, 30 Jan 2018 08:41:20 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id q1sm9647522wrf.40.2018.01.30.08.41.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 30 Jan 2018 08:41:20 -0800 (PST) Date: Tue, 30 Jan 2018 17:41:07 +0100 From: Adrien Mazarguil To: Moti Haimovsky Cc: dev@dpdk.org, stable@dpdk.org Message-ID: <20180130164107.GP4256@6wind.com> References: <1517327640-182072-1-git-send-email-motih@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1517327640-182072-1-git-send-email-motih@mellanox.com> Subject: Re: [dpdk-stable] [PATCH] net/mlx4: fix drop flow resources not freed X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Jan 2018 16:41:21 -0000 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 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. -- Adrien Mazarguil 6WIND