[ovs-dev] [PATCH v9 2/3] dpif-netdev: Detailed performance stats for PMDs

Stokes, Ian ian.stokes at intel.com
Wed Mar 21 13:40:59 UTC 2018


> -----Original Message-----
> From: Jan Scheurich [mailto:jan.scheurich at ericsson.com]
> Sent: Sunday, March 18, 2018 5:38 PM
> To: Stokes, Ian <ian.stokes at intel.com>; dev at openvswitch.org
> Cc: i.maximets at samsung.com
> Subject: RE: [ovs-dev] [PATCH v9 2/3] dpif-netdev: Detailed performance
> stats for PMDs
> 
> > Checkpatch reports the following:
> >
> > WARNING: Line lacks whitespace around operator
> > #560 FILE: lib/dpif-netdev-perf.h:113:
> >     uint64_t cycles;            /* Number of TSC cycles spent in it/ms.
> */
> >
> > WARNING: Line lacks whitespace around operator
> > #563 FILE: lib/dpif-netdev-perf.h:116:
> >     uint32_t pkts;              /* Packets processed in iteration/ms. */
> >
> > WARNING: Line lacks whitespace around operator
> > #564 FILE: lib/dpif-netdev-perf.h:117:
> >     uint32_t upcalls;           /* Number of upcalls in iteration/ms. */
> >
> > WARNING: Line lacks whitespace around operator
> > #565 FILE: lib/dpif-netdev-perf.h:118:
> >     uint32_t upcall_cycles;     /* Cycles spent in upcalls in
> iteration/ms. */
> >
> > WARNING: Line lacks whitespace around operator
> > #566 FILE: lib/dpif-netdev-perf.h:119:
> >     uint32_t batches;           /* Number of rx batches in iteration/ms.
> */
> >
> > WARNING: Line lacks whitespace around operator
> > #567 FILE: lib/dpif-netdev-perf.h:120:
> >     uint32_t max_vhost_qfill;   /* Maximum fill level encountered in
> it/ms. */
> 
> These warnings are pretty silly, given that they complain about comments.
> Somebody should improve checkpatch.py to skip checking comments for coding
> style! For now I have modified these comments to avoid the warnings.
> 

In this case I agree, it probably doesn't make sense.

> > WARNING: Line lacks whitespace around operator
> > #1218 FILE: lib/dpif-netdev.c:3396:
> >     int rem_qlen = 0, *qlen_p= NULL;
> 
> OK, this one was real.
> 
> > > +    } else {
> > > +        ds_put_format(str,
> > > +                "  Rx packets:      %12"PRIu64"\n",
> > > +                0UL);
> >
> > Will cause compilation error ovs OVS travis
> >
> > lib/dpif-netdev-perf.c:194:17: error: format '%llu' expects argument
> > of type 'long long unsigned int', but argument 3 has type 'long unsigned
> int' [-Werror=format=]
> >                  0UL);
> >
> > See link below for further info
> >
> > https://travis-ci.org/istokes/ovs/jobs/354225095
> >
> > > +    }
> > > +    if (tx_packets > 0) {
> > > +        ds_put_format(str,
> > > +            "  Tx packets:      %12"PRIu64"  (%.0f Kpps)\n"
> > > +            "  Tx batches:      %12"PRIu64"  (%.2f pkts/batch)"
> > > +            "\n",
> > > +            tx_packets, (tx_packets / duration) / 1000,
> > > +            tx_batches, 1.0 * tx_packets / tx_batches);
> > > +    } else {
> > > +        ds_put_format(str,
> > > +                "  Tx packets:      %12"PRIu64"\n"
> > > +                "\n",
> > > +                0UL);
> >
> > Same as above.
> 
> Fixed the constants to 0ULL. Hope that does the trick.
> 
> > > +/* This function clears the PMD performance counters from within
> > > +the PMD
> > > + * thread or from another thread when the PMD thread is not
> > > +executing its
> > > + * poll loop. */
> > >  void
> > > -pmd_perf_stats_clear(struct pmd_perf_stats *s)
> > > +pmd_perf_stats_clear_lock(struct pmd_perf_stats *s)
> > > +    OVS_REQUIRES(pmd->stats_mutex)
> >
> > Will cause compilation error for OVS Travis build
> >
> > lib/dpif-netdev-perf.c:365:18: error: use of undeclared identifier 'pmd'
> >     OVS_REQUIRES(pmd->stats_mutex)
> >
> > https://travis-ci.org/istokes/ovs/jobs/354225106
> 
> Yes, that was a leftover that normal compilation with GCC didn't catch.
> Should read OVS_REQUIRES(s->stats_mutex).
> 
> Will include the fixes in v10.
> 
> Thanks, Jan


More information about the dev mailing list