From mboxrd@z Thu Jan 1 00:00:00 1970 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) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=mL3ctRsItJiHCD5DR2K4/Bwg6g7k1fXxun1/lq9ZA8o=; b=S/3hYSqBNEZJWrrt8ND+HOQOqx54002wL9/cbZ6pbcjbwu7otA1eS173+SMx2ossmG Au57Zw0BlT0m5xeVcQoHulx2/PvoAhTDY7wQYcbnBjFD/CggWi05KZTkvWoCd/aiX3tt V+NRcbaRRK5utp1lgRDaDBjr1ctE/aIT9HCF5dfqXxgDU7aizK9inEBaP2iCvnX3hhBt ZFfw9CsB9KEsdLRmMKWY4PcTPnooMF/iPSc4IwD6hQZvkJ9dR0iLRIzaJ5A7B21ZdzbM 9E82fWD/ulalkvfC8KE5bmqrDENC6edVm4A3BfE8v0/A+ZOlzJiTsO8FdV3HQTwt1KS1 REOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=mL3ctRsItJiHCD5DR2K4/Bwg6g7k1fXxun1/lq9ZA8o=; b=NFidDfiVDXvBlG7+iKni+d+FMbb/26zlGt2Bi0NU6ti9vyadto845CXXXGBkJWU5VD Hv5oKDCdhZQawSLmImmCDWtm5mOxDqjSHskxzXo1/UlCDPa12y17PSFzQNCkBfS//v8r TM5ZM5cF2XVyDgJ9v8rJWMKtP1ZjNjkfLX4AcxU6jJVASEHqj3mMeS8PXSdt+rjE5jco ja5X5+53DS1a9hYugxHYPZqW5IQrP6PrcGgfarcUMLCrH1uSlkPFn+RkNaQL9zEgeKTm IgsC9uz70MZK6cH0/UlOT44Vqu33kL8cEQFvVkxMLpdjydEMz+obDA940i0YbFP4Fx4V 2bFQ== X-Gm-Message-State: AD7BkJJ3pwfgkQOghyCeQNYey0uyOUJ6LWyHmIQA7rEH0dDE+o5gqIN3qbL9wb4UuvwIiw== X-Received: by 10.98.42.211 with SMTP id q202mr1836876pfq.13.1457132514310; Fri, 04 Mar 2016 15:01:54 -0800 (PST) Received: from xeon-e3 (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by smtp.gmail.com with ESMTPSA id g74sm7849054pfj.1.2016.03.04.15.01.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 04 Mar 2016 15:01:54 -0800 (PST) Date: Fri, 4 Mar 2016 15:02:07 -0800 From: Stephen Hemminger To: Stephen Hurd Message-ID: <20160304150207.49d859f4@xeon-e3> In-Reply-To: <1457125528-128877-4-git-send-email-stephen.hurd@broadcom.com> 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 Subject: Re: [dpdk-dev] [PATCH v3 3/7] drivers/net/bnxt new driver for Broadcom bnxt X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Mar 2016 23:01:55 -0000 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.