netdev - Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo and return it

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B87D31B.4000001@garzik.org>
Date:	Fri, 26 Feb 2010 08:56:43 -0500
From:	Jeff Garzik <jeff@...zik.org>
To:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
CC:	davem@...emloft.net, netdev@...r.kernel.org, gospo@...hat.com,
	Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@...el.com>
Subject: Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo
 and return it

On 02/26/2010 06:54 AM, Jeff Kirsher wrote:
> From: Peter Waskiewicz<peter.p.waskiewicz.jr@...el.com>
>
> The drvinfo struct should include the number of strings that
> get_rx_ntuple will return.  It will be variable if an underlying
> driver implements its own get_rx_ntuple routine, so userspace
> needs to know how much data is coming.
>
> Signed-off-by: Peter P Waskiewicz Jr<peter.p.waskiewicz.jr@...el.com>
> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@...el.com>
> ---
>
>   include/linux/ethtool.h |    1 +
>   net/core/ethtool.c      |    3 +++
>   2 files changed, 4 insertions(+), 0 deletions(-)

(resending reply, standard patch-sending box is having trouble sending 
to vger)


As noted in the other email, your patch breaks ABI.  The proper path is
to decrease the size of reserved struct member, and NOT shift the offset
of other members.



However, perhaps consider the following patch for returning n-tuple
count, for four reasons:

1) space in ethtool_drvinfo is limited

2) the patch below permits trivial string set addition, without
    ABI changes beyond adding a new ETH_SS_xxx constant.

3) the patch below permits direct access to ops->get_sset_count(),
    rather than implicit access via ethtool_drvinfo

4) ethtool_drvinfo interface does not permit indication of
    ops->get_sset_count() failure, versus returning zero value.  The
    patch below does so, via output sset_mask.

WARNING: this patch is compile-tested only.

NOTE: I added a cosmetic fix to ETHTOOL_[GS]RXNTUPLE constants, making
their indentation consistent with the rest of the list of constants.

Signed-off-by: Jeff Garzik <jgarzik@...hat.com>



View attachment "patch" of type "text/plain" (3582 bytes)