netdev - Re: [PATCH net-next 7/8] ixgbe: add pause frame stats

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKgT0UccY586mhxRjf+W5gKZdhDMOCXW=p+reEivPnqyFryUbQ@mail.gmail.com>
Date:   Fri, 11 Sep 2020 14:12:50 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>,
        Michal Kubecek <mkubecek@...e.cz>,
        Michael Chan <michael.chan@...adcom.com>, tariqt@...dia.com,
        saeedm@...dia.com, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH net-next 7/8] ixgbe: add pause frame stats

On Fri, Sep 11, 2020 at 12:53 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> Report standard pause frame stats. They are already aggregated
> in struct ixgbe_hw_stats.
>
> The combination of the registers is suggested as equivalent to
> PAUSEMACCtrlFramesTransmitted / PAUSEMACCtrlFramesReceived
> by the Intel 82576EB datasheet, I could not find any information
> in the HW actually supported by ixgbe.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index 71ec908266a6..a280aa34ca1d 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -531,6 +531,16 @@ static int ixgbe_set_link_ksettings(struct net_device *netdev,
>         return err;
>  }
>
> +static void ixgbe_get_pause_stats(struct net_device *netdev,
> +                                 struct ethtool_pause_stats *stats)
> +{
> +       struct ixgbe_adapter *adapter = netdev_priv(netdev);
> +       struct ixgbe_hw_stats *hwstats = &adapter->stats;
> +
> +       stats->tx_pause_frames = hwstats->lxontxc + hwstats->lxofftxc;
> +       stats->rx_pause_frames = hwstats->lxonrxc + hwstats->lxoffrxc;
> +}
> +
>  static void ixgbe_get_pauseparam(struct net_device *netdev,
>                                  struct ethtool_pauseparam *pause)
>  {
> @@ -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.

Reviewed-by: Alexander Duyck <alexander.h.duyck@...ux.intel.com>