From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 720521DB8 for ; Mon, 8 Oct 2018 11:13:52 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 13627140064; Mon, 8 Oct 2018 09:13:51 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Mon, 8 Oct 2018 10:13:45 +0100 To: Thomas Monjalon CC: , , , References: <20181007222554.4886-1-thomas@monjalon.net> <20181007222554.4886-4-thomas@monjalon.net> <3841557.hIBcaeYvXo@xps> From: Andrew Rybchenko Message-ID: Date: Mon, 8 Oct 2018 12:13:04 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <3841557.hIBcaeYvXo@xps> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24142.003 X-TM-AS-Result: No-12.825700-8.000000-10 X-TMASE-MatchedRID: 6lay9u8oTUMOwH4pD14DsPHkpkyUphL9Ct59Uh3p/NVgg+UjPGL1RcVA 08S977kgSuqwC2iSnbI4To4+gy/tYNS7ezKc1Aok9UVHiwLx0/LljSRvSGpq3BQLE16MmfDLlgB 5OJUfvnUX65lVYyVW07BuBtU8b2WS+uobhwVz9gAcsSroYI5AViT0t/+p7zHHWUNXQqAdJYcBQW mzDz8Zj5CsMxjPvrHHoIXao5DzMhZIm8dlggjEyG0PWqD0pliRv8yqWnJhUsEGW3hFnC9N1WqgH jofzSuJLfdJWgMEbJlJBXfF88zVLn1GcR5AeEs7aDgPZBX/bMvqvccKLF+4pwwv1ZvdCH+FhmPN hzYtFr72vPhulcaXWIFoPFs7wHUtBawxeDgsyEmdOp1TjDrlaZ03C0fPk8xKeI8u4tqjXL3dKUS BW7I322+5ieh24ZYRkZOl7WKIImrvXOvQVlExsFZ0V5tYhzdWbGVEmIfjf3vBhzWBzbBEzVCI4a Mc+cqx0qOPY0cLdPus+9u20EzM0gNN2xFAM/4r++t5Ttqu3VLmTa9ralKi+unVjJnb+wyZUJGMr zn2oZ8= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--12.825700-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24142.003 X-MDID: 1538990032-TXmDBeT8-kv0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 09:13:52 -0000 On 10/8/18 11:07 AM, Thomas Monjalon wrote: > 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. OK, I see now. > 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? Yes, it makes sense. If so, I think it is better to remove Fixes line. With Fixes I would expect to be able to apply it before previous patch. > [...] >>> + /* 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. I'd suggest to highligh rte_devargs_parse() limitations here to explain why special handling is required here.