Re: [PATCH net-next] selftests: drv-net: rss: validate min RSS table size

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Jakub Kicinski wrote:
> Add a test which checks that the RSS table is at least 4x the max
> queue count supported by the device. The original RSS spec from
> Microsoft stated that the RSS indirection table should be 2 to 8
> times the CPU count, presumably assuming queue per CPU. If the
> CPU count is not a power of two, however, a power-of-2 table
> 2x larger than queue count results in a 33% traffic imbalance.
> Validate that the indirection table is at least 4x the queue
> count. This lowers the imbalance to 16% which empirically
> appears to be more acceptable to memcache-like workloads.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

> +def _test_rss_indir_size(cfg, qcnt, context=0):
> +    """Test that indirection table size is at least 4x queue count."""
> +    ethtool(f"-L {cfg.ifname} combined {qcnt}")

Remind me: does this work with devices that advertise RX N TX N rather
than combined N?

> +
> +    rss = _get_rss(cfg, context=context)
> +    indir = rss['rss-indirection-table']
> +    ksft_ge(len(indir), 4 * qcnt, "Table smaller than 4x")
> +    return len(indir)
> +
> +
> +@ksft_variants([
> +    KsftNamedVariant("main", False),
> +    KsftNamedVariant("ctx", True),
> +])
> +def indir_size_4x(cfg, create_context):
> +    """
> +    Test that the indirection table has at least 4 entries per queue.
> +    Empirically network-heavy workloads like memcache suffer with the 33%
> +    imbalance of a 2x indirection table size.
> +    4x table translates to a 16% imbalance.
> +    """
> +    channels = cfg.ethnl.channels_get({'header': {'dev-index': cfg.ifindex}})
> +    ch_max = channels.get('combined-max', 0)

Same here: not all drivers set this.

Perhaps we should skip if absent?

And does combined-max mean all queues across all contexts, or per
context? The test seems to imply the second. My intuition was the
first. Is it clearly defined across devices. per ethtool_channels,
seems per device?

  * @max_combined: Read only. Maximum number of combined channel the driver
  *      support. Set of queues RX, TX or other.


> +    qcnt = channels['combined-count']
> +
> +    if ch_max < 3:
> +        raise KsftSkipEx(f"Not enough queues for the test: max={ch_max}")
> +
> +    defer(ethtool, f"-L {cfg.ifname} combined {qcnt}")
> +    ethtool(f"-L {cfg.ifname} combined 3")
> +
> +    ctx_id = _maybe_create_context(cfg, create_context)
> +
> +    indir_sz = _test_rss_indir_size(cfg, 3, context=ctx_id)
> +
> +    # Test with max queue count (max - 1 if max is a power of two)
> +    test_max = ch_max - 1 if _is_power_of_two(ch_max) else ch_max
> +    if test_max > 3 and indir_sz < test_max * 4:
> +        _test_rss_indir_size(cfg, test_max, context=ctx_id)
> +