Re: [PATCH V2 08/12] net/eipoib: Add ethtool file support

On Thu, 2012-08-02 at 11:35 +0300, Erez Shitrit wrote:

On 8/2/2012 3:22 AM, Ben Hutchings wrote:

quoted

On Wed, 2012-08-01 at 20:09 +0300, Or Gerlitz wrote:

quoted

From: Erez Shitrit <redacted>

Via ethtool the driver describes its version, ABI version, on what PIF
interface it runs and various statistics.

[...]

quoted

--- /dev/null
+++ b/drivers/net/eipoib/eth_ipoib_ethtool.c

[...]

quoted

+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.

quoted

quoted

+	/* 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.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.