From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH v7 0/4] User-space Ethtool Date: Wed, 17 Jun 2015 22:04:01 -0400 Message-ID: References: <1432946276-9424-1-git-send-email-liang-min.wang@intel.com> <1434579735-15496-1-git-send-email-liang-min.wang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: "dev@dpdk.org" To: Liang-Min Larry Wang Return-path: Received: from mail-ie0-f178.google.com (mail-ie0-f178.google.com [209.85.223.178]) by dpdk.org (Postfix) with ESMTP id 4C012C522 for ; Thu, 18 Jun 2015 04:04:02 +0200 (CEST) Received: by iebmu5 with SMTP id mu5so45932697ieb.1 for ; Wed, 17 Jun 2015 19:04:01 -0700 (PDT) In-Reply-To: <1434579735-15496-1-git-send-email-liang-min.wang@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Jun 17, 2015 at 6:22 PM, Liang-Min Larry Wang < liang-min.wang@intel.com> wrote: > This implementation is designed to provide a familar interface for > applications that rely on kernel-space driver to support ethtool_op and > net_device_op for device management. The initial implementation focuses on > ops that can be implemented through existing netdev APIs. More ops will be > supported in latter release. > > v7 change: > - Remove rte_eth_dev_get_ringparam implementation > v6 change: > - Rebase to match new changes over librte_ether > v5 change: > - Change API name from 'leng' to 'length' > - Remove unused data structure rte_dev_vf_info > - Remove placeholder API rte_eth_dev_set_ringparam > - Clean up set_mac_addr implementation > v4 change: > - Add rte_eth_xxx apis and respective ops over igb and ixgbe > to support ethtool and net device alike ops > - Add an example to demonstrate the use of ethtool library > v3 change: > - Fix a build issue > v2 change: > - Implement rte_eth_dev_default_mac_addr_set through dev_ops::mac_addr_set > so it would support NIC devices other than ixgbe and igb > > Liang-Min Larry Wang (4): > ethdev: add apis to support access device info > ixgbe: add ops to support ethtool ops > igb: add ops to support ethtool ops > examples: new example: l2fwd-ethtool > > drivers/net/e1000/igb_ethdev.c | 186 ++++ > drivers/net/e1000/igb_regs.h | 217 +++++ > drivers/net/ixgbe/ixgbe_ethdev.c | 183 ++++ > drivers/net/ixgbe/ixgbe_regs.h | 357 ++++++++ > examples/l2fwd-ethtool/Makefile | 55 ++ > examples/l2fwd-ethtool/l2fwd-app/Makefile | 58 ++ > examples/l2fwd-ethtool/l2fwd-app/main.c | 1030 > ++++++++++++++++++++++ > examples/l2fwd-ethtool/l2fwd-app/netdev_api.h | 781 ++++++++++++++++ > examples/l2fwd-ethtool/l2fwd-app/shared_fifo.h | 151 ++++ > examples/l2fwd-ethtool/lib/Makefile | 55 ++ > examples/l2fwd-ethtool/lib/rte_ethtool.c | 301 +++++++ > examples/l2fwd-ethtool/lib/rte_ethtool.h | 378 ++++++++ > examples/l2fwd-ethtool/nic-control/Makefile | 55 ++ > examples/l2fwd-ethtool/nic-control/nic_control.c | 412 +++++++++ > lib/librte_ether/Makefile | 1 + > lib/librte_ether/rte_eth_dev_info.h | 57 ++ > lib/librte_ether/rte_ethdev.c | 115 +++ > lib/librte_ether/rte_ethdev.h | 117 +++ > lib/librte_ether/rte_ether_version.map | 6 + > 19 files changed, 4515 insertions(+) > create mode 100644 drivers/net/e1000/igb_regs.h > create mode 100644 drivers/net/ixgbe/ixgbe_regs.h > create mode 100644 examples/l2fwd-ethtool/Makefile > create mode 100644 examples/l2fwd-ethtool/l2fwd-app/Makefile > create mode 100644 examples/l2fwd-ethtool/l2fwd-app/main.c > create mode 100644 examples/l2fwd-ethtool/l2fwd-app/netdev_api.h > create mode 100644 examples/l2fwd-ethtool/l2fwd-app/shared_fifo.h > create mode 100644 examples/l2fwd-ethtool/lib/Makefile > create mode 100644 examples/l2fwd-ethtool/lib/rte_ethtool.c > create mode 100644 examples/l2fwd-ethtool/lib/rte_ethtool.h > create mode 100644 examples/l2fwd-ethtool/nic-control/Makefile > create mode 100644 examples/l2fwd-ethtool/nic-control/nic_control.c > create mode 100644 lib/librte_ether/rte_eth_dev_info.h > > -- > 2.1.4 > > I agree with having a more complete API, but have some nits to pick. Could the API be more abstract to reduce ABI issues in future? I know choosing names is hard, but as a Linux developer ethtool has a very specific meaning to me. This API encompasses things broader than Linux ethtool and has different semantics therefore not sure having something in DPDK with same name is really a good idea. It would be better to call it something else like netdev_?? Or dpnet_??