| Message ID | 20200911195258.1048468-1-kuba@kernel.org |
|---|---|
| Headers | show |
| Series | ethtool: add pause frame stats | expand |
On Fri, 11 Sep 2020 14:12:50 -0700 Alexander Duyck wrote: > On Fri, Sep 11, 2020 at 12:53 PM Jakub Kicinski <kuba@kernel.org> wrote: > > @@ -3546,6 +3556,7 @@ static const struct ethtool_ops ixgbe_ethtool_ops = { > > .set_eeprom = ixgbe_set_eeprom, > > .get_ringparam = ixgbe_get_ringparam, > > .set_ringparam = ixgbe_set_ringparam, > > + .get_pause_stats = ixgbe_get_pause_stats, > > .get_pauseparam = ixgbe_get_pauseparam, > > .set_pauseparam = ixgbe_set_pauseparam, > > .get_msglevel = ixgbe_get_msglevel, > > So the count for this is simpler in igb than it is for ixgbe. I'm > assuming you want just standard link flow control frames. If so then > this patch is correct. Otherwise if you are wanting to capture > priority flow control data then those are a seperate array of stats > prefixed with a "p" instead of an "l". Otherwise this looks fine to > me. That's my interpretation, although I haven't found any place the standard would address this directly. Non-PFC pause has a different opcode, so I'm reasonably certain this makes sense. BTW I'm not entirely clear on what "global PFC pause" is either. Maybe someone can clarify? Mellanox folks? > Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com> Thanks!
On Sun, 13 Sep 2020 12:14:14 +0300 Ido Schimmel wrote: > On Fri, Sep 11, 2020 at 03:13:43PM -0700, Jakub Kicinski wrote: > > On Fri, 11 Sep 2020 14:12:50 -0700 Alexander Duyck wrote: > > > On Fri, Sep 11, 2020 at 12:53 PM Jakub Kicinski <kuba@kernel.org> wrote: > > > > @@ -3546,6 +3556,7 @@ static const struct ethtool_ops ixgbe_ethtool_ops = { > > > > .set_eeprom = ixgbe_set_eeprom, > > > > .get_ringparam = ixgbe_get_ringparam, > > > > .set_ringparam = ixgbe_set_ringparam, > > > > + .get_pause_stats = ixgbe_get_pause_stats, > > > > .get_pauseparam = ixgbe_get_pauseparam, > > > > .set_pauseparam = ixgbe_set_pauseparam, > > > > .get_msglevel = ixgbe_get_msglevel, > > > > > > So the count for this is simpler in igb than it is for ixgbe. I'm > > > assuming you want just standard link flow control frames. If so then > > > this patch is correct. Otherwise if you are wanting to capture > > > priority flow control data then those are a seperate array of stats > > > prefixed with a "p" instead of an "l". Otherwise this looks fine to > > > me. > > > > That's my interpretation, although I haven't found any place the > > standard would address this directly. Non-PFC pause has a different > > opcode, so I'm reasonably certain this makes sense. > > > > BTW I'm not entirely clear on what "global PFC pause" is either. > > > > Maybe someone can clarify? Mellanox folks? > > I checked IEEE 802.1Qaz and could not find anything relevant. My only > guess is that it might be a PFC frame with all the priorities set. > > Where did you see it? I think I saw it in MLX5 and I thought it's something implementation-specific. But then I noticed 802.1-2018 has a ieee8021PfcGlobalReqdGroup group.