Re: [PATCH net-next v5 3/7] bnxt_en: add support for tcp-data-split ethtool command

Jakub Kicinski
2024-11-15T04:15:32Z

On Wed, 13 Nov 2024 17:32:17 +0000 Taehee Yoo wrote:
> NICs that uses bnxt_en driver supports tcp-data-split feature by the
> name of HDS(header-data-split).
> But there is no implementation for the HDS to enable by ethtool.
> Only getting the current HDS status is implemented and The HDS is just
> automatically enabled only when either LRO, HW-GRO, or JUMBO is enabled.
> The hds_threshold follows rx-copybreak value. and it was unchangeable.
>
> This implements `ethtool -G <interface name> tcp-data-split <value>`
> command option.
> The value can be <on> and <auto>.
> The value is <auto> and one of LRO/GRO/JUMBO is enabled, HDS is
> automatically enabled and all LRO/GRO/JUMBO are disabled, HDS is
> automatically disabled.
>
> HDS feature relies on the aggregation ring.
> So, if HDS is enabled, the bnxt_en driver initializes the aggregation ring.
> This is the reason why BNXT_FLAG_AGG_RINGS contains HDS condition.

I may be missing some existing check but doesn't enabling XDP force
page_mode which in turn clears BNXT_FLAG_AGG_RINGS, including HDS ?
If user specifically requested HDS we should refuse to install XDP
in non-multibuf mode.

TBH a selftest under tools/testing/drivers/net would go a long way
to make it clear we caught all cases. You can add a dummy netdevsim
implementation for testing without bnxt present (some of the existing
python tests can work with real drivers and netdevsim):