From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH v2 17/25] bnxt: add support for tx loopback, set vf mac and queues drop Date: Mon, 29 May 2017 18:43:34 +0100 Message-ID: References: <20170526183941.80678-1-ajit.khaparde@broadcom.com> <20170526183941.80678-18-ajit.khaparde@broadcom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Steeven Li To: Ajit Khaparde , dev@dpdk.org 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) In-Reply-To: <20170526183941.80678-18-ajit.khaparde@broadcom.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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)