| Message ID | 20220920151419.76050-3-simon.horman@corigine.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | nfp: support VF multi-queues configuration | expand |
On 20/09/2022 16:14, Simon Horman wrote: > From: Peng Zhang <peng.zhang@corigine.com> > > VF max-queue-number is the MAX num of queues which the VF has. > > Add new device generic parameter to configure the max-queue-number > of the each VF to be generated dynamically. > > The string format is decided ad vendor specific. The suggested > format is ...-V-W-X-Y-Z, the V represents generating V VFs that > have 16 queues, the W represents generating W VFs that have 8 > queues, and so on, the Z represents generating Z VFs that have > 1 queue. I don't like this. If I'm correctly understanding, it hardcodes an assumption that lower-numbered VFs will be the ones with more queues, and also makes it difficult to change a VF's max-queues on the fly. Why not instead have a per-VF operation to set that VF's max queue count? Ideally through the VF representor, perhaps as an ethtool param/tunable, rather than devlink. Then the mechanism is flexible and makes no assumptions about policy. -ed
On Tue, 20 Sep 2022 19:27:48 +0100, Edward Cree wrote: > On 20/09/2022 16:14, Simon Horman wrote: > > From: Peng Zhang <peng.zhang@corigine.com> > > > > VF max-queue-number is the MAX num of queues which the VF has. > > > > Add new device generic parameter to configure the max-queue-number > > of the each VF to be generated dynamically. > > > > The string format is decided ad vendor specific. The suggested > > format is ...-V-W-X-Y-Z, the V represents generating V VFs that > > have 16 queues, the W represents generating W VFs that have 8 > > queues, and so on, the Z represents generating Z VFs that have > > 1 queue. > > I don't like this. > If I'm correctly understanding, it hardcodes an assumption that > lower-numbered VFs will be the ones with more queues, and also Usually all VFs have same max-q-num, so config like "...-0-N-0-.." will be the most case. If you want some VFs have more queues than the others, then yes, the lower-numbered VFs always have more. We think it's OK, since the user can decide which VFs for what use. If you need VFs with fewer queues, then use those higher-numbered VFs. And the format is just a recommendation, the parse process is implemented in NIC driver, so it can be vendor specific. > makes it difficult to change a VF's max-queues on the fly. > Why not instead have a per-VF operation to set that VF's max > queue count? Ideally through the VF representor, perhaps as > an ethtool param/tunable, rather than devlink. Then the > mechanism is flexible and makes no assumptions about policy. The background of this configuration proposal is that we need a way to reallocate the queue resource that is shared by all VFs. So it should be pre-configured before VFs are created, that's why the configuration is per-PF or per-pcidev. We're not supposed to change the max-q-number once the VF is created. Thanks. > > -ed
diff --git a/Documentation/networking/devlink/devlink-params.rst b/Documentation/networking/devlink/devlink-params.rst index 4e01dc32bc08..4b415b1acc9d 100644 --- a/Documentation/networking/devlink/devlink-params.rst +++ b/Documentation/networking/devlink/devlink-params.rst @@ -137,3 +137,8 @@ own name. * - ``event_eq_size`` - u32 - Control the size of asynchronous control events EQ. + * - ``max_vf_queue`` + - String + - Configure the queue of the each VF to be generated dynamically. When + created VF number is bigger than that is configured by this parameter, + the extra VFs' max-queue-number is decided as vendor specific. diff --git a/include/net/devlink.h b/include/net/devlink.h index 264aa98e6da6..b756be29f824 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -496,6 +496,7 @@ enum devlink_param_generic_id { DEVLINK_PARAM_GENERIC_ID_ENABLE_IWARP, DEVLINK_PARAM_GENERIC_ID_IO_EQ_SIZE, DEVLINK_PARAM_GENERIC_ID_EVENT_EQ_SIZE, + DEVLINK_PARAM_GENERIC_ID_MAX_VF_QUEUE, /* add new param generic ids above here*/ __DEVLINK_PARAM_GENERIC_ID_MAX, @@ -554,6 +555,9 @@ enum devlink_param_generic_id { #define DEVLINK_PARAM_GENERIC_EVENT_EQ_SIZE_NAME "event_eq_size" #define DEVLINK_PARAM_GENERIC_EVENT_EQ_SIZE_TYPE DEVLINK_PARAM_TYPE_U32 +#define DEVLINK_PARAM_GENERIC_MAX_VF_QUEUE_NAME "max_vf_queue" +#define DEVLINK_PARAM_GENERIC_MAX_VF_QUEUE_TYPE DEVLINK_PARAM_TYPE_STRING + #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate) \ { \ .id = DEVLINK_PARAM_GENERIC_ID_##_id, \ diff --git a/net/core/devlink.c b/net/core/devlink.c index 7776dc82f88d..f447e8b11a80 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -5145,6 +5145,11 @@ static const struct devlink_param devlink_param_generic[] = { .name = DEVLINK_PARAM_GENERIC_EVENT_EQ_SIZE_NAME, .type = DEVLINK_PARAM_GENERIC_EVENT_EQ_SIZE_TYPE, }, + { + .id = DEVLINK_PARAM_GENERIC_ID_MAX_VF_QUEUE, + .name = DEVLINK_PARAM_GENERIC_MAX_VF_QUEUE_NAME, + .type = DEVLINK_PARAM_GENERIC_MAX_VF_QUEUE_TYPE, + }, }; static int devlink_param_generic_verify(const struct devlink_param *param)