| 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
| ||
|
Message-ID: <Z2WE3u2xrBw8XYxr@lzaremba-mobl.ger.corp.intel.com>
Date: Fri, 20 Dec 2024 15:53:18 +0100
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: <davem@...emloft.net>, <netdev@...r.kernel.org>, <edumazet@...gle.com>,
<pabeni@...hat.com>
Subject: Re: [PATCH net-next 01/10] eth: fbnic: reorder ethtool code
On Thu, Dec 19, 2024 at 06:52:32PM -0800, Jakub Kicinski wrote:
> Define ethtool callback handlers in order in which they are defined
> in the ops struct. It doesn't really matter what the order is,
> but it's good to have an order.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 160 +++++++++---------
> 1 file changed, 80 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> index cc8ca94529ca..777e083acae9 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> @@ -40,6 +40,68 @@ static const struct fbnic_stat fbnic_gstrings_hw_stats[] = {
> #define FBNIC_HW_FIXED_STATS_LEN ARRAY_SIZE(fbnic_gstrings_hw_stats)
> #define FBNIC_HW_STATS_LEN FBNIC_HW_FIXED_STATS_LEN
>
> +static void
I thought type and name on separate lines are not desirable, it could be moved
to a single line in this commit. Assuming such adjustment,
Reviewed-by: Larysa Zaremba <larysa.zaremba@...el.com>
Also, this would be a little bit out of scope for this commit, but seeing
relatively new code that does not use `for (int i = 0,...)` is surprising.
> +fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
> +{
> + struct fbnic_net *fbn = netdev_priv(netdev);
> + struct fbnic_dev *fbd = fbn->fbd;
> +
> + fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
> + sizeof(drvinfo->fw_version));
> +}
> +
> +static int fbnic_get_regs_len(struct net_device *netdev)
> +{
> + struct fbnic_net *fbn = netdev_priv(netdev);
> +
> + return fbnic_csr_regs_len(fbn->fbd) * sizeof(u32);
> +}
> +
> +static void fbnic_get_regs(struct net_device *netdev,
> + struct ethtool_regs *regs, void *data)
> +{
> + struct fbnic_net *fbn = netdev_priv(netdev);
> +
> + fbnic_csr_get_regs(fbn->fbd, data, ®s->version);
> +}
> +
> +static void fbnic_get_strings(struct net_device *dev, u32 sset, u8 *data)
> +{
> + int i;
> +
> + switch (sset) {
> + case ETH_SS_STATS:
> + for (i = 0; i < FBNIC_HW_STATS_LEN; i++)
> + ethtool_puts(&data, fbnic_gstrings_hw_stats[i].string);
> + break;
> + }
> +}
> +
> +static void fbnic_get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct fbnic_net *fbn = netdev_priv(dev);
> + const struct fbnic_stat *stat;
> + int i;
> +
> + fbnic_get_hw_stats(fbn->fbd);
> +
> + for (i = 0; i < FBNIC_HW_STATS_LEN; i++) {
> + stat = &fbnic_gstrings_hw_stats[i];
> + data[i] = *(u64 *)((u8 *)&fbn->fbd->hw_stats + stat->offset);
> + }
> +}
> +
> +static int fbnic_get_sset_count(struct net_device *dev, int sset)
> +{
> + switch (sset) {
> + case ETH_SS_STATS:
> + return FBNIC_HW_STATS_LEN;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> static int
> fbnic_get_ts_info(struct net_device *netdev,
> struct kernel_ethtool_ts_info *tsinfo)
> @@ -69,14 +131,27 @@ fbnic_get_ts_info(struct net_device *netdev,
> return 0;
> }
>
> -static void
> -fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
> +static void fbnic_get_ts_stats(struct net_device *netdev,
> + struct ethtool_ts_stats *ts_stats)
> {
> struct fbnic_net *fbn = netdev_priv(netdev);
> - struct fbnic_dev *fbd = fbn->fbd;
> + u64 ts_packets, ts_lost;
> + struct fbnic_ring *ring;
> + unsigned int start;
> + int i;
>
> - fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
> - sizeof(drvinfo->fw_version));
> + ts_stats->pkts = fbn->tx_stats.ts_packets;
> + ts_stats->lost = fbn->tx_stats.ts_lost;
> + for (i = 0; i < fbn->num_tx_queues; i++) {
> + ring = fbn->tx[i];
> + do {
> + start = u64_stats_fetch_begin(&ring->stats.syncp);
> + ts_packets = ring->stats.ts_packets;
> + ts_lost = ring->stats.ts_lost;
> + } while (u64_stats_fetch_retry(&ring->stats.syncp, start));
> + ts_stats->pkts += ts_packets;
> + ts_stats->lost += ts_lost;
> + }
> }
>
> static void fbnic_set_counter(u64 *stat, struct fbnic_stat_counter *counter)
> @@ -85,43 +160,6 @@ static void fbnic_set_counter(u64 *stat, struct fbnic_stat_counter *counter)
> *stat = counter->value;
> }
>
> -static void fbnic_get_strings(struct net_device *dev, u32 sset, u8 *data)
> -{
> - int i;
> -
> - switch (sset) {
> - case ETH_SS_STATS:
> - for (i = 0; i < FBNIC_HW_STATS_LEN; i++)
> - ethtool_puts(&data, fbnic_gstrings_hw_stats[i].string);
> - break;
> - }
> -}
> -
> -static int fbnic_get_sset_count(struct net_device *dev, int sset)
> -{
> - switch (sset) {
> - case ETH_SS_STATS:
> - return FBNIC_HW_STATS_LEN;
> - default:
> - return -EOPNOTSUPP;
> - }
> -}
> -
> -static void fbnic_get_ethtool_stats(struct net_device *dev,
> - struct ethtool_stats *stats, u64 *data)
> -{
> - struct fbnic_net *fbn = netdev_priv(dev);
> - const struct fbnic_stat *stat;
> - int i;
> -
> - fbnic_get_hw_stats(fbn->fbd);
> -
> - for (i = 0; i < FBNIC_HW_STATS_LEN; i++) {
> - stat = &fbnic_gstrings_hw_stats[i];
> - data[i] = *(u64 *)((u8 *)&fbn->fbd->hw_stats + stat->offset);
> - }
> -}
> -
> static void
> fbnic_get_eth_mac_stats(struct net_device *netdev,
> struct ethtool_eth_mac_stats *eth_mac_stats)
> @@ -164,44 +202,6 @@ fbnic_get_eth_mac_stats(struct net_device *netdev,
> &mac_stats->eth_mac.FrameTooLongErrors);
> }
>
> -static void fbnic_get_ts_stats(struct net_device *netdev,
> - struct ethtool_ts_stats *ts_stats)
> -{
> - struct fbnic_net *fbn = netdev_priv(netdev);
> - u64 ts_packets, ts_lost;
> - struct fbnic_ring *ring;
> - unsigned int start;
> - int i;
> -
> - ts_stats->pkts = fbn->tx_stats.ts_packets;
> - ts_stats->lost = fbn->tx_stats.ts_lost;
> - for (i = 0; i < fbn->num_tx_queues; i++) {
> - ring = fbn->tx[i];
> - do {
> - start = u64_stats_fetch_begin(&ring->stats.syncp);
> - ts_packets = ring->stats.ts_packets;
> - ts_lost = ring->stats.ts_lost;
> - } while (u64_stats_fetch_retry(&ring->stats.syncp, start));
> - ts_stats->pkts += ts_packets;
> - ts_stats->lost += ts_lost;
> - }
> -}
> -
> -static void fbnic_get_regs(struct net_device *netdev,
> - struct ethtool_regs *regs, void *data)
> -{
> - struct fbnic_net *fbn = netdev_priv(netdev);
> -
> - fbnic_csr_get_regs(fbn->fbd, data, ®s->version);
> -}
> -
> -static int fbnic_get_regs_len(struct net_device *netdev)
> -{
> - struct fbnic_net *fbn = netdev_priv(netdev);
> -
> - return fbnic_csr_regs_len(fbn->fbd) * sizeof(u32);
> -}
> -
> static const struct ethtool_ops fbnic_ethtool_ops = {
> .get_drvinfo = fbnic_get_drvinfo,
> .get_regs_len = fbnic_get_regs_len,
> --
> 2.47.1
>
>
Powered by blists - more mailing lists