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

Reply via email to