On 12/13/2021 5:07 AM, Eli Cohen wrote:
On Mon, Dec 13, 2021 at 08:44:53AM +0200, Eli Cohen wrote:
On Fri, Dec 10, 2021 at 10:29:43AM +0800, Jason Wang wrote:
On Fri, Dec 10, 2021 at 5:51 AM Si-Wei Liu <si-wei.liu@xxxxxxxxxx> wrote:
On 12/8/2021 9:36 PM, Jason Wang wrote:
On Thu, Dec 9, 2021 at 8:25 AM Si-Wei Liu <si-wei.liu@xxxxxxxxxx> wrote:
On 12/8/2021 12:14 PM, Eli Cohen wrote:
Add netlink support to configure the max virtqueue pairs for a device.
At least one pair is required. The maximum is dictated by the device.
Example:
$ vdpa dev add name vdpa-a mgmtdev auxiliary/mlx5_core.sf.1 max_vqp 5
Not this patch, but I think there should be a mega attribute defined
ahead to specify the virtio class/type to create vdpa instance with.
Like the existing ones e.g. mac_addr and mtu, max_vqp is also vdpa net
specific attribute.
Probably, but this only works for the case when a single parent is
allowed to create different types of devices. It looks to me the
current model to have a per type parent.
Yes, that is the current situation and implementation, although the
model does allow multi-type parent through
VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES.
Right, so I agree with you, we need to specify the class first.
Or maybe fail device creation if the parent device does not support net
class.
BTW, we already have the mechanism in place to enforce that. If a device
does not support configuration of max_vqp, it will not set
VDPA_ATTR_DEV_NET_CFG_MACADDR in its config_attr_mask so you will not be
able add it if you attempt to specify max_vqp upon creation.
Yes, but that is another level of validation down to the specific class
of vdpa config. It should be completely fine for user not to specify
max_vqp or mac address when vdpa is created. Though they may not create
the expected vdpa class as they wish in the first place if the class
validation is missing.
Having said, I guess it would be great if users who want to create vdpa
can get known of the parent's supported class and capability ahead
through some mgmtdev command, like what I suggested earlier:
$ vdpa mgmtdev capab show
pci/0000:41:00.2:
class: net
mac: off
mtu: on
max_mtu: 9000
max_vqp: 1
auxiliary/mlx5_core.sf.1:
class: net
mac: on
mtu: off
max_mtu: 1500
max_vqp: 256
Regards,
-Siwei
You check first the supported class with "vdpa mgmtdev show" and if net
is supported you may use max_vqp.
Regardless, even though with
single-type parent, so far on vdpa creation there's no validation done
yet against which class/type the parent can support. The max-vqp is
essentially vdpa network device only, but command line users don't have
a means to infer it is structured this way, and/or which vdpa parent may
support this config attribute. That said, were the command line usage
structured like below, I would have less worry.
$ vdpa dev add name vdpa-a mgmtdev auxiliary/mlx5_core.sf.1 net.max_vqp 5
It might be easier to specify class as a dedicated parameter, since we
may want to specify mtu and mac.
Thanks
Alternatively, if the goal is to keep the attribute flat, we may show
the mapping for the parent capability and the supported class:
$ vdpa mgmtdev capab show
pci/0000:41:00.2:
class: net
mac: off
mtu: on
max_mtu: 9000
max_vqp: 1
auxiliary/mlx5_core.sf.1:
class: net
mac: on
mtu: off
max_mtu: 1500
max_vqp: 256
Thanks,
-Siwei
Thanks
-Siwei
Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
---
v0 -> v1:
1. fix typo
2. move max_vq_pairs to net specific struct
drivers/vdpa/vdpa.c | 14 +++++++++++++-
include/linux/vdpa.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index c37d384c0f33..3bf016e03512 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -480,7 +480,8 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb)
}
#define VDPA_DEV_NET_ATTRS_MASK ((1 << VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
- (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
+ (1 << VDPA_ATTR_DEV_NET_CFG_MTU) | \
+ (1 << VDPA_ATTR_DEV_NET_CFG_MAX_VQP))
static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *info)
{
@@ -506,6 +507,17 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU);
}
+ if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MAX_VQP]) {
+ config.net.max_vq_pairs =
+ nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MAX_VQP]);
+ if (!config.net.max_vq_pairs) {
+ NL_SET_ERR_MSG_MOD(info->extack,
+ "At least one pair of VQs is required");
+ err = -EINVAL;
+ goto err;
+ }
+ config.mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
+ }
/* Skip checking capability if user didn't prefer to configure any
* device networking attributes. It is likely that user might have used
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index db24317d61b6..b62032573780 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -99,6 +99,7 @@ struct vdpa_dev_set_config {
struct {
u8 mac[ETH_ALEN];
u16 mtu;
+ u16 max_vq_pairs;
} net;
u64 mask;
};
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
