From patchwork Fri Feb 26 13:56:43 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Garzik X-Patchwork-Id: 46330 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 2B7A1B7CFB for ; Sat, 27 Feb 2010 00:56:53 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964784Ab0BZN4r (ORCPT ); Fri, 26 Feb 2010 08:56:47 -0500 Received: from mail-yw0-f197.google.com ([209.85.211.197]:46054 "EHLO mail-yw0-f197.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S936217Ab0BZN4q (ORCPT ); Fri, 26 Feb 2010 08:56:46 -0500 Received: by ywh35 with SMTP id 35so42466ywh.4 for ; Fri, 26 Feb 2010 05:56:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type; bh=Ux49aPD2Whm5tdjt9uBYSwHj9k0+Do0jajgu9Z8gg4w=; b=i5DYGiKyZ99D6JhuE2YfU6Vt57u5Ds39dOnAz/nLUoO3yXU7faVWHogSz8da5iZX1h FwzOI0tuQoc0Cp+wAV14C+4dg3fenIHAQL7ih09gCUWFsGeNufd++WpgF+opZfivHdkG h+41E1s56n9wcfNfQP8bF3ixtGyRAV27FQ2c4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type; b=trmE+W9yhGxjrey3ml9Fh8rshrayy/HwYaQloJretejdVggi/KLICGbLGqInOdVIpE ZMkHgAIPRaIf1BEO5pZZ8r9G8AErjPHnSd+P7C5+Im8KtSw7MJGj3133V8eHL2HMXF7t nZRdCyK14F1Er3DgvuEIpZ1j8MY2aMF6kM0ik= Received: by 10.101.56.7 with SMTP id i7mr583062ank.227.1267192605196; Fri, 26 Feb 2010 05:56:45 -0800 (PST) Received: from bd.yyz.us (cpe-069-134-158-197.nc.res.rr.com [69.134.158.197]) by mx.google.com with ESMTPS id 14sm74246gxk.11.2010.02.26.05.56.43 (version=TLSv1/SSLv3 cipher=RC4-MD5); Fri, 26 Feb 2010 05:56:44 -0800 (PST) Message-ID: <4B87D31B.4000001@garzik.org> Date: Fri, 26 Feb 2010 08:56:43 -0500 From: Jeff Garzik User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.7) Gecko/20100120 Fedora/3.0.1-1.fc11 Thunderbird/3.0.1 MIME-Version: 1.0 To: Jeff Kirsher CC: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com, Peter P Waskiewicz Jr Subject: Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo and return it References: <20100226115355.20213.59254.stgit@localhost.localdomain> In-Reply-To: <20100226115355.20213.59254.stgit@localhost.localdomain> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 02/26/2010 06:54 AM, Jeff Kirsher wrote: > From: Peter Waskiewicz > > 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 > Signed-off-by: Jeff Kirsher > --- > > 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 diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index f7992a2..04ecf18 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -254,6 +254,17 @@ struct ethtool_gstrings { __u8 data[0]; }; +struct ethtool_sset_info { + __u32 cmd; /* ETHTOOL_GSSET_INFO */ + __u32 reserved; + __u64 sset_mask; /* input: each bit selects an sset to query */ + /* output: each bit a returned sset */ + __u32 data[0]; /* ETH_SS_xxx count, in order, based on bits + in sset_mask. One bit implies one + __u32, two bits implies two + __u32's, etc. */ +}; + enum ethtool_test_flags { ETH_TEST_FL_OFFLINE = (1 << 0), /* online / offline */ ETH_TEST_FL_FAILED = (1 << 1), /* test passed / failed */ @@ -607,9 +618,9 @@ struct ethtool_ops { #define ETHTOOL_SRXCLSRLINS 0x00000032 /* Insert RX classification rule */ #define ETHTOOL_FLASHDEV 0x00000033 /* Flash firmware to device */ #define ETHTOOL_RESET 0x00000034 /* Reset hardware */ - -#define ETHTOOL_SRXNTUPLE 0x00000035 /* Add an n-tuple filter to device */ -#define ETHTOOL_GRXNTUPLE 0x00000036 /* Get n-tuple filters from device */ +#define ETHTOOL_SRXNTUPLE 0x00000035 /* Add an n-tuple filter to device */ +#define ETHTOOL_GRXNTUPLE 0x00000036 /* Get n-tuple filters from device */ +#define ETHTOOL_GSSET_INFO 0x00000037 /* Get string set info */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSET diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 1c94f48..c019f92 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -241,6 +241,69 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use /* * noinline attribute so that gcc doesnt use too much stack in dev_ethtool() */ +static noinline int ethtool_get_sset_info(struct net_device *dev, void __user *useraddr) +{ + struct ethtool_sset_info info; + const struct ethtool_ops *ops = dev->ethtool_ops; + u64 sset_mask; + int i, idx = 0, n_bits = 0, ret, rc; + u32 *info_buf = NULL; + + if (!ops->get_sset_count) + return -EOPNOTSUPP; + + if (copy_from_user(&info, useraddr, sizeof(info))) + return -EFAULT; + + /* store copy of mask, because we zero struct later on */ + sset_mask = info.sset_mask; + if (!sset_mask) + return 0; + + /* calculate size of return buffer */ + for (i = 0; i < 64; i++) + if (sset_mask & (1ULL << i)) + n_bits++; + + memset(&info, 0, sizeof(info)); + info.cmd = ETHTOOL_GSSET_INFO; + + info_buf = kzalloc(n_bits * sizeof(u32), GFP_USER); + if (!info_buf) + return -ENOMEM; + + /* fill return buffer based on input bitmask and successful + * get_sset_count return + */ + for (i = 0; i < 64; i++) { + if (!(sset_mask & (1ULL << i))) + continue; + + rc = ops->get_sset_count(dev, i); + if (rc >= 0) { + info.sset_mask |= (1ULL << i); + info_buf[idx++] = rc; + } + } + + ret = -EFAULT; + if (copy_to_user(useraddr, &info, sizeof(info))) + goto out; + + useraddr += offsetof(struct ethtool_sset_info, data); + if (copy_to_user(useraddr, info_buf, idx * sizeof(u32))) + goto out; + + ret = 0; + +out: + kfree(info_buf); + return ret; +} + +/* + * noinline attribute so that gcc doesnt use too much stack in dev_ethtool() + */ static noinline int ethtool_set_rxnfc(struct net_device *dev, void __user *useraddr) { struct ethtool_rxnfc cmd; @@ -1472,6 +1535,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr) case ETHTOOL_GRXNTUPLE: rc = ethtool_get_rx_ntuple(dev, useraddr); break; + case ETHTOOL_GSSET_INFO: + rc = ethtool_get_sset_info(dev, useraddr); + break; default: rc = -EOPNOTSUPP; }