add an iqdrops view to systat to show interface queue drops

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

add an iqdrops view to systat to show interface queue drops

David Gwynne-5
im adding numbers to input and output qdrops in the kernel, so im
aware that they exist now. however, i don't really see these values
in userland. it seems netstat and systat think errors are more
important.

i tried adding qdrops to the ifstat view, but it got too cluttered.
so i made a new view called iqdrops that shows qdrops instead of
errors. i used iqdrops instead of ifqdrops so "if" on its own is
still not ambiguous.

ok?

Index: systat.1
===================================================================
RCS file: /cvs/src/usr.bin/systat/systat.1,v
retrieving revision 1.102
diff -u -p -r1.102 systat.1
--- systat.1 15 Jun 2017 03:47:07 -0000 1.102
+++ systat.1 15 Nov 2017 03:24:53 -0000
@@ -126,6 +126,7 @@ argument expects to be one of:
 .Ic vmstat ,
 .Ic pigs ,
 .Ic ifstat ,
+.Ic iqdrops ,
 .Ic iostat ,
 .Ic sensors ,
 .Ic mbufs ,
@@ -287,6 +288,10 @@ between display refreshes.
 changes the counters to show the average per second over
 the display refresh interval;
 this is the default.
+.It Ic iqdrops
+Displays a similar view to
+.Ic ifstat ,
+but shows queue drops instead of errors.
 .It Ic iostat
 Display statistics about disk throughput.
 Statistics
Index: systat.h
===================================================================
RCS file: /cvs/src/usr.bin/systat/systat.h,v
retrieving revision 1.21
diff -u -p -r1.21 systat.h
--- systat.h 12 Mar 2015 01:03:00 -0000 1.21
+++ systat.h 15 Nov 2017 03:24:53 -0000
@@ -103,9 +103,11 @@ struct ifcount {
  u_int64_t ifc_ib; /* input bytes */
  u_int64_t ifc_ip; /* input packets */
  u_int64_t ifc_ie; /* input errors */
+ u_int64_t ifc_iq; /* input qdrops */
  u_int64_t ifc_ob; /* output bytes */
  u_int64_t ifc_op; /* output packets */
  u_int64_t ifc_oe; /* output errors */
+ u_int64_t ifc_oq; /* output qdrops */
  u_int64_t ifc_co; /* collisions */
  int ifc_flags; /* up / down */
  int ifc_state; /* link state */
Index: if.c
===================================================================
RCS file: /cvs/src/usr.bin/systat/if.c,v
retrieving revision 1.23
diff -u -p -r1.23 if.c
--- if.c 16 Jan 2015 00:03:37 -0000 1.23
+++ if.c 15 Nov 2017 03:24:53 -0000
@@ -69,6 +69,8 @@ field_def fields_if[] = {
  {"OERRS", 5, 8, 1, FLD_ALIGN_RIGHT, -1, 0, 0, 0},
  {"COLLS", 5, 8, 1, FLD_ALIGN_RIGHT, -1, 0, 0, 0},
  {"DESC", 14, 64, 1, FLD_ALIGN_LEFT, -1, 0, 0, 0},
+ {"IDROP", 5, 8, 1, FLD_ALIGN_RIGHT, -1, 0, 0, 0},
+ {"ODROP", 5, 8, 1, FLD_ALIGN_RIGHT, -1, 0, 0, 0},
 };
 
 
@@ -82,6 +84,8 @@ field_def fields_if[] = {
 #define FLD_IF_OERRS FIELD_ADDR(fields_if,7)
 #define FLD_IF_COLLS FIELD_ADDR(fields_if,8)
 #define FLD_IF_DESC FIELD_ADDR(fields_if,9)
+#define FLD_IF_IDROP FIELD_ADDR(fields_if,10)
+#define FLD_IF_ODROP FIELD_ADDR(fields_if,11)
 
 
 /* Define views */
@@ -91,6 +95,12 @@ field_def *view_if_0[] = {
  FLD_IF_OERRS, FLD_IF_COLLS, NULL
 };
 
+field_def *view_if_1[] = {
+ FLD_IF_IFACE, FLD_IF_STATE, FLD_IF_DESC, FLD_IF_IPKTS,
+ FLD_IF_IBYTES, FLD_IF_IDROP, FLD_IF_OPKTS, FLD_IF_OBYTES,
+ FLD_IF_ODROP, FLD_IF_COLLS, NULL
+};
+
 /* Define view managers */
 
 struct view_manager ifstat_mgr = {
@@ -100,6 +110,7 @@ struct view_manager ifstat_mgr = {
 
 field_view views_if[] = {
  {view_if_0, "ifstat", '1', &ifstat_mgr},
+ {view_if_1, "iqdrops", '1', &ifstat_mgr},
  {NULL, NULL, 0, NULL}
 };
 
@@ -264,9 +275,11 @@ fetchifstat(void)
  UPDATE(ifc_ip, ifm_data.ifi_ipackets);
  UPDATE(ifc_ib, ifm_data.ifi_ibytes);
  UPDATE(ifc_ie, ifm_data.ifi_ierrors);
+ UPDATE(ifc_iq, ifm_data.ifi_iqdrops);
  UPDATE(ifc_op, ifm_data.ifi_opackets);
  UPDATE(ifc_ob, ifm_data.ifi_obytes);
  UPDATE(ifc_oe, ifm_data.ifi_oerrors);
+ UPDATE(ifc_oq, ifm_data.ifi_oqdrops);
  UPDATE(ifc_co, ifm_data.ifi_collisions);
  ifs->ifs_cur.ifc_flags = ifm.ifm_flags;
  ifs->ifs_cur.ifc_state = ifm.ifm_data.ifi_link_state;
@@ -316,10 +329,12 @@ showifstat(struct ifstat *ifs)
  print_fld_sdiv(FLD_IF_IBYTES, ifs->ifs_cur.ifc_ib * conv, div);
  print_fld_size(FLD_IF_IPKTS, ifs->ifs_cur.ifc_ip);
  print_fld_size(FLD_IF_IERRS, ifs->ifs_cur.ifc_ie);
+ print_fld_size(FLD_IF_IDROP, ifs->ifs_cur.ifc_iq);
 
  print_fld_sdiv(FLD_IF_OBYTES, ifs->ifs_cur.ifc_ob * conv, div);
  print_fld_size(FLD_IF_OPKTS, ifs->ifs_cur.ifc_op);
  print_fld_size(FLD_IF_OERRS, ifs->ifs_cur.ifc_oe);
+ print_fld_size(FLD_IF_ODROP, ifs->ifs_cur.ifc_oq);
 
  print_fld_size(FLD_IF_COLLS, ifs->ifs_cur.ifc_co);
 
@@ -337,10 +352,12 @@ showtotal(void)
  print_fld_sdiv(FLD_IF_IBYTES, sum.ifc_ib * conv, div);
  print_fld_size(FLD_IF_IPKTS, sum.ifc_ip);
  print_fld_size(FLD_IF_IERRS, sum.ifc_ie);
+ print_fld_size(FLD_IF_IDROP, sum.ifc_iq);
 
  print_fld_sdiv(FLD_IF_OBYTES, sum.ifc_ob * conv, div);
  print_fld_size(FLD_IF_OPKTS, sum.ifc_op);
  print_fld_size(FLD_IF_OERRS, sum.ifc_oe);
+ print_fld_size(FLD_IF_ODROP, sum.ifc_oq);
 
  print_fld_size(FLD_IF_COLLS, sum.ifc_co);
 

Reply | Threaded
Open this post in threaded view
|

Re: add an iqdrops view to systat to show interface queue drops

Alexander Bluhm
On Wed, Nov 15, 2017 at 01:29:42PM +1000, David Gwynne wrote:
> im adding numbers to input and output qdrops in the kernel, so im
> aware that they exist now. however, i don't really see these values
> in userland. it seems netstat and systat think errors are more
> important.
>
> i tried adding qdrops to the ifstat view, but it got too cluttered.
> so i made a new view called iqdrops that shows qdrops instead of
> errors. i used iqdrops instead of ifqdrops so "if" on its own is
> still not ambiguous.

Now ifstat and iqdrops show almost the same information.  Just two
commns are different, eight culumns are redundant.  I think one
page would be better.  Do we need DESC?  It takes a lot of space
that could be used for output of dynamic counters.

bluhm

Reply | Threaded
Open this post in threaded view
|

Re: add an iqdrops view to systat to show interface queue drops

Theo de Raadt-2
> Now ifstat and iqdrops show almost the same information.  Just two
> commns are different, eight culumns are redundant.  I think one
> page would be better.  Do we need DESC?  It takes a lot of space
> that could be used for output of dynamic counters.

I use DESC every single day.

Reply | Threaded
Open this post in threaded view
|

Re: add an iqdrops view to systat to show interface queue drops

Claudio Jeker
In reply to this post by Alexander Bluhm
On Thu, Nov 16, 2017 at 03:21:20PM +0100, Alexander Bluhm wrote:

> On Wed, Nov 15, 2017 at 01:29:42PM +1000, David Gwynne wrote:
> > im adding numbers to input and output qdrops in the kernel, so im
> > aware that they exist now. however, i don't really see these values
> > in userland. it seems netstat and systat think errors are more
> > important.
> >
> > i tried adding qdrops to the ifstat view, but it got too cluttered.
> > so i made a new view called iqdrops that shows qdrops instead of
> > errors. i used iqdrops instead of ifqdrops so "if" on its own is
> > still not ambiguous.
>
> Now ifstat and iqdrops show almost the same information.  Just two
> commns are different, eight culumns are redundant.  I think one
> page would be better.  Do we need DESC?  It takes a lot of space
> that could be used for output of dynamic counters.
>

Would it make sense to extend the mbuf page instead? It has the ring size
and so it may make sense to show drops there.

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: add an iqdrops view to systat to show interface queue drops

David Gwynne-5
In reply to this post by Theo de Raadt-2
On Thu, Nov 16, 2017 at 07:22:48AM -0700, Theo de Raadt wrote:
> > Now ifstat and iqdrops show almost the same information.  Just two
> > commns are different, eight culumns are redundant.  I think one
> > page would be better.  Do we need DESC?  It takes a lot of space
> > that could be used for output of dynamic counters.
>
> I use DESC every single day.

me too.

Reply | Threaded
Open this post in threaded view
|

Re: add an iqdrops view to systat to show interface queue drops

David Gwynne-5
In reply to this post by Claudio Jeker

> On 17 Nov 2017, at 05:39, Claudio Jeker <[hidden email]> wrote:
>
> On Thu, Nov 16, 2017 at 03:21:20PM +0100, Alexander Bluhm wrote:
>> On Wed, Nov 15, 2017 at 01:29:42PM +1000, David Gwynne wrote:
>>> im adding numbers to input and output qdrops in the kernel, so im
>>> aware that they exist now. however, i don't really see these values
>>> in userland. it seems netstat and systat think errors are more
>>> important.
>>>
>>> i tried adding qdrops to the ifstat view, but it got too cluttered.
>>> so i made a new view called iqdrops that shows qdrops instead of
>>> errors. i used iqdrops instead of ifqdrops so "if" on its own is
>>> still not ambiguous.
>>
>> Now ifstat and iqdrops show almost the same information.  Just two
>> commns are different, eight culumns are redundant.  I think one
>> page would be better.  Do we need DESC?  It takes a lot of space
>> that could be used for output of dynamic counters.

can we have modified displays within a view? kind of like how some views change their ordering.

i agree that everything should be on the ifstat page, but there really isnt enough space. if we can have tweaked displays, id like the main one to aggregate the qdrops and errs values into a single column, and then have separate displays within that view to show errors and drops as separate values.

> Would it make sense to extend the mbuf page instead? It has the ring size
> and so it may make sense to show drops there.

i dont think that would make sense for interfaces like vlan.


Reply | Threaded
Open this post in threaded view
|

Re: add an iqdrops view to systat to show interface queue drops

Christian Weisgerber
On 2017-11-17, David Gwynne <[hidden email]> wrote:

> can we have modified displays within a view?

We already have this for the ifstat view.

         The character B changes the counter view between bytes and
         bits.  Pressing b displays statistics as calculated from boot
         time.  r changes the counters to show their totals as
         calculated between display refreshes.  t changes the counters
         to show the average per second over the display refresh
         interval; this is the default.

--
Christian "naddy" Weisgerber                          [hidden email]