netdev - Re: [PATCH iwl-net] ice: Block switchdev mode when ADQ is acvite and vice versa

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZMvnWIhrpOJA8bG+@nanopsycho>
Date: Thu, 3 Aug 2023 19:43:52 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Marcin Szycik <marcin.szycik@...ux.intel.com>
Cc: Leon Romanovsky <leon@...nel.org>, intel-wired-lan@...ts.osuosl.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH iwl-net] ice: Block switchdev mode when ADQ is acvite and
 vice versa

Thu, Aug 03, 2023 at 05:11:16PM CEST, marcin.szycik@...ux.intel.com wrote:
>
>
>On 03.08.2023 15:26, Jiri Pirko wrote:
>> Thu, Aug 03, 2023 at 03:11:26PM CEST, leon@...nel.org wrote:
>>> On Tue, Aug 01, 2023 at 01:52:35PM +0200, Marcin Szycik wrote:
>>>> ADQ and switchdev are not supported simultaneously. Enabling both at the
>>>> same time can result in nullptr dereference.
>>>>
>>>> To prevent this, check if ADQ is active when changing devlink mode to
>>>> switchdev mode, and check if switchdev is active when enabling ADQ.
>>>>
>>>> Fixes: fbc7b27af0f9 ("ice: enable ndo_setup_tc support for mqprio_qdisc")
>>>> Signed-off-by: Marcin Szycik <marcin.szycik@...ux.intel.com>
>>>> ---
>>>>  drivers/net/ethernet/intel/ice/ice_eswitch.c | 5 +++++
>>>>  drivers/net/ethernet/intel/ice/ice_main.c    | 6 ++++++
>>>>  2 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c b/drivers/net/ethernet/intel/ice/ice_eswitch.c
>>>> index ad0a007b7398..2ea5aaceee11 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
>>>> @@ -538,6 +538,11 @@ ice_eswitch_mode_set(struct devlink *devlink, u16 mode,
>>>>  		break;
>>>>  	case DEVLINK_ESWITCH_MODE_SWITCHDEV:
>>>>  	{
>>>> +		if (ice_is_adq_active(pf)) {
>>>> +			dev_err(ice_pf_to_dev(pf), "switchdev cannot be configured - ADQ is active. Delete ADQ configs using TC and try again\n");
>> 
>> Does this provide sufficient hint to the user? I mean, what's ADQ and
>> how it is related to TC objects? Please be more precise.
>
>Application Device Queues, a conflicting feature unrelated to switchdev.
>If it's enabled, there's a good chance the user knows what it is because
>they configured it.
>
>Could you suggest a better error message?

The user would need to know what he needs to do in order to make this
work. So it would be nice to hint what rules need to be removed.

>
>> 
>> 
>>>
>>> It needs to be reported through netlink extack.
>>>
>>>> +			return -EOPNOTSUPP;
>>>> +		}
>>>> +
>>>>  		dev_info(ice_pf_to_dev(pf), "PF %d changed eswitch mode to switchdev",
>>>>  			 pf->hw.pf_id);
>>>>  		NL_SET_ERR_MSG_MOD(extack, "Changed eswitch mode to switchdev");
>>>
>>> Thanks
>>>
>> 
>
>Regards,
>Marcin