RE: [PATCH v7 5/8] net/gve: add support for MTU setting

["Guo, Junfeng" <junfeng.guo@intel.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, October 24, 2022 18:48
> To: Guo, Junfeng <junfeng.guo@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>;
> awogbemila@google.com; Richardson, Bruce
> <bruce.richardson@intel.com>; hemant.agrawal@nxp.com;
> stephen@networkplumber.org; Xia, Chenbo <chenbo.xia@intel.com>;
> Zhang, Helin <helin.zhang@intel.com>
> Subject: Re: [PATCH v7 5/8] net/gve: add support for MTU setting
> 
> On 10/24/2022 6:04 AM, Guo, Junfeng wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> Sent: Friday, October 21, 2022 17:50
> >> To: Guo, Junfeng <junfeng.guo@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> >> Beilei <beilei.xing@intel.com>
> >> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>;
> >> awogbemila@google.com; Richardson, Bruce
> >> <bruce.richardson@intel.com>; hemant.agrawal@nxp.com;
> >> stephen@networkplumber.org; Xia, Chenbo <chenbo.xia@intel.com>;
> >> Zhang, Helin <helin.zhang@intel.com>
> >> Subject: Re: [PATCH v7 5/8] net/gve: add support for MTU setting
> >>
> >> On 10/21/2022 10:19 AM, Junfeng Guo wrote:
> >>
> >>>
> >>> Support dev_ops mtu_set.
> >>>
> >>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> >>> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
> >>
> >> <...>
> >>
> >>> +static int
> >>> +gve_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> >>> +{
> >>> +       struct gve_priv *priv = dev->data->dev_private;
> >>> +       int err;
> >>> +
> >>> +       if (mtu < RTE_ETHER_MIN_MTU || mtu > priv->max_mtu) {
> >>> +               PMD_DRV_LOG(ERR, "MIN MTU is %u, MAX MTU is %u",
> >>> +                           RTE_ETHER_MIN_MTU, priv->max_mtu);
> >>> +               return -EINVAL;
> >>> +       }
> >>> +
> >>> +       /* mtu setting is forbidden if port is start */
> >>> +       if (dev->data->dev_started) {
> >>> +               PMD_DRV_LOG(ERR, "Port must be stopped before
> >> configuration");
> >>> +               return -EBUSY;
> >>> +       }
> >>> +
> >>> +       err = gve_adminq_set_mtu(priv, mtu);
> >>> +       if (err) {
> >>> +               PMD_DRV_LOG(ERR, "Failed to set mtu as %u err = %d",
> mtu,
> >> err);
> >>> +               return err;
> >>> +       }
> >>> +
> >>> +       return 0;
> >>> +}
> >>> +
> >>
> >> [copy/paste from previous version]
> >>
> >> configure() (gve_dev_configure()) also get 'mtu' as user config
> >> ('eth_conf->rxmode.mtu') which is ignored right now,
> >>
> >> since there is 'gve_adminq_set_mtu()' command already what do you
> >> think
> >> to use it within 'gve_dev_configure()'?
> >
> > There may be issues to set mtu with ('eth_conf->rxmode.mtu').
> > So better to keep this ignored at this stage.
> >
> 
> What do you mean by issues?
> 
> 'eth_conf->rxmode.mtu' is user provided config parameter, so user may
> prefer to provide this value and not call 'rte_eth_dev_set_mtu()' at all
> and still can expect correct MTU value.

Yes, it should be like this. But on current GCP, the fetched MTU value is
1460, which is smaller than 1500. And if we set MTU with the value
1500, the backend will return a failed message.
I think it would be better to comment this as an limitation or known issue.