[dpdk-dev,v2,3/3] net/vhost: vHost support to free consumed buffers

  Add support to the vHostdriver for the new API to force free consumed
buffers on Tx ring. vHost does not cache the mbufs so there is no work
to do.

Signed-off-by: Billy McFall <bmcfall@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
  

On Wed, 11 Jan 2017 15:03:23 -0500
Billy McFall <bmcfall@redhat.com> wrote:

> Add support to the vHostdriver for the new API to force free consumed
> buffers on Tx ring. vHost does not cache the mbufs so there is no work
> to do.
> 
> Signed-off-by: Billy McFall <bmcfall@redhat.com>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index 766d4ef..6493d56 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -939,6 +939,16 @@ eth_queue_release(void *q)
>  }
>  
>  static int
> +eth_tx_done_cleanup(void *txq __rte_unused, uint32_t free_cnt __rte_unused)
> +{
> +	/*
> +	 * vHost does not hang onto mbuf. eth_vhost_tx() copies packet data
> +	 * and releases mbuf, so nothing to cleanup.
> +	 */
> +	return 0;
> +}
> +
> +static int
>  eth_link_update(struct rte_eth_dev *dev __rte_unused,
>  		int wait_to_complete __rte_unused)
>  {
> @@ -979,6 +989,7 @@ static const struct eth_dev_ops ops = {
>  	.tx_queue_setup = eth_tx_queue_setup,
>  	.rx_queue_release = eth_queue_release,
>  	.tx_queue_release = eth_queue_release,
> +	.tx_done_cleanup = eth_tx_done_cleanup,
>  	.link_update = eth_link_update,
>  	.stats_get = eth_stats_get,
>  	.stats_reset = eth_stats_reset,

Rather than change drivers, since this is not critical path, make
it optional to have tx_done_cleanup.
  

rte_eth_tx_done_cleanup(..) checks to see if .tx_done_cleanup is set, so in
that since it is optional. The reason for this function does no work and
returns 0 is to indicate to the application calling it that the mbufs are
free and the application can continue. The application needs to know if the
driver is attempting to the free the mbufs (return value >= 0) or not
implemented (return value < 0).

If return value is < 0, the application needs to take alternative action,
like make a copy of the packet in the flooding case, because the mbuf is
not going to be returned anytime soon. If return value is >= 0, keep
polling until the desired mbuf is returned (reference count decremented).

Let me know if this doesn't answer the question or I am missing your point.

Thanks,
Billy McFall

On Wed, Jan 11, 2017 at 8:39 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Wed, 11 Jan 2017 15:03:23 -0500
> Billy McFall <bmcfall@redhat.com> wrote:
>
> > Add support to the vHostdriver for the new API to force free consumed
> > buffers on Tx ring. vHost does not cache the mbufs so there is no work
> > to do.
> >
> > Signed-off-by: Billy McFall <bmcfall@redhat.com>
> > ---
> >  drivers/net/vhost/rte_eth_vhost.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/net/vhost/rte_eth_vhost.c
> b/drivers/net/vhost/rte_eth_vhost.c
> > index 766d4ef..6493d56 100644
> > --- a/drivers/net/vhost/rte_eth_vhost.c
> > +++ b/drivers/net/vhost/rte_eth_vhost.c
> > @@ -939,6 +939,16 @@ eth_queue_release(void *q)
> >  }
> >
> >  static int
> > +eth_tx_done_cleanup(void *txq __rte_unused, uint32_t free_cnt
> __rte_unused)
> > +{
> > +     /*
> > +      * vHost does not hang onto mbuf. eth_vhost_tx() copies packet data
> > +      * and releases mbuf, so nothing to cleanup.
> > +      */
> > +     return 0;
> > +}
> > +
> > +static int
> >  eth_link_update(struct rte_eth_dev *dev __rte_unused,
> >               int wait_to_complete __rte_unused)
> >  {
> > @@ -979,6 +989,7 @@ static const struct eth_dev_ops ops = {
> >       .tx_queue_setup = eth_tx_queue_setup,
> >       .rx_queue_release = eth_queue_release,
> >       .tx_queue_release = eth_queue_release,
> > +     .tx_done_cleanup = eth_tx_done_cleanup,
> >       .link_update = eth_link_update,
> >       .stats_get = eth_stats_get,
> >       .stats_reset = eth_stats_reset,
>
> Rather than change drivers, since this is not critical path, make
> it optional to have tx_done_cleanup.
>