| Message ID | 20170722220942.78852-8-salil.mehta@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 07/22/2017 03:09 PM, Salil Mehta wrote: > This patch adds the support of the Ethtool interface to > the HNS3 Ethernet driver. Various commands to read the > statistics, configure the offloading, loopback selftest etc. > are supported. > > Signed-off-by: Daode Huang <huangdaode@hisilicon.com> > Signed-off-by: lipeng <lipeng321@huawei.com> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > Signed-off-by: Yisen Zhuang <yisen.zhuang@huawei.com> > --- > Patch V4: addressed below comments > 1. Andrew Lunn > Removed the support of loop PHY back for now > Patch V3: Address below comments > 1. Stephen Hemminger > https://lkml.org/lkml/2017/6/13/974 > 2. Andrew Lunn > https://lkml.org/lkml/2017/6/13/1037 > Patch V2: No change > Patch V1: Initial Submit > --- > .../ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c | 543 +++++++++++++++++++++ > 1 file changed, 543 insertions(+) > create mode 100644 drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > new file mode 100644 > index 000000000000..82b0d4d829f8 > --- /dev/null > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > @@ -0,0 +1,543 @@ > +/* > + * Copyright (c) 2016~2017 Hisilicon Limited. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include <linux/etherdevice.h> > +#include "hns3_enet.h" > + > +struct hns3_stats { > + char stats_string[ETH_GSTRING_LEN]; > + int stats_size; > + int stats_offset; > +}; > + > +/* netdev related stats */ > +#define HNS3_NETDEV_STAT(_string, _member) \ > + { _string, \ > + FIELD_SIZEOF(struct rtnl_link_stats64, _member), \ > + offsetof(struct rtnl_link_stats64, _member), \ > + } Can you make this macro use named initializers? > + > +static const struct hns3_stats hns3_netdev_stats[] = { > + /* misc. Rx/Tx statistics */ > + HNS3_NETDEV_STAT("rx_packets", rx_packets), > + HNS3_NETDEV_STAT("tx_packets", tx_packets), > + HNS3_NETDEV_STAT("rx_bytes", rx_bytes), > + HNS3_NETDEV_STAT("tx_bytes", tx_bytes), > + HNS3_NETDEV_STAT("rx_errors", rx_errors), > + HNS3_NETDEV_STAT("tx_errors", tx_errors), > + HNS3_NETDEV_STAT("rx_dropped", rx_dropped), > + HNS3_NETDEV_STAT("tx_dropped", tx_dropped), > + HNS3_NETDEV_STAT("multicast", multicast), > + HNS3_NETDEV_STAT("collisions", collisions), > + > + /* detailed Rx errors */ > + HNS3_NETDEV_STAT("rx_length_errors", rx_length_errors), > + HNS3_NETDEV_STAT("rx_over_errors", rx_over_errors), > + HNS3_NETDEV_STAT("rx_crc_errors", rx_crc_errors), > + HNS3_NETDEV_STAT("rx_frame_errors", rx_frame_errors), > + HNS3_NETDEV_STAT("rx_fifo_errors", rx_fifo_errors), > + HNS3_NETDEV_STAT("rx_missed_errors", rx_missed_errors), > + > + /* detailed Tx errors */ > + HNS3_NETDEV_STAT("tx_aborted_errors", tx_aborted_errors), > + HNS3_NETDEV_STAT("tx_carrier_errors", tx_carrier_errors), > + HNS3_NETDEV_STAT("tx_fifo_errors", tx_fifo_errors), > + HNS3_NETDEV_STAT("tx_heartbeat_errors", tx_heartbeat_errors), > + HNS3_NETDEV_STAT("tx_window_errors", tx_window_errors), > + > + /* for cslip etc */ > + HNS3_NETDEV_STAT("rx_compressed", rx_compressed), > + HNS3_NETDEV_STAT("tx_compressed", tx_compressed), > +}; > + > +#define HNS3_NETDEV_STATS_COUNT ARRAY_SIZE(hns3_netdev_stats) > + > +/* tqp related stats */ > +#define HNS3_TQP_STAT(_string, _member) \ > + { _string, \ > + FIELD_SIZEOF(struct ring_stats, _member), \ > + offsetof(struct hns3_enet_ring, stats), \ > + } > + Same here. > +static const struct hns3_stats hns3_txq_stats[] = { > + /* Tx per-queue statistics */ > + HNS3_TQP_STAT("tx_io_err_cnt", io_err_cnt), > + HNS3_TQP_STAT("tx_sw_err_cnt", sw_err_cnt), > + HNS3_TQP_STAT("tx_seg_pkt_cnt", seg_pkt_cnt), > + HNS3_TQP_STAT("tx_pkts", tx_pkts), > + HNS3_TQP_STAT("tx_bytes", tx_bytes), > + HNS3_TQP_STAT("tx_err_cnt", tx_err_cnt), > + HNS3_TQP_STAT("tx_restart_queue", restart_queue), > + HNS3_TQP_STAT("tx_busy", tx_busy), > +}; > + > +#define HNS3_TXQ_STATS_COUNT ARRAY_SIZE(hns3_txq_stats) > + > +static const struct hns3_stats hns3_rxq_stats[] = { > + /* Rx per-queue statistics */ > + HNS3_TQP_STAT("rx_io_err_cnt", io_err_cnt), > + HNS3_TQP_STAT("rx_sw_err_cnt", sw_err_cnt), > + HNS3_TQP_STAT("rx_seg_pkt_cnt", seg_pkt_cnt), > + HNS3_TQP_STAT("rx_pkts", rx_pkts), > + HNS3_TQP_STAT("rx_bytes", rx_bytes), > + HNS3_TQP_STAT("rx_err_cnt", rx_err_cnt), > + HNS3_TQP_STAT("rx_reuse_pg_cnt", reuse_pg_cnt), > + HNS3_TQP_STAT("rx_err_pkt_len", err_pkt_len), > + HNS3_TQP_STAT("rx_non_vld_descs", non_vld_descs), > + HNS3_TQP_STAT("rx_err_bd_num", err_bd_num), > + HNS3_TQP_STAT("rx_l2_err", l2_err), > + HNS3_TQP_STAT("rx_l3l4_csum_err", l3l4_csum_err), > +}; > + > +#define HNS3_RXQ_STATS_COUNT ARRAY_SIZE(hns3_rxq_stats) > + > +#define HNS3_TQP_STATS_COUNT (HNS3_TXQ_STATS_COUNT + HNS3_RXQ_STATS_COUNT) > + > +struct hns3_link_mode_mapping { > + u32 hns3_link_mode; > + u32 ethtool_link_mode; > +}; > + > +static const struct hns3_link_mode_mapping hns3_lm_map[] = { > + {HNS3_LM_FIBRE_BIT, ETHTOOL_LINK_MODE_FIBRE_BIT}, > + {HNS3_LM_AUTONEG_BIT, ETHTOOL_LINK_MODE_Autoneg_BIT}, > + {HNS3_LM_TP_BIT, ETHTOOL_LINK_MODE_TP_BIT}, > + {HNS3_LM_PAUSE_BIT, ETHTOOL_LINK_MODE_Pause_BIT}, > + {HNS3_LM_BACKPLANE_BIT, ETHTOOL_LINK_MODE_Backplane_BIT}, > + {HNS3_LM_10BASET_HALF_BIT, ETHTOOL_LINK_MODE_10baseT_Half_BIT}, > + {HNS3_LM_10BASET_FULL_BIT, ETHTOOL_LINK_MODE_10baseT_Full_BIT}, > + {HNS3_LM_100BASET_HALF_BIT, ETHTOOL_LINK_MODE_100baseT_Half_BIT}, > + {HNS3_LM_100BASET_FULL_BIT, ETHTOOL_LINK_MODE_100baseT_Full_BIT}, > + {HNS3_LM_1000BASET_FULL_BIT, ETHTOOL_LINK_MODE_1000baseT_Full_BIT}, > +}; > + > +#define HNS3_DRV_TO_ETHTOOL_CAPS(caps, lk_ksettings, name) \ > +{ \ > + int i; \ > + \ > + for (i = 0; i < ARRAY_SIZE(hns3_lm_map); i++) { \ > + if ((caps) & hns3_lm_map[i].hns3_link_mode) \ > + __set_bit(hns3_lm_map[i].ethtool_link_mode,\ > + (lk_ksettings)->link_modes.name); \ > + } \ > +} How about making this an inline function such that you would get proper type checking? > + > +static int hns3_get_sset_count(struct net_device *netdev, int stringset) > +{ > + struct hns3_nic_priv *priv = netdev_priv(netdev); > + struct hnae3_handle *h = priv->ae_handle; > + const struct hnae3_ae_ops *ops = h->ae_algo->ops; > + > + if (!ops->get_sset_count) { > + netdev_err(netdev, "could not get string set count\n"); > + return -EOPNOTSUPP; > + } Is it really worth the error message? This might be called several times during the driver's lifecycle. > + > + switch (stringset) { > + case ETH_SS_STATS: > + return (HNS3_NETDEV_STATS_COUNT + > + (HNS3_TQP_STATS_COUNT * h->kinfo.num_tqps) + > + ops->get_sset_count(h, stringset)); > + > + case ETH_SS_TEST: > + return ops->get_sset_count(h, stringset); > + } > + > + return 0; > +} > + > +static u8 *hns3_get_strings_netdev(u8 *data) > +{ > + int i; unsigned int i. > + > + for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) { > + memcpy(data, hns3_netdev_stats[i].stats_string, > + ETH_GSTRING_LEN); > + data += ETH_GSTRING_LEN; > + } > + > + return data; > +} > + > +static u8 *hns3_get_strings_tqps(struct hnae3_handle *handle, u8 *data) > +{ > + struct hnae3_knic_private_info *kinfo = &handle->kinfo; > + int i, j; Likewise, unsigned int i, j. > + > + /* get strings for Tx */ > + for (i = 0; i < kinfo->num_tqps; i++) { > + for (j = 0; j < HNS3_TXQ_STATS_COUNT; j++) { > + u8 gstr[ETH_GSTRING_LEN]; char gstr[ETH_GSTRING_LEN] and you can move it out of the loop since it is just a temporary buffer for both loops here. > + > + sprintf(gstr, "rcb_q%d_", i); snprintf() just to be on the safe side. > + strcat(gstr, hns3_txq_stats[j].stats_string); > + > + memcpy(data, gstr, ETH_GSTRING_LEN); What ensures that you are NULL terminating this string? > + data += ETH_GSTRING_LEN; > + } > + } > + > + /* get strings for Rx */ > + for (i = 0; i < kinfo->num_tqps; i++) { > + for (j = 0; j < HNS3_RXQ_STATS_COUNT; j++) { > + u8 gstr[ETH_GSTRING_LEN]; > + > + sprintf(gstr, "rcb_q%d_", i); > + strcat(gstr, hns3_rxq_stats[j].stats_string); > + > + memcpy(data, gstr, ETH_GSTRING_LEN); > + data += ETH_GSTRING_LEN; > + } > + } > + > + return data; > +} > + > +static void hns3_get_strings(struct net_device *netdev, u32 stringset, u8 *data) > +{ > + struct hns3_nic_priv *priv = netdev_priv(netdev); > + struct hnae3_handle *h = priv->ae_handle; > + const struct hnae3_ae_ops *ops = h->ae_algo->ops; > + char *buff = (char *)data; > + > + if (!ops->get_strings) { > + netdev_err(netdev, "could not get strings!\n"); > + return; > + } Same here, does not sound like something you would want to print more than once? > + > + switch (stringset) { > + case ETH_SS_STATS: > + buff = hns3_get_strings_netdev(buff); > + buff = hns3_get_strings_tqps(h, buff); > + h->ae_algo->ops->get_strings(h, stringset, (u8 *)buff); > + break; > + case ETH_SS_TEST: > + ops->get_strings(h, stringset, data); > + break; > + } > +} > + > +static u64 *hns3_get_stats_netdev(struct net_device *netdev, u64 *data) > +{ You should implement the 64-bit version of this. > + const struct rtnl_link_stats64 *net_stats; > + struct rtnl_link_stats64 temp; > + u8 *stat; > + int i; unsigned int i > + > + net_stats = dev_get_stats(netdev, &temp); > + > + for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) { > + stat = (u8 *)net_stats + hns3_netdev_stats[i].stats_offset; > + *data++ = *(u64 *)stat; > + } > + > + return data; > +} > + > +static u64 *hns3_get_stats_tqps(struct hnae3_handle *handle, u64 *data) > +{ > + struct hns3_nic_priv *nic_priv = (struct hns3_nic_priv *)handle->priv; > + struct hnae3_knic_private_info *kinfo = &handle->kinfo; > + struct hns3_enet_ring *ring; > + u8 *stat; > + int i; Same here. > + > + /* get stats for Tx */ > + for (i = 0; i < kinfo->num_tqps; i++) { > + ring = nic_priv->ring_data[i].ring; > + for (i = 0; i < HNS3_TXQ_STATS_COUNT; i++) { > + stat = (u8 *)ring + hns3_txq_stats[i].stats_offset; > + *data++ = *(u64 *)stat; > + } > + } > + > + /* get stats for Rx */ > + for (i = 0; i < kinfo->num_tqps; i++) { > + ring = nic_priv->ring_data[i + kinfo->num_tqps].ring; > + for (i = 0; i < HNS3_RXQ_STATS_COUNT; i++) { > + stat = (u8 *)ring + hns3_rxq_stats[i].stats_offset; > + *data++ = *(u64 *)stat; > + } > + } > + > + return data; > +} > + > +/* hns3_get_stats - get detail statistics. > + * @netdev: net device > + * @stats: statistics info. > + * @data: statistics data. > + */ > +void hns3_get_stats(struct net_device *netdev, struct ethtool_stats *stats, > + u64 *data) > +{ > + struct hns3_nic_priv *priv = netdev_priv(netdev); > + struct hnae3_handle *h = priv->ae_handle; > + u64 *p = data; > + > + if (!h->ae_algo->ops->get_stats || !h->ae_algo->ops->update_stats) { > + netdev_err(netdev, "could not get any statistics\n"); > + return; > + } > + > + h->ae_algo->ops->update_stats(h, &netdev->stats); > + > + /* get netdev related stats */ > + p = hns3_get_stats_netdev(netdev, p); > + > + /* get per-queue stats */ > + p = hns3_get_stats_tqps(h, p); > + > + /* get MAC & other misc hardware stats */ > + h->ae_algo->ops->get_stats(h, p); > +} > + > +static void hns3_get_drvinfo(struct net_device *netdev, > + struct ethtool_drvinfo *drvinfo) > +{ > + struct hns3_nic_priv *priv = netdev_priv(netdev); > + struct hnae3_handle *h = priv->ae_handle; > + > + strncpy(drvinfo->version, HNAE_DRIVER_VERSION, > + sizeof(drvinfo->version)); > + drvinfo->version[sizeof(drvinfo->version) - 1] = '\0'; strlcpy() would probably do that for you. > + > + strncpy(drvinfo->driver, HNAE_DRIVER_NAME, sizeof(drvinfo->driver)); > + drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0'; Same here > + > + strncpy(drvinfo->bus_info, priv->dev->bus->name, > + sizeof(drvinfo->bus_info));> + drvinfo->bus_info[ETHTOOL_BUSINFO_LEN - 1] = '\0'; And here. The rest looked fine.
On Sat, 22 Jul 2017 23:09:41 +0100 Salil Mehta <salil.mehta@huawei.com> wrote: > + HNS3_NETDEV_STAT("rx_packets", rx_packets), > + HNS3_NETDEV_STAT("tx_packets", tx_packets), > + HNS3_NETDEV_STAT("rx_bytes", rx_bytes), > + HNS3_NETDEV_STAT("tx_bytes", tx_bytes), > + HNS3_NETDEV_STAT("rx_errors", rx_errors), > + HNS3_NETDEV_STAT("tx_errors", tx_errors), > + HNS3_NETDEV_STAT("rx_dropped", rx_dropped), > + HNS3_NETDEV_STAT("tx_dropped", tx_dropped), > + HNS3_NETDEV_STAT("multicast", multicast), > + HNS3_NETDEV_STAT("collisions", collisions), > + > + /* detailed Rx errors */ Do not put network statistics in ethtool statistics. This is redundant and unnecessary. Yes some other drivers may do it but it is not best practice. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Jul 23, 2017, at 10:05 AM, Florian Fainelli <f.fainelli@gmail.com> wrote: >> + >> + strncpy(drvinfo->version, HNAE_DRIVER_VERSION, >> + sizeof(drvinfo->version)); >> + drvinfo->version[sizeof(drvinfo->version) - 1] = '\0'; > > strlcpy() would probably do that for you. You need to be careful about strlcpy - it does not completely write the destination buffer as strncpy does, and so can result in a kernel memory leak if the destination is not known to already be cleared. >> + >> + strncpy(drvinfo->driver, HNAE_DRIVER_NAME, sizeof(drvinfo->driver)); >> + drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0'; > > Same here Same here >> + >> + strncpy(drvinfo->bus_info, priv->dev->bus->name, >> + sizeof(drvinfo->bus_info));> + drvinfo->bus_info[ETHTOOL_BUSINFO_LEN - 1] = '\0'; > > And here. And here too. I haven't looked at this deeply enough to know whether a leak could be created by strlcpy here, but I wanted to raise it as something to be considered before switching to it. Blindly adopting strlcpy is hazardous as are tools that unconditionally recommend it. -- Mark Rustad, Networking Division, Intel Corporation
From: Rustad, Mark D > Sent: 24 July 2017 21:32 > > On Jul 23, 2017, at 10:05 AM, Florian Fainelli <f.fainelli@gmail.com> wrote: > >> + > >> + strncpy(drvinfo->version, HNAE_DRIVER_VERSION, > >> + sizeof(drvinfo->version)); > >> + drvinfo->version[sizeof(drvinfo->version) - 1] = '\0'; > > > > strlcpy() would probably do that for you. > > You need to be careful about strlcpy - it does not completely write the destination buffer as strncpy > does, and so can result in a kernel memory leak if the destination is not known to already be cleared. Not only that, strlcpy() is defined to return the size of the source string. So if the source buffers isn't '\0' terminated it can fault. (Not a problem above.) Neither function is the one you were looking for. David -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Stephen, > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Sunday, July 23, 2017 6:26 PM > To: Salil Mehta > Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); > mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm > Subject: Re: [PATCH V4 net-next 7/8] net: hns3: Add Ethtool support to > HNS3 driver > > On Sat, 22 Jul 2017 23:09:41 +0100 > Salil Mehta <salil.mehta@huawei.com> wrote: > > > + HNS3_NETDEV_STAT("rx_packets", rx_packets), > > + HNS3_NETDEV_STAT("tx_packets", tx_packets), > > + HNS3_NETDEV_STAT("rx_bytes", rx_bytes), > > + HNS3_NETDEV_STAT("tx_bytes", tx_bytes), > > + HNS3_NETDEV_STAT("rx_errors", rx_errors), > > + HNS3_NETDEV_STAT("tx_errors", tx_errors), > > + HNS3_NETDEV_STAT("rx_dropped", rx_dropped), > > + HNS3_NETDEV_STAT("tx_dropped", tx_dropped), > > + HNS3_NETDEV_STAT("multicast", multicast), > > + HNS3_NETDEV_STAT("collisions", collisions), > > + > > + /* detailed Rx errors */ > > Do not put network statistics in ethtool statistics. > This is redundant and unnecessary. Yes some other drivers may do it > but it is not best practice. Ok sure, will remove netdev stats from the ethtool. Thanks Salil -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Florian, > -----Original Message----- > From: Florian Fainelli [mailto:f.fainelli@gmail.com] > Sent: Sunday, July 23, 2017 6:05 PM > To: Salil Mehta; davem@davemloft.net > Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); > mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm > Subject: Re: [PATCH V4 net-next 7/8] net: hns3: Add Ethtool support to > HNS3 driver > > > > On 07/22/2017 03:09 PM, Salil Mehta wrote: > > This patch adds the support of the Ethtool interface to > > the HNS3 Ethernet driver. Various commands to read the > > statistics, configure the offloading, loopback selftest etc. > > are supported. > > > > Signed-off-by: Daode Huang <huangdaode@hisilicon.com> > > Signed-off-by: lipeng <lipeng321@huawei.com> > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > > Signed-off-by: Yisen Zhuang <yisen.zhuang@huawei.com> > > --- > > Patch V4: addressed below comments > > 1. Andrew Lunn > > Removed the support of loop PHY back for now > > Patch V3: Address below comments > > 1. Stephen Hemminger > > https://lkml.org/lkml/2017/6/13/974 > > 2. Andrew Lunn > > https://lkml.org/lkml/2017/6/13/1037 > > Patch V2: No change > > Patch V1: Initial Submit > > --- > > .../ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c | 543 > +++++++++++++++++++++ > > 1 file changed, 543 insertions(+) > > create mode 100644 > drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > > > > diff --git > a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > > new file mode 100644 > > index 000000000000..82b0d4d829f8 > > --- /dev/null > > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > > @@ -0,0 +1,543 @@ > > +/* > > + * Copyright (c) 2016~2017 Hisilicon Limited. > > + * > > + * This program is free software; you can redistribute it and/or > modify > > + * it under the terms of the GNU General Public License as published > by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include <linux/etherdevice.h> > > +#include "hns3_enet.h" > > + > > +struct hns3_stats { > > + char stats_string[ETH_GSTRING_LEN]; > > + int stats_size; > > + int stats_offset; > > +}; > > + > > +/* netdev related stats */ > > +#define HNS3_NETDEV_STAT(_string, _member) \ > > + { _string, \ > > + FIELD_SIZEOF(struct rtnl_link_stats64, _member), \ > > + offsetof(struct rtnl_link_stats64, _member), \ > > + } > > Can you make this macro use named initializers? Can you please explain bit more or point out some example. This would be very handy. Thanks > > > + > > +static const struct hns3_stats hns3_netdev_stats[] = { > > + /* misc. Rx/Tx statistics */ > > + HNS3_NETDEV_STAT("rx_packets", rx_packets), > > + HNS3_NETDEV_STAT("tx_packets", tx_packets), > > + HNS3_NETDEV_STAT("rx_bytes", rx_bytes), > > + HNS3_NETDEV_STAT("tx_bytes", tx_bytes), > > + HNS3_NETDEV_STAT("rx_errors", rx_errors), > > + HNS3_NETDEV_STAT("tx_errors", tx_errors), > > + HNS3_NETDEV_STAT("rx_dropped", rx_dropped), > > + HNS3_NETDEV_STAT("tx_dropped", tx_dropped), > > + HNS3_NETDEV_STAT("multicast", multicast), > > + HNS3_NETDEV_STAT("collisions", collisions), > > + > > + /* detailed Rx errors */ > > + HNS3_NETDEV_STAT("rx_length_errors", rx_length_errors), > > + HNS3_NETDEV_STAT("rx_over_errors", rx_over_errors), > > + HNS3_NETDEV_STAT("rx_crc_errors", rx_crc_errors), > > + HNS3_NETDEV_STAT("rx_frame_errors", rx_frame_errors), > > + HNS3_NETDEV_STAT("rx_fifo_errors", rx_fifo_errors), > > + HNS3_NETDEV_STAT("rx_missed_errors", rx_missed_errors), > > + > > + /* detailed Tx errors */ > > + HNS3_NETDEV_STAT("tx_aborted_errors", tx_aborted_errors), > > + HNS3_NETDEV_STAT("tx_carrier_errors", tx_carrier_errors), > > + HNS3_NETDEV_STAT("tx_fifo_errors", tx_fifo_errors), > > + HNS3_NETDEV_STAT("tx_heartbeat_errors", tx_heartbeat_errors), > > + HNS3_NETDEV_STAT("tx_window_errors", tx_window_errors), > > + > > + /* for cslip etc */ > > + HNS3_NETDEV_STAT("rx_compressed", rx_compressed), > > + HNS3_NETDEV_STAT("tx_compressed", tx_compressed), > > +}; > > + > > +#define HNS3_NETDEV_STATS_COUNT ARRAY_SIZE(hns3_netdev_stats) > > + > > +/* tqp related stats */ > > +#define HNS3_TQP_STAT(_string, _member) \ > > + { _string, \ > > + FIELD_SIZEOF(struct ring_stats, _member), \ > > + offsetof(struct hns3_enet_ring, stats), \ > > + } > > + > > Same here. Ok. > > > +static const struct hns3_stats hns3_txq_stats[] = { > > + /* Tx per-queue statistics */ > > + HNS3_TQP_STAT("tx_io_err_cnt", io_err_cnt), > > + HNS3_TQP_STAT("tx_sw_err_cnt", sw_err_cnt), > > + HNS3_TQP_STAT("tx_seg_pkt_cnt", seg_pkt_cnt), > > + HNS3_TQP_STAT("tx_pkts", tx_pkts), > > + HNS3_TQP_STAT("tx_bytes", tx_bytes), > > + HNS3_TQP_STAT("tx_err_cnt", tx_err_cnt), > > + HNS3_TQP_STAT("tx_restart_queue", restart_queue), > > + HNS3_TQP_STAT("tx_busy", tx_busy), > > +}; > > + > > +#define HNS3_TXQ_STATS_COUNT ARRAY_SIZE(hns3_txq_stats) > > + > > +static const struct hns3_stats hns3_rxq_stats[] = { > > + /* Rx per-queue statistics */ > > + HNS3_TQP_STAT("rx_io_err_cnt", io_err_cnt), > > + HNS3_TQP_STAT("rx_sw_err_cnt", sw_err_cnt), > > + HNS3_TQP_STAT("rx_seg_pkt_cnt", seg_pkt_cnt), > > + HNS3_TQP_STAT("rx_pkts", rx_pkts), > > + HNS3_TQP_STAT("rx_bytes", rx_bytes), > > + HNS3_TQP_STAT("rx_err_cnt", rx_err_cnt), > > + HNS3_TQP_STAT("rx_reuse_pg_cnt", reuse_pg_cnt), > > + HNS3_TQP_STAT("rx_err_pkt_len", err_pkt_len), > > + HNS3_TQP_STAT("rx_non_vld_descs", non_vld_descs), > > + HNS3_TQP_STAT("rx_err_bd_num", err_bd_num), > > + HNS3_TQP_STAT("rx_l2_err", l2_err), > > + HNS3_TQP_STAT("rx_l3l4_csum_err", l3l4_csum_err), > > +}; > > + > > +#define HNS3_RXQ_STATS_COUNT ARRAY_SIZE(hns3_rxq_stats) > > + > > +#define HNS3_TQP_STATS_COUNT (HNS3_TXQ_STATS_COUNT + > HNS3_RXQ_STATS_COUNT) > > + > > +struct hns3_link_mode_mapping { > > + u32 hns3_link_mode; > > + u32 ethtool_link_mode; > > +}; > > + > > +static const struct hns3_link_mode_mapping hns3_lm_map[] = { > > + {HNS3_LM_FIBRE_BIT, ETHTOOL_LINK_MODE_FIBRE_BIT}, > > + {HNS3_LM_AUTONEG_BIT, ETHTOOL_LINK_MODE_Autoneg_BIT}, > > + {HNS3_LM_TP_BIT, ETHTOOL_LINK_MODE_TP_BIT}, > > + {HNS3_LM_PAUSE_BIT, ETHTOOL_LINK_MODE_Pause_BIT}, > > + {HNS3_LM_BACKPLANE_BIT, ETHTOOL_LINK_MODE_Backplane_BIT}, > > + {HNS3_LM_10BASET_HALF_BIT, ETHTOOL_LINK_MODE_10baseT_Half_BIT}, > > + {HNS3_LM_10BASET_FULL_BIT, ETHTOOL_LINK_MODE_10baseT_Full_BIT}, > > + {HNS3_LM_100BASET_HALF_BIT, ETHTOOL_LINK_MODE_100baseT_Half_BIT}, > > + {HNS3_LM_100BASET_FULL_BIT, ETHTOOL_LINK_MODE_100baseT_Full_BIT}, > > + {HNS3_LM_1000BASET_FULL_BIT, > ETHTOOL_LINK_MODE_1000baseT_Full_BIT}, > > +}; > > + > > +#define HNS3_DRV_TO_ETHTOOL_CAPS(caps, lk_ksettings, name) \ > > +{ \ > > + int i; \ > > + \ > > + for (i = 0; i < ARRAY_SIZE(hns3_lm_map); i++) { \ > > + if ((caps) & hns3_lm_map[i].hns3_link_mode) \ > > + __set_bit(hns3_lm_map[i].ethtool_link_mode,\ > > + (lk_ksettings)->link_modes.name); \ > > + } \ > > +} > > How about making this an inline function such that you would get proper > type checking? Sure will try doing that. Thanks > > > + > > +static int hns3_get_sset_count(struct net_device *netdev, int > stringset) > > +{ > > + struct hns3_nic_priv *priv = netdev_priv(netdev); > > + struct hnae3_handle *h = priv->ae_handle; > > + const struct hnae3_ae_ops *ops = h->ae_algo->ops; > > + > > + if (!ops->get_sset_count) { > > + netdev_err(netdev, "could not get string set count\n"); > > + return -EOPNOTSUPP; > > + } > > Is it really worth the error message? This might be called several > times > during the driver's lifecycle. Fine, will remove. > > > + > > + switch (stringset) { > > + case ETH_SS_STATS: > > + return (HNS3_NETDEV_STATS_COUNT + > > + (HNS3_TQP_STATS_COUNT * h->kinfo.num_tqps) + > > + ops->get_sset_count(h, stringset)); > > + > > + case ETH_SS_TEST: > > + return ops->get_sset_count(h, stringset); > > + } > > + > > + return 0; > > +} > > + > > +static u8 *hns3_get_strings_netdev(u8 *data) > > +{ > > + int i; > > unsigned int i. Ok. > > > + > > + for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) { > > + memcpy(data, hns3_netdev_stats[i].stats_string, > > + ETH_GSTRING_LEN); > > + data += ETH_GSTRING_LEN; > > + } > > + > > + return data; > > +} > > + > > +static u8 *hns3_get_strings_tqps(struct hnae3_handle *handle, u8 > *data) > > +{ > > + struct hnae3_knic_private_info *kinfo = &handle->kinfo; > > + int i, j; > > Likewise, unsigned int i, j. Ok. > > > + > > + /* get strings for Tx */ > > + for (i = 0; i < kinfo->num_tqps; i++) { > > + for (j = 0; j < HNS3_TXQ_STATS_COUNT; j++) { > > + u8 gstr[ETH_GSTRING_LEN]; > > char gstr[ETH_GSTRING_LEN] and you can move it out of the loop since it > is just a temporary buffer for both loops here. Fine. > > > + > > + sprintf(gstr, "rcb_q%d_", i); > > snprintf() just to be on the safe side. > > > + strcat(gstr, hns3_txq_stats[j].stats_string); > > + > > + memcpy(data, gstr, ETH_GSTRING_LEN); > > What ensures that you are NULL terminating this string? Yes, point! Will fix this. Thanks > > > + data += ETH_GSTRING_LEN; > > + } > > + } > > + > > + /* get strings for Rx */ > > + for (i = 0; i < kinfo->num_tqps; i++) { > > + for (j = 0; j < HNS3_RXQ_STATS_COUNT; j++) { > > + u8 gstr[ETH_GSTRING_LEN]; > > + > > + sprintf(gstr, "rcb_q%d_", i); > > + strcat(gstr, hns3_rxq_stats[j].stats_string); > > + > > + memcpy(data, gstr, ETH_GSTRING_LEN); > > + data += ETH_GSTRING_LEN; > > + } > > + } > > + > > + return data; > > +} > > + > > +static void hns3_get_strings(struct net_device *netdev, u32 > stringset, u8 *data) > > +{ > > + struct hns3_nic_priv *priv = netdev_priv(netdev); > > + struct hnae3_handle *h = priv->ae_handle; > > + const struct hnae3_ae_ops *ops = h->ae_algo->ops; > > + char *buff = (char *)data; > > + > > + if (!ops->get_strings) { > > + netdev_err(netdev, "could not get strings!\n"); > > + return; > > + } > > Same here, does not sound like something you would want to print more > than once? Sure. Will remove. > > > + > > + switch (stringset) { > > + case ETH_SS_STATS: > > + buff = hns3_get_strings_netdev(buff); > > + buff = hns3_get_strings_tqps(h, buff); > > + h->ae_algo->ops->get_strings(h, stringset, (u8 *)buff); > > + break; > > + case ETH_SS_TEST: > > + ops->get_strings(h, stringset, data); > > + break; > > + } > > +} > > + > > +static u64 *hns3_get_stats_netdev(struct net_device *netdev, u64 > *data) > > +{ > > You should implement the 64-bit version of this. This is already taking care of 64bit stats? > > > + const struct rtnl_link_stats64 *net_stats; > > + struct rtnl_link_stats64 temp; > > + u8 *stat; > > + int i; > > unsigned int i Ok. > > > + > > + net_stats = dev_get_stats(netdev, &temp); > > + > > + for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) { > > + stat = (u8 *)net_stats + hns3_netdev_stats[i].stats_offset; > > + *data++ = *(u64 *)stat; > > + } > > + > > + return data; > > +} > > + > > +static u64 *hns3_get_stats_tqps(struct hnae3_handle *handle, u64 > *data) > > +{ > > + struct hns3_nic_priv *nic_priv = (struct hns3_nic_priv *)handle- > >priv; > > + struct hnae3_knic_private_info *kinfo = &handle->kinfo; > > + struct hns3_enet_ring *ring; > > + u8 *stat; > > + int i; > > Same here. Ok. > > > + > > + /* get stats for Tx */ > > + for (i = 0; i < kinfo->num_tqps; i++) { > > + ring = nic_priv->ring_data[i].ring; > > + for (i = 0; i < HNS3_TXQ_STATS_COUNT; i++) { > > + stat = (u8 *)ring + hns3_txq_stats[i].stats_offset; > > + *data++ = *(u64 *)stat; > > + } > > + } > > + > > + /* get stats for Rx */ > > + for (i = 0; i < kinfo->num_tqps; i++) { > > + ring = nic_priv->ring_data[i + kinfo->num_tqps].ring; > > + for (i = 0; i < HNS3_RXQ_STATS_COUNT; i++) { > > + stat = (u8 *)ring + hns3_rxq_stats[i].stats_offset; > > + *data++ = *(u64 *)stat; > > + } > > + } > > + > > + return data; > > +} > > + > > +/* hns3_get_stats - get detail statistics. > > + * @netdev: net device > > + * @stats: statistics info. > > + * @data: statistics data. > > + */ > > +void hns3_get_stats(struct net_device *netdev, struct ethtool_stats > *stats, > > + u64 *data) > > +{ > > + struct hns3_nic_priv *priv = netdev_priv(netdev); > > + struct hnae3_handle *h = priv->ae_handle; > > + u64 *p = data; > > + > > + if (!h->ae_algo->ops->get_stats || !h->ae_algo->ops- > >update_stats) { > > + netdev_err(netdev, "could not get any statistics\n"); > > + return; > > + } > > + > > + h->ae_algo->ops->update_stats(h, &netdev->stats); > > + > > + /* get netdev related stats */ > > + p = hns3_get_stats_netdev(netdev, p); > > + > > + /* get per-queue stats */ > > + p = hns3_get_stats_tqps(h, p); > > + > > + /* get MAC & other misc hardware stats */ > > + h->ae_algo->ops->get_stats(h, p); > > +} > > + > > +static void hns3_get_drvinfo(struct net_device *netdev, > > + struct ethtool_drvinfo *drvinfo) > > +{ > > + struct hns3_nic_priv *priv = netdev_priv(netdev); > > + struct hnae3_handle *h = priv->ae_handle; > > + > > + strncpy(drvinfo->version, HNAE_DRIVER_VERSION, > > + sizeof(drvinfo->version)); > > + drvinfo->version[sizeof(drvinfo->version) - 1] = '\0'; > > strlcpy() would probably do that for you. Not changing this for now. > > > + > > + strncpy(drvinfo->driver, HNAE_DRIVER_NAME, sizeof(drvinfo- > >driver)); > > + drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0'; > > Same here > > > + > > + strncpy(drvinfo->bus_info, priv->dev->bus->name, > > + sizeof(drvinfo->bus_info));> + drvinfo- > >bus_info[ETHTOOL_BUSINFO_LEN - 1] = '\0'; > > And here. > > The rest looked fine. Sure thanks > -- > Florian -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/27/2017 11:01 AM, Salil Mehta wrote: > Hi Florian, > >> -----Original Message----- >> From: Florian Fainelli [mailto:f.fainelli@gmail.com] >> Sent: Sunday, July 23, 2017 6:05 PM >> To: Salil Mehta; davem@davemloft.net >> Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); >> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux- >> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm >> Subject: Re: [PATCH V4 net-next 7/8] net: hns3: Add Ethtool support to >> HNS3 driver >> >> >> >> On 07/22/2017 03:09 PM, Salil Mehta wrote: >>> This patch adds the support of the Ethtool interface to >>> the HNS3 Ethernet driver. Various commands to read the >>> statistics, configure the offloading, loopback selftest etc. >>> are supported. >>> >>> Signed-off-by: Daode Huang <huangdaode@hisilicon.com> >>> Signed-off-by: lipeng <lipeng321@huawei.com> >>> Signed-off-by: Salil Mehta <salil.mehta@huawei.com> >>> Signed-off-by: Yisen Zhuang <yisen.zhuang@huawei.com> >>> --- >>> Patch V4: addressed below comments >>> 1. Andrew Lunn >>> Removed the support of loop PHY back for now >>> Patch V3: Address below comments >>> 1. Stephen Hemminger >>> https://lkml.org/lkml/2017/6/13/974 >>> 2. Andrew Lunn >>> https://lkml.org/lkml/2017/6/13/1037 >>> Patch V2: No change >>> Patch V1: Initial Submit >>> --- >>> .../ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c | 543 >> +++++++++++++++++++++ >>> 1 file changed, 543 insertions(+) >>> create mode 100644 >> drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c >>> >>> diff --git >> a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c >> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c >>> new file mode 100644 >>> index 000000000000..82b0d4d829f8 >>> --- /dev/null >>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c >>> @@ -0,0 +1,543 @@ >>> +/* >>> + * Copyright (c) 2016~2017 Hisilicon Limited. >>> + * >>> + * This program is free software; you can redistribute it and/or >> modify >>> + * it under the terms of the GNU General Public License as published >> by >>> + * the Free Software Foundation; either version 2 of the License, or >>> + * (at your option) any later version. >>> + */ >>> + >>> +#include <linux/etherdevice.h> >>> +#include "hns3_enet.h" >>> + >>> +struct hns3_stats { >>> + char stats_string[ETH_GSTRING_LEN]; >>> + int stats_size; >>> + int stats_offset; >>> +}; >>> + >>> +/* netdev related stats */ >>> +#define HNS3_NETDEV_STAT(_string, _member) \ >>> + { _string, \ >>> + FIELD_SIZEOF(struct rtnl_link_stats64, _member), \ >>> + offsetof(struct rtnl_link_stats64, _member), \ >>> + } >> >> Can you make this macro use named initializers? > Can you please explain bit more or point out some > example. This would be very handy. .stat_string = _string, .stats_size = FIELD_SIZEOF(struct rtnl_link_stat64, _member), .stats_offset = offsetof(struct rtnl_link_stats64, _member), https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
Hi Florian, > -----Original Message----- > From: Florian Fainelli [mailto:f.fainelli@gmail.com] > Sent: Thursday, July 27, 2017 7:05 PM > To: Salil Mehta; davem@davemloft.net > Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); > mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm > Subject: Re: [PATCH V4 net-next 7/8] net: hns3: Add Ethtool support to > HNS3 driver > > On 07/27/2017 11:01 AM, Salil Mehta wrote: > > Hi Florian, > > > >> -----Original Message----- > >> From: Florian Fainelli [mailto:f.fainelli@gmail.com] > >> Sent: Sunday, July 23, 2017 6:05 PM > >> To: Salil Mehta; davem@davemloft.net > >> Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); > >> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux- > >> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm > >> Subject: Re: [PATCH V4 net-next 7/8] net: hns3: Add Ethtool support > to > >> HNS3 driver > >> > >> > >> > >> On 07/22/2017 03:09 PM, Salil Mehta wrote: > >>> This patch adds the support of the Ethtool interface to > >>> the HNS3 Ethernet driver. Various commands to read the > >>> statistics, configure the offloading, loopback selftest etc. > >>> are supported. > >>> > >>> Signed-off-by: Daode Huang <huangdaode@hisilicon.com> > >>> Signed-off-by: lipeng <lipeng321@huawei.com> > >>> Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > >>> Signed-off-by: Yisen Zhuang <yisen.zhuang@huawei.com> > >>> --- > >>> Patch V4: addressed below comments > >>> 1. Andrew Lunn > >>> Removed the support of loop PHY back for now > >>> Patch V3: Address below comments > >>> 1. Stephen Hemminger > >>> https://lkml.org/lkml/2017/6/13/974 > >>> 2. Andrew Lunn > >>> https://lkml.org/lkml/2017/6/13/1037 > >>> Patch V2: No change > >>> Patch V1: Initial Submit > >>> --- > >>> .../ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c | 543 > >> +++++++++++++++++++++ > >>> 1 file changed, 543 insertions(+) > >>> create mode 100644 > >> drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > >>> > >>> diff --git > >> a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > >> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > >>> new file mode 100644 > >>> index 000000000000..82b0d4d829f8 > >>> --- /dev/null > >>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c > >>> @@ -0,0 +1,543 @@ > >>> +/* > >>> + * Copyright (c) 2016~2017 Hisilicon Limited. > >>> + * > >>> + * This program is free software; you can redistribute it and/or > >> modify > >>> + * it under the terms of the GNU General Public License as > published > >> by > >>> + * the Free Software Foundation; either version 2 of the License, > or > >>> + * (at your option) any later version. > >>> + */ > >>> + > >>> +#include <linux/etherdevice.h> > >>> +#include "hns3_enet.h" > >>> + > >>> +struct hns3_stats { > >>> + char stats_string[ETH_GSTRING_LEN]; > >>> + int stats_size; > >>> + int stats_offset; > >>> +}; > >>> + > >>> +/* netdev related stats */ > >>> +#define HNS3_NETDEV_STAT(_string, _member) \ > >>> + { _string, \ > >>> + FIELD_SIZEOF(struct rtnl_link_stats64, _member), \ > >>> + offsetof(struct rtnl_link_stats64, _member), \ > >>> + } > >> > >> Can you make this macro use named initializers? > > Can you please explain bit more or point out some > > example. This would be very handy. > > .stat_string = _string, > .stats_size = FIELD_SIZEOF(struct rtnl_link_stat64, _member), > .stats_offset = offsetof(struct rtnl_link_stats64, _member), > > https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html Ok got it, thanks! > -- > Florian
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c new file mode 100644 index 000000000000..82b0d4d829f8 --- /dev/null +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c @@ -0,0 +1,543 @@ +/* + * Copyright (c) 2016~2017 Hisilicon Limited. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/etherdevice.h> +#include "hns3_enet.h" + +struct hns3_stats { + char stats_string[ETH_GSTRING_LEN]; + int stats_size; + int stats_offset; +}; + +/* netdev related stats */ +#define HNS3_NETDEV_STAT(_string, _member) \ + { _string, \ + FIELD_SIZEOF(struct rtnl_link_stats64, _member), \ + offsetof(struct rtnl_link_stats64, _member), \ + } + +static const struct hns3_stats hns3_netdev_stats[] = { + /* misc. Rx/Tx statistics */ + HNS3_NETDEV_STAT("rx_packets", rx_packets), + HNS3_NETDEV_STAT("tx_packets", tx_packets), + HNS3_NETDEV_STAT("rx_bytes", rx_bytes), + HNS3_NETDEV_STAT("tx_bytes", tx_bytes), + HNS3_NETDEV_STAT("rx_errors", rx_errors), + HNS3_NETDEV_STAT("tx_errors", tx_errors), + HNS3_NETDEV_STAT("rx_dropped", rx_dropped), + HNS3_NETDEV_STAT("tx_dropped", tx_dropped), + HNS3_NETDEV_STAT("multicast", multicast), + HNS3_NETDEV_STAT("collisions", collisions), + + /* detailed Rx errors */ + HNS3_NETDEV_STAT("rx_length_errors", rx_length_errors), + HNS3_NETDEV_STAT("rx_over_errors", rx_over_errors), + HNS3_NETDEV_STAT("rx_crc_errors", rx_crc_errors), + HNS3_NETDEV_STAT("rx_frame_errors", rx_frame_errors), + HNS3_NETDEV_STAT("rx_fifo_errors", rx_fifo_errors), + HNS3_NETDEV_STAT("rx_missed_errors", rx_missed_errors), + + /* detailed Tx errors */ + HNS3_NETDEV_STAT("tx_aborted_errors", tx_aborted_errors), + HNS3_NETDEV_STAT("tx_carrier_errors", tx_carrier_errors), + HNS3_NETDEV_STAT("tx_fifo_errors", tx_fifo_errors), + HNS3_NETDEV_STAT("tx_heartbeat_errors", tx_heartbeat_errors), + HNS3_NETDEV_STAT("tx_window_errors", tx_window_errors), + + /* for cslip etc */ + HNS3_NETDEV_STAT("rx_compressed", rx_compressed), + HNS3_NETDEV_STAT("tx_compressed", tx_compressed), +}; + +#define HNS3_NETDEV_STATS_COUNT ARRAY_SIZE(hns3_netdev_stats) + +/* tqp related stats */ +#define HNS3_TQP_STAT(_string, _member) \ + { _string, \ + FIELD_SIZEOF(struct ring_stats, _member), \ + offsetof(struct hns3_enet_ring, stats), \ + } + +static const struct hns3_stats hns3_txq_stats[] = { + /* Tx per-queue statistics */ + HNS3_TQP_STAT("tx_io_err_cnt", io_err_cnt), + HNS3_TQP_STAT("tx_sw_err_cnt", sw_err_cnt), + HNS3_TQP_STAT("tx_seg_pkt_cnt", seg_pkt_cnt), + HNS3_TQP_STAT("tx_pkts", tx_pkts), + HNS3_TQP_STAT("tx_bytes", tx_bytes), + HNS3_TQP_STAT("tx_err_cnt", tx_err_cnt), + HNS3_TQP_STAT("tx_restart_queue", restart_queue), + HNS3_TQP_STAT("tx_busy", tx_busy), +}; + +#define HNS3_TXQ_STATS_COUNT ARRAY_SIZE(hns3_txq_stats) + +static const struct hns3_stats hns3_rxq_stats[] = { + /* Rx per-queue statistics */ + HNS3_TQP_STAT("rx_io_err_cnt", io_err_cnt), + HNS3_TQP_STAT("rx_sw_err_cnt", sw_err_cnt), + HNS3_TQP_STAT("rx_seg_pkt_cnt", seg_pkt_cnt), + HNS3_TQP_STAT("rx_pkts", rx_pkts), + HNS3_TQP_STAT("rx_bytes", rx_bytes), + HNS3_TQP_STAT("rx_err_cnt", rx_err_cnt), + HNS3_TQP_STAT("rx_reuse_pg_cnt", reuse_pg_cnt), + HNS3_TQP_STAT("rx_err_pkt_len", err_pkt_len), + HNS3_TQP_STAT("rx_non_vld_descs", non_vld_descs), + HNS3_TQP_STAT("rx_err_bd_num", err_bd_num), + HNS3_TQP_STAT("rx_l2_err", l2_err), + HNS3_TQP_STAT("rx_l3l4_csum_err", l3l4_csum_err), +}; + +#define HNS3_RXQ_STATS_COUNT ARRAY_SIZE(hns3_rxq_stats) + +#define HNS3_TQP_STATS_COUNT (HNS3_TXQ_STATS_COUNT + HNS3_RXQ_STATS_COUNT) + +struct hns3_link_mode_mapping { + u32 hns3_link_mode; + u32 ethtool_link_mode; +}; + +static const struct hns3_link_mode_mapping hns3_lm_map[] = { + {HNS3_LM_FIBRE_BIT, ETHTOOL_LINK_MODE_FIBRE_BIT}, + {HNS3_LM_AUTONEG_BIT, ETHTOOL_LINK_MODE_Autoneg_BIT}, + {HNS3_LM_TP_BIT, ETHTOOL_LINK_MODE_TP_BIT}, + {HNS3_LM_PAUSE_BIT, ETHTOOL_LINK_MODE_Pause_BIT}, + {HNS3_LM_BACKPLANE_BIT, ETHTOOL_LINK_MODE_Backplane_BIT}, + {HNS3_LM_10BASET_HALF_BIT, ETHTOOL_LINK_MODE_10baseT_Half_BIT}, + {HNS3_LM_10BASET_FULL_BIT, ETHTOOL_LINK_MODE_10baseT_Full_BIT}, + {HNS3_LM_100BASET_HALF_BIT, ETHTOOL_LINK_MODE_100baseT_Half_BIT}, + {HNS3_LM_100BASET_FULL_BIT, ETHTOOL_LINK_MODE_100baseT_Full_BIT}, + {HNS3_LM_1000BASET_FULL_BIT, ETHTOOL_LINK_MODE_1000baseT_Full_BIT}, +}; + +#define HNS3_DRV_TO_ETHTOOL_CAPS(caps, lk_ksettings, name) \ +{ \ + int i; \ + \ + for (i = 0; i < ARRAY_SIZE(hns3_lm_map); i++) { \ + if ((caps) & hns3_lm_map[i].hns3_link_mode) \ + __set_bit(hns3_lm_map[i].ethtool_link_mode,\ + (lk_ksettings)->link_modes.name); \ + } \ +} + +static int hns3_get_sset_count(struct net_device *netdev, int stringset) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + const struct hnae3_ae_ops *ops = h->ae_algo->ops; + + if (!ops->get_sset_count) { + netdev_err(netdev, "could not get string set count\n"); + return -EOPNOTSUPP; + } + + switch (stringset) { + case ETH_SS_STATS: + return (HNS3_NETDEV_STATS_COUNT + + (HNS3_TQP_STATS_COUNT * h->kinfo.num_tqps) + + ops->get_sset_count(h, stringset)); + + case ETH_SS_TEST: + return ops->get_sset_count(h, stringset); + } + + return 0; +} + +static u8 *hns3_get_strings_netdev(u8 *data) +{ + int i; + + for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) { + memcpy(data, hns3_netdev_stats[i].stats_string, + ETH_GSTRING_LEN); + data += ETH_GSTRING_LEN; + } + + return data; +} + +static u8 *hns3_get_strings_tqps(struct hnae3_handle *handle, u8 *data) +{ + struct hnae3_knic_private_info *kinfo = &handle->kinfo; + int i, j; + + /* get strings for Tx */ + for (i = 0; i < kinfo->num_tqps; i++) { + for (j = 0; j < HNS3_TXQ_STATS_COUNT; j++) { + u8 gstr[ETH_GSTRING_LEN]; + + sprintf(gstr, "rcb_q%d_", i); + strcat(gstr, hns3_txq_stats[j].stats_string); + + memcpy(data, gstr, ETH_GSTRING_LEN); + data += ETH_GSTRING_LEN; + } + } + + /* get strings for Rx */ + for (i = 0; i < kinfo->num_tqps; i++) { + for (j = 0; j < HNS3_RXQ_STATS_COUNT; j++) { + u8 gstr[ETH_GSTRING_LEN]; + + sprintf(gstr, "rcb_q%d_", i); + strcat(gstr, hns3_rxq_stats[j].stats_string); + + memcpy(data, gstr, ETH_GSTRING_LEN); + data += ETH_GSTRING_LEN; + } + } + + return data; +} + +static void hns3_get_strings(struct net_device *netdev, u32 stringset, u8 *data) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + const struct hnae3_ae_ops *ops = h->ae_algo->ops; + char *buff = (char *)data; + + if (!ops->get_strings) { + netdev_err(netdev, "could not get strings!\n"); + return; + } + + switch (stringset) { + case ETH_SS_STATS: + buff = hns3_get_strings_netdev(buff); + buff = hns3_get_strings_tqps(h, buff); + h->ae_algo->ops->get_strings(h, stringset, (u8 *)buff); + break; + case ETH_SS_TEST: + ops->get_strings(h, stringset, data); + break; + } +} + +static u64 *hns3_get_stats_netdev(struct net_device *netdev, u64 *data) +{ + const struct rtnl_link_stats64 *net_stats; + struct rtnl_link_stats64 temp; + u8 *stat; + int i; + + net_stats = dev_get_stats(netdev, &temp); + + for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) { + stat = (u8 *)net_stats + hns3_netdev_stats[i].stats_offset; + *data++ = *(u64 *)stat; + } + + return data; +} + +static u64 *hns3_get_stats_tqps(struct hnae3_handle *handle, u64 *data) +{ + struct hns3_nic_priv *nic_priv = (struct hns3_nic_priv *)handle->priv; + struct hnae3_knic_private_info *kinfo = &handle->kinfo; + struct hns3_enet_ring *ring; + u8 *stat; + int i; + + /* get stats for Tx */ + for (i = 0; i < kinfo->num_tqps; i++) { + ring = nic_priv->ring_data[i].ring; + for (i = 0; i < HNS3_TXQ_STATS_COUNT; i++) { + stat = (u8 *)ring + hns3_txq_stats[i].stats_offset; + *data++ = *(u64 *)stat; + } + } + + /* get stats for Rx */ + for (i = 0; i < kinfo->num_tqps; i++) { + ring = nic_priv->ring_data[i + kinfo->num_tqps].ring; + for (i = 0; i < HNS3_RXQ_STATS_COUNT; i++) { + stat = (u8 *)ring + hns3_rxq_stats[i].stats_offset; + *data++ = *(u64 *)stat; + } + } + + return data; +} + +/* hns3_get_stats - get detail statistics. + * @netdev: net device + * @stats: statistics info. + * @data: statistics data. + */ +void hns3_get_stats(struct net_device *netdev, struct ethtool_stats *stats, + u64 *data) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + u64 *p = data; + + if (!h->ae_algo->ops->get_stats || !h->ae_algo->ops->update_stats) { + netdev_err(netdev, "could not get any statistics\n"); + return; + } + + h->ae_algo->ops->update_stats(h, &netdev->stats); + + /* get netdev related stats */ + p = hns3_get_stats_netdev(netdev, p); + + /* get per-queue stats */ + p = hns3_get_stats_tqps(h, p); + + /* get MAC & other misc hardware stats */ + h->ae_algo->ops->get_stats(h, p); +} + +static void hns3_get_drvinfo(struct net_device *netdev, + struct ethtool_drvinfo *drvinfo) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + + strncpy(drvinfo->version, HNAE_DRIVER_VERSION, + sizeof(drvinfo->version)); + drvinfo->version[sizeof(drvinfo->version) - 1] = '\0'; + + strncpy(drvinfo->driver, HNAE_DRIVER_NAME, sizeof(drvinfo->driver)); + drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0'; + + strncpy(drvinfo->bus_info, priv->dev->bus->name, + sizeof(drvinfo->bus_info)); + drvinfo->bus_info[ETHTOOL_BUSINFO_LEN - 1] = '\0'; + + snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version), "0x%08x", + priv->ae_handle->ae_algo->ops->get_fw_version(h)); +} + +static u32 hns3_get_link(struct net_device *netdev) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h; + + h = priv->ae_handle; + + if (h->ae_algo && h->ae_algo->ops && h->ae_algo->ops->get_status) + return h->ae_algo->ops->get_status(h); + else + return 0; +} + +static void hns3_get_ringparam(struct net_device *netdev, + struct ethtool_ringparam *param) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + int queue_num = priv->ae_handle->kinfo.num_tqps; + + param->tx_max_pending = HNS3_RING_MAX_PENDING; + param->rx_max_pending = HNS3_RING_MAX_PENDING; + + param->tx_pending = priv->ring_data[0].ring->desc_num; + param->rx_pending = priv->ring_data[queue_num].ring->desc_num; +} + +static void hns3_get_pauseparam(struct net_device *netdev, + struct ethtool_pauseparam *param) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + + if (h->ae_algo && h->ae_algo->ops && h->ae_algo->ops->get_pauseparam) + h->ae_algo->ops->get_pauseparam(h, ¶m->autoneg, + ¶m->rx_pause, ¶m->tx_pause); +} + +static int hns3_get_link_ksettings(struct net_device *netdev, + struct ethtool_link_ksettings *cmd) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + u32 supported_caps; + u32 advertised_caps; + u8 media_type; + u8 link_stat; + u8 auto_neg; + u8 duplex; + u32 speed; + + if (!h->ae_algo || !h->ae_algo->ops) + return -ESRCH; + + /* 1.auto_neg&speed&duplex from cmd */ + if (h->ae_algo->ops->get_ksettings_an_result) { + h->ae_algo->ops->get_ksettings_an_result(h, &auto_neg, + &speed, &duplex); + cmd->base.autoneg = auto_neg; + cmd->base.speed = speed; + cmd->base.duplex = duplex; + + link_stat = hns3_get_link(netdev); + if (!link_stat) { + cmd->base.speed = (u32)SPEED_UNKNOWN; + cmd->base.duplex = DUPLEX_UNKNOWN; + } + } + + /* 2.media_type get from bios parameter block */ + if (h->ae_algo->ops->get_media_type) + h->ae_algo->ops->get_media_type(h, &media_type); + + switch (media_type) { + case HNAE3_MEDIA_TYPE_FIBER: + cmd->base.port = PORT_FIBRE; + supported_caps = HNS3_LM_FIBRE_BIT | HNS3_LM_AUTONEG_BIT | + HNS3_LM_PAUSE_BIT | HNS3_LM_1000BASET_FULL_BIT; + + advertised_caps = supported_caps; + break; + case HNAE3_MEDIA_TYPE_COPPER: + cmd->base.port = PORT_TP; + supported_caps = HNS3_LM_TP_BIT | HNS3_LM_AUTONEG_BIT | + HNS3_LM_PAUSE_BIT | HNS3_LM_1000BASET_FULL_BIT | + HNS3_LM_100BASET_FULL_BIT | HNS3_LM_100BASET_HALF_BIT | + HNS3_LM_10BASET_FULL_BIT | HNS3_LM_10BASET_HALF_BIT; + advertised_caps = supported_caps; + break; + case HNAE3_MEDIA_TYPE_BACKPLANE: + cmd->base.port = PORT_NONE; + supported_caps = HNS3_LM_BACKPLANE_BIT | HNS3_LM_PAUSE_BIT | + HNS3_LM_AUTONEG_BIT | HNS3_LM_1000BASET_FULL_BIT | + HNS3_LM_100BASET_FULL_BIT | HNS3_LM_100BASET_HALF_BIT | + HNS3_LM_10BASET_FULL_BIT | HNS3_LM_10BASET_HALF_BIT; + + advertised_caps = supported_caps; + break; + case HNAE3_MEDIA_TYPE_UNKNOWN: + default: + cmd->base.port = PORT_OTHER; + supported_caps = 0; + advertised_caps = 0; + break; + } + + ethtool_link_ksettings_zero_link_mode(cmd, supported); + HNS3_DRV_TO_ETHTOOL_CAPS(supported_caps, cmd, supported) + + ethtool_link_ksettings_zero_link_mode(cmd, advertising); + HNS3_DRV_TO_ETHTOOL_CAPS(advertised_caps, cmd, advertising) + + /* 3.mdix_ctrl&mdix get from phy reg */ + if (h->ae_algo->ops->get_mdix_mode) + h->ae_algo->ops->get_mdix_mode(h, &cmd->base.eth_tp_mdix_ctrl, + &cmd->base.eth_tp_mdix); + /* 4.mdio_support */ + cmd->base.mdio_support = ETH_MDIO_SUPPORTS_C22; + + return 0; +} + +static u32 hns3_get_rss_key_size(struct net_device *netdev) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + + if (!h->ae_algo || !h->ae_algo->ops || + !h->ae_algo->ops->get_rss_key_size) + return -EOPNOTSUPP; + + return h->ae_algo->ops->get_rss_key_size(h); +} + +static u32 hns3_get_rss_indir_size(struct net_device *netdev) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + + if (!h->ae_algo || !h->ae_algo->ops || + !h->ae_algo->ops->get_rss_indir_size) + return -EOPNOTSUPP; + + return h->ae_algo->ops->get_rss_indir_size(h); +} + +static int hns3_get_rss(struct net_device *netdev, u32 *indir, u8 *key, + u8 *hfunc) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + + if (!h->ae_algo || !h->ae_algo->ops || !h->ae_algo->ops->get_rss) + return -EOPNOTSUPP; + + return h->ae_algo->ops->get_rss(h, indir, key, hfunc); +} + +static int hns3_set_rss(struct net_device *netdev, const u32 *indir, + const u8 *key, const u8 hfunc) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + + if (!h->ae_algo || !h->ae_algo->ops || !h->ae_algo->ops->set_rss) + return -EOPNOTSUPP; + + /* currently we only support Toeplitz hash */ + if ((hfunc != ETH_RSS_HASH_NO_CHANGE) && (hfunc != ETH_RSS_HASH_TOP)) { + netdev_err(netdev, + "hash func not supported (only Toeplitz hash)\n"); + return -EOPNOTSUPP; + } + if (!indir) { + netdev_err(netdev, + "set rss failed for indir is empty\n"); + return -EOPNOTSUPP; + } + + return h->ae_algo->ops->set_rss(h, indir, key, hfunc); +} + +static int hns3_get_rxnfc(struct net_device *netdev, + struct ethtool_rxnfc *cmd, + u32 *rule_locs) +{ + struct hns3_nic_priv *priv = netdev_priv(netdev); + struct hnae3_handle *h = priv->ae_handle; + + if (!h->ae_algo || !h->ae_algo->ops || !h->ae_algo->ops->get_tc_size) + return -EOPNOTSUPP; + + switch (cmd->cmd) { + case ETHTOOL_GRXRINGS: + cmd->data = h->ae_algo->ops->get_tc_size(h); + break; + default: + return -EOPNOTSUPP; + } + + return 0; +} + +static const struct ethtool_ops hns3_ethtool_ops = { + .get_drvinfo = hns3_get_drvinfo, + .get_link = hns3_get_link, + .get_ringparam = hns3_get_ringparam, + .get_pauseparam = hns3_get_pauseparam, + .get_strings = hns3_get_strings, + .get_ethtool_stats = hns3_get_stats, + .get_sset_count = hns3_get_sset_count, + .get_rxnfc = hns3_get_rxnfc, + .get_rxfh_key_size = hns3_get_rss_key_size, + .get_rxfh_indir_size = hns3_get_rss_indir_size, + .get_rxfh = hns3_get_rss, + .set_rxfh = hns3_set_rss, + .get_link_ksettings = hns3_get_link_ksettings, +}; + +void hns3_ethtool_set_ops(struct net_device *netdev) +{ + netdev->ethtool_ops = &hns3_ethtool_ops; +}