Use iflib_if_get_counter_default() to get output dropped packets

[zlei]

Summary

PR: 280386
MFC after: 1 week

Test Plan

Stress test with iperf3, monitor the output dropped packets with netstat -di .

Note: iperf3 -u can overflow the output queue excessively.

[zlei]

mgb(4) employs iflib(4) but does not have ifdi_get_counter method. Not sure how it report network statistics counters. CC @emaste .

[erj]

ixl(4) and ice(4) could integrate this function, too:

ixl(4) needs to remove the unused "vsi->oqdrops" variable and just not handle the IFCOUNTER_OQDROPS case in its switch statement in ixl_if_get_counter(), then call your new iflib_if_get_counter_default().

ice(4) is more complex because it does actually include a stat from the hardware to include in OQDROPS; I'm not sure how to get that added to the iflib stat. Maybe it should be added to the ifnet stat instead of being directly reported by ice_if_get_counter()?

[emaste]

In D46187#1052940, @zlei wrote:

mgb(4) employs iflib(4) but does not have ifdi_get_counter method. Not sure how it report network statistics counters. CC @emaste .

It is possible that it just doesn't, right now.

[zlei]

In D46187#1052954, @erj wrote:

ixl(4) and ice(4) could integrate this function, too:

ixl(4) needs to remove the unused "vsi->oqdrops" variable and just not handle the IFCOUNTER_OQDROPS case in its switch statement in ixl_if_get_counter(), then call your new iflib_if_get_counter_default().

That make senses.

ice(4) is more complex because it does actually include a stat from the hardware to include in OQDROPS; I'm not sure how to get that added to the iflib stat. Maybe it should be added to the ifnet stat instead of being directly reported by ice_if_get_counter()?

I think generally OQDROPS = ( hardware drops ) + ( iflib drops ) + ( ifnet drops ) . ALTQ drops can be integrated into ifnet drops, if ALTQ is enabled. I have not looked throughout but some drivers simply ignore ALTQ drops. I have no idea but I think fixing ALTQ drops is out of scope of this change.

Some driver has admin task that retrieve hardware drops into ifnet stats, then OQDROPS can be simplified to ( iflib drops ) + ( ifnet drops ) .

[zlei]

Update ixl(4), iavf(4), ice(4) to take iflib managed OQDROPS into account.

[zlei]

In D46187#1052954, @erj wrote:

ixl(4) and ice(4) could integrate this function, too:

ixl(4) needs to remove the unused "vsi->oqdrops" variable and just not handle the IFCOUNTER_OQDROPS case in its switch statement in ixl_if_get_counter(), then call your new iflib_if_get_counter_default().

"vsi->oqdrops" is actually in use. It is set via IXL_SET_OQDROPS in function ixl_update_vsi_stats().

ice(4) is more complex because it does actually include a stat from the hardware to include in OQDROPS; I'm not sure how to get that added to the iflib stat. Maybe it should be added to the ifnet stat instead of being directly reported by ice_if_get_counter()?

See the diff. It is quite simple but I do not have devices to do the test ;)

[erj]

In D46187#1053418, @zlei wrote:

In D46187#1052954, @erj wrote:

ixl(4) and ice(4) could integrate this function, too:

ixl(4) needs to remove the unused "vsi->oqdrops" variable and just not handle the IFCOUNTER_OQDROPS case in its switch statement in ixl_if_get_counter(), then call your new iflib_if_get_counter_default().

"vsi->oqdrops" is actually in use. It is set via IXL_SET_OQDROPS in function ixl_update_vsi_stats().

You're right; that's what I get for not looking at the code too closely...

ice(4) is more complex because it does actually include a stat from the hardware to include in OQDROPS; I'm not sure how to get that added to the iflib stat. Maybe it should be added to the ifnet stat instead of being directly reported by ice_if_get_counter()?

See the diff. It is quite simple but I do not have devices to do the test ;)

I think the changes are logically correct.

But now I'm thinking that these driver stats functions should change so that for each counter that counts errors, we return the sum of the hardware stats and the result of iflib_if_get_counter_default() so that we never ignore stats collected in either. But that should be something done in some other patch.