[dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id

2016-09-18 Thread Yuanhan Liu
On Wed, Sep 14, 2016 at 10:35:53AM +0200, Thomas Monjalon wrote:
> 2016-09-14 15:21, Yuanhan Liu:
> > On Wed, Sep 14, 2016 at 09:10:48AM +0200, Thomas Monjalon wrote:
> > > 2016-09-14 12:43, Yuanhan Liu:
> > > > On Tue, Sep 13, 2016 at 05:10:09PM +0200, Thomas Monjalon wrote:
> > > > > 2016-09-13 14:47, Ciara Loftus:
> > > > > > In some cases when using the vHost PMD, certain vHost library 
> > > > > > functions
> > > > > > may still need to be accessed. One such example is the
> > > > > > rte_vhost_get_queue_num function which returns the number of 
> > > > > > virtqueues
> > > > > > reported by the guest - information which is not exposed by the PMD.
> > > > > > 
> > > > > > This commit introduces a new rte_eth_vhost function that returns the
> > > > > > 'vid' associated with a given port id. This allows the PMD user to 
> > > > > > call
> > > > > > vHost library functions which require the 'vid' value.
> > > > > 
> > > > > I think we should not add any API to the PMDs.
> > > > 
> > > > In general, I agree with you.
> > > > 
> > > > > Maybe you are looking for a generic API in ethdev.
> > > > 
> > > > But maybe it's a bit hard to define a "right" generic API here. For this
> > > > case, the generic API I can think of could be:
> > > > 
> > > > - an API to get queue num, like rte_eth_get_queue_enabled_num
> > > >   I barely know NIC pmd drivers, but I doubt it's useful/meaningful for 
> > > > them.
> > > > 
> > > > - an API to get a PMD driver private (or specific) data.
> > > >   For vhost-pmd, it's vid. Again, I don't know what it could be for 
> > > > other nic
> > > >   drivers.
> > > > 
> > > >   This one may be a better option here, because it expose a key field to
> > > >   the application, vid, with which the application can invoke more vhost
> > > >   APIs. And apparently, it's not feasible to try to define a generic API
> > > >   for some (if not each) vhost APIs.
> > > 
> > > 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?

--yliu

> Just a thought, not sure yet.


[dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id

2016-09-14 Thread Yuanhan Liu
On Wed, Sep 14, 2016 at 09:10:48AM +0200, Thomas Monjalon wrote:
> 2016-09-14 12:43, Yuanhan Liu:
> > On Tue, Sep 13, 2016 at 05:10:09PM +0200, Thomas Monjalon wrote:
> > > 2016-09-13 14:47, Ciara Loftus:
> > > > In some cases when using the vHost PMD, certain vHost library functions
> > > > may still need to be accessed. One such example is the
> > > > rte_vhost_get_queue_num function which returns the number of virtqueues
> > > > reported by the guest - information which is not exposed by the PMD.
> > > > 
> > > > This commit introduces a new rte_eth_vhost function that returns the
> > > > 'vid' associated with a given port id. This allows the PMD user to call
> > > > vHost library functions which require the 'vid' value.
> > > 
> > > I think we should not add any API to the PMDs.
> > 
> > In general, I agree with you.
> > 
> > > Maybe you are looking for a generic API in ethdev.
> > 
> > But maybe it's a bit hard to define a "right" generic API here. For this
> > case, the generic API I can think of could be:
> > 
> > - an API to get queue num, like rte_eth_get_queue_enabled_num
> >   I barely know NIC pmd drivers, but I doubt it's useful/meaningful for 
> > them.
> > 
> > - an API to get a PMD driver private (or specific) data.
> >   For vhost-pmd, it's vid. Again, I don't know what it could be for other 
> > nic
> >   drivers.
> > 
> >   This one may be a better option here, because it expose a key field to
> >   the application, vid, with which the application can invoke more vhost
> >   APIs. And apparently, it's not feasible to try to define a generic API
> >   for some (if not each) vhost APIs.
> 
> 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" ;)

--yliu


[dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id

2016-09-14 Thread Yuanhan Liu
On Tue, Sep 13, 2016 at 05:10:09PM +0200, Thomas Monjalon wrote:
> 2016-09-13 14:47, Ciara Loftus:
> > In some cases when using the vHost PMD, certain vHost library functions
> > may still need to be accessed. One such example is the
> > rte_vhost_get_queue_num function which returns the number of virtqueues
> > reported by the guest - information which is not exposed by the PMD.
> > 
> > This commit introduces a new rte_eth_vhost function that returns the
> > 'vid' associated with a given port id. This allows the PMD user to call
> > vHost library functions which require the 'vid' value.
> 
> I think we should not add any API to the PMDs.

In general, I agree with you.

> Maybe you are looking for a generic API in ethdev.

But maybe it's a bit hard to define a "right" generic API here. For this
case, the generic API I can think of could be:

- an API to get queue num, like rte_eth_get_queue_enabled_num
  I barely know NIC pmd drivers, but I doubt it's useful/meaningful for them.

- an API to get a PMD driver private (or specific) data.
  For vhost-pmd, it's vid. Again, I don't know what it could be for other nic
  drivers.

  This one may be a better option here, because it expose a key field to
  the application, vid, with which the application can invoke more vhost
  APIs. And apparently, it's not feasible to try to define a generic API
  for some (if not each) vhost APIs.


Or, better ideas?

--yliu


[dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id

2016-09-14 Thread Thomas Monjalon
2016-09-14 15:21, Yuanhan Liu:
> On Wed, Sep 14, 2016 at 09:10:48AM +0200, Thomas Monjalon wrote:
> > 2016-09-14 12:43, Yuanhan Liu:
> > > On Tue, Sep 13, 2016 at 05:10:09PM +0200, Thomas Monjalon wrote:
> > > > 2016-09-13 14:47, Ciara Loftus:
> > > > > In some cases when using the vHost PMD, certain vHost library 
> > > > > functions
> > > > > may still need to be accessed. One such example is the
> > > > > rte_vhost_get_queue_num function which returns the number of 
> > > > > virtqueues
> > > > > reported by the guest - information which is not exposed by the PMD.
> > > > > 
> > > > > This commit introduces a new rte_eth_vhost function that returns the
> > > > > 'vid' associated with a given port id. This allows the PMD user to 
> > > > > call
> > > > > vHost library functions which require the 'vid' value.
> > > > 
> > > > I think we should not add any API to the PMDs.
> > > 
> > > In general, I agree with you.
> > > 
> > > > Maybe you are looking for a generic API in ethdev.
> > > 
> > > But maybe it's a bit hard to define a "right" generic API here. For this
> > > case, the generic API I can think of could be:
> > > 
> > > - an API to get queue num, like rte_eth_get_queue_enabled_num
> > >   I barely know NIC pmd drivers, but I doubt it's useful/meaningful for 
> > > them.
> > > 
> > > - an API to get a PMD driver private (or specific) data.
> > >   For vhost-pmd, it's vid. Again, I don't know what it could be for other 
> > > nic
> > >   drivers.
> > > 
> > >   This one may be a better option here, because it expose a key field to
> > >   the application, vid, with which the application can invoke more vhost
> > >   APIs. And apparently, it's not feasible to try to define a generic API
> > >   for some (if not each) vhost APIs.
> > 
> > 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.
Just a thought, not sure yet.


[dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id

2016-09-14 Thread Thomas Monjalon
2016-09-14 12:43, Yuanhan Liu:
> On Tue, Sep 13, 2016 at 05:10:09PM +0200, Thomas Monjalon wrote:
> > 2016-09-13 14:47, Ciara Loftus:
> > > In some cases when using the vHost PMD, certain vHost library functions
> > > may still need to be accessed. One such example is the
> > > rte_vhost_get_queue_num function which returns the number of virtqueues
> > > reported by the guest - information which is not exposed by the PMD.
> > > 
> > > This commit introduces a new rte_eth_vhost function that returns the
> > > 'vid' associated with a given port id. This allows the PMD user to call
> > > vHost library functions which require the 'vid' value.
> > 
> > I think we should not add any API to the PMDs.
> 
> In general, I agree with you.
> 
> > Maybe you are looking for a generic API in ethdev.
> 
> But maybe it's a bit hard to define a "right" generic API here. For this
> case, the generic API I can think of could be:
> 
> - an API to get queue num, like rte_eth_get_queue_enabled_num
>   I barely know NIC pmd drivers, but I doubt it's useful/meaningful for them.
> 
> - an API to get a PMD driver private (or specific) data.
>   For vhost-pmd, it's vid. Again, I don't know what it could be for other nic
>   drivers.
> 
>   This one may be a better option here, because it expose a key field to
>   the application, vid, with which the application can invoke more vhost
>   APIs. And apparently, it's not feasible to try to define a generic API
>   for some (if not each) vhost APIs.

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.


[dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id.

2016-09-13 Thread Thomas Monjalon
2016-09-13 14:47, Ciara Loftus:
> In some cases when using the vHost PMD, certain vHost library functions
> may still need to be accessed. One such example is the
> rte_vhost_get_queue_num function which returns the number of virtqueues
> reported by the guest - information which is not exposed by the PMD.
> 
> This commit introduces a new rte_eth_vhost function that returns the
> 'vid' associated with a given port id. This allows the PMD user to call
> vHost library functions which require the 'vid' value.

I think we should not add any API to the PMDs.
Maybe you are looking for a generic API in ethdev.


[dpdk-dev] [PATCH] net/vhost: Add function to retreive the 'vid' for a given port id.

2016-09-13 Thread Ciara Loftus
In some cases when using the vHost PMD, certain vHost library functions
may still need to be accessed. One such example is the
rte_vhost_get_queue_num function which returns the number of virtqueues
reported by the guest - information which is not exposed by the PMD.

This commit introduces a new rte_eth_vhost function that returns the
'vid' associated with a given port id. This allows the PMD user to call
vHost library functions which require the 'vid' value.

Signed-off-by: Ciara Loftus 
---
 drivers/net/vhost/rte_eth_vhost.c   | 29 +
 drivers/net/vhost/rte_eth_vhost.h   |  9 +
 drivers/net/vhost/rte_pmd_vhost_version.map |  6 ++
 3 files changed, 44 insertions(+)

diff --git a/drivers/net/vhost/rte_eth_vhost.c 
b/drivers/net/vhost/rte_eth_vhost.c
index 7539cd4..091517d 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -428,6 +428,35 @@ rte_eth_vhost_get_queue_event(uint8_t port_id,
return -1;
 }

+int
+rte_eth_vhost_get_vid_from_port_id(uint8_t port_id)
+{
+   struct internal_list *list;
+   struct rte_eth_dev *eth_dev;
+   struct vhost_queue *vq;
+   int vid = -1;
+
+   if (!rte_eth_dev_is_valid_port(port_id))
+   return -1;
+
+   pthread_mutex_lock(_list_lock);
+
+   TAILQ_FOREACH(list, _list, next) {
+   eth_dev = list->eth_dev;
+   if (eth_dev->data->port_id == port_id) {
+   vq = eth_dev->data->rx_queues[0];
+   if (vq) {
+   vid = vq->vid;
+   }
+   break;
+   }
+   }
+
+   pthread_mutex_unlock(_list_lock);
+
+   return vid;
+}
+
 static void *
 vhost_driver_session(void *param __rte_unused)
 {
diff --git a/drivers/net/vhost/rte_eth_vhost.h 
b/drivers/net/vhost/rte_eth_vhost.h
index ff5d877..7c98b1a 100644
--- a/drivers/net/vhost/rte_eth_vhost.h
+++ b/drivers/net/vhost/rte_eth_vhost.h
@@ -102,6 +102,15 @@ struct rte_eth_vhost_queue_event {
 int rte_eth_vhost_get_queue_event(uint8_t port_id,
struct rte_eth_vhost_queue_event *event);

+/**
+ * Get the 'vid' value associated with the specified port.
+ *
+ * @return
+ *  - On success, the 'vid' associated with 'port_id'.
+ *  - On failure, a negative value.
+ */
+int rte_eth_vhost_get_vid_from_port_id(uint8_t port_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/drivers/net/vhost/rte_pmd_vhost_version.map 
b/drivers/net/vhost/rte_pmd_vhost_version.map
index 65bf3a8..0533a83 100644
--- a/drivers/net/vhost/rte_pmd_vhost_version.map
+++ b/drivers/net/vhost/rte_pmd_vhost_version.map
@@ -8,3 +8,9 @@ DPDK_16.04 {

local: *;
 };
+
+DPDK_16.11 {
+   global:
+
+   rte_eth_vhost_get_vid_from_port_id;
+}
-- 
2.4.3