Virtualization - Feb 2022 - [PATCH] net/mlx5: Add support for configuring max device MTU

On 2/16/2022 10:20 PM, Eli Cohen wrote:
> On Wed, Feb 16, 2022 at 10:19:52AM -0800, Si-Wei Liu wrote:
>>
>> On 2/16/2022 3:33 AM, Eli Cohen wrote:
>>> Allow an admin creating a vdpa device to specify the max MTU for
the
>>> net device.
>>>
>>> For example, to create a device with max MTU of 1000, the following
>>> command can be used:
>>>
>>> $ vdpa dev add name vdpa-a mgmtdev auxiliary/mlx5_core.sf.1 mtu
1000
>>>
>>> Signed-off-by: Eli Cohen <elic at nvidia.com>
>>> ---
>>>    drivers/vdpa/mlx5/net/mlx5_vnet.c | 32
++++++++++++++++++++++++++++++-
>>>    1 file changed, 31 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> index 6156cf6e9377..be095dc1134e 100644
>>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> @@ -2689,6 +2689,28 @@ static int event_handler(struct
notifier_block *nb, unsigned long event, void *p
>>>    	return ret;
>>>    }
>>> +static int config_func_mtu(struct mlx5_core_dev *mdev, u16 mtu)
>>> +{
>>> +	int inlen = MLX5_ST_SZ_BYTES(modify_nic_vport_context_in);
>>> +	void *in;
>>> +	int err;
>>> +
>>> +	in = kvzalloc(inlen, GFP_KERNEL);
>>> +	if (!in)
>>> +		return -ENOMEM;
>>> +
>>> +	MLX5_SET(modify_nic_vport_context_in, in, field_select.mtu, 1);
>>> +	MLX5_SET(modify_nic_vport_context_in, in, nic_vport_context.mtu,
>>> +		 mtu + MLX5V_ETH_HARD_MTU);
>>> +	MLX5_SET(modify_nic_vport_context_in, in, opcode,
>>> +		 MLX5_CMD_OP_MODIFY_NIC_VPORT_CONTEXT);
>>> +
>>> +	err = mlx5_cmd_exec_in(mdev, modify_nic_vport_context, in);
>>> +
>>> +	kvfree(in);
>>> +	return err;
>>> +}
>>> +
>>>    static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const
char *name,
>>>    			     const struct vdpa_dev_set_config *add_config)
>>>    {
>>> @@ -2749,6 +2771,13 @@ static int mlx5_vdpa_dev_add(struct
vdpa_mgmt_dev *v_mdev, const char *name,
>>>    	mutex_init(&ndev->reslock);
>>>    	mutex_init(&ndev->numq_lock);
>>>    	config = &ndev->config;
>>> +
>>> +	if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU))
{
>>> +		err = config_func_mtu(mdev, add_config->net.mtu);
>>> +		if (err)
>>> +			goto err_mtu;
>>> +	}
>>> +
>> The code change here looks fine. Maybe not this patch, does this vDPA
MTU
>> option assume the exclusive use of parent SF device? I wonder if worth
>> restoring the MTU upon any subsequent failure in this function, could
the
>> updated MTU value affect other SF children (e.g. Ethernet netdev,
rdma), as
>> vport's context has changed but the MTU change is not notified to
other
>> sibling drivers?
> We assume that if mlx5_vdpa is using the function it is the only real
> user of the function. e.g. the mlx5_core net device is still there but
> should not be used.
Could you capture this assumption in the commit log? Otherwise you can add

Reviewed-by: Si-Wei Liu<si-wei.liu at oracle.com>


Thanks,
-Siwei

>> -Siwei
>>
>>>    	err = query_mtu(mdev, &mtu);
>>>    	if (err)
>>>    		goto err_mtu;
>>> @@ -2867,7 +2896,8 @@ static int mlx5v_probe(struct
auxiliary_device *adev,
>>>    	mgtdev->mgtdev.device = mdev->device;
>>>    	mgtdev->mgtdev.id_table = id_table;
>>>    	mgtdev->mgtdev.config_attr_mask =
BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR) |
>>> -					  BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
>>> +					  BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP) |
>>> +					  BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU);
>>>    	mgtdev->mgtdev.max_supported_vqs >>>    	
MLX5_CAP_DEV_VDPA_EMULATION(mdev, max_num_virtio_queues) + 1;
>>>    	mgtdev->mgtdev.supported_features =
get_supported_features(mdev);