| 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: <1237240381.3106.79.camel@achroite>
Date: Mon, 16 Mar 2009 21:53:01 +0000
From: Ben Hutchings <bhutchings@...arflare.com>
To: ram.vepa@...erion.com
Cc: Netdev <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>
Subject: Re: [net-2.6 PATCH 9/10] Neterion: New driver: Ethtool related
On Sat, 2009-03-14 at 00:22 -0800, Ramkrishna Vepa wrote:
> This patch implements all ethtool related entry point functions for the driver.
>
> Signed-off-by: Sivakumar Subramani <sivakumar.subramani@...erion.com>
> Signed-off-by: Rastapur Santosh <santosh.rastapur@...erion.com>
> Signed-off-by: Ramkrishna Vepa <ram.vepa@...erion.com>
> ---
> diff -urpN patch_8/drivers/net/vxge/vxge-ethtool.c patch_9/drivers/net/vxge/vxge-ethtool.c
> --- patch_8/drivers/net/vxge/vxge-ethtool.c 1969-12-31 16:00:00.000000000 -0800
> +++ patch_9/drivers/net/vxge/vxge-ethtool.c 2009-03-13 00:15:35.000000000 -0700
[...
> +/*
> + * vxge_ethtool_sset - Sets different link parameters.
> + * @dev: device pointer.
> + * @info: pointer to the structure with parameters given by ethtool to set
> + * link information.
> + *
> + * The function sets different link parameters provided by the user onto
> + * the NIC.
> + * Return value:
> + * 0 on success.
> + */
> +
> +static int vxge_ethtool_sset(struct net_device *dev, struct ethtool_cmd *info)
> +{
> + if ((info->autoneg == AUTONEG_ENABLE) ||
> + (info->speed != SPEED_10000) || (info->duplex != DUPLEX_FULL))
> + return -EINVAL;
> +
> + if (netif_running(dev)) {
> + vxge_close(dev);
> + vxge_open(dev);
Why?
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * vxge_ethtool_gset - Return link specific information.
> + * @dev: device pointer.
> + * @info: pointer to the structure with parameters given by ethtool
> + * to return link information.
> + *
> + * Returns link specific information like speed, duplex etc.. to ethtool.
> + * Return value :
> + * return 0 on success.
> + */
> +int vxge_ethtool_gset(struct net_device *dev, struct ethtool_cmd
> +*info)
> +{
> + info->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE);
> +
> +#ifdef ADVERTISED_10000baseT_Full
> + info->advertising = ADVERTISED_10000baseT_Full;
> +#else
> + info->advertising = SUPPORTED_10000baseT_Full;
> +#endif
> +
> +#ifdef ADVERTISED_FIBRE
> + info->advertising |= ADVERTISED_FIBRE;
> +#else
> + info->advertising |= SUPPORTED_FIBRE;
> +#endif
Do not use #ifdef here. You can be sure that these definitions aren't
going away.
[...]
> +/*
> + * vxge_ethtool_gdrvinfo - Returns driver specific information.
> + * @dev: device pointer.
> + * @info: pointer to the structure with parameters given by ethtool to
> + * return driver information.
> + *
> + * Returns driver specefic information like name, version etc.. to ethtool.
> + */
> +static void vxge_ethtool_gdrvinfo(struct net_device *dev,
> + struct ethtool_drvinfo *info)
> +{
> + struct vxgedev *vdev;
> + char version[80];
> + vdev = (struct vxgedev *)netdev_priv(dev);
> + sprintf(version, "%s", DRV_VERSION);
What is the point of this intermediate copy?
> + strncpy(info->driver, VXGE_DRIVER_NAME, sizeof(VXGE_DRIVER_NAME));
> + strncpy(info->version, version, sizeof(version));
> + strncpy(info->fw_version, vdev->fw_version, VXGE_HW_FW_VERSION_LEN);
> + strncpy(info->bus_info, pci_name(vdev->pdev), sizeof(info->bus_info));
Use strlcpy() to ensure null-termination.
[...]
> +static int vxge_ethtool_idnic(struct net_device *dev, u32 data)
> +{
> + u32 port[3] = {0, 1, 2};
> + int i;
> + struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + struct __hw_device *hldev = (struct __hw_device *)
> + pci_get_drvdata(vdev->pdev);
> +
> + if (vdev->id_timer.function == NULL) {
> + init_timer(&vdev->id_timer);
> + vdev->id_timer.function = vxge_phy_id;
> + vdev->id_timer.data = (unsigned long) vdev;
> + }
> + mod_timer(&vdev->id_timer, jiffies);
> + set_current_state(TASK_INTERRUPTIBLE);
> + if (data)
> + schedule_timeout(data * HZ);
> + else
> + schedule_timeout(MAX_SCHEDULE_TIMEOUT);
Just schedule() will do.
[...]
> +static u32 vxge_ethtool_op_get_tso(struct net_device *dev)
> +{
> + return (dev->features & NETIF_F_TSO) != 0;
> +}
This is the same as ethtool_op_get_tso; just point to that.
[...]
> +int vxge_ethtool_self_test_count(struct net_device *dev)
> +{
> + return VXGE_TEST_LEN;
> +}
> +
> +static int vxge_ethtool_get_stats_count(struct net_device *dev)
> +{
> + struct vxgedev *vdev = (struct vxgedev *)netdev_priv(dev);
> + return VXGE_TITLE_LEN + (vdev->no_of_vpath *
> + VXGE_HW_VPATH_STATS_LEN) +
> + (vdev->max_config_port * VXGE_HW_AGGR_STATS_LEN) +
> + (vdev->max_config_port * VXGE_HW_PORT_STATS_LEN) +
> + (vdev->no_of_vpath * VXGE_HW_VPATH_TX_STATS_LEN) +
> + (vdev->no_of_vpath * VXGE_HW_VPATH_RX_STATS_LEN) +
> +
> + (vdev->no_of_vpath * VXGE_SW_STATS_LEN) + DRIVER_STAT_LEN;
> +
> +}
These operations are deprecated; you should implement get_strings_count
instead.
> +int vxge_ethtool_op_set_tx_csum(struct net_device *dev, u32 data)
> +{
> + if (data)
> + dev->features |= NETIF_F_HW_CSUM;
> + else
> + dev->features &= ~NETIF_F_HW_CSUM;
> +
> + return 0;
> +}
This is the same as ethtool_op_set_tx_hw_csum; just point to that.
[...]
> +struct ethtool_ops netdev_ethtool_ops = {
> + .get_settings = vxge_ethtool_gset,
> + .set_settings = vxge_ethtool_sset,
> + .get_drvinfo = vxge_ethtool_gdrvinfo,
> + .get_regs_len = vxge_ethtool_get_regs_len,
> + .get_regs = vxge_ethtool_gregs,
> + .get_link = ethtool_op_get_link,
> +
> + .get_eeprom_len = vxge_get_eeprom_len,
> +
> + .get_eeprom = NULL,
> + .set_eeprom = NULL,
So what is the point of reporting the EEPROM length?
> + .get_pauseparam = vxge_ethtool_getpause_data,
> + .set_pauseparam = vxge_ethtool_setpause_data,
> + .get_rx_csum = vxge_get_rx_csum,
> + .set_rx_csum = vxge_set_rx_csum,
> + .get_tx_csum = ethtool_op_get_tx_csum,
> + .set_tx_csum = vxge_ethtool_op_set_tx_csum,
> + .get_sg = ethtool_op_get_sg,
> + .set_sg = ethtool_op_set_sg,
> +
> + .get_tso = vxge_ethtool_op_get_tso,
> + .set_tso = vxge_ethtool_op_set_tso,
> +
> + .self_test_count = vxge_ethtool_self_test_count,
> + .self_test = NULL,
[...]
Don't claim to have tests you don't actually implement.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
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