ON-16037: added functions to read stats for interfaces

[rdrinkwa-xilinx]

Added two functions to read the drop stats from the physical interfaces used by TCPDirect. Tested as working with physical interfaces themselves or a bond. The zfsink app was updated to make use of this additional functionality for testing and demo purposes.

The design mimics that of ef_vi itself with the two calls to get the layout and then the actual stats themselves. It could be changed to hold a copy of the layout internally within the stack so passing it back in wouldn't be required but it would add complexity and necessitate code changes in other files beyond the minimal updates to existing files here.

Testing with 'efsend' sending 1000 1-byte packets as a burst that is known to exceed zfsink's ability to avoid drops.

# testing physical interface with regular drops
[server1:static]$ ZF_ATTR="interface=enp1s0f0" ./zfsink -s 224.1.2.3:12345
# pkt-rate  bandwidth(Mbps)       total-pkts  RX CRC errors  RX trunk errors  RX no descriptor errors  RX abort errors
         0                0                0              0                0                        0                0
         0                0                0              0                0                        0                0
       710                0              710              0                0                      290                0
         0                0              710              0                0                        0                0
         0                0              710              0                0                        0                0
^C
# physical interface and backgrounded to cause more drops
[server1:static]$ ZF_ATTR="interface=enp1s0f0" ./zfsink -s 224.1.2.3:12345
# pkt-rate  bandwidth(Mbps)       total-pkts  RX CRC errors  RX trunk errors  RX no descriptor errors  RX abort errors
         0                0                0              0                0                        0                0
^Z
[1]+  Stopped                 ./zfsink -s 224.1.2.3:12345
[server1:static]$ fg
ZF_ATTR="interface=enp1s0f0" ./zfsink -s 224.1.2.3:12345
         0                0                0              0                0                      496                0
       504                0              504              0                0                        0                0
         0                0              504              0                0                        0                0
^C
 # running on a bond with burst on first then second intf. Output changed to show one line per intf.
[server1:static]$ ZF_ATTR="interface=bond0" ./zfsink -s 224.1.2.3:12345
# pkt-rate  bandwidth(Mbps)       total-pkts
#  RX CRC errors RX trunk errors RX no descriptor errors RX abort errors
         0                0                0
              0                0                        0                0
              0                0                        0                0
         0                0                0
              0                0                        0                0
              0                0                        0                0
       744                0              744
              0                0                      256                0
              0                0                        0                0
         0                0              744
              0                0                        0                0
              0                0                        0                0
       760                0             1504
              0                0                        0                0
              0                0                      240                0
         0                0             1504
              0                0                        0                0
              0                0                        0                0
         0                0             1504
              0                0                        0                0
              0                0                        0                0
^C
[server1:static]$

[ligallag-amd]

I think you need to make static method which does the following.

pthread_mutex_lock(&printf_mutex);
printf(...)
pthread_mutex_unlock(&printf_mutex);

There is too much duplicated code. Please update #50 to do this also.

[pemberso-xilinx]

If the intention is to always mirror the equivalent ef_vi data structures with same fields (and you want them to have their own name), I suggest just typedef'ing original structures to a new name.

However, you could just use the original ef_vi ones. There is a precedent in use of ef_vi_transmit_alt_overhead as part of TCPDirect API.

[rdrinkwa-xilinx]

When it's used with ef_vi_transmit_alt_overhead that is because the user application will be making an ef_vi call. That's not the case here and I wanted to avoid having a user application pull in vi.h just to get two structure definitions.

[ligallag-amd]

I think I agree with Paul here. The typedef will achieve the same thing. Perhaps it would be useful to do something like this if you wanted to change the struct field names.

Also, typedef-ing shouldn't mean the user needs to pull any more headers than they already have since zf_stats.c will include vi.h (if that's what contains ef_vi_stats_layout)

[pemberso-xilinx]

I was confused by passing empty stats data into a function name ..._print(). I would pull call to get the stats here before the print.

[pemberso-xilinx]

coding style doesn't use braces for if condition containing single statement

[pemberso-xilinx]

Think this should have layout in name as doesn't contains stats.
could perhaps just call this zf_stats_layout and name the data structures above zf_nic_stats_...

[rdrinkwa-xilinx]

The zf_stats_field_layout and zf_stats_layout relate directly to the ef_vi_... ones of the same name. I've updated this to zf_layout_collection, since that is what it's mostly used for.

[pemberso-xilinx]

It would be cleaner to move the zf_stats_query outside of the printf lock as it doesn't need it

[ligallag-amd]

I think I agree with Paul here. The typedef will achieve the same thing. Perhaps it would be useful to do something like this if you wanted to change the struct field names.

Also, typedef-ing shouldn't mean the user needs to pull any more headers than they already have since zf_stats.c will include vi.h (if that's what contains ef_vi_stats_layout)

[pemberso-xilinx]

The fact this doesn't include stats in the name bothers me slightly. Wondering if you could name it zf_vi_stats (for consistency with onload) or possibly zf_nic_stats or zf_interface_stats with appropriate renaming elsewhere (e.g. zf_alloc_vi_stats)?