On Wed, Aug 06, 2025 at 11:44:49AM -0700, Mingming Cao wrote:
> POWER8 support a maximum of 16 subcrq indirect descriptor entries per
> H_SEND_SUB_CRQ_INDIRECT call, while POWER9 and newer hypervisors
> support up to 128 entries. Increasing the max number of indirect
> descriptor entries improves batching efficiency and reduces
> hcall overhead, which enhances throughput under large workload on POWER9+.
>
> Currently, ibmvnic driver always uses a fixed number of max indirect
> descriptor entries (16). send_subcrq_indirect() treats all hypervisor
> errors the same:
> - Cleanup and Drop the entire batch of descriptors.
> - Return an error to the caller.
> - Rely on TCP/IP retransmissions to recover.
> - If the hypervisor returns H_PARAMETER (e.g., because 128
> entries are not supported on POWER8), the driver will continue
> to drop batches, resulting in unnecessary packet loss.
>
> In this patch:
> Raise the default maximum indirect entries to 128 to improve ibmvnic
> batching on morden platform. But also gracefully fall back to
> 16 entries for Power 8 systems.
>
> Since there is no VIO interface to query the hypervisor’s supported
> limit, vnic handles send_subcrq_indirect() H_PARAMETER errors:
> - On first H_PARAMETER failure, log the failure context
> - Reduce max_indirect_entries to 16 and allow the single batch to drop.
> - Subsequent calls automatically use the correct lower limit,
> avoiding repeated drops.
>
> The goal is to optimizes performance on modern systems while handles
> falling back for older POWER8 hypervisors.
>
> Performance shows 40% improvements with MTU (1500) on largework load.
>
> --------------------------------------
> Changes since v3:
> Link to v3: https://www.spinics.net/lists/netdev/msg1112828.html
> - consolidate H_PARAMTER handling & subcrq ind desc limit reset for RX/TX
> into a helper function
> - Cleanup and clarify comments in post migration case
> - Renamed the limits to be a clear and simple name
Thanks for the updates.
I'm sorry for not mentioning this in my review of v3, but net-next
is currently closed for the merge window. Could you please repost,
or post a v4, once it re-opens. That should happen once v6.17-rc1
has been released. Probably early next week (week of 11th August).
My minor nits below notwithstanding this looks good to me.
So feel free to include.
Reviewed-by: Simon Horman <ho...@kernel.org>
N.b.: I will be on a break when net-next reopens.
So please don't wait for feedback from me then.
>
> Changes since v2:
> link to v2: https://www.spinics.net/lists/netdev/msg1104669.html
>
> -- was Patch 4 from a patch series v2. v2 introduced a module parameter
> for backward compatibility. Based on review feedback, This patch handles
> older systems fall back case without adding a module parameter.
>
> Signed-off-by: Mingming Cao <m...@linux.ibm.com>
> Reviewed-by: Brian King <bjki...@linux.ibm.com>
> Reviewed-by: Haren Myneni <ha...@linux.ibm.com>
> ---
These days it is preferable to put the revision history here.
Rather than above your Signed-off-by line, as is currently the case.
> drivers/net/ethernet/ibm/ibmvnic.c | 59 ++++++++++++++++++++++++++----
> drivers/net/ethernet/ibm/ibmvnic.h | 6 ++-
> 2 files changed, 56 insertions(+), 9 deletions(-)
Or here.
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
...
> @@ -6369,6 +6400,19 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter
> *adapter, bool reset)
> rc = reset_sub_crq_queues(adapter);
> }
> } else {
> + if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
> + /* After an LPM, reset the max number of indirect
> + * subcrq descriptors per H_SEND_SUB_CRQ_INDIRECT
> + * hcall to the default max (e.g POWER8 -> POWER10)
> + *
> + * If the new destination platform does not support
> + * the higher limit max (e.g. POWER10-> POWER8 LPM)
> + * H_PARAMETER will trigger automatic fallback to the
> + * safe minimium limit.
minimum
> + */
> + adapter->cur_max_ind_descs = IBMVNIC_MAX_IND_DESCS;
> + }
> +
> rc = init_sub_crqs(adapter);
> }
...
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.h
> b/drivers/net/ethernet/ibm/ibmvnic.h
> index 246ddce753f9..480dc587078f 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.h
> +++ b/drivers/net/ethernet/ibm/ibmvnic.h
> @@ -29,8 +29,9 @@
> #define IBMVNIC_BUFFS_PER_POOL 100
> #define IBMVNIC_MAX_QUEUES 16
> #define IBMVNIC_MAX_QUEUE_SZ 4096
> -#define IBMVNIC_MAX_IND_DESCS 16
> -#define IBMVNIC_IND_ARR_SZ (IBMVNIC_MAX_IND_DESCS * 32)
> +#define IBMVNIC_MAX_IND_DESCS 128
> +#define IBMVNIC_SAFE_IND_DESC 16
> +#define IBMVNIC_IND_MAX_ARR_SZ (IBMVNIC_MAX_IND_DESCS * 32)
nit: maybe move towards using tabs before the values here?
+#define IBMVNIC_MAX_IND_DESCS 128
+#define IBMVNIC_SAFE_IND_DESC 16
+#define IBMVNIC_IND_MAX_ARR_SZ (IBMVNIC_MAX_IND_DESCS * 32)
...
--
pw-bot: deferred
