> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@amd.com>
> Sent: Thursday, October 20, 2022 19:20
> To: Guo, Junfeng <junfeng....@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>; Wu, Jingjing <jingjing...@intel.com>
> Cc: ferruh.yi...@xilinx.com; dev@dpdk.org; Li, Xiaoyun
> <xiaoyun...@intel.com>; awogbem...@google.com; Richardson, Bruce
> <bruce.richard...@intel.com>; Lin, Xueqin <xueqin....@intel.com>
> Subject: Re: [PATCH v5 6/8] net/gve: add support for dev info get and dev
> configure
>
> On 10/20/2022 10:29 AM, Guo, Junfeng wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yi...@amd.com>
> >> Sent: Wednesday, October 19, 2022 21:49
> >> To: Guo, Junfeng <junfeng....@intel.com>; Zhang, Qi Z
> >> <qi.z.zh...@intel.com>; Wu, Jingjing <jingjing...@intel.com>
> >> Cc: ferruh.yi...@xilinx.com; dev@dpdk.org; Li, Xiaoyun
> >> <xiaoyun...@intel.com>; awogbem...@google.com; Richardson, Bruce
> >> <bruce.richard...@intel.com>; Lin, Xueqin <xueqin....@intel.com>
> >> Subject: Re: [PATCH v5 6/8] net/gve: add support for dev info get and
> dev
> >> configure
> >>
> >> On 10/10/2022 11:17 AM, Junfeng Guo wrote:
> >>
> >>>
> >>> Add dev_ops dev_infos_get.
> >>> Complete dev_configure with RX offloads configuration.
> >>>
> >>> Signed-off-by: Xiaoyun Li <xiaoyun...@intel.com>
> >>> Signed-off-by: Junfeng Guo <junfeng....@intel.com>
> >>
> >> <...>
> >>
> >>> --- a/drivers/net/gve/gve_ethdev.c
> >>> +++ b/drivers/net/gve/gve_ethdev.c
> >>> @@ -29,8 +29,16 @@ gve_write_version(uint8_t
> >> *driver_version_register)
> >>> }
> >>>
> >>> static int
> >>> -gve_dev_configure(__rte_unused struct rte_eth_dev *dev)
> >>> +gve_dev_configure(struct rte_eth_dev *dev)
> >>> {
> >>> + struct gve_priv *priv = dev->data->dev_private;
> >>> +
> >>> + if (dev->data->dev_conf.rxmode.mq_mode &
> >> RTE_ETH_MQ_RX_RSS_FLAG)
> >>> + dev->data->dev_conf.rxmode.offloads |=
> >> RTE_ETH_RX_OFFLOAD_RSS_HASH;
> >>> +
> >>
> >> This is force enabling the feature, we are doing this for PMDs that has
> >> the hash value anyway and no additional work or performance loss
> >> observed to enable this offload. Otherwise drivers shouldn't update
> >> 'dev_conf.rxmode'.
> >>
> >> Can you please confirm this PMD fits above description? And can you
> >> please add a coment that says force enabling the feature?
> >
> > Yes, it seems force enabling this offloading is not quite reasonable here.
> > This may just follow previous PMD convention, so we decided to
> remove
> > this part in the coming version. Thanks!
> >
> >>
> >>> + if (dev->data->dev_conf.rxmode.offloads &
> >> RTE_ETH_RX_OFFLOAD_TCP_LRO)
> >>> + priv->enable_rsc = 1;
> >>> +
> >>> return 0;
> >>> }
> >>>
> >>> @@ -94,6 +102,60 @@ gve_dev_close(struct rte_eth_dev *dev)
> >>> return err;
> >>> }
> >>>
> >>> +static int
> >>> +gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info
> >> *dev_info)
> >>> +{
> >>> + struct gve_priv *priv = dev->data->dev_private;
> >>> +
> >>> + dev_info->device = dev->device;
> >>> + dev_info->max_mac_addrs = 1;
> >>> + dev_info->max_rx_queues = priv->max_nb_rxq;
> >>> + dev_info->max_tx_queues = priv->max_nb_txq;
> >>> + dev_info->min_rx_bufsize = GVE_MIN_BUF_SIZE;
> >>> + dev_info->max_rx_pktlen = GVE_MAX_RX_PKTLEN;
> >>> + dev_info->max_mtu = RTE_ETHER_MTU;
> >>
> >> Can you please confirm max MTU this PMD supports is 1500? Meaning
> it
> >> doesn't support jumbo frames etc...
> >
> > Actually here is just a workaround solution for the max_mtu info...
> > We can only get the max_mtu value via adminq message from the
> backend.
> > But the real one (i.e., priv->max_mtu) we get is 1460, which is less than
> 1500
> > Seems it is the GCP bug or something.
> > If we use "dev_info->max_mtu = priv->max_mtu", the testpmd cannot
> even
> > be launched successfully...
> > I'll keep this part unchanged with some comments here if no other
> solutions.
> > Please help correct me if you have any other idea. Thanks a lot!
> >
>
> Getting actual value from device is correct thing to do, but it seems
> received value is not good, so OK to keep as it is.
> Can you please follow this with GVE?
Sure, will update this in the coming version. Thanks!
>
> >>
> >>> + dev_info->min_mtu = RTE_ETHER_MIN_MTU;
> >>> +
> >>> + dev_info->rx_offload_capa = 0;
> >>> + dev_info->tx_offload_capa =
> >>> + RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
> >>> + RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |
> >>> + RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
> >>> + RTE_ETH_TX_OFFLOAD_TCP_CKSUM |
> >>> + RTE_ETH_TX_OFFLOAD_SCTP_CKSUM |
> >>> + RTE_ETH_TX_OFFLOAD_TCP_TSO;
> >>
> >> Can you adverstise these capabilities in the patch that implements
> them?
> >
> > Will move this to the corresponding patch, thanks!
> >
