| Message ID | 1343840975-3252-9-git-send-email-ogerlitz@mellanox.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2012-08-01 at 20:09 +0300, Or Gerlitz wrote: > From: Erez Shitrit <erezsh@mellanox.co.il> > > Via ethtool the driver describes its version, ABI version, on what PIF > interface it runs and various statistics. [...] > --- /dev/null > +++ b/drivers/net/eipoib/eth_ipoib_ethtool.c [...] > +static void parent_ethtool_get_drvinfo(struct net_device *parent_dev, > + struct ethtool_drvinfo *drvinfo) > +{ > + struct parent *parent = netdev_priv(parent_dev); > + > + strncpy(drvinfo->driver, DRV_NAME, 32); > + > + strncpy(drvinfo->version, DRV_VERSION, 32); > + > + strncpy(drvinfo->bus_info, parent->ipoib_main_interface, > + ETHTOOL_BUSINFO_LEN); These must be null-terminated; therefore use strlcpy(). > + /* indicates ABI version */ > + snprintf(drvinfo->fw_version, 32, "%d", EIPOIB_ABI_VER); [...] This is an abuse of fw_version. Ben.
On 8/2/2012 3:22 AM, Ben Hutchings wrote: > On Wed, 2012-08-01 at 20:09 +0300, Or Gerlitz wrote: >> From: Erez Shitrit <erezsh@mellanox.co.il> >> >> Via ethtool the driver describes its version, ABI version, on what PIF >> interface it runs and various statistics. > [...] >> --- /dev/null >> +++ b/drivers/net/eipoib/eth_ipoib_ethtool.c > [...] >> +static void parent_ethtool_get_drvinfo(struct net_device *parent_dev, >> + struct ethtool_drvinfo *drvinfo) >> +{ >> + struct parent *parent = netdev_priv(parent_dev); >> + >> + strncpy(drvinfo->driver, DRV_NAME, 32); >> + >> + strncpy(drvinfo->version, DRV_VERSION, 32); >> + >> + strncpy(drvinfo->bus_info, parent->ipoib_main_interface, >> + ETHTOOL_BUSINFO_LEN); > These must be null-terminated; therefore use strlcpy(). ok, will fix. >> + /* indicates ABI version */ >> + snprintf(drvinfo->fw_version, 32, "%d", EIPOIB_ABI_VER); > [...] > > This is an abuse of fw_version. > > Ben. we took the idea from the bonding driver, (snprintf(drvinfo->fw_version, 32, "%d", BOND_ABI_VERSION);) Do you have any idea where can we keep the abi version? Thanks, Erez -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2012-08-02 at 11:35 +0300, Erez Shitrit wrote: > On 8/2/2012 3:22 AM, Ben Hutchings wrote: > > On Wed, 2012-08-01 at 20:09 +0300, Or Gerlitz wrote: > >> From: Erez Shitrit <erezsh@mellanox.co.il> > >> > >> Via ethtool the driver describes its version, ABI version, on what PIF > >> interface it runs and various statistics. > > [...] > >> --- /dev/null > >> +++ b/drivers/net/eipoib/eth_ipoib_ethtool.c > > [...] > >> +static void parent_ethtool_get_drvinfo(struct net_device *parent_dev, > >> + struct ethtool_drvinfo *drvinfo) > >> +{ > >> + struct parent *parent = netdev_priv(parent_dev); > >> + > >> + strncpy(drvinfo->driver, DRV_NAME, 32); > >> + > >> + strncpy(drvinfo->version, DRV_VERSION, 32); > >> + > >> + strncpy(drvinfo->bus_info, parent->ipoib_main_interface, > >> + ETHTOOL_BUSINFO_LEN); > > These must be null-terminated; therefore use strlcpy(). > > ok, will fix. > > >> + /* indicates ABI version */ > >> + snprintf(drvinfo->fw_version, 32, "%d", EIPOIB_ABI_VER); > > [...] > > > > This is an abuse of fw_version. > > > > Ben. > we took the idea from the bonding driver, > (snprintf(drvinfo->fw_version, 32, "%d", BOND_ABI_VERSION);) The bonding driver has lots of warts. > Do you have any idea where can we keep the abi version? You don't need to, because David will insist that you will only change the ABI in a backward-compatible way. :-) (The bonding ABI version hasn't changed since Linux 2.6.3, and even then it provided backward compatibility.) Ben.
diff --git a/drivers/net/eipoib/eth_ipoib_ethtool.c b/drivers/net/eipoib/eth_ipoib_ethtool.c new file mode 100644 index 0000000..0731d94 --- /dev/null +++ b/drivers/net/eipoib/eth_ipoib_ethtool.c @@ -0,0 +1,114 @@ +/* + * Copyright (c) 2012 Mellanox Technologies. All rights reserved + * + * This software is available to you under a choice of one of two + * licenses. You may choose to be licensed under the terms of the GNU + * General Public License (GPL) Version 2, available from the file + * COPYING in the main directory of this source tree, or the + * openfabric.org BSD license below: + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above + * copyright notice, this list of conditions and the following + * disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#include "eth_ipoib.h" + +static void parent_ethtool_get_drvinfo(struct net_device *parent_dev, + struct ethtool_drvinfo *drvinfo) +{ + struct parent *parent = netdev_priv(parent_dev); + + strncpy(drvinfo->driver, DRV_NAME, 32); + + strncpy(drvinfo->version, DRV_VERSION, 32); + + strncpy(drvinfo->bus_info, parent->ipoib_main_interface, + ETHTOOL_BUSINFO_LEN); + + /* indicates ABI version */ + snprintf(drvinfo->fw_version, 32, "%d", EIPOIB_ABI_VER); +} + +static const char parent_strings[][ETH_GSTRING_LEN] = { + /* private statistics */ + "tx_parent_dropped", + "tx_vif_miss", + "tx_neigh_miss", + "tx_vlan", + "tx_shared", + "tx_proto_errors", + "tx_skb_errors", + "tx_slave_err", + + "rx_parent_dropped", + "rx_vif_miss", + "rx_neigh_miss", + "rx_vlan", + "rx_shared", + "rx_proto_errors", + "rx_skb_errors", + "rx_slave_err", +}; + +#define PORT_STATS_LEN (sizeof(parent_strings) / ETH_GSTRING_LEN) + +static void parent_get_strings(struct net_device *parent_dev, + uint32_t stringset, uint8_t *data) +{ + if (stringset != ETH_SS_STATS) + return; + memcpy(data, parent_strings, sizeof(parent_strings)); +} + +static void parent_get_ethtool_stats(struct net_device *parent_dev, + struct ethtool_stats *stats, + uint64_t *data) +{ + struct parent *parent = netdev_priv(parent_dev); + + read_lock_bh(&parent->lock); + memcpy(data, &parent->port_stats, sizeof(parent->port_stats)); + read_unlock_bh(&parent->lock); +} + +static int parent_get_sset_count(struct net_device *parent_dev, int sset) +{ + switch (sset) { + case ETH_SS_STATS: + return PORT_STATS_LEN; + default: + return -EOPNOTSUPP; + } +} + +static const struct ethtool_ops parent_ethtool_ops = { + .get_drvinfo = parent_ethtool_get_drvinfo, + .get_strings = parent_get_strings, + .get_ethtool_stats = parent_get_ethtool_stats, + .get_sset_count = parent_get_sset_count, + .get_link = ethtool_op_get_link, +}; + +void parent_set_ethtool_ops(struct net_device *dev) +{ + SET_ETHTOOL_OPS(dev, &parent_ethtool_ops); +}