Virtualization - Aug 2021 - [PATCH linux-next v3 0/6] vdpa: enable user to set mac, mtu

? 2021/8/5 ??5:57, Michael S. Tsirkin ??:
> On Wed, Jun 16, 2021 at 10:11:49PM +0300, Parav Pandit wrote:
>> Currently user cannot set the mac address and mtu of the vdpa device.
>> This patchset enables users to set the mac address and mtu of the vdpa
>> device once the device is created.
>> If a vendor driver supports such configuration user can set it
otherwise
>> user gets unsupported error.
> This makes sense to me overall. People are used to
> use netlink to set these parameters, and virtio does
> not necessarily have a way to set all device
> parameters - they can be RO in the config space.

I don't get here, we need to care RO as well (e.g the max_virtqueue_pairs).

And do we really want netlink uAPI for virtio like:

  enum vdpa_attr {
@@ -33,6 +34,16 @@ enum vdpa_attr {
  	VDPA_ATTR_DEV_MAX_VQS,			/* u32 */
  	VDPA_ATTR_DEV_MAX_VQ_SIZE,		/* u16 */
  
+	VDPA_ATTR_DEV_NET_CFG_MACADDR,		/* binary */
+	VDPA_ATTR_DEV_NET_STATUS,		/* u8 */
+	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_SPEED,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_DUPLEX,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,	/* u8 */
+	VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN,	/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,	/* u32 */
+
  	/* new attributes must be added above here */
  	VDPA_ATTR_MAX,
  };

Or virtio uAPI and make netlink a transport?

I prefer the latter since we will meet the similar issue at the hardware 
level when we want to create and provision virtio device dynamically.

Thanks

>
>

On Fri, Aug 06, 2021 at 10:50:27AM +0800, Jason Wang
wrote:
> 
> ? 2021/8/5 ??5:57, Michael S. Tsirkin ??:
> > On Wed, Jun 16, 2021 at 10:11:49PM +0300, Parav Pandit wrote:
> > > Currently user cannot set the mac address and mtu of the vdpa
device.
> > > This patchset enables users to set the mac address and mtu of the
vdpa
> > > device once the device is created.
> > > If a vendor driver supports such configuration user can set it
otherwise
> > > user gets unsupported error.
> > This makes sense to me overall. People are used to
> > use netlink to set these parameters, and virtio does
> > not necessarily have a way to set all device
> > parameters - they can be RO in the config space.
> 
> 
> I don't get here, we need to care RO as well (e.g the
max_virtqueue_pairs).
Point I tried to make is, a virtio transport will not allow writing
max_virtqueue_pairs, but when managing virtio VFs from a PF we do need
to set it.  Thus virtio devices need a new set of interfaces for
managing them, it is not just a virtio transport.
> And do we really want netlink uAPI for virtio like:
> 
>  enum vdpa_attr {
> @@ -33,6 +34,16 @@ enum vdpa_attr {
>  	VDPA_ATTR_DEV_MAX_VQS,			/* u32 */
>  	VDPA_ATTR_DEV_MAX_VQ_SIZE,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_MACADDR,		/* binary */
> +	VDPA_ATTR_DEV_NET_STATUS,		/* u8 */
> +	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_SPEED,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_DUPLEX,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,	/* u8 */
> +	VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN,	/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,	/* u32 */
> +
>  	/* new attributes must be added above here */
>  	VDPA_ATTR_MAX,
>  };
The point is to try and not reinvent a dedicated vpda interface
where a generic one exits.
E.g. for phy things such as mac speed etc, I think most people are using
ethtool things right?
> Or virtio uAPI and make netlink a transport?
> 
> I prefer the latter since we will meet the similar issue at the hardware
> level when we want to create and provision virtio device dynamically.
> 
> Thanks
Creating devices dynamically exists with e.g. vxlan.
That is using IFLA_MTU IFLA_ADDRESS etc.

> 
> > 
> >