On Fri, Sep 23, 2016 at 09:23:11PM +0000, Wiles, Keith wrote:

Regards,
Keith

quoted

On Sep 23, 2016, at 12:26 AM, Yuanhan Liu [off-list ref] wrote:

On Thu, Sep 22, 2016 at 06:43:55PM +0200, Thomas Monjalon wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

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.

Yes, that's what I meant.

Not sure what any application would need with the FD here, as it could cause some problems.

Me, neither.

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);

Looks like a very clean solution to me!

Thomas?

	--yliu

quoted

If so, we may go that way.

Another thought is that, it may be a bit weird to me to introduce an API
to get an opaque pointer. I mean, it's a bit hard to document it, because
it has different meaning for different drivers. Should we list all of
them then?

	--yliu