Hi, PATCH v2 was sent
> -----Original Message----- > From: Gaëtan Rivet [mailto:gaetan.ri...@6wind.com] > Sent: Wednesday, May 09, 2018 3:00 PM > To: Ophir Munk <ophi...@mellanox.com> > Cc: dev@dpdk.org; Thomas Monjalon <tho...@monjalon.net>; Olga Shern > <ol...@mellanox.com>; Shahaf Shuler <shah...@mellanox.com> > Subject: Re: [PATCH v1] net/failsafe: report on supported RSS functions > > Hi Ophir, > > The commit title could read: > > net/failsafe: advertize supported RSS functions > > Some nitpicks in the commit log: > > On Tue, May 08, 2018 at 06:02:41PM +0000, Ophir Munk wrote: > > Report on failsafe supported RSS functions as part of dev_infos_get > ^^^^^^^^^ > Advertize > > > callback. Set failsafe default RSS hash functions to be: ETH_RSS_IP, > > ETH_RSS_UDP and ETH_RSS_TCP. > ^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^ > > Better on its own line: > > ETH_RSS_IP, ETH_RSS_UDP, and ETH_RSS_TCP. > > > > The net result of failsafe RSS hash functions is the logical AND of > ^^^ > should be removed > > > the RSS hash functions among all failsafe sub_devices and failsafe own > > defaults. > > > > Previous to this commit RSS support was reported as none. Since the > > introduction of [1] it is required that all RSS configurations will be > ^^^^ > should be > removed > > verified. > > > > [1] commit 8863a1fbfc66 ("ethdev: add supported hash function check") > > > > Signed-off-by: Ophir Munk <ophi...@mellanox.com> > > --- > > drivers/net/failsafe/failsafe_ops.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/failsafe/failsafe_ops.c > > b/drivers/net/failsafe/failsafe_ops.c > > index 6d44884..d18b793 100644 > > --- a/drivers/net/failsafe/failsafe_ops.c > > +++ b/drivers/net/failsafe/failsafe_ops.c > > @@ -83,7 +83,10 @@ static struct rte_eth_dev_info default_infos = { > > DEV_TX_OFFLOAD_UDP_CKSUM | > > DEV_TX_OFFLOAD_TCP_CKSUM | > > DEV_TX_OFFLOAD_TCP_TSO, > > - .flow_type_rss_offloads = 0x0, > > + .flow_type_rss_offloads = > > + ETH_RSS_IP | > > + ETH_RSS_UDP | > > + ETH_RSS_TCP, > > }; > > > > static int > > @@ -805,26 +808,29 @@ fs_dev_infos_get(struct rte_eth_dev *dev, > > } else { > > uint64_t rx_offload_capa; > > uint64_t rxq_offload_capa; > > + uint64_t rss_offloads_hf; > > The name would read better as rss_hf_offload_capa. > > rss_hash_function_offload_capa is easier to understand than > rss_offloads_hash_function. > > > > > rx_offload_capa = default_infos.rx_offload_capa; > > rxq_offload_capa = default_infos.rx_queue_offload_capa; > > + rss_offloads_hf = default_infos.flow_type_rss_offloads; > > FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) { > > rte_eth_dev_info_get(PORT_ID(sdev), > > &PRIV(dev)->infos); > > rx_offload_capa &= PRIV(dev)- > >infos.rx_offload_capa; > > rxq_offload_capa &= > > PRIV(dev)- > >infos.rx_queue_offload_capa; > > + rss_offloads_hf &= > > + PRIV(dev)- > >infos.flow_type_rss_offloads; > > } > > sdev = TX_SUBDEV(dev); > > rte_eth_dev_info_get(PORT_ID(sdev), &PRIV(dev)->infos); > > PRIV(dev)->infos.rx_offload_capa = rx_offload_capa; > > PRIV(dev)->infos.rx_queue_offload_capa = > rxq_offload_capa; > > + PRIV(dev)->infos.flow_type_rss_offloads = rss_offloads_hf; > > PRIV(dev)->infos.tx_offload_capa &= > > default_infos.tx_offload_capa; > > PRIV(dev)->infos.tx_queue_offload_capa &= > > > default_infos.tx_queue_offload_capa; > > - PRIV(dev)->infos.flow_type_rss_offloads &= > > - > default_infos.flow_type_rss_offloads; > > } > > rte_memcpy(infos, &PRIV(dev)->infos, sizeof(*infos)); } > > -- > > 2.7.4 > > > > With those nitpicks fixed, > Acked-by: Gaetan Rivet <gaetan.ri...@6wind.com> > > -- > Gaëtan Rivet > 6WIND