From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH v3 3/7] drivers/net/bnxt new driver for Broadcom bnxt Date: Fri, 4 Mar 2016 15:02:07 -0800 Message-ID: <20160304150207.49d859f4@xeon-e3> References: <1457125528-128877-1-git-send-email-stephen.hurd@broadcom.com> <1457125528-128877-4-git-send-email-stephen.hurd@broadcom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org To: Stephen Hurd Return-path: Received: from mail-pf0-f180.google.com (mail-pf0-f180.google.com [209.85.192.180]) by dpdk.org (Postfix) with ESMTP id E4B7C2BAC for ; Sat, 5 Mar 2016 00:01:54 +0100 (CET) Received: by mail-pf0-f180.google.com with SMTP id 63so42605694pfe.3 for ; Fri, 04 Mar 2016 15:01:54 -0800 (PST) In-Reply-To: <1457125528-128877-4-git-send-email-stephen.hurd@broadcom.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 Fri, 4 Mar 2016 13:05:24 -0800 Stephen Hurd wrote: > New driver for Broadcom bnxt (NexXtreme C-series) devices. > > Standards-compliant 10/25/50G support with 30MPPS full-duplex throughput > http://www.broadcom.com/press/release.php?id=s923886 > > Signed-off-by: Stephen Hurd > --- > v3: > * Fix incorrect format specifier compilation error on i686 > (PRIx64 instead of lx for uint64_t) on line 1337 > > drivers/net/bnxt/Makefile | 79 ++ > drivers/net/bnxt/bnxt.h | 217 ++++ > drivers/net/bnxt/bnxt_cpr.c | 138 +++ > drivers/net/bnxt/bnxt_cpr.h | 117 ++ > drivers/net/bnxt/bnxt_ethdev.c | 1381 ++++++++++++++++++++++ > drivers/net/bnxt/bnxt_filter.c | 175 +++ > drivers/net/bnxt/bnxt_filter.h | 74 ++ > drivers/net/bnxt/bnxt_hwrm.c | 1554 ++++++++++++++++++++++++ > drivers/net/bnxt/bnxt_hwrm.h | 105 ++ > drivers/net/bnxt/bnxt_irq.c | 154 +++ > drivers/net/bnxt/bnxt_irq.h | 51 + > drivers/net/bnxt/bnxt_ring.c | 306 +++++ > drivers/net/bnxt/bnxt_ring.h | 104 ++ > drivers/net/bnxt/bnxt_rxq.c | 383 ++++++ > drivers/net/bnxt/bnxt_rxq.h | 75 ++ > drivers/net/bnxt/bnxt_rxr.c | 369 ++++++ > drivers/net/bnxt/bnxt_rxr.h | 73 ++ > drivers/net/bnxt/bnxt_stats.c | 190 +++ > drivers/net/bnxt/bnxt_stats.h | 44 + > drivers/net/bnxt/bnxt_txq.c | 164 +++ > drivers/net/bnxt/bnxt_txq.h | 76 ++ > drivers/net/bnxt/bnxt_txr.c | 326 +++++ > drivers/net/bnxt/bnxt_txr.h | 71 ++ > drivers/net/bnxt/bnxt_vnic.c | 285 +++++ > drivers/net/bnxt/bnxt_vnic.h | 80 ++ > drivers/net/bnxt/hsi_struct_def_dpdk.h | 1832 +++++++++++++++++++++++++++++ > drivers/net/bnxt/rte_pmd_bnxt_version.map | 4 + > 27 files changed, 8427 insertions(+) > create mode 100644 drivers/net/bnxt/Makefile > create mode 100644 drivers/net/bnxt/bnxt.h > create mode 100644 drivers/net/bnxt/bnxt_cpr.c > create mode 100644 drivers/net/bnxt/bnxt_cpr.h > create mode 100644 drivers/net/bnxt/bnxt_ethdev.c > create mode 100644 drivers/net/bnxt/bnxt_filter.c > create mode 100644 drivers/net/bnxt/bnxt_filter.h > create mode 100644 drivers/net/bnxt/bnxt_hwrm.c > create mode 100644 drivers/net/bnxt/bnxt_hwrm.h > create mode 100644 drivers/net/bnxt/bnxt_irq.c > create mode 100644 drivers/net/bnxt/bnxt_irq.h > create mode 100644 drivers/net/bnxt/bnxt_ring.c > create mode 100644 drivers/net/bnxt/bnxt_ring.h > create mode 100644 drivers/net/bnxt/bnxt_rxq.c > create mode 100644 drivers/net/bnxt/bnxt_rxq.h > create mode 100644 drivers/net/bnxt/bnxt_rxr.c > create mode 100644 drivers/net/bnxt/bnxt_rxr.h > create mode 100644 drivers/net/bnxt/bnxt_stats.c > create mode 100644 drivers/net/bnxt/bnxt_stats.h > create mode 100644 drivers/net/bnxt/bnxt_txq.c > create mode 100644 drivers/net/bnxt/bnxt_txq.h > create mode 100644 drivers/net/bnxt/bnxt_txr.c > create mode 100644 drivers/net/bnxt/bnxt_txr.h > create mode 100644 drivers/net/bnxt/bnxt_vnic.c > create mode 100644 drivers/net/bnxt/bnxt_vnic.h > create mode 100644 drivers/net/bnxt/hsi_struct_def_dpdk.h > create mode 100644 drivers/net/bnxt/rte_pmd_bnxt_version.map Looks good, I just have a couple of functionality comments. 1. Driver does not appear to support Link State interrupt. Not a big deal, but would be good to have. 2. Driver does not support hotplug 3. Since driver does not support scattered receive, it should check and error out on enable_scatter (ditto for other features in rxmode). This will save pain in future when some application asks device to do something it can not do. 4. Does driver support SECONDARY process model, it appears secondary device will reset hardware. 5. Driver does not supper per-receive queue data interrupts. This is necessary for power-saving NAPI like code.