From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 1DE457CBF for ; Mon, 29 May 2017 19:43:36 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 May 2017 10:43:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,415,1491289200"; d="scan'208";a="267757250" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.81]) ([10.237.220.81]) by fmsmga004.fm.intel.com with ESMTP; 29 May 2017 10:43:35 -0700 To: Ajit Khaparde , dev@dpdk.org Cc: Steeven Li References: <20170526183941.80678-1-ajit.khaparde@broadcom.com> <20170526183941.80678-18-ajit.khaparde@broadcom.com> From: Ferruh Yigit Message-ID: Date: Mon, 29 May 2017 18:43:34 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170526183941.80678-18-ajit.khaparde@broadcom.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 17/25] bnxt: add support for tx loopback, set vf mac and queues drop X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 May 2017 17:43:37 -0000 On 5/26/2017 7:39 PM, Ajit Khaparde wrote: > Add functions rte_pmd_bnxt_set_tx_loopback, > rte_pmd_bnxt_set_all_queues_drop_en and > rte_pmd_bnxt_set_vf_mac_addr to configure tx_loopback, > queue_drop and VF MAC address setting in the hardware. > It also adds the necessary functions to send the HWRM commands > to the firmware. > > Signed-off-by: Steeven Li > Signed-off-by: Ajit Khaparde > > -- > v1->v2: regroup related patches and incorporate other review comments > --- > drivers/net/bnxt/Makefile | 4 + > drivers/net/bnxt/bnxt_hwrm.c | 107 ++++++++++++++++++++ > drivers/net/bnxt/bnxt_hwrm.h | 4 + > drivers/net/bnxt/rte_pmd_bnxt.c | 163 ++++++++++++++++++++++++++++++ > drivers/net/bnxt/rte_pmd_bnxt.h | 87 ++++++++++++++++ > drivers/net/bnxt/rte_pmd_bnxt_version.map | 9 ++ > 6 files changed, 374 insertions(+) > create mode 100644 drivers/net/bnxt/rte_pmd_bnxt.c > create mode 100644 drivers/net/bnxt/rte_pmd_bnxt.h How did you test these new PMD specific APIs? In testpmd there are already functions for these APIs, can you please update testpmd to use these APIs, this also makes them compiled so it will be easy to find any issues. <...> > + if (req.vnic_id_tbl_addr == 0) { > + RTE_LOG(ERR, PMD, > + "unable to map VNIC ID table address to physical memory\n"); > + //rte_free(vnic_ids); Please remove unused code. <...> > + if (vnic.mru == 4) // Unallocated And please don't use // comment style. <...> > +/*- > + * BSD LICENSE > + * > + * Copyright(c) Broadcom Limited. Is year required here? I don't know what it is good for, but files tend to have it. <...> > +int rte_pmd_bnxt_set_tx_loopback(uint8_t port, uint8_t on) > +{ > + struct rte_eth_dev *eth_dev; > + struct bnxt *bp; > + int rc; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV); > + > + if (on > 1) > + return -EINVAL; > + > + eth_dev = &rte_eth_devices[port]; For PMD specific APIs, you need to add a protection against application call the API with port_id of different vendor NIC. Please check samples on ixgbe or i40e drivers. <...> > diff --git a/drivers/net/bnxt/rte_pmd_bnxt_version.map b/drivers/net/bnxt/rte_pmd_bnxt_version.map > index 349c6e1..8b77163 100644 > --- a/drivers/net/bnxt/rte_pmd_bnxt_version.map > +++ b/drivers/net/bnxt/rte_pmd_bnxt_version.map > @@ -1,4 +1,13 @@ > DPDK_16.04 { > > local: *; > + > +}; > + > +DPDK_17.08 { > + global: > + > + rte_pmd_bnxt_set_tx_loopback; > + rte_pmd_bnxt_set_all_queues_drop_en; > + rte_pmd_bnxt_set_vf_mac_addr; > }; Since DPDK_16.04 doesn't have any APIs, I think it can be removed completely this one replace it (meaning moving local: *; to here)