[v7,5/8] net/gve: add support for MTU setting
Checks
Commit Message
Support dev_ops mtu_set.
Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
---
doc/guides/nics/features/gve.ini | 1 +
drivers/net/gve/gve_ethdev.c | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
Comments
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()'?
> -----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.
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.
> -----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.
@@ -5,6 +5,7 @@
;
[Features]
Link status = Y
+MTU update = Y
Linux = Y
x86-32 = Y
x86-64 = Y
@@ -96,12 +96,40 @@ gve_dev_close(struct rte_eth_dev *dev)
return err;
}
+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;
+}
+
static const struct eth_dev_ops gve_eth_dev_ops = {
.dev_configure = gve_dev_configure,
.dev_start = gve_dev_start,
.dev_stop = gve_dev_stop,
.dev_close = gve_dev_close,
.link_update = gve_link_update,
+ .mtu_set = gve_dev_mtu_set,
};
static void