Hi Vipin,

> -----Original Message-----
> From: Varghese, Vipin
> Sent: Tuesday, December 4, 2018 12:53 PM
> To: Lu, Wenzhuo <wenzhuo...@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; 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
> > +
> > +static int ice_init_rss(struct ice_pf *pf) {
> > +   struct ice_hw *hw = ICE_PF_TO_HW(pf);
> > +   struct ice_vsi *vsi = pf->main_vsi;
> > +   struct rte_eth_dev *dev = pf->adapter->eth_dev;
> > +   struct rte_eth_rss_conf *rss_conf;
> > +   struct ice_aqc_get_set_rss_keys key;
> > +   uint16_t i, nb_q;
> > +   int ret = 0;
> > +
> > +   rss_conf = &dev->data->dev_conf.rx_adv_conf.rss_conf;
> > +   nb_q = dev->data->nb_rx_queues;
> > +   vsi->rss_key_size =
> ICE_AQC_GET_SET_RSS_KEY_DATA_RSS_KEY_SIZE;
> > +   vsi->rss_lut_size = hw->func_caps.common_cap.rss_table_size;
> > +
> > +   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?

> 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.

> 
> 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?
> 
> Snipped
> 
> > +
> > +static void
> > +ice_dev_stop(struct rte_eth_dev *dev) {
> > +   struct rte_eth_dev_data *data = dev->data;
> > +   struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data-
> >dev_private);
> > +   uint16_t i;
> > +
> > +   /* avoid stopping again */
> > +   if (pf->adapter_stopped)
> > +           return;
> > +
> > +   ICE_PROC_SECONDARY_CHECK_NO_ERR;
> 
> Same as above.
> 
> snipped

Reply via email to