From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f45.google.com (mail-wm0-f45.google.com [74.125.82.45]) by dpdk.org (Postfix) with ESMTP id CF9C61AEF4 for ; Mon, 9 Oct 2017 10:05:32 +0200 (CEST) Received: by mail-wm0-f45.google.com with SMTP id m72so20065286wmc.1 for ; Mon, 09 Oct 2017 01:05:32 -0700 (PDT) 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:content-transfer-encoding:in-reply-to :user-agent; bh=y7OVsVQuZ7/7t3kTRsGypOOBoLPNiw09hIHIMZfmuH8=; b=bWqezjFHfloMAEcbuLoNnmdOS2GaYtmOjAO7yC8+qsfdTbCS1QSfgYAVkb+X4L8oI6 1psILmn6eTOqc2QqhvYJAWWx2a4m0D6AtgN7TKC0H6wwaaQPKaFiky7vUR6aocKEKv+k cihG0IetUeI2ReqVh7bP4VwNoDwlvQi7t1Of0aDrUmz+ZqmjOcBSMzTfdtAXFsjCLe1+ 2RjHY8dWFrLPCkiPDJBjFSyiIIB9GmgJ3ZMoNKHsp3fV02ASttlJfu9CHUM62zBXUlAb cYuzdrz50fW5KqRPMQe4pzw9gxocJyktoI48pVPIRcWwAFBjHzACcIB5H1Y+1fbjRs4R IPHA== 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:content-transfer-encoding :in-reply-to:user-agent; bh=y7OVsVQuZ7/7t3kTRsGypOOBoLPNiw09hIHIMZfmuH8=; b=EYtpARsyY6DhtkbxPo8P4zmgI7Yy85CXYOT+vvvQClpeDb9eJn+uAJwPV1+z7JkGDu cNA0QgTjsymVd1YW15O/JThpnyXii94HIVRPm/jyzxyAHiGDHMIt5kUdZWMum6W3q8/C ISIzrNoXWxSX3ZmrTJQsDO3GvhKOtEsmtl3F5WOIDbNPxeVH5vRyNujjb/r+c9SBCHRt iNQtphAsCiar3R8fTiV7d4FZCPX0Yn6abbJOSHT6mmPxoPuv/4Ga3WKxLgHx3LoMtBFQ Wcdq26t1LrVOSCyJBH+20/znVn8n0qzebbHqmH7VweepKhXP5rRNKpAwhjggJD6M/u6H Y+3A== X-Gm-Message-State: AMCzsaW2pVNyejFS69NJtquLioCIwZ8I3WHl+zXaUpUAJTCaQYtaBdX1 cqT+DeXiBwlu0ARILbKYU/Xl X-Google-Smtp-Source: AOwi7QBuoV6H89sf0KKJlLK4D1+FzANyYqJM0PiJ/klHBvpIXm1cxYRxJ4sH9Sx7lQq1on5v6iS8yw== X-Received: by 10.80.148.37 with SMTP id p34mr3701121eda.254.1507536332520; Mon, 09 Oct 2017 01:05:32 -0700 (PDT) Received: from autoinstall.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 41sm1506883edz.66.2017.10.09.01.05.31 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 09 Oct 2017 01:05:32 -0700 (PDT) Date: Mon, 9 Oct 2017 10:05:21 +0200 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro To: Yongseok Koh Cc: dev@dpdk.org, adrien.mazarguil@6wind.com, ferruh.yigit@intel.com Message-ID: <20171009080521.GB30412@autoinstall.dev.6wind.com> References: <20171006045956.GF19330@yongseok-MBP.local> <20171006070325.GI15330@autoinstall.dev.6wind.com> <20171006225005.GA20117@yongseok-MBP.local> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20171006225005.GA20117@yongseok-MBP.local> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v2 15/30] net/mlx5: add Hash Rx queue object X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Oct 2017 08:05:33 -0000 On Fri, Oct 06, 2017 at 03:50:06PM -0700, Yongseok Koh wrote: > On Fri, Oct 06, 2017 at 09:03:25AM +0200, Nélio Laranjeiro wrote: > > On Thu, Oct 05, 2017 at 09:59:58PM -0700, Yongseok Koh wrote: > > > On Thu, Oct 05, 2017 at 02:49:47PM +0200, Nelio Laranjeiro wrote: > > > [...] > > > > +struct mlx5_hrxq* > > > > +mlx5_priv_hrxq_get(struct priv *priv, uint8_t *rss_key, uint8_t rss_key_len, > > > > + uint64_t hash_fields, uint16_t queues[], uint16_t queues_n) > > > > +{ > > > > + struct mlx5_hrxq *hrxq; > > > > + > > > > + LIST_FOREACH(hrxq, &priv->hrxqs, next) { > > > > + struct mlx5_ind_table_ibv *ind_tbl; > > > > + > > > > + if (hrxq->rss_key_len != rss_key_len) > > > > + continue; > > > > + if (memcmp(hrxq->rss_key, rss_key, rss_key_len)) > > > > + continue; > > > > + if (hrxq->hash_fields != hash_fields) > > > > + continue; > > > > + ind_tbl = mlx5_priv_ind_table_ibv_get(priv, queues, queues_n); > > > > + if (!ind_tbl) > > > > + continue; > > > > + if (ind_tbl != hrxq->ind_table) { > > > > + mlx5_priv_ind_table_ibv_release(priv, ind_tbl); > > > > > > As one hrxq can have only one ind_tbl, it looks unnecessary to increment refcnt > > > of ind_tbl. As long as a hrxq exist, its ind_tbl can't be destroyed. So, it's > > > safe. How about moving up this _release() outside of this if-clause and remove > > > _release() in _hrxq_release()? > > > > This is right, but in the other side, an indirection table can be used > > by several hash rx queues, that is the main reason why they have their > > own reference counter. > > > > > > +-------+ +-------+ > > | Hrxq | | Hrxq | > > | r = 1 | | r = 1 | > > +-------+ +-------+ > > | | > > v v > > +-------------------+ > > | indirection table | > > | r = 2 | > > +-------------------+ > > > > Seems logical to make the Indirection table counter evolve the same way > > as the hash rx queue, otherwise a second hash rx queue using this > > indirection may release it whereas it is still in use by another hash rx > > queue. > > Whenever a hash Rx queue is created, it gets to have a ind_tbl either by > mlx5_priv_ind_table_ibv_get() or by mlx5_priv_ind_table_ibv_new(). So, the > refcnt of the ind_tbl is already increased. So, even if other hash RxQ which > have had the ind_tbl releases it, it is safe. That's why I don't think > ind_tbl->refcnt needs to get increased on calling mlx5_priv_hrxq_get(). Makes > sense? It make sense, but in this situation, the whole patches needs to be modified to follow this design, the current one being, it needs an object it gets a reference, it does not need it anymore, it release the reference. Which mean a get() in a high level object causes a get() on underlying ones. A release on high level objects causes a release() on underlying ones. In this case, a flow will handle a reference on all objects which contains a reference counter and used by it, even the hidden ones. Currently it won't hurt as it is a control plane point which already rely on a lot of system calls. Can we agree on letting the design as is for this release and maybe changing it in the next one? Thanks, -- Nélio Laranjeiro 6WIND