| 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
| ||
|
Date: Fri, 23 Aug 2013 19:08:39 +0200
From: Stefan Assmann <sassmann@...nic.de>
To: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
CC: davem@...emloft.net, Jesse Brandeburg <jesse.brandeburg@...el.com>,
netdev@...r.kernel.org, gospo@...hat.com,
Shannon Nelson <shannon.nelson@...el.com>,
PJ Waskiewicz <peter.p.waskiewicz.jr@...el.com>,
e1000-devel@...ts.sourceforge.net
Subject: Re: [net-next v2 3/8] i40e: driver ethtool core
On 23.08.2013 04:15, Jeff Kirsher wrote:
> From: Jesse Brandeburg <jesse.brandeburg@...el.com>
>
> This patch contains the ethtool interface and implementation.
>
> The goal in this patch series is minimal functionality while not
> including much in the way of "set support."
[...]
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
[...]
> +#define I40E_QUEUE_STATS_LEN(n) \
> + ((((struct i40e_netdev_priv *)netdev_priv((n)))->vsi->num_queue_pairs + \
> + ((struct i40e_netdev_priv *)netdev_priv((n)))->vsi->num_queue_pairs) * 2)
> +#define I40E_GLOBAL_STATS_LEN ARRAY_SIZE(i40e_gstrings_stats)
> +#define I40E_NETDEV_STATS_LEN ARRAY_SIZE(i40e_gstrings_net_stats)
> +#define I40E_VSI_STATS_LEN(n) (I40E_NETDEV_STATS_LEN + \
Please use tabs for spacing here.
> + I40E_QUEUE_STATS_LEN((n)))
> +#define I40E_PFC_STATS_LEN ( \
> + (FIELD_SIZEOF(struct i40e_pf, stats.priority_xoff_rx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_rx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xoff_tx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_tx) + \
> + FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_2_xoff)) \
> + / sizeof(u64))
> +#define I40E_PF_STATS_LEN(n) (I40E_GLOBAL_STATS_LEN + \
Here as well.
[...]
> +static void i40e_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
> + void *p)
> +{
> + struct i40e_netdev_priv *np = netdev_priv(netdev);
> + struct i40e_pf *pf = np->vsi->back;
> + struct i40e_hw *hw = &pf->hw;
> + u32 *reg_buf = p;
> + int i, j, ri;
> + u32 reg;
> +
> + /* Tell ethtool which driver-version-specific regs output we have.
> + *
> + * At some point, if we have ethtool doing special formatting of
> + * this data, it will rely on this version number to know how to
> + * interpret things. Hence, this needs to be updated if/when the
> + * diags register table is changed.
> + */
> + regs->version = 1;
> +
> + /* loop through the diags reg table for what to print */
> + ri = 0;
> + for (i = 0; i40e_reg_list[i].offset != 0; i++) {
> + for (j = 0; j < i40e_reg_list[i].elements; j++) {
> + reg = i40e_reg_list[i].offset
> + + (j * i40e_reg_list[i].stride);
> + reg_buf[ri++] = rd32(hw, reg);
> + }
> + }
> +
> + return;
void function, no return necessary.
> +}
[...]
> +static void i40e_get_ethtool_stats(struct net_device *netdev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct i40e_netdev_priv *np = netdev_priv(netdev);
> + struct i40e_vsi *vsi = np->vsi;
> + struct i40e_pf *pf = vsi->back;
> + struct rtnl_link_stats64 *net_stats = i40e_get_vsi_stats_struct(vsi);
> + char *p;
> + int i, j;
> +
> + i40e_update_stats(vsi);
> +
> + i = 0;
This could be avoided by int i = 0 few lines above.
> + for (j = 0; j < I40E_NETDEV_STATS_LEN; j++) {
> + p = (char *)net_stats + i40e_gstrings_net_stats[j].stat_offset;
> + data[i++] = (i40e_gstrings_net_stats[j].sizeof_stat ==
> + sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
> + }
> + for (j = 0; j < vsi->num_queue_pairs; j++) {
> + data[i++] = vsi->tx_rings[j].tx_stats.packets;
> + data[i++] = vsi->tx_rings[j].tx_stats.bytes;
> + }
> + for (j = 0; j < vsi->num_queue_pairs; j++) {
> + data[i++] = vsi->rx_rings[j].rx_stats.packets;
> + data[i++] = vsi->rx_rings[j].rx_stats.bytes;
> + }
> + if (vsi == pf->vsi[pf->lan_vsi]) {
> + for (j = 0; j < I40E_GLOBAL_STATS_LEN; j++) {
> + p = (char *)pf + i40e_gstrings_stats[j].stat_offset;
> + data[i++] = (i40e_gstrings_stats[j].sizeof_stat ==
> + sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++) {
> + data[i++] = pf->stats.priority_xon_tx[j];
> + data[i++] = pf->stats.priority_xoff_tx[j];
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++) {
> + data[i++] = pf->stats.priority_xon_rx[j];
> + data[i++] = pf->stats.priority_xoff_rx[j];
> + }
> + for (j = 0; j < I40E_MAX_USER_PRIORITY; j++)
> + data[i++] = pf->stats.priority_xon_2_xoff[j];
> + }
> +
> + return;
Another void function.
[...]
> +static struct ethtool_ops i40e_ethtool_ops = {
> + .get_settings = i40e_get_settings,
> + .get_drvinfo = i40e_get_drvinfo,
> + .get_regs_len = i40e_get_regs_len,
> + .get_regs = i40e_get_regs,
> + .nway_reset = i40e_nway_reset,
> + .get_link = ethtool_op_get_link,
> + .get_wol = i40e_get_wol,
> + .get_ringparam = i40e_get_ringparam,
> + .set_ringparam = i40e_set_ringparam,
> + .get_pauseparam = i40e_get_pauseparam,
> + .get_msglevel = i40e_get_msglevel,
> + .set_msglevel = i40e_set_msglevel,
> + .get_rxnfc = i40e_get_rxnfc,
> + .set_rxnfc = i40e_set_rxnfc,
> + .self_test = i40e_diag_test,
> + .get_strings = i40e_get_strings,
> + .set_phys_id = i40e_set_phys_id,
> + .get_sset_count = i40e_get_sset_count,
> + .get_ethtool_stats = i40e_get_ethtool_stats,
> + .get_coalesce = i40e_get_coalesce,
> + .set_coalesce = i40e_set_coalesce,
> + .get_ts_info = i40e_get_ts_info,
> +};
It would be nice if you could use tabs for spacing here.
Stefan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists