On Wed, Nov 30, 2022 at 1:55 PM Kevin Traynor <[email protected]> wrote:
> >> @@ -1463,5 +1479,16 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> >> argc, const char *argv[],
> >>           }
> >>           if (type == PMD_INFO_SHOW_RXQ) {
> >> -            pmd_info_show_rxq(&reply, pmd);
> >> +            if (first_show_rxq) {
> >> +                if (!secs || secs > max_secs) {
> >> +                    secs = max_secs;
> >> +                } else {
> >> +                    secs = ROUND_UP(secs,
> >> +                                    PMD_INTERVAL_LEN / 
> >> INTERVAL_USEC_TO_SEC);
> >> +                }
> >> +                ds_put_format(&reply, "Displaying last %u seconds "
> >> +                              "pmd usage %%\n", secs);
> >> +                first_show_rxq = false;
> >
> > Always displaying this banner regardless of a pmd matching would make
> > this code smaller.
>
> If I got you right, wouldn't that lead to this?
>
> # ovs-appctl dpif-netdev/pmd-rxq-show -pmd 8
> Displaying last 60 seconds pmd usage %
> pmd thread numa_id 0 core_id 8:
>    isolated : false
>    port: myport            queue-id:  0 (enabled)   pmd usage:  0 %
>    overhead:  0 %
>
> # ovs-appctl dpif-netdev/pmd-rxq-show -pmd 5
> Displaying last 60 seconds pmd usage %
> #
>
> I think it looks a bit off to give info about the stats for the case
> where there are then no pmds matching.

I have no strong opinion, just that I preferred simpler code.


>
> > Plus, secs can be computed once, before this per pmd loop.
> > Wdyt?
> >
>
> Secs are only calculated once as is. It could be moved outside the loop
> but then it might be calculated and not used for other commands or no
> pmd being displayed. So not sure it's worth it considering i think i'd
> still want to the keep the 'if (first_show_rxq)' for the banner based on
> above.
>
> Does it sound ok?

Yes, so keep it as is.


-- 
David Marchand

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to