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

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 1 Nov 2024 02:34:59 +0900 Taehee Yoo wrote:
> While I'm writing a patch I face an ambiguous problem here.
> ethnl_set_ring() first calls .get_ringparam() to get current config.
> Then it calls .set_ringparam() after it sets the current config + new
> config to param structures.
> The bnxt_set_ringparam() may receive ETHTOOL_TCP_DATA_SPLIT_ENABLED
> because two cases.
> 1. from user
> 2. from bnxt_get_ringparam() because of UNKNWON.
> The problem is that the bnxt_set_ringparam() can't distinguish them.
> The problem scenario is here.
> 1. tcp-data-split is UNKNOWN mode.
> 2. HDS is automatically enabled because one of LRO or GRO is enabled.
> 3. user changes ring parameter with following command
> `ethtool -G eth0 rx 1024`
> 4. ethnl_set_rings() calls .get_ringparam() to get current config.
> 5. bnxt_get_ringparam() returns ENABLE of HDS because of UNKNWON mode.
> 6. ethnl_set_rings() calls .set_ringparam() after setting param with
> configs comes from .get_ringparam().
> 7. bnxt_set_ringparam() is passed ETHTOOL_TCP_DATA_SPLIT_ENABLED but
> the user didn't set it explicitly.
> 8. bnxt_set_ringparam() eventually force enables tcp-data-split.
> 
> I couldn't find a way to distinguish them so far.
> I'm not sure if this is acceptable or not.
> Maybe we need to modify a scenario?

I thought we discussed this, but I may be misremembering.
You may need to record in the core whether the setting came 
from the user or not (similarly to IFF_RXFH_CONFIGURED).
User setting UNKNWON would mean "reset".
Maybe I'm misunderstanding..