| Message ID | 20220207125537.174619-4-elic@nvidia.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | vdpa tool support for configuring max VQs | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote: > > Add support to configure max supported virtqueue pairs for a vdpa > device. For this to be possible, add support for reading management > device's capabilities. Management device capabilities give the user a > hint as to how many virtqueue pairs at most he can ask for. Using this > information the user can choose a valid number of virtqueue pairs when > creating the device. > > Examples: > - Show management device capabiliteis: > $ vdpa mgmtdev show > auxiliary/mlx5_core.sf.1: > supported_classes net > max_supported_vqs 257 > dev_features CSUM GUEST_CSUM MTU HOST_TSO4 HOST_TSO6 STATUS CTRL_VQ \ > MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM > > A user can now create a device based on the above information. In the > above case 128 virtqueue pairs at most. The other VQ being for the > control virtqueue. > > - Add a vdpa device with 16 data virtqueue pairs > $ vdpa dev add name vdpa-a mgmtdev auxiliary/mlx5_core.sf.1 max_vqp 16 > > After feature negotiation has been completed, one can read the vdpa > configuration using: > $ vdpa dev config show > vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 16 mtu 1500 > negotiated_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS > CTRL_VQ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM I wonder if lower case is better. > > Reviewed-by: Jianbo Liu <jianbol@nvidia.com> > Signed-off-by: Eli Cohen <elic@nvidia.com> > --- > vdpa/include/uapi/linux/vdpa.h | 4 ++ > vdpa/vdpa.c | 113 ++++++++++++++++++++++++++++++++- > 2 files changed, 114 insertions(+), 3 deletions(-) > > diff --git a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h > index b7eab069988a..171122dd03c9 100644 > --- a/vdpa/include/uapi/linux/vdpa.h > +++ b/vdpa/include/uapi/linux/vdpa.h > @@ -40,6 +40,10 @@ enum vdpa_attr { > VDPA_ATTR_DEV_NET_CFG_MAX_VQP, /* u16 */ > VDPA_ATTR_DEV_NET_CFG_MTU, /* u16 */ > > + VDPA_ATTR_DEV_NEGOTIATED_FEATURES, /* u64 */ > + VDPA_ATTR_DEV_MGMTDEV_MAX_VQS, /* u32 */ > + VDPA_ATTR_DEV_SUPPORTED_FEATURES, /* u64 */ I wonder if it's better to split the patches into three where the above command could be implemented separately. > + > /* new attributes must be added above here */ > VDPA_ATTR_MAX, > }; > diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c > index 4ccb564872a0..d0dd4196610f 100644 > --- a/vdpa/vdpa.c > +++ b/vdpa/vdpa.c > @@ -23,6 +23,7 @@ > #define VDPA_OPT_VDEV_HANDLE BIT(3) > #define VDPA_OPT_VDEV_MAC BIT(4) > #define VDPA_OPT_VDEV_MTU BIT(5) > +#define VDPA_OPT_MAX_VQP BIT(6) > > struct vdpa_opts { > uint64_t present; /* flags of present items */ > @@ -32,6 +33,7 @@ struct vdpa_opts { > unsigned int device_id; > char mac[ETH_ALEN]; > uint16_t mtu; > + uint16_t max_vqp; > }; > > struct vdpa { > @@ -78,6 +80,9 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = { > [VDPA_ATTR_DEV_VENDOR_ID] = MNL_TYPE_U32, > [VDPA_ATTR_DEV_MAX_VQS] = MNL_TYPE_U32, > [VDPA_ATTR_DEV_MAX_VQ_SIZE] = MNL_TYPE_U16, > + [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64, > + [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32, > + [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64, > }; > > static int attr_cb(const struct nlattr *attr, void *data) > @@ -219,6 +224,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, struct vdpa *vdpa) > sizeof(opts->mac), opts->mac); > if (opts->present & VDPA_OPT_VDEV_MTU) > mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, opts->mtu); > + if (opts->present & VDPA_OPT_MAX_VQP) > + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, opts->max_vqp); > } > > static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, > @@ -287,6 +294,14 @@ static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, > > NEXT_ARG_FWD(); > o_found |= VDPA_OPT_VDEV_MTU; > + } else if ((matches(*argv, "max_vqp") == 0) && (o_optional & VDPA_OPT_MAX_VQP)) { > + NEXT_ARG_FWD(); > + err = vdpa_argv_u16(vdpa, argc, argv, &opts->max_vqp); > + if (err) > + return err; > + > + NEXT_ARG_FWD(); > + o_found |= VDPA_OPT_MAX_VQP; > } else { > fprintf(stderr, "Unknown option \"%s\"\n", *argv); > return -EINVAL; > @@ -385,6 +400,77 @@ static const char *parse_class(int num) > return class ? class : "< unknown class >"; > } > > +static const char * const net_feature_strs[64] = { > + [VIRTIO_NET_F_CSUM] = "CSUM", > + [VIRTIO_NET_F_GUEST_CSUM] = "GUEST_CSUM", > + [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "CTRL_GUEST_OFFLOADS", > + [VIRTIO_NET_F_MTU] = "MTU", > + [VIRTIO_NET_F_MAC] = "MAC", > + [VIRTIO_NET_F_GUEST_TSO4] = "GUEST_TSO4", > + [VIRTIO_NET_F_GUEST_TSO6] = "GUEST_TSO6", > + [VIRTIO_NET_F_GUEST_ECN] = "GUEST_ECN", > + [VIRTIO_NET_F_GUEST_UFO] = "GUEST_UFO", > + [VIRTIO_NET_F_HOST_TSO4] = "HOST_TSO4", > + [VIRTIO_NET_F_HOST_TSO6] = "HOST_TSO6", > + [VIRTIO_NET_F_HOST_ECN] = "HOST_ECN", > + [VIRTIO_NET_F_HOST_UFO] = "HOST_UFO", > + [VIRTIO_NET_F_MRG_RXBUF] = "MRG_RXBUF", > + [VIRTIO_NET_F_STATUS] = "STATUS", > + [VIRTIO_NET_F_CTRL_VQ] = "CTRL_VQ", > + [VIRTIO_NET_F_CTRL_RX] = "CTRL_RX", > + [VIRTIO_NET_F_CTRL_VLAN] = "CTRL_VLAN", > + [VIRTIO_NET_F_CTRL_RX_EXTRA] = "CTRL_RX_EXTRA", > + [VIRTIO_NET_F_GUEST_ANNOUNCE] = "GUEST_ANNOUNCE", > + [VIRTIO_NET_F_MQ] = "MQ", > + [VIRTIO_F_NOTIFY_ON_EMPTY] = "NOTIFY_ON_EMPTY", > + [VIRTIO_NET_F_CTRL_MAC_ADDR] = "CTRL_MAC_ADDR", > + [VIRTIO_F_ANY_LAYOUT] = "ANY_LAYOUT", > + [VIRTIO_NET_F_RSC_EXT] = "RSC_EXT", > + [VIRTIO_NET_F_STANDBY] = "STANDBY", > +}; It seems we are still missing things that are already supported in the Linux uapi. I think it's better to support them. E.g the RSS and SPEED_DUPLEX etc. > + > +#define VDPA_EXT_FEATURES_SZ (VIRTIO_DEV_INDEPENDENT_F_END - \ > + VIRTIO_DEV_INDEPENDENT_F_START + 1) > + > +static const char * const ext_feature_strs[VDPA_EXT_FEATURES_SZ] = { > + [VIRTIO_F_RING_INDIRECT_DESC - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_INDIRECT_DESC", > + [VIRTIO_F_RING_EVENT_IDX - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_EVENT_IDX", > + [VIRTIO_F_VERSION_1 - VIRTIO_DEV_INDEPENDENT_F_START] = "VERSION_1", > + [VIRTIO_F_ACCESS_PLATFORM - VIRTIO_DEV_INDEPENDENT_F_START] = "ACCESS_PLATFORM", > + [VIRTIO_F_RING_PACKED - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_PACKED", > + [VIRTIO_F_IN_ORDER - VIRTIO_DEV_INDEPENDENT_F_START] = "IN_ORDER", > + [VIRTIO_F_ORDER_PLATFORM - VIRTIO_DEV_INDEPENDENT_F_START] = "ORDER_PLATFORM", > + [VIRTIO_F_SR_IOV - VIRTIO_DEV_INDEPENDENT_F_START] = "SR_IOV", > + [VIRTIO_F_NOTIFICATION_DATA - VIRTIO_DEV_INDEPENDENT_F_START] = "NOTIFICATION_DATA", > +}; > + > +static void print_net_features(struct vdpa *vdpa, uint64_t features, bool maxf) > +{ > + const char *s; > + int i; > + > + if (maxf) > + pr_out_array_start(vdpa, "dev_features"); > + else > + pr_out_array_start(vdpa, "negotiated_features"); > + > + for (i = 0; i < 64; i++) { > + if (!(features & (1ULL << i))) > + continue; > + > + if (i >= VIRTIO_DEV_INDEPENDENT_F_START && i <= VIRTIO_DEV_INDEPENDENT_F_END) I don't see any issue that just use VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END (even if END can change). > + s = ext_feature_strs[i - VIRTIO_DEV_INDEPENDENT_F_START]; > + else > + s = net_feature_strs[i]; > + > + if (!s) > + print_uint(PRINT_ANY, NULL, " unrecognized_bit_%d", i); > + else > + print_string(PRINT_ANY, NULL, " %s", s); > + } > + pr_out_array_end(vdpa); > +} > + > static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh, > struct nlattr **tb) > { > @@ -408,6 +494,22 @@ static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh, > pr_out_array_end(vdpa); > } > > + if (tb[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS]) { > + uint16_t num_vqs; > + > + if (!vdpa->json_output) > + printf("\n"); > + num_vqs = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS]); > + print_uint(PRINT_ANY, "max_supported_vqs", " max_supported_vqs %d", num_vqs); > + } > + > + if (tb[VDPA_ATTR_DEV_SUPPORTED_FEATURES]) { > + uint64_t features; > + > + features = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_SUPPORTED_FEATURES]); > + print_net_features(vdpa, features, true); Do we need to check whether it's a networking device before trying to print the feature and for other type devices we can simply print the bit number as a startup? Thanks > + } > + > pr_out_handle_end(vdpa); > } > > @@ -557,7 +659,7 @@ static int cmd_dev_add(struct vdpa *vdpa, int argc, char **argv) > NLM_F_REQUEST | NLM_F_ACK); > err = vdpa_argv_parse_put(nlh, vdpa, argc, argv, > VDPA_OPT_VDEV_MGMTDEV_HANDLE | VDPA_OPT_VDEV_NAME, > - VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU); > + VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU | VDPA_OPT_MAX_VQP); > if (err) > return err; > > @@ -579,9 +681,10 @@ static int cmd_dev_del(struct vdpa *vdpa, int argc, char **argv) > return mnlu_gen_socket_sndrcv(&vdpa->nlg, nlh, NULL, NULL); > } > > -static void pr_out_dev_net_config(struct nlattr **tb) > +static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb) > { > SPRINT_BUF(macaddr); > + uint64_t val_u64; > uint16_t val_u16; > > if (tb[VDPA_ATTR_DEV_NET_CFG_MACADDR]) { > @@ -610,6 +713,10 @@ static void pr_out_dev_net_config(struct nlattr **tb) > val_u16 = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_NET_CFG_MTU]); > print_uint(PRINT_ANY, "mtu", "mtu %d ", val_u16); > } > + if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]) { > + val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]); > + print_net_features(vdpa, val_u64, false); > + } > } > > static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb) > @@ -619,7 +726,7 @@ static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb) > pr_out_vdev_handle_start(vdpa, tb); > switch (device_id) { > case VIRTIO_ID_NET: > - pr_out_dev_net_config(tb); > + pr_out_dev_net_config(vdpa, tb); > break; > default: > break; > -- > 2.34.1 >
On Thu, Feb 10, 2022 at 04:07:24PM +0800, Jason Wang wrote: > On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote: > > > > Add support to configure max supported virtqueue pairs for a vdpa > > device. For this to be possible, add support for reading management > > device's capabilities. Management device capabilities give the user a > > hint as to how many virtqueue pairs at most he can ask for. Using this > > information the user can choose a valid number of virtqueue pairs when > > creating the device. > > > > Examples: > > - Show management device capabiliteis: > > $ vdpa mgmtdev show > > auxiliary/mlx5_core.sf.1: > > supported_classes net > > max_supported_vqs 257 > > dev_features CSUM GUEST_CSUM MTU HOST_TSO4 HOST_TSO6 STATUS CTRL_VQ \ > > MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM > > > > A user can now create a device based on the above information. In the > > above case 128 virtqueue pairs at most. The other VQ being for the > > control virtqueue. > > > > - Add a vdpa device with 16 data virtqueue pairs > > $ vdpa dev add name vdpa-a mgmtdev auxiliary/mlx5_core.sf.1 max_vqp 16 > > > > After feature negotiation has been completed, one can read the vdpa > > configuration using: > > $ vdpa dev config show > > vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 16 mtu 1500 > > negotiated_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS > > CTRL_VQ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM > > I wonder if lower case is better. > I thought the capital letters will emphasize the fact that these are flag bits. Also, note the matching kernel patches have this documented in the change log with capital letters. > > > > Reviewed-by: Jianbo Liu <jianbol@nvidia.com> > > Signed-off-by: Eli Cohen <elic@nvidia.com> > > --- > > vdpa/include/uapi/linux/vdpa.h | 4 ++ > > vdpa/vdpa.c | 113 ++++++++++++++++++++++++++++++++- > > 2 files changed, 114 insertions(+), 3 deletions(-) > > > > diff --git a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h > > index b7eab069988a..171122dd03c9 100644 > > --- a/vdpa/include/uapi/linux/vdpa.h > > +++ b/vdpa/include/uapi/linux/vdpa.h > > @@ -40,6 +40,10 @@ enum vdpa_attr { > > VDPA_ATTR_DEV_NET_CFG_MAX_VQP, /* u16 */ > > VDPA_ATTR_DEV_NET_CFG_MTU, /* u16 */ > > > > + VDPA_ATTR_DEV_NEGOTIATED_FEATURES, /* u64 */ > > + VDPA_ATTR_DEV_MGMTDEV_MAX_VQS, /* u32 */ > > + VDPA_ATTR_DEV_SUPPORTED_FEATURES, /* u64 */ > > I wonder if it's better to split the patches into three where the > above command could be implemented separately. I already sent three. You mean split the third patch into three? > > > + > > /* new attributes must be added above here */ > > VDPA_ATTR_MAX, > > }; > > diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c > > index 4ccb564872a0..d0dd4196610f 100644 > > --- a/vdpa/vdpa.c > > +++ b/vdpa/vdpa.c > > @@ -23,6 +23,7 @@ > > #define VDPA_OPT_VDEV_HANDLE BIT(3) > > #define VDPA_OPT_VDEV_MAC BIT(4) > > #define VDPA_OPT_VDEV_MTU BIT(5) > > +#define VDPA_OPT_MAX_VQP BIT(6) > > > > struct vdpa_opts { > > uint64_t present; /* flags of present items */ > > @@ -32,6 +33,7 @@ struct vdpa_opts { > > unsigned int device_id; > > char mac[ETH_ALEN]; > > uint16_t mtu; > > + uint16_t max_vqp; > > }; > > > > struct vdpa { > > @@ -78,6 +80,9 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = { > > [VDPA_ATTR_DEV_VENDOR_ID] = MNL_TYPE_U32, > > [VDPA_ATTR_DEV_MAX_VQS] = MNL_TYPE_U32, > > [VDPA_ATTR_DEV_MAX_VQ_SIZE] = MNL_TYPE_U16, > > + [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64, > > + [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32, > > + [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64, > > }; > > > > static int attr_cb(const struct nlattr *attr, void *data) > > @@ -219,6 +224,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, struct vdpa *vdpa) > > sizeof(opts->mac), opts->mac); > > if (opts->present & VDPA_OPT_VDEV_MTU) > > mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, opts->mtu); > > + if (opts->present & VDPA_OPT_MAX_VQP) > > + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, opts->max_vqp); > > } > > > > static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, > > @@ -287,6 +294,14 @@ static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, > > > > NEXT_ARG_FWD(); > > o_found |= VDPA_OPT_VDEV_MTU; > > + } else if ((matches(*argv, "max_vqp") == 0) && (o_optional & VDPA_OPT_MAX_VQP)) { > > + NEXT_ARG_FWD(); > > + err = vdpa_argv_u16(vdpa, argc, argv, &opts->max_vqp); > > + if (err) > > + return err; > > + > > + NEXT_ARG_FWD(); > > + o_found |= VDPA_OPT_MAX_VQP; > > } else { > > fprintf(stderr, "Unknown option \"%s\"\n", *argv); > > return -EINVAL; > > @@ -385,6 +400,77 @@ static const char *parse_class(int num) > > return class ? class : "< unknown class >"; > > } > > > > +static const char * const net_feature_strs[64] = { > > + [VIRTIO_NET_F_CSUM] = "CSUM", > > + [VIRTIO_NET_F_GUEST_CSUM] = "GUEST_CSUM", > > + [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "CTRL_GUEST_OFFLOADS", > > + [VIRTIO_NET_F_MTU] = "MTU", > > + [VIRTIO_NET_F_MAC] = "MAC", > > + [VIRTIO_NET_F_GUEST_TSO4] = "GUEST_TSO4", > > + [VIRTIO_NET_F_GUEST_TSO6] = "GUEST_TSO6", > > + [VIRTIO_NET_F_GUEST_ECN] = "GUEST_ECN", > > + [VIRTIO_NET_F_GUEST_UFO] = "GUEST_UFO", > > + [VIRTIO_NET_F_HOST_TSO4] = "HOST_TSO4", > > + [VIRTIO_NET_F_HOST_TSO6] = "HOST_TSO6", > > + [VIRTIO_NET_F_HOST_ECN] = "HOST_ECN", > > + [VIRTIO_NET_F_HOST_UFO] = "HOST_UFO", > > + [VIRTIO_NET_F_MRG_RXBUF] = "MRG_RXBUF", > > + [VIRTIO_NET_F_STATUS] = "STATUS", > > + [VIRTIO_NET_F_CTRL_VQ] = "CTRL_VQ", > > + [VIRTIO_NET_F_CTRL_RX] = "CTRL_RX", > > + [VIRTIO_NET_F_CTRL_VLAN] = "CTRL_VLAN", > > + [VIRTIO_NET_F_CTRL_RX_EXTRA] = "CTRL_RX_EXTRA", > > + [VIRTIO_NET_F_GUEST_ANNOUNCE] = "GUEST_ANNOUNCE", > > + [VIRTIO_NET_F_MQ] = "MQ", > > + [VIRTIO_F_NOTIFY_ON_EMPTY] = "NOTIFY_ON_EMPTY", > > + [VIRTIO_NET_F_CTRL_MAC_ADDR] = "CTRL_MAC_ADDR", > > + [VIRTIO_F_ANY_LAYOUT] = "ANY_LAYOUT", > > + [VIRTIO_NET_F_RSC_EXT] = "RSC_EXT", > > + [VIRTIO_NET_F_STANDBY] = "STANDBY", > > +}; > > It seems we are still missing things that are already supported in the > Linux uapi. I think it's better to support them. E.g the RSS and > SPEED_DUPLEX etc. > Will do. > > + > > +#define VDPA_EXT_FEATURES_SZ (VIRTIO_DEV_INDEPENDENT_F_END - \ > > + VIRTIO_DEV_INDEPENDENT_F_START + 1) > > + > > +static const char * const ext_feature_strs[VDPA_EXT_FEATURES_SZ] = { > > + [VIRTIO_F_RING_INDIRECT_DESC - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_INDIRECT_DESC", > > + [VIRTIO_F_RING_EVENT_IDX - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_EVENT_IDX", > > + [VIRTIO_F_VERSION_1 - VIRTIO_DEV_INDEPENDENT_F_START] = "VERSION_1", > > + [VIRTIO_F_ACCESS_PLATFORM - VIRTIO_DEV_INDEPENDENT_F_START] = "ACCESS_PLATFORM", > > + [VIRTIO_F_RING_PACKED - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_PACKED", > > + [VIRTIO_F_IN_ORDER - VIRTIO_DEV_INDEPENDENT_F_START] = "IN_ORDER", > > + [VIRTIO_F_ORDER_PLATFORM - VIRTIO_DEV_INDEPENDENT_F_START] = "ORDER_PLATFORM", > > + [VIRTIO_F_SR_IOV - VIRTIO_DEV_INDEPENDENT_F_START] = "SR_IOV", > > + [VIRTIO_F_NOTIFICATION_DATA - VIRTIO_DEV_INDEPENDENT_F_START] = "NOTIFICATION_DATA", > > +}; > > + > > +static void print_net_features(struct vdpa *vdpa, uint64_t features, bool maxf) > > +{ > > + const char *s; > > + int i; > > + > > + if (maxf) > > + pr_out_array_start(vdpa, "dev_features"); > > + else > > + pr_out_array_start(vdpa, "negotiated_features"); > > + > > + for (i = 0; i < 64; i++) { > > + if (!(features & (1ULL << i))) > > + continue; > > + > > + if (i >= VIRTIO_DEV_INDEPENDENT_F_START && i <= VIRTIO_DEV_INDEPENDENT_F_END) > > I don't see any issue that just use VIRTIO_TRANSPORT_F_START and > VIRTIO_TRANSPORT_F_END (even if END can change). I don't get you > > > + s = ext_feature_strs[i - VIRTIO_DEV_INDEPENDENT_F_START]; > > + else > > + s = net_feature_strs[i]; > > + > > + if (!s) > > + print_uint(PRINT_ANY, NULL, " unrecognized_bit_%d", i); > > + else > > + print_string(PRINT_ANY, NULL, " %s", s); > > + } > > + pr_out_array_end(vdpa); > > +} > > + > > static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh, > > struct nlattr **tb) > > { > > @@ -408,6 +494,22 @@ static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh, > > pr_out_array_end(vdpa); > > } > > > > + if (tb[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS]) { > > + uint16_t num_vqs; > > + > > + if (!vdpa->json_output) > > + printf("\n"); > > + num_vqs = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS]); > > + print_uint(PRINT_ANY, "max_supported_vqs", " max_supported_vqs %d", num_vqs); > > + } > > + > > + if (tb[VDPA_ATTR_DEV_SUPPORTED_FEATURES]) { > > + uint64_t features; > > + > > + features = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_SUPPORTED_FEATURES]); > > + print_net_features(vdpa, features, true); > > Do we need to check whether it's a networking device before trying to > print the feature Yes, will fix > and for other type devices we can simply print the > bit number as a startup? > Why not add proper support (e.g. strings) for other types of devices when intoduced? > Thanks > > > + } > > + > > pr_out_handle_end(vdpa); > > } > > > > @@ -557,7 +659,7 @@ static int cmd_dev_add(struct vdpa *vdpa, int argc, char **argv) > > NLM_F_REQUEST | NLM_F_ACK); > > err = vdpa_argv_parse_put(nlh, vdpa, argc, argv, > > VDPA_OPT_VDEV_MGMTDEV_HANDLE | VDPA_OPT_VDEV_NAME, > > - VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU); > > + VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU | VDPA_OPT_MAX_VQP); > > if (err) > > return err; > > > > @@ -579,9 +681,10 @@ static int cmd_dev_del(struct vdpa *vdpa, int argc, char **argv) > > return mnlu_gen_socket_sndrcv(&vdpa->nlg, nlh, NULL, NULL); > > } > > > > -static void pr_out_dev_net_config(struct nlattr **tb) > > +static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb) > > { > > SPRINT_BUF(macaddr); > > + uint64_t val_u64; > > uint16_t val_u16; > > > > if (tb[VDPA_ATTR_DEV_NET_CFG_MACADDR]) { > > @@ -610,6 +713,10 @@ static void pr_out_dev_net_config(struct nlattr **tb) > > val_u16 = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_NET_CFG_MTU]); > > print_uint(PRINT_ANY, "mtu", "mtu %d ", val_u16); > > } > > + if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]) { > > + val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]); > > + print_net_features(vdpa, val_u64, false); > > + } > > } > > > > static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb) > > @@ -619,7 +726,7 @@ static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb) > > pr_out_vdev_handle_start(vdpa, tb); > > switch (device_id) { > > case VIRTIO_ID_NET: > > - pr_out_dev_net_config(tb); > > + pr_out_dev_net_config(vdpa, tb); > > break; > > default: > > break; > > -- > > 2.34.1 > > >
On Thu, Feb 10, 2022 at 4:44 PM Eli Cohen <elic@nvidia.com> wrote: > > On Thu, Feb 10, 2022 at 04:07:24PM +0800, Jason Wang wrote: > > On Mon, Feb 7, 2022 at 8:56 PM Eli Cohen <elic@nvidia.com> wrote: > > > > > > Add support to configure max supported virtqueue pairs for a vdpa > > > device. For this to be possible, add support for reading management > > > device's capabilities. Management device capabilities give the user a > > > hint as to how many virtqueue pairs at most he can ask for. Using this > > > information the user can choose a valid number of virtqueue pairs when > > > creating the device. > > > > > > Examples: > > > - Show management device capabiliteis: > > > $ vdpa mgmtdev show > > > auxiliary/mlx5_core.sf.1: > > > supported_classes net > > > max_supported_vqs 257 > > > dev_features CSUM GUEST_CSUM MTU HOST_TSO4 HOST_TSO6 STATUS CTRL_VQ \ > > > MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM > > > > > > A user can now create a device based on the above information. In the > > > above case 128 virtqueue pairs at most. The other VQ being for the > > > control virtqueue. > > > > > > - Add a vdpa device with 16 data virtqueue pairs > > > $ vdpa dev add name vdpa-a mgmtdev auxiliary/mlx5_core.sf.1 max_vqp 16 > > > > > > After feature negotiation has been completed, one can read the vdpa > > > configuration using: > > > $ vdpa dev config show > > > vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 16 mtu 1500 > > > negotiated_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS > > > CTRL_VQ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM > > > > I wonder if lower case is better. > > > > I thought the capital letters will emphasize the fact that these are > flag bits. Also, note the matching kernel patches have this documented > in the change log with capital letters. Ok, that's fine. > > > > > > > Reviewed-by: Jianbo Liu <jianbol@nvidia.com> > > > Signed-off-by: Eli Cohen <elic@nvidia.com> > > > --- > > > vdpa/include/uapi/linux/vdpa.h | 4 ++ > > > vdpa/vdpa.c | 113 ++++++++++++++++++++++++++++++++- > > > 2 files changed, 114 insertions(+), 3 deletions(-) > > > > > > diff --git a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h > > > index b7eab069988a..171122dd03c9 100644 > > > --- a/vdpa/include/uapi/linux/vdpa.h > > > +++ b/vdpa/include/uapi/linux/vdpa.h > > > @@ -40,6 +40,10 @@ enum vdpa_attr { > > > VDPA_ATTR_DEV_NET_CFG_MAX_VQP, /* u16 */ > > > VDPA_ATTR_DEV_NET_CFG_MTU, /* u16 */ > > > > > > + VDPA_ATTR_DEV_NEGOTIATED_FEATURES, /* u64 */ > > > + VDPA_ATTR_DEV_MGMTDEV_MAX_VQS, /* u32 */ > > > + VDPA_ATTR_DEV_SUPPORTED_FEATURES, /* u64 */ > > > > I wonder if it's better to split the patches into three where the > > above command could be implemented separately. > > I already sent three. You mean split the third patch into three? Yes, since it introduces three functions 1) specify max_vqp 2) get supported features 3) get negotiated features (Or at least two, 2 and 3 can be merged somehow) > > > > > > + > > > /* new attributes must be added above here */ > > > VDPA_ATTR_MAX, > > > }; > > > diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c > > > index 4ccb564872a0..d0dd4196610f 100644 > > > --- a/vdpa/vdpa.c > > > +++ b/vdpa/vdpa.c > > > @@ -23,6 +23,7 @@ > > > #define VDPA_OPT_VDEV_HANDLE BIT(3) > > > #define VDPA_OPT_VDEV_MAC BIT(4) > > > #define VDPA_OPT_VDEV_MTU BIT(5) > > > +#define VDPA_OPT_MAX_VQP BIT(6) > > > > > > struct vdpa_opts { > > > uint64_t present; /* flags of present items */ > > > @@ -32,6 +33,7 @@ struct vdpa_opts { > > > unsigned int device_id; > > > char mac[ETH_ALEN]; > > > uint16_t mtu; > > > + uint16_t max_vqp; > > > }; > > > > > > struct vdpa { > > > @@ -78,6 +80,9 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = { > > > [VDPA_ATTR_DEV_VENDOR_ID] = MNL_TYPE_U32, > > > [VDPA_ATTR_DEV_MAX_VQS] = MNL_TYPE_U32, > > > [VDPA_ATTR_DEV_MAX_VQ_SIZE] = MNL_TYPE_U16, > > > + [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64, > > > + [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32, > > > + [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64, > > > }; > > > > > > static int attr_cb(const struct nlattr *attr, void *data) > > > @@ -219,6 +224,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, struct vdpa *vdpa) > > > sizeof(opts->mac), opts->mac); > > > if (opts->present & VDPA_OPT_VDEV_MTU) > > > mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, opts->mtu); > > > + if (opts->present & VDPA_OPT_MAX_VQP) > > > + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, opts->max_vqp); > > > } > > > > > > static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, > > > @@ -287,6 +294,14 @@ static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, > > > > > > NEXT_ARG_FWD(); > > > o_found |= VDPA_OPT_VDEV_MTU; > > > + } else if ((matches(*argv, "max_vqp") == 0) && (o_optional & VDPA_OPT_MAX_VQP)) { > > > + NEXT_ARG_FWD(); > > > + err = vdpa_argv_u16(vdpa, argc, argv, &opts->max_vqp); > > > + if (err) > > > + return err; > > > + > > > + NEXT_ARG_FWD(); > > > + o_found |= VDPA_OPT_MAX_VQP; > > > } else { > > > fprintf(stderr, "Unknown option \"%s\"\n", *argv); > > > return -EINVAL; > > > @@ -385,6 +400,77 @@ static const char *parse_class(int num) > > > return class ? class : "< unknown class >"; > > > } > > > > > > +static const char * const net_feature_strs[64] = { > > > + [VIRTIO_NET_F_CSUM] = "CSUM", > > > + [VIRTIO_NET_F_GUEST_CSUM] = "GUEST_CSUM", > > > + [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "CTRL_GUEST_OFFLOADS", > > > + [VIRTIO_NET_F_MTU] = "MTU", > > > + [VIRTIO_NET_F_MAC] = "MAC", > > > + [VIRTIO_NET_F_GUEST_TSO4] = "GUEST_TSO4", > > > + [VIRTIO_NET_F_GUEST_TSO6] = "GUEST_TSO6", > > > + [VIRTIO_NET_F_GUEST_ECN] = "GUEST_ECN", > > > + [VIRTIO_NET_F_GUEST_UFO] = "GUEST_UFO", > > > + [VIRTIO_NET_F_HOST_TSO4] = "HOST_TSO4", > > > + [VIRTIO_NET_F_HOST_TSO6] = "HOST_TSO6", > > > + [VIRTIO_NET_F_HOST_ECN] = "HOST_ECN", > > > + [VIRTIO_NET_F_HOST_UFO] = "HOST_UFO", > > > + [VIRTIO_NET_F_MRG_RXBUF] = "MRG_RXBUF", > > > + [VIRTIO_NET_F_STATUS] = "STATUS", > > > + [VIRTIO_NET_F_CTRL_VQ] = "CTRL_VQ", > > > + [VIRTIO_NET_F_CTRL_RX] = "CTRL_RX", > > > + [VIRTIO_NET_F_CTRL_VLAN] = "CTRL_VLAN", > > > + [VIRTIO_NET_F_CTRL_RX_EXTRA] = "CTRL_RX_EXTRA", > > > + [VIRTIO_NET_F_GUEST_ANNOUNCE] = "GUEST_ANNOUNCE", > > > + [VIRTIO_NET_F_MQ] = "MQ", > > > + [VIRTIO_F_NOTIFY_ON_EMPTY] = "NOTIFY_ON_EMPTY", > > > + [VIRTIO_NET_F_CTRL_MAC_ADDR] = "CTRL_MAC_ADDR", > > > + [VIRTIO_F_ANY_LAYOUT] = "ANY_LAYOUT", > > > + [VIRTIO_NET_F_RSC_EXT] = "RSC_EXT", > > > + [VIRTIO_NET_F_STANDBY] = "STANDBY", > > > +}; > > > > It seems we are still missing things that are already supported in the > > Linux uapi. I think it's better to support them. E.g the RSS and > > SPEED_DUPLEX etc. > > > > Will do. > > > > + > > > +#define VDPA_EXT_FEATURES_SZ (VIRTIO_DEV_INDEPENDENT_F_END - \ > > > + VIRTIO_DEV_INDEPENDENT_F_START + 1) > > > + > > > +static const char * const ext_feature_strs[VDPA_EXT_FEATURES_SZ] = { > > > + [VIRTIO_F_RING_INDIRECT_DESC - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_INDIRECT_DESC", > > > + [VIRTIO_F_RING_EVENT_IDX - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_EVENT_IDX", > > > + [VIRTIO_F_VERSION_1 - VIRTIO_DEV_INDEPENDENT_F_START] = "VERSION_1", > > > + [VIRTIO_F_ACCESS_PLATFORM - VIRTIO_DEV_INDEPENDENT_F_START] = "ACCESS_PLATFORM", > > > + [VIRTIO_F_RING_PACKED - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_PACKED", > > > + [VIRTIO_F_IN_ORDER - VIRTIO_DEV_INDEPENDENT_F_START] = "IN_ORDER", > > > + [VIRTIO_F_ORDER_PLATFORM - VIRTIO_DEV_INDEPENDENT_F_START] = "ORDER_PLATFORM", > > > + [VIRTIO_F_SR_IOV - VIRTIO_DEV_INDEPENDENT_F_START] = "SR_IOV", > > > + [VIRTIO_F_NOTIFICATION_DATA - VIRTIO_DEV_INDEPENDENT_F_START] = "NOTIFICATION_DATA", > > > +}; > > > + > > > +static void print_net_features(struct vdpa *vdpa, uint64_t features, bool maxf) > > > +{ > > > + const char *s; > > > + int i; > > > + > > > + if (maxf) > > > + pr_out_array_start(vdpa, "dev_features"); > > > + else > > > + pr_out_array_start(vdpa, "negotiated_features"); > > > + > > > + for (i = 0; i < 64; i++) { > > > + if (!(features & (1ULL << i))) > > > + continue; > > > + > > > + if (i >= VIRTIO_DEV_INDEPENDENT_F_START && i <= VIRTIO_DEV_INDEPENDENT_F_END) > > > > I don't see any issue that just use VIRTIO_TRANSPORT_F_START and > > VIRTIO_TRANSPORT_F_END (even if END can change). > > I don't get you I meant any reason we can't simply use VIRTIO_TRANSPORT_F_START/END? > > > > > > + s = ext_feature_strs[i - VIRTIO_DEV_INDEPENDENT_F_START]; > > > + else > > > + s = net_feature_strs[i]; > > > + > > > + if (!s) > > > + print_uint(PRINT_ANY, NULL, " unrecognized_bit_%d", i); > > > + else > > > + print_string(PRINT_ANY, NULL, " %s", s); > > > + } > > > + pr_out_array_end(vdpa); > > > +} > > > + > > > static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh, > > > struct nlattr **tb) > > > { > > > @@ -408,6 +494,22 @@ static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh, > > > pr_out_array_end(vdpa); > > > } > > > > > > + if (tb[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS]) { > > > + uint16_t num_vqs; > > > + > > > + if (!vdpa->json_output) > > > + printf("\n"); > > > + num_vqs = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS]); > > > + print_uint(PRINT_ANY, "max_supported_vqs", " max_supported_vqs %d", num_vqs); > > > + } > > > + > > > + if (tb[VDPA_ATTR_DEV_SUPPORTED_FEATURES]) { > > > + uint64_t features; > > > + > > > + features = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_SUPPORTED_FEATURES]); > > > + print_net_features(vdpa, features, true); > > > > Do we need to check whether it's a networking device before trying to > > print the feature > > Yes, will fix > > > and for other type devices we can simply print the > > bit number as a startup? > > > > Why not add proper support (e.g. strings) for other types of devices when intoduced? Then the vdpa tool won't show any features which seems sub-optimal. Note that we've already had virtio-blk parent: IFCVF, simulator and VDUSE. Thanks > > > Thanks > > > > > + } > > > + > > > pr_out_handle_end(vdpa); > > > } > > > > > > @@ -557,7 +659,7 @@ static int cmd_dev_add(struct vdpa *vdpa, int argc, char **argv) > > > NLM_F_REQUEST | NLM_F_ACK); > > > err = vdpa_argv_parse_put(nlh, vdpa, argc, argv, > > > VDPA_OPT_VDEV_MGMTDEV_HANDLE | VDPA_OPT_VDEV_NAME, > > > - VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU); > > > + VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU | VDPA_OPT_MAX_VQP); > > > if (err) > > > return err; > > > > > > @@ -579,9 +681,10 @@ static int cmd_dev_del(struct vdpa *vdpa, int argc, char **argv) > > > return mnlu_gen_socket_sndrcv(&vdpa->nlg, nlh, NULL, NULL); > > > } > > > > > > -static void pr_out_dev_net_config(struct nlattr **tb) > > > +static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb) > > > { > > > SPRINT_BUF(macaddr); > > > + uint64_t val_u64; > > > uint16_t val_u16; > > > > > > if (tb[VDPA_ATTR_DEV_NET_CFG_MACADDR]) { > > > @@ -610,6 +713,10 @@ static void pr_out_dev_net_config(struct nlattr **tb) > > > val_u16 = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_NET_CFG_MTU]); > > > print_uint(PRINT_ANY, "mtu", "mtu %d ", val_u16); > > > } > > > + if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]) { > > > + val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]); > > > + print_net_features(vdpa, val_u64, false); > > > + } > > > } > > > > > > static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb) > > > @@ -619,7 +726,7 @@ static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb) > > > pr_out_vdev_handle_start(vdpa, tb); > > > switch (device_id) { > > > case VIRTIO_ID_NET: > > > - pr_out_dev_net_config(tb); > > > + pr_out_dev_net_config(vdpa, tb); > > > break; > > > default: > > > break; > > > -- > > > 2.34.1 > > > > > >
diff --git a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h index b7eab069988a..171122dd03c9 100644 --- a/vdpa/include/uapi/linux/vdpa.h +++ b/vdpa/include/uapi/linux/vdpa.h @@ -40,6 +40,10 @@ enum vdpa_attr { VDPA_ATTR_DEV_NET_CFG_MAX_VQP, /* u16 */ VDPA_ATTR_DEV_NET_CFG_MTU, /* u16 */ + VDPA_ATTR_DEV_NEGOTIATED_FEATURES, /* u64 */ + VDPA_ATTR_DEV_MGMTDEV_MAX_VQS, /* u32 */ + VDPA_ATTR_DEV_SUPPORTED_FEATURES, /* u64 */ + /* new attributes must be added above here */ VDPA_ATTR_MAX, }; diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c index 4ccb564872a0..d0dd4196610f 100644 --- a/vdpa/vdpa.c +++ b/vdpa/vdpa.c @@ -23,6 +23,7 @@ #define VDPA_OPT_VDEV_HANDLE BIT(3) #define VDPA_OPT_VDEV_MAC BIT(4) #define VDPA_OPT_VDEV_MTU BIT(5) +#define VDPA_OPT_MAX_VQP BIT(6) struct vdpa_opts { uint64_t present; /* flags of present items */ @@ -32,6 +33,7 @@ struct vdpa_opts { unsigned int device_id; char mac[ETH_ALEN]; uint16_t mtu; + uint16_t max_vqp; }; struct vdpa { @@ -78,6 +80,9 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = { [VDPA_ATTR_DEV_VENDOR_ID] = MNL_TYPE_U32, [VDPA_ATTR_DEV_MAX_VQS] = MNL_TYPE_U32, [VDPA_ATTR_DEV_MAX_VQ_SIZE] = MNL_TYPE_U16, + [VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64, + [VDPA_ATTR_DEV_MGMTDEV_MAX_VQS] = MNL_TYPE_U32, + [VDPA_ATTR_DEV_SUPPORTED_FEATURES] = MNL_TYPE_U64, }; static int attr_cb(const struct nlattr *attr, void *data) @@ -219,6 +224,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, struct vdpa *vdpa) sizeof(opts->mac), opts->mac); if (opts->present & VDPA_OPT_VDEV_MTU) mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, opts->mtu); + if (opts->present & VDPA_OPT_MAX_VQP) + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, opts->max_vqp); } static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, @@ -287,6 +294,14 @@ static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, NEXT_ARG_FWD(); o_found |= VDPA_OPT_VDEV_MTU; + } else if ((matches(*argv, "max_vqp") == 0) && (o_optional & VDPA_OPT_MAX_VQP)) { + NEXT_ARG_FWD(); + err = vdpa_argv_u16(vdpa, argc, argv, &opts->max_vqp); + if (err) + return err; + + NEXT_ARG_FWD(); + o_found |= VDPA_OPT_MAX_VQP; } else { fprintf(stderr, "Unknown option \"%s\"\n", *argv); return -EINVAL; @@ -385,6 +400,77 @@ static const char *parse_class(int num) return class ? class : "< unknown class >"; } +static const char * const net_feature_strs[64] = { + [VIRTIO_NET_F_CSUM] = "CSUM", + [VIRTIO_NET_F_GUEST_CSUM] = "GUEST_CSUM", + [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "CTRL_GUEST_OFFLOADS", + [VIRTIO_NET_F_MTU] = "MTU", + [VIRTIO_NET_F_MAC] = "MAC", + [VIRTIO_NET_F_GUEST_TSO4] = "GUEST_TSO4", + [VIRTIO_NET_F_GUEST_TSO6] = "GUEST_TSO6", + [VIRTIO_NET_F_GUEST_ECN] = "GUEST_ECN", + [VIRTIO_NET_F_GUEST_UFO] = "GUEST_UFO", + [VIRTIO_NET_F_HOST_TSO4] = "HOST_TSO4", + [VIRTIO_NET_F_HOST_TSO6] = "HOST_TSO6", + [VIRTIO_NET_F_HOST_ECN] = "HOST_ECN", + [VIRTIO_NET_F_HOST_UFO] = "HOST_UFO", + [VIRTIO_NET_F_MRG_RXBUF] = "MRG_RXBUF", + [VIRTIO_NET_F_STATUS] = "STATUS", + [VIRTIO_NET_F_CTRL_VQ] = "CTRL_VQ", + [VIRTIO_NET_F_CTRL_RX] = "CTRL_RX", + [VIRTIO_NET_F_CTRL_VLAN] = "CTRL_VLAN", + [VIRTIO_NET_F_CTRL_RX_EXTRA] = "CTRL_RX_EXTRA", + [VIRTIO_NET_F_GUEST_ANNOUNCE] = "GUEST_ANNOUNCE", + [VIRTIO_NET_F_MQ] = "MQ", + [VIRTIO_F_NOTIFY_ON_EMPTY] = "NOTIFY_ON_EMPTY", + [VIRTIO_NET_F_CTRL_MAC_ADDR] = "CTRL_MAC_ADDR", + [VIRTIO_F_ANY_LAYOUT] = "ANY_LAYOUT", + [VIRTIO_NET_F_RSC_EXT] = "RSC_EXT", + [VIRTIO_NET_F_STANDBY] = "STANDBY", +}; + +#define VDPA_EXT_FEATURES_SZ (VIRTIO_DEV_INDEPENDENT_F_END - \ + VIRTIO_DEV_INDEPENDENT_F_START + 1) + +static const char * const ext_feature_strs[VDPA_EXT_FEATURES_SZ] = { + [VIRTIO_F_RING_INDIRECT_DESC - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_INDIRECT_DESC", + [VIRTIO_F_RING_EVENT_IDX - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_EVENT_IDX", + [VIRTIO_F_VERSION_1 - VIRTIO_DEV_INDEPENDENT_F_START] = "VERSION_1", + [VIRTIO_F_ACCESS_PLATFORM - VIRTIO_DEV_INDEPENDENT_F_START] = "ACCESS_PLATFORM", + [VIRTIO_F_RING_PACKED - VIRTIO_DEV_INDEPENDENT_F_START] = "RING_PACKED", + [VIRTIO_F_IN_ORDER - VIRTIO_DEV_INDEPENDENT_F_START] = "IN_ORDER", + [VIRTIO_F_ORDER_PLATFORM - VIRTIO_DEV_INDEPENDENT_F_START] = "ORDER_PLATFORM", + [VIRTIO_F_SR_IOV - VIRTIO_DEV_INDEPENDENT_F_START] = "SR_IOV", + [VIRTIO_F_NOTIFICATION_DATA - VIRTIO_DEV_INDEPENDENT_F_START] = "NOTIFICATION_DATA", +}; + +static void print_net_features(struct vdpa *vdpa, uint64_t features, bool maxf) +{ + const char *s; + int i; + + if (maxf) + pr_out_array_start(vdpa, "dev_features"); + else + pr_out_array_start(vdpa, "negotiated_features"); + + for (i = 0; i < 64; i++) { + if (!(features & (1ULL << i))) + continue; + + if (i >= VIRTIO_DEV_INDEPENDENT_F_START && i <= VIRTIO_DEV_INDEPENDENT_F_END) + s = ext_feature_strs[i - VIRTIO_DEV_INDEPENDENT_F_START]; + else + s = net_feature_strs[i]; + + if (!s) + print_uint(PRINT_ANY, NULL, " unrecognized_bit_%d", i); + else + print_string(PRINT_ANY, NULL, " %s", s); + } + pr_out_array_end(vdpa); +} + static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh, struct nlattr **tb) { @@ -408,6 +494,22 @@ static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh, pr_out_array_end(vdpa); } + if (tb[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS]) { + uint16_t num_vqs; + + if (!vdpa->json_output) + printf("\n"); + num_vqs = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_MGMTDEV_MAX_VQS]); + print_uint(PRINT_ANY, "max_supported_vqs", " max_supported_vqs %d", num_vqs); + } + + if (tb[VDPA_ATTR_DEV_SUPPORTED_FEATURES]) { + uint64_t features; + + features = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_SUPPORTED_FEATURES]); + print_net_features(vdpa, features, true); + } + pr_out_handle_end(vdpa); } @@ -557,7 +659,7 @@ static int cmd_dev_add(struct vdpa *vdpa, int argc, char **argv) NLM_F_REQUEST | NLM_F_ACK); err = vdpa_argv_parse_put(nlh, vdpa, argc, argv, VDPA_OPT_VDEV_MGMTDEV_HANDLE | VDPA_OPT_VDEV_NAME, - VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU); + VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU | VDPA_OPT_MAX_VQP); if (err) return err; @@ -579,9 +681,10 @@ static int cmd_dev_del(struct vdpa *vdpa, int argc, char **argv) return mnlu_gen_socket_sndrcv(&vdpa->nlg, nlh, NULL, NULL); } -static void pr_out_dev_net_config(struct nlattr **tb) +static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb) { SPRINT_BUF(macaddr); + uint64_t val_u64; uint16_t val_u16; if (tb[VDPA_ATTR_DEV_NET_CFG_MACADDR]) { @@ -610,6 +713,10 @@ static void pr_out_dev_net_config(struct nlattr **tb) val_u16 = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_NET_CFG_MTU]); print_uint(PRINT_ANY, "mtu", "mtu %d ", val_u16); } + if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]) { + val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]); + print_net_features(vdpa, val_u64, false); + } } static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb) @@ -619,7 +726,7 @@ static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb) pr_out_vdev_handle_start(vdpa, tb); switch (device_id) { case VIRTIO_ID_NET: - pr_out_dev_net_config(tb); + pr_out_dev_net_config(vdpa, tb); break; default: break;
Add support to configure max supported virtqueue pairs for a vdpa device. For this to be possible, add support for reading management device's capabilities. Management device capabilities give the user a hint as to how many virtqueue pairs at most he can ask for. Using this information the user can choose a valid number of virtqueue pairs when creating the device. Examples: - Show management device capabiliteis: $ vdpa mgmtdev show auxiliary/mlx5_core.sf.1: supported_classes net max_supported_vqs 257 dev_features CSUM GUEST_CSUM MTU HOST_TSO4 HOST_TSO6 STATUS CTRL_VQ \ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM A user can now create a device based on the above information. In the above case 128 virtqueue pairs at most. The other VQ being for the control virtqueue. - Add a vdpa device with 16 data virtqueue pairs $ vdpa dev add name vdpa-a mgmtdev auxiliary/mlx5_core.sf.1 max_vqp 16 After feature negotiation has been completed, one can read the vdpa configuration using: $ vdpa dev config show vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 16 mtu 1500 negotiated_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS CTRL_VQ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM Reviewed-by: Jianbo Liu <jianbol@nvidia.com> Signed-off-by: Eli Cohen <elic@nvidia.com> --- vdpa/include/uapi/linux/vdpa.h | 4 ++ vdpa/vdpa.c | 113 ++++++++++++++++++++++++++++++++- 2 files changed, 114 insertions(+), 3 deletions(-)