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

David Marchand david.marchand at redhat.com
Thu Apr 18 14:05:32 UTC 2019


On Wed, Apr 17, 2019 at 4:16 PM Kevin Traynor <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 %
>
> 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


More information about the dev mailing list