| 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
| ||
|
Date: Thu, 10 Jul 2014 09:31:31 +0800
From: ethan zhao <ethan.zhao@...cle.com>
To: vaughan <vaughan.cao@...cle.com>
CC: Manish Chopra <manish.chopra@...gic.com>,
David Miller <davem@...emloft.net>,
"balbi@...com" <balbi@...com>, netdev <netdev@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
"sriharsha.devdas@...cle.com" <sriharsha.devdas@...cle.com>,
"ethan.kernel@...il.com" <ethan.kernel@...il.com>
Subject: Re: [PATCH NET-NEXT V2] netxen: fix ethtool rx_dropped information
in ethtool get_ethtool_stats()
On 2014/7/9 19:45, vaughan wrote:
> On 07/09/2014 07:32 PM, Manish Chopra wrote:
>>> -----Original Message-----
>>> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
>>> On Behalf Of ethan zhao
>>> Sent: Wednesday, July 09, 2014 8:11 AM
>>> To: David Miller
>>> Cc: balbi@...com; netdev; linux-kernel; sriharsha.devdas@...cle.com;
>>> ethan.kernel@...il.com; vaughan
>>> Subject: Re: [PATCH NET-NEXT V2] netxen: fix ethtool rx_dropped information in
>>> ethtool get_ethtool_stats()
>>>
>>> David,
>>>
>>> Please help to review and confirm this patch.
>>>
>>> Thanks,
>>> Ethan
>>>
>>> On 2014/7/8 21:57, Ethan Zhao wrote:
>>>> netxen driver has implemented netxen_nic_get_ethtool_stats()
>>>> interface, but it doesn't collect stats.rxdropped in driver, so we
>>>> will get different rx_dropped statistic information while using ifconfig and
>>> ethtool.
>>>> this patch fills stats.rxdropped field with data from net core with
>>>> dev_get_stats() just as ixgbe driver did for some of its stats.
>>>>
>>>> V2: only fix the stats.rxdropped field.
>>>>
>>>> Tested with last netxen 4.0.82
>>>> Compiled with net-next branch 3.16-rc2
>>>>
>>>> Signed-off-by: Ethan Zhao <ethan.zhao@...cle.com>
>>>> Tested-by: Sriharsha Yadagudde <sriharsha.devdas@...cle.com>
>>>> ---
>>>> .../ethernet/qlogic/netxen/netxen_nic_ethtool.c | 34 +++++++++++++++++-
>>> --
>>>> 1 files changed, 29 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c
>>>> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c
>>>> index 4ca2c19..49e6a1b 100644
>>>> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c
>>>> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ethtool.c
>>>> @@ -33,22 +33,30 @@
>>>> #include "netxen_nic.h"
>>>> #include "netxen_nic_hw.h"
>>>>
>>>> +enum {NETDEV_STATS, NETXEN_STATS};
>>>> +
>>>> struct netxen_nic_stats {
>>>> char stat_string[ETH_GSTRING_LEN];
>>>> + int type;
>>>> int sizeof_stat;
>>>> int stat_offset;
>>>> };
>>>>
>>>> -#define NETXEN_NIC_STAT(m) sizeof(((struct netxen_adapter *)0)->m), \
>>>> +#define NETXEN_NIC_STAT(m) NETXEN_STATS, \
>>>> + sizeof(((struct netxen_adapter *)0)->m), \
>>>> offsetof(struct netxen_adapter, m)
>>>>
>>>> +#define NETXEN_NETDEV_STAT(m) NETDEV_STATS, \
>>>> + sizeof(((struct rtnl_link_stats64 *)0)->m), \
>>>> + offsetof(struct rtnl_link_stats64, m)
>>>> +
>>>> #define NETXEN_NIC_PORT_WINDOW 0x10000
>>>> #define NETXEN_NIC_INVALID_DATA 0xDEADBEEF
>>>>
>>>> static const struct netxen_nic_stats netxen_nic_gstrings_stats[] = {
>>>> {"xmit_called", NETXEN_NIC_STAT(stats.xmitcalled)},
>>>> {"xmit_finished", NETXEN_NIC_STAT(stats.xmitfinished)},
>>>> - {"rx_dropped", NETXEN_NIC_STAT(stats.rxdropped)},
>>>> + {"rx_dropped", NETXEN_NETDEV_STAT(rx_dropped)},
>>>> {"tx_dropped", NETXEN_NIC_STAT(stats.txdropped)},
>>>> {"csummed", NETXEN_NIC_STAT(stats.csummed)},
>>>> {"rx_pkts", NETXEN_NIC_STAT(stats.rx_pkts)}, @@ -679,11 +687,27 @@
>>>> netxen_nic_get_ethtool_stats(struct net_device *dev,
>>>> {
>>>> struct netxen_adapter *adapter = netdev_priv(dev);
>>>> int index;
>>>> + struct rtnl_link_stats64 temp;
>>>> + const struct rtnl_link_stats64 *net_stats;
>>>> + char *p = NULL;
>>>>
>>>> + net_stats = dev_get_stats(dev, &temp);
>>>> for (index = 0; index < NETXEN_NIC_STATS_LEN; index++) {
>>>> - char *p =
>>>> - (char *)adapter +
>>>> - netxen_nic_gstrings_stats[index].stat_offset;
>>>> +
>>>> + switch (netxen_nic_gstrings_stats[index].type) {
>>>> + case NETDEV_STATS:
>>>> + p = (char *)net_stats +
>>>> + netxen_nic_gstrings_stats[index].stat_offset;
>>>> + break;
>>>> + case NETXEN_STATS:
>>>> + p = (char *)adapter +
>>>> + netxen_nic_gstrings_stats[index].stat_offset;
>>>> + break;
>>>> + default:
>>>> + data[index] = 0;
>>>> + continue;
>>>> + }
>>>> +
>>>> data[index] =
>>>> (netxen_nic_gstrings_stats[index].sizeof_stat ==
>>>> sizeof(u64)) ? *(u64 *) p : *(u32 *) p;
>> Ethan, I can't download this patch as I don’t see it in netdev patchwork. Just looking at the diff I think there is style issue with macro definition of NETXEN_NIC_STAT and NETXEN_NETDEV_STAT.
>> Enclose macro definition in parentheses as they are containing complex values.
> It's not an error as checkpatch.pl indicates. As the definition of
> netxen_nic_stats shows,
>
> struct netxen_nic_stats {
> char stat_string[ETH_GSTRING_LEN];
> + int type;
> int sizeof_stat;
> int stat_offset;
> };
>
> This macros consists of multiple elements, which shouldn't be enclosed
> in parentheses.
Agree, I am sure it is error of checkpatch.pl.
Thanks,
Ethan
>> Send this patch targeted for net instead of net-next being a bug fix. Please also run checkpatch.pl to correct any style warning/errors in the patch.
>>
>> Thanks!!
>>
--
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