Hi Beilei,
On 05/13/2016 10:15 AM, Beilei Xing wrote:
> This patch enables configuring MTU for i40e.
> Since changing MTU needs to reconfigure queue, stop port first
> before configuring MTU.
>
> Signed-off-by: Beilei Xing <beilei.xing at intel.com>
> ---
> v3 changes:
> Add frame size with extra I40E_VLAN_TAG_SIZE.
> Delete i40e_dev_rx_init(pf) cause it will be called when port starts.
>
> v2 changes:
> If mtu is not within the allowed range, return -EINVAL instead of -EBUSY.
> Delete rxq reconfigure cause rxq reconfigure will be finished in
> i40e_dev_rx_init.
>
> drivers/net/i40e/i40e_ethdev.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> [...]
> +static int
> +i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> + struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> + struct rte_eth_dev_data *dev_data = pf->dev_data;
> + uint32_t frame_size = mtu + ETHER_HDR_LEN
> + + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
> + int ret = 0;
> +
> + /* check if mtu is within the allowed range */
> + if ((mtu < ETHER_MIN_MTU) || (frame_size > I40E_FRAME_SIZE_MAX))
> + return -EINVAL;
> +
> + /* mtu setting is forbidden if port is start */
> + if (dev_data->dev_started) {
> + PMD_DRV_LOG(ERR,
> + "port %d must be stopped before configuration\n",
> + dev_data->port_id);
> + return -ENOTSUP;
> + }
I'm not convinced that ENOTSUP is the proper return value here.
It is usually returned when a function is not implemented, which
is not the case here: the function is implemented but is forbidden
because the port is running.
I saw that Julien commented on your v1 that the return value should
be one of:
- (0) if successful.
- (-ENOTSUP) if operation is not supported.
- (-ENODEV) if *port_id* invalid.
- (-EINVAL) if *mtu* invalid.
But I think your initial value (-EBUSY) was fine. Maybe it should be
added in the API instead, with the following description:
(-EBUSY) if the operation is not allowed when the port is running
This would allow the application to take its dispositions to stop the
port and restart it with the proper jumbo_frame argument.
+CC Thomas which maintains ethdev API.
Regards,
Olivier
