Hi Vipin, Konstantin, > -----Original Message----- > From: Varghese, Vipin > Sent: Thursday, December 6, 2018 10:16 PM > To: Ananyev, Konstantin <konstantin.anan...@intel.com>; Lu, Wenzhuo > <wenzhuo...@intel.com>; dev@dpdk.org > Cc: Yang, Qiming <qiming.y...@intel.com>; Li, Xiaoyun > <xiaoyun...@intel.com>; Wu, Jingjing <jingjing...@intel.com> > Subject: RE: [dpdk-dev] [PATCH v2 03/20] net/ice: support device and queue > ops > > snipped > > > -----Original Message----- > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Varghese, Vipin > > > Sent: Thursday, December 6, 2018 5:27 AM > > > To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org > > > Cc: Yang, Qiming <qiming.y...@intel.com>; Li, Xiaoyun > > > <xiaoyun...@intel.com>; Wu, Jingjing <jingjing...@intel.com> > > > Subject: Re: [dpdk-dev] [PATCH v2 03/20] net/ice: support device and > > > queue ops > > > > > > Hi Wenzhuo, > > > > > > Please find my updates below > > > > > > snipped > > > > > > + if (!vsi->rss_key) > > > > > > + vsi->rss_key = rte_zmalloc("rss_key", > > > > > > + vsi->rss_key_size, 0); > > > > > > + if (!vsi->rss_lut) > > > > > > + vsi->rss_lut = rte_zmalloc("rss_lut", > > > > > > + vsi->rss_lut_size, 0); > > > > > > > > > > 2 suggestions > > > > > 1. should the name be macro? > > > > Sorry, which name? > > > Would you like to convert and use as? > > > #define ICE_RSS_KEY "rss_key" > > > #define ICE_RSS_LUT "rss_lut" > > > > > > And replace ' rte_zmalloc("rss_key",' as ' rte_zmalloc(ICE_RSS_KEY,' > > > > > > > > > 2. if there are multiple 810 NIC under DPDK, should not each rss > > > > > be different like "rss_key-%u" where it is port number? > > > > Sorry, don't understand the question. > > > Let assume we have 2 ICE_DSI NIC on PCIe bus. Then crating ' > > rte_zmalloc("rss_key",' for port 1 will fail since malloc region "rss_key" > > > already exist for port 0 > > > > It wouldn't. > > rte_malloc() simply ignores name argument. > > You can even put NULL here. > > As I remember, Anatoly suggested to remove it completely in future. > > Konstantin > Ohh, this I was not aware. Then the suggestion from my end would be to > pass 'NULL'. Wenzhuo you can ignore the MACRO and safe thing is passing > NULL. Checked the code, currently someone already passes NULL to the function. I'd like to change it to NULL. Thanks.
> > > > > > > > > > > > > > > > > > > > Snipped > > > > > > > > > > > + > > > > > > +static int > > > > > > +ice_dev_start(struct rte_eth_dev *dev) { > > > > > > + struct rte_eth_dev_data *data = dev->data; > > > > > > + struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data- > > > > > > >dev_private); > > > > > > + struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data- > > > > > >dev_private); > > > > > > + uint16_t nb_rxq = 0; > > > > > > + uint16_t nb_txq, i; > > > > > > + int ret; > > > > > > + > > > > > > + ICE_PROC_SECONDARY_CHECK; > > > > > > > > > > Device start is not supported, but how is this differentiated > > > > > from primary configured device vs secondary configured device. > > > > > > > > > > Ie: primary uses black list '-b BB:DD:F' while secondary uses > > > > > '-w BB:DD:F'. In this case since we are checking process type > > > > > this will return without > > > > start? > > > Two updates with respect to your comment, 1. tool and application > > > like dpdk-procinfo will no longer be able pull data since you are > > > asking to black > > list. > > > 2. If there are functions which needs to shared, like primary using > > > rx-0 and tx- > > 0 then secondary rx-1 and tx-1 how to make this work? > > > > > > snipped