From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id C0C7037B1 for ; Mon, 26 Sep 2016 16:26:29 +0200 (CEST) Received: by mail-wm0-f49.google.com with SMTP id b130so152056168wmc.0 for ; Mon, 26 Sep 2016 07:26:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=CHBI5cR3Ya6b1Kx4436qdWUrx4SndP9VY6XlL+G9acE=; b=GW5VTg0cXEEhehnh4cpl8amyB7UhXlYaBV2ojqHH3pzSvmGAfy3DdgN5qimQrKkudG 2I+6KqquOBYX2vB7grt5yXZqvje6gmZN1Kgkz3M5B3lMgvxrHXL8DS/curIkvPB7PSNP CTB3apQQ0hemqciHn2RTkSFwi9ePoHmNyONwiDnrncmr6Pz4sVRLQNkSPj7Y+PwntqaB wyKJXpAqFkXwXRrvpLTvOa555eb5dZwp7EL1PDkRkTY2XDQjn+0neFG8hAXEhCo2yS14 l4h1H77Vserh5XPH3I2sNqMkfPsxPBsy+l9CvifzLbFowd3ETVLixO7nbydmsxJITe0Q 3ghg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=CHBI5cR3Ya6b1Kx4436qdWUrx4SndP9VY6XlL+G9acE=; b=fUD4/3UEgCAB5LbmxOqJMt88wq646+5FV7hi9+8aVzxXJv8gyQgcYrPQ0rRD/Xlxl/ dycpJpleyhn3rvwtiLRKp7GP24k6T6mqf9v0U3cWkEcQOB1ot/vXmYFKvBfwnx9c8m1J jbU9THohfVuKf1gw1HZPJbXNfNdo+pR545BPJdRPsLVbKNf8nCxT8y1m/S0EF2cIO0Qr 7SS6GmEaA0OucU7L46ZExZgH6zmQfQ6IRf8OW/v0IFrH3ONhiTbSSIszLmMSNQHW8ZvO Tc1CyDQyeT2FHFCBDWWB/ZICY2ZnwbdcNNzxLYP/STZlFeuNUVAAmI8eRH8QCrziyFTE nazA== X-Gm-Message-State: AA6/9RmM4EFTkKiyKJNi9Nh0HZA4elPPVT6pUdd07HlC2+v0xcef/KRUBKDRjsjpPMWTJiRS X-Received: by 10.28.184.216 with SMTP id i207mr13737233wmf.63.1474899989479; Mon, 26 Sep 2016 07:26:29 -0700 (PDT) Received: from xps13.localnet (184.203.134.77.rev.sfr.net. [77.134.203.184]) by smtp.gmail.com with ESMTPSA id bk7sm22698295wjc.36.2016.09.26.07.26.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 26 Sep 2016 07:26:28 -0700 (PDT) From: Thomas Monjalon To: Bruce Richardson Cc: "Wiles, Keith" , Yuanhan Liu , "Loftus, Ciara" , dev@dpdk.org Date: Mon, 26 Sep 2016 16:26:27 +0200 Message-ID: <1649619.cdBSfnkg7V@xps13> User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <20160926131835.GA21308@bricha3-MOBL3> References: <1473774463-26966-1-git-send-email-ciara.loftus@intel.com> <1895719.0W7iI9zMqJ@xps13> <20160926131835.GA21308@bricha3-MOBL3> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Sep 2016 14:26:30 -0000 2016-09-26 14:18, Bruce Richardson: > On Mon, Sep 26, 2016 at 03:12:01PM +0200, Thomas Monjalon wrote: > > 2016-09-23 21:23, Wiles, Keith: > > > On Sep 23, 2016, at 12:26 AM, Yuanhan Liu wrote: > > > > On Thu, Sep 22, 2016 at 06:43:55PM +0200, Thomas Monjalon wrote: > > > >>>>>>>> There could be a similar need in other PMD. > > > >>>>>>>> If we can get an opaque identifier of the device which is not the port id, > > > >>>>>>>> we could call some specific functions of the driver not implemented in > > > >>>>>>>> the generic ethdev API. > > > >>>>>>> > > > >>>>>>> That means you have to add/export the PMD API first. Isn't it against what > > > >>>>>>> you are proposing -- "I think we should not add any API to the PMDs" ;) > > > >>>>>> > > > >>>>>> Yes you are totally right :) > > > >>>>>> Except that in vhost case, we would not have any API in the PMD. > > > >>>>>> But it would allow to have some specific API in other PMDs for the features > > > >>>>>> which do not fit in a generic API. > > > >>>>> > > > >>>>> So, does that mean you are okay with this patch now? I mean, okay to introduce > > > >>>>> a vhost PMD API? > > > >>>> > > > >>>> It means I would be in favor of introducing API in drivers for very specific > > > >>>> features. > > > >>>> In this case, I am not sure that retrieving an internal id is very specific. > > > >>> > > > >>> It's not, instead, it's very generic. The "internal id" is actually the > > > >>> public interface to vhost-user application, like "fd" to file APIs. > > > >>> > > > >>> Instead of introducing a few specific wrappers/APIs, I'd prefer to > > > >>> introduce a generic one to get the handle, and let the application to > > > >>> call other vhost APIs. > > > >> > > > >> Yes it makes sense. > > > >> I was thinking of introducing a function to get an internal id from ethdev, > > > >> in order to use it with any driver or underlying library. > > > >> But it would be an opaque pointer and you need an int. > > > >> Note that we can cast an int into a pointer, so I am not sure what is best. > > > > > > > > Yes, that should work. But I just doubt what the "opaque pointer" could be > > > > for other PMD drivers, and what the application could do with it. For a > > > > typical nic PMD driver, I can think of nothing is valuable to export to > > > > user applications. > > > > > > > > But maybe it's valuable to other virtual PMD drives as well, like the TAP > > > > pmd from Keith? > > > > > > I do not see a need in the TAP PMD other then returning the FD for TUN/TAP device. Not sure what any application would need with the FD here, as it could cause some problems. > > > > > > This feels like we are talking about a IOCTL like generic interface into the PMD. Then we can add new one types and reject types in the PMD that are not supported. Would this not be a better method for all future PMD APIs? > > > > > > Here is just a thought as to how to solve this problem without a PMD specific API. A number of current ethdev APIs could be removed to use the API below. The APIs would be removed from ethdev structure and have the current APIs use the API below. I know some are not happy with number of APIs in the ethdev structure. > > > > > > The API could be something like this: > > > struct rte_tlv { /* Type/Length/Value like structure */ > > > uint16_t type; /* Type of command */ > > > uint16_t len; /* Length of data section on input and on output */ > > > uint16_t tlen; /* Total or max length of data buffer */ > > > uint8_t data[0]; > > > }; > > > > > > int rte_eth_dev_ioctl(int pid, int qid, struct rte_tlv *tlv); > > > > Yes we are talking about having some specific functions per driver which > > are not defined in the generic ethdev layer. > > We need only one function in ethdev to give access to driver-specific API. > > My idea is to convert the port id into an opaque handler. > > Your idea is to use the port id in an ioctl like function. > > > > About the implementation, these are the 2 differences between my proposal > > and yours: > > - You use the well known port id, whereas I need another handler which is > > understood by the driver. > > - You need to build a message string which will be decoded by the driver. > > I propose to directly offer some specific functions in the drivers which > > are more convenient to use and easier for code review/debug. > > > > No conclusion here. I just want to make sure that we are on the same page, > > and would like to have feedback from others. Thanks > > I personally don't like the idea of having a generic IOCTL in ethdev. If you > want to have NIC-specific functions provided by a driver, that is fine, but > any app using those is going to be limited to working only with that driver. > > In that case, since the driver in question is known, I don't see any reason > to go through the ethdev layer. I think it would be much clearer to have the > app instead include the driver's header file and call the driver function > directly. The #include at the top of the file makes the dependency very clear, > and having a function name instead of IOCTL with magic command numbers allows > the action take by the function to be clearer too. So you are against an IOCTL API. Me too. You agree that an application can be NIC-specific and include an header file given by the driver to offer very specific features. Me too. My proposal was to convert the port id to an opaque pointer as handler of these driver APIs. After an offline discussion, we agreed that it is not necessary because drivers manage rte_eth_dev struct and port_id through lib/librte_ether/rte_ethdev.h: extern struct rte_eth_dev rte_eth_devices[]; Now let's come back to the vhost discussion. The vhost library uses a vid which could be retrieved from a port id thanks to a new function from a vhost PMD header file. I think there is no more objection against this new specific function?