From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 3E811E5D for ; Mon, 4 Dec 2017 20:56:46 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Dec 2017 11:56:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,361,1508828400"; d="scan'208";a="8988318" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.241.225.178]) ([10.241.225.178]) by FMSMGA003.fm.intel.com with ESMTP; 04 Dec 2017 11:56:45 -0800 To: Jingjing Wu , dev@dpdk.org Cc: wenzhuo.lu@intel.com References: <1508488012-82704-1-git-send-email-jingjing.wu@intel.com> <1511505206-97333-1-git-send-email-jingjing.wu@intel.com> <1511505206-97333-4-git-send-email-jingjing.wu@intel.com> From: Ferruh Yigit Message-ID: <516cba6e-51ea-cb61-1d56-7dfcc6e434a9@intel.com> Date: Mon, 4 Dec 2017 11:56:45 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <1511505206-97333-4-git-send-email-jingjing.wu@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 03/14] net/avf: enable queue and device 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, 04 Dec 2017 19:56:47 -0000 On 11/23/2017 10:33 PM, Jingjing Wu wrote: > enable device and queue setup ops like: > > - dev_configure > - dev_start > - dev_stop > - dev_close > - dev_infos_get > - rx_queue_start > - rx_queue_stop > - tx_queue_start > - tx_queue_stop > - rx_queue_setup > - rx_queue_release > - tx_queue_setup > - tx_queue_release > > Signed-off-by: Jingjing Wu <...> > +/* HW desc structure, both 16-byte and 32-byte types are supported */ > +#ifdef RTE_LIBRTE_AVF_16BYTE_RX_DESC Do you want to add this config option in this patch? <...> > +/* Structure associated with each Rx queue. */ > +struct avf_rx_queue { > + struct rte_mempool *mp; /* mbuf pool to populate Rx ring */ > + const struct rte_memzone *mz; /* memzone for Rx ring */ > + volatile union avf_rx_desc *rx_ring; /* Rx ring virtual address */ > + uint64_t rx_ring_phys_addr; /* Rx ring DMA address */ > + struct rte_mbuf **sw_ring; /* address of SW ring */ > + uint16_t nb_rx_desc; /* ring length */ > + uint16_t rx_tail; /* current value of tail */ > + volatile uint8_t *qrx_tail; /* register address of tail */ > + uint16_t rx_free_thresh; /* max free RX desc to hold */ > + uint16_t nb_rx_hold; /* number of held free RX desc */ > + struct rte_mbuf *pkt_first_seg; /* first segment of current packet */ > + struct rte_mbuf *pkt_last_seg; /* last segment of current packet */ > + struct rte_mbuf fake_mbuf; /* dummy mbuf */ > + > + uint8_t port_id; /* device port ID */ If this is ethdev port_id, this needs to be 16bits now. <...> > +/* Structure associated with each TX queue. */ > +struct avf_tx_queue { > + const struct rte_memzone *mz; /* memzone for Tx ring */ > + volatile struct avf_tx_desc *tx_ring; /* Tx ring virtual address */ > + uint64_t tx_ring_phys_addr; /* Tx ring DMA address */ > + struct avf_tx_entry *sw_ring; /* address array of SW ring */ > + uint16_t nb_tx_desc; /* ring length */ > + uint16_t tx_tail; /* current value of tail */ > + volatile uint8_t *qtx_tail; /* register address of tail */ > + uint16_t nb_used; /* number of used desc since RS bit set */ > + uint16_t nb_free; > + uint16_t last_desc_cleaned; /* last desc have been cleaned*/ > + uint16_t free_thresh; > + uint16_t rs_thresh; > + > + uint8_t port_id; Same here. <...> > + > +#ifdef RTE_LIBRTE_AVF_RX_DUMP > +#define AVF_DUMP_RX_DESC(rxq, desc, rx_id) \ > + avf_dump_rx_descriptor(rxq, desc, rx_id); > +#else > +#define AVF_DUMP_RX_DESC(rxq, desc, rx_id) do { } while (0) > +#endif > + > +#ifdef RTE_LIBRTE_AVF_TX_DUMP > +#define AVF_DUMP_TX_DESC(txq, desc, tx_id) \ > + avf_dump_tx_descriptor(txq, desc, tx_id); > +#else > +#define AVF_DUMP_TX_DESC(txq, desc, tx_id) do { } while (0) > +#endif These are not defined anywhere and will be replaced in next patch, so why not completely removed in this patch, and add correct one in next patch? <...>