Re: [RFC PATCH 0/1] vmxnet3: Adjust maximum Rx ring buffer size

[Aaron Tomlin]

On Tue, 7 Jan 2025, Jakub Kicinski wrote:
> This is a bit of a weird driver. But we should distinguish the default
> ring size, which yes, should not be too large, and max ring size which
> can be large but user setting a large size risks the fact the
> allocations will fail and device will not open.
>
> This driver seems to read the default size from the hypervisor, is that
> the value that is too large in your case? Maybe we should min() it with
> something reasonable? The max allowed to be set via ethtool can remain
> high IMO
>

See vmxnet3_get_ringparam(). If I understand correctly, since commit
50a5ce3e7116a ("vmxnet3: add receive data ring support"), if the specified
VMXNET3 adapter has support for the Rx Data ring feature then the maximum
Rx Data buffer size is reported as VMXNET3_RXDATA_DESC_MAX_SIZE (i.e. 2048)
by 'ethtool'. Furthermore, See vmxnet3_set_ringparam(). A user specified Rx
mini value cannot be more than VMXNET3_RXDATA_DESC_MAX_SIZE. Indeed the Rx
mini value in the context of VMXNET3 would be the size of the Rx Data ring
buffer. See the following excerpt from vmxnet3_set_ringparam(). As far as I
can tell, the Rx Data buffer cannot be more than
VMXNET3_RXDATA_DESC_MAX_SIZE:

 686 static int
 687 vmxnet3_set_ringparam(struct net_device *netdev,
 688                       struct ethtool_ringparam *param,
 689                       struct kernel_ethtool_ringparam *kernel_param,
 690                       struct netlink_ext_ack *extack)
 691 {
  :
 760         new_rxdata_desc_size =
 761                 (param->rx_mini_pending + VMXNET3_RXDATA_DESC_SIZE_MASK) &
 762                 ~VMXNET3_RXDATA_DESC_SIZE_MASK;
 763         new_rxdata_desc_size = min_t(u16, new_rxdata_desc_size,
 764                                      VMXNET3_RXDATA_DESC_MAX_SIZE);


Have I missed something?


-- 
Aaron Tomlin