[ovs-dev] [PATCH v2 1/3] dpif-netdev: only poll enabled vhost queues

[Ilya Maximets]

On 18.04.2019 17:05, David Marchand wrote:
> 
> 
> On Wed, Apr 17, 2019 at 4:16 PM Kevin Traynor <ktraynor at redhat.com <mailto:ktraynor at redhat.com>> wrote:
> 
>     On 16/04/2019 10:45, David Marchand wrote:
>     > @@ -1171,6 +1173,9 @@ pmd_info_show_rxq(struct ds *reply, struct dp_netdev_pmd_thread *pmd)
>     >              } else {
>     >                  ds_put_format(reply, "%s", "NOT AVAIL");
>     >              }
>     > +            if (!netdev_rxq_enabled(list[i].rxq->rx)) {
>     > +                ds_put_cstr(reply, "  polling: disabled");
>     > +            }
> 
>     It's just a personal preference but I'm not crazy about the additional
>     columns appearing/disappearing. Also it seems like it's more fundamental
>     than the % usage and should be closer to the queue-id. It's currently
> 
>     port: v0        queue-id:  0  pmd usage: 13 %
>     port: v0        queue-id:  1  pmd usage:  0 %  polling: disabled
>     port: v1        queue-id:  0  pmd usage: 13 %
>     port: v1        queue-id:  1  pmd usage:  0 %  polling: disabled
> 
>     As suggestion, could be:
> 
>     port: v0        queue-id:  0   enabled  pmd usage: 13 %
>     port: v0        queue-id:  1  disabled  pmd usage:  0 %
>     port: v1        queue-id:  0   enabled  pmd usage: 13 %
>     port: v1        queue-id:  1  disabled  pmd usage:  0 %

Maybe:

      port: v0        queue-id:  0             pmd usage: 13 %
      port: v0        queue-id:  1 (disabled)  pmd usage:  0 %
      port: v1        queue-id:  0             pmd usage: 13 %
      port: v1        queue-id:  1 (disabled)  pmd usage:  0 %

?

> 
>     or else just remove the % usage if a queue is disabled:
> 
>     port: v0        queue-id:  0  pmd usage: 13 %
>     port: v0        queue-id:  1  disabled
>     port: v1        queue-id:  0  pmd usage: 13 %
>     port: v1        queue-id:  1  disabled
> 
> 
> If we want to have all the informations (pmd usage and queue state), then your first suggestion seems the best and is close to what I had in my v1.
> Since I got no hard comment on how we must keep compatibility on those commands output, I am all to go with this, and I would fix the unit tests accordingly.
> 
> Waiting until tomorrow and I will do this if I don't hear from anyone.
>  
> 
> 
>     > diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
>     > index fb0c27e..5faae0d 100644
>     > --- a/lib/netdev-provider.h
>     > +++ b/lib/netdev-provider.h
>     > @@ -789,6 +789,11 @@ struct netdev_class {
>     >      void (*rxq_destruct)(struct netdev_rxq *);
>     >      void (*rxq_dealloc)(struct netdev_rxq *);
>     > 
>     > +    /* A netdev can report if a queue won't get traffic and should be excluded
>     > +     * from polling (no callback implicitely means that the queue is enabled).
> 
>     nit: implicitly
> 
> 
> Will disappear in next version anyway :-)
> 
> -- 
> David Marchand