Re: [Intel-wired-lan] [PATCH net] ice: fix setting RSS VSI hash for E830

[Marcin Szycik <marcin.szycik@linux.intel.com>]

On 12.01.2026 12:15, Vadim Fedorenko wrote:
> On 12/01/2026 09:36, Marcin Szycik wrote:
>>
>>
>> On 09.01.2026 17:44, Vadim Fedorenko wrote:
>>> On 09/01/2026 08:53, Marcin Szycik wrote:
>>>> ice_set_rss_hfunc() performs a VSI update, in which it sets hashing
>>>> function, leaving other VSI options unchanged. However, ::q_opt_flags is
>>>> mistakenly set to the value of another field, instead of its original
>>>> value, probably due to a typo. What happens next is hardware-dependent:
>>>>
>>>> On E810, only the first bit is meaningful (see
>>>> ICE_AQ_VSI_Q_OPT_PE_FLTR_EN) and can potentially end up in a different
>>>> state than before VSI update.
>>>>
>>>> On E830, some of the remaining bits are not reserved. Setting them
>>>> to some unrelated values can cause the firmware to reject the update
>>>> because of invalid settings, or worse - succeed.
>>>>
>>>> Reproducer:
>>>>     sudo ethtool -X $PF1 equal 8
>>>>
>>>> Output in dmesg:
>>>>     Failed to configure RSS hash for VSI 6, error -5
>>>>
>>>> Fixes: 352e9bf23813 ("ice: enable symmetric-xor RSS for Toeplitz hash function")
>>>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>>>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>>> Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
>>>> ---
>>>>    drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>>>> index cf8ba5a85384..08268f1a03da 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>>> @@ -8038,7 +8038,7 @@ int ice_set_rss_hfunc(struct ice_vsi *vsi, u8 hfunc)
>>>>        ctx->info.q_opt_rss |=
>>>>            FIELD_PREP(ICE_AQ_VSI_Q_OPT_RSS_HASH_M, hfunc);
>>>>        ctx->info.q_opt_tc = vsi->info.q_opt_tc;
>>>> -    ctx->info.q_opt_flags = vsi->info.q_opt_rss;
>>>> +    ctx->info.q_opt_flags = vsi->info.q_opt_flags;
>>>
>>> The very same typo pattern is in ice_vc_handle_rss_cfg() in
>>> ice/virt/rss.c
>>>
>>> I believe both places have to be fixed.
>>
>> Hmm... where exactly? ice_vc_rss_hash_update() (called from ice_vc_handle_rss_cfg()) looks correct.
> 
> Sorry, it was fixed in 3a6d87e2eaac ("ice: implement GTP RSS context
> tracking and configuration") when the logic was moved to
> ice_vc_rss_hash_update(), but this code was never backported, the
> problem exists in 6.18...

Sorry for late reply, I don't know why, but I assumed the patch you
mention would be independently picked up to stable. Now that I've
looked into it, I see it's a feature implementation that accidentally (?)
also fixed that one line in ice_vc_handle_rss_cfg(), so it won't be
backported.

Do we know if the mistake in ice_vc_handle_rss_cfg() is an actual issue?
So far I wasn't even able to enter that branch (rss_cfg->rss_algorithm is
never VIRTCHNL_RSS_ALG_R_ASYMMETRIC, in fact this is the only usage of
VIRTCHNL_RSS_ALG_R_ASYMMETRIC - dead code?). If I understand correctly,
"theoretical" fixes are not welcome in stable.

Thanks,
Marcin

> 
>>
>> Thanks for reviewing,
>> Marcin
>>
>>>
>>>>          err = ice_update_vsi(hw, vsi->idx, ctx, NULL);
>>>>        if (err) {
>>>
>>
>