From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id AD3B31DB8 for ; Mon, 8 Oct 2018 10:07:19 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 3F0882203B; Mon, 8 Oct 2018 04:07:19 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 08 Oct 2018 04:07:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=/AHeXcjJqVJn1KNzC7iru5HSfvpxfGKMfwNls5tfXHI=; b=Ps/8npKdmCij P5ry9YBAUhwC2ZkuPz4qGzaBKip9nblHb/fnNilqUpAgzXKMoabSSTsBex0z+L0g M0EcOeM0W0dlM3WKYk8d6mTW1oAD/X1MThb4Ng4WBnAX0Is5YT0sYWmzHkI1FfTd OYi17rLkPgaiS6wRlg2lA9DaV1KvzGE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=/AHeXcjJqVJn1KNzC7iru5HSfvpxfGKMfwNls5tfX HI=; b=OuJcHIZS1T5kbPMSIsUzzGdQnSnS9O49Xw2mj5JmYs5b6kvj1Q9b1Z2tG LXVS2MpM/AI4vNFqaLrNYJXIH4M8q5F20ZRGXiYUkc6sxKPpqi2h2IRx6xzHOuvU jAjatoPWhgzJzssrAsskyJLGlI6zd55tCjPAZfAe0+DxUYYskvqhePAP75YsTKf3 NJ3LRhdHkFW8DmI7XA6gGMXyG2dlpJAHrNfJI5CFjKEe0Pxr76CpNQy7ZN9Cab90 VxuvbBCBERsfQjmQOhO9w4XQDuvA8S2BFZWk6f0Id3vcXn9s0R3agV7xHBYzI4M1 dj305KjMKSZqwQL5tANsrhY86L3eA== X-ME-Sender: X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 8D9A9102EA; Mon, 8 Oct 2018 04:07:17 -0400 (EDT) From: Thomas Monjalon To: Andrew Rybchenko Cc: dev@dpdk.org, gaetan.rivet@6wind.com, ophirmu@mellanox.com, ferruh.yigit@intel.com Date: Mon, 08 Oct 2018 10:07:16 +0200 Message-ID: <3841557.hIBcaeYvXo@xps> In-Reply-To: References: <20181007222554.4886-1-thomas@monjalon.net> <20181007222554.4886-4-thomas@monjalon.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH 3/5] ethdev: allow iterating with only class filter 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, 08 Oct 2018 08:07:20 -0000 08/10/2018 09:20, Andrew Rybchenko: > On 10/8/18 1:25 AM, Thomas Monjalon wrote: > > If no rte_device is given in the iterator, > > eth_dev_match() is looking at all ports without any restriction, > > except the ethdev kvargs filter. > > > > It allows to iterate with a devargs filter referencing only > > some ethdev parameters. The format (from the new devargs syntax) is: > > class=eth,paramY=Y > > > > Fixes: e815a7f69371 ("ethdev: register as a class") > > > > Signed-off-by: Thomas Monjalon > > --- > > --- a/lib/librte_ethdev/rte_class_eth.c > > +++ b/lib/librte_ethdev/rte_class_eth.c > > @@ -42,7 +42,7 @@ eth_dev_match(const struct rte_eth_dev *edev, > > > > if (edev->state == RTE_ETH_DEV_UNUSED) > > return -1; > > - if (edev->device != arg->device) > > + if (arg->device != NULL && arg->device != edev->device) > > return -1; > > if (kvlist == NULL) > > /* Empty string matches everything. */ > > It looks like it is the only hunk which > Fixes: e815a7f69371 ("ethdev: register as a class") Yes this hunk is fixing above commit. > everything else adjusts the previous patch. Yes but the whole goal of this patch is to allow ethdev pure filter. All is related in this patch. > I think this fix should go before and the rest should be squashed > in the previous patch. It was really questionable why it is safe > to dereference iter->bus without checking that it is not NULL. No iter->bus was safe because iter->cls was checked before and implied that iter->bus was successfully set. I still think it is better for understanding to split different kind of filters in 2 patches. I may remove the Fixes line however. Opinion? [...] > > + /* Handle a case from future syntax, without any bus-level argument. */ > > + if (strncmp(devargs_str, iter_anybus_str, > > + strlen(iter_anybus_str)) == 0) { > > + iter->cls_str = devargs_str + strlen(iter_anybus_str); > > + goto end; > > + } > > + > > It looks like a hack, but I guess we need it since rte_devargs_parse() > cannot handle the case. May be it is acceptable if we have no time > to solve it, but it would be good to highlight it better in the comments. This function is a mix of old and new syntax. When only new syntax will be supported, it will be better. It is a work in progress. I do not know how to better explain it.