linux-kernel - Re: [PATCH net] octeontx2-af: Consider the action set by PF

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]

Date: Wed, 21 Feb 2024 12:24:22 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Subbaraya Sundeep <sbhatta@...vell.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <sgoutham@...vell.com>, <lcherian@...vell.com>,
	<gakula@...vell.com>, <hkelam@...vell.com>, <naveenm@...vell.com>,
	<horms@...nel.org>
Subject: Re: [PATCH net] octeontx2-af: Consider the action set by PF

On 2/19/24 13:55, Subbaraya Sundeep wrote:
> AF reserves MCAM entries for each PF, VF present in the
> system and populates the entry with DMAC and action with
> default RSS so that basic packet I/O works. Since PF/VF is
> not aware of the RSS action installed by AF, AF only fixup
> the actions of the rules installed by PF/VF with corresponding
> default RSS action. This worked well for rules installed by
> PF/VF for features like RX VLAN offload and DMAC filters but
> rules involving action like drop/forward to queue are also
> getting modified by AF. Hence fix it by setting the default
> RSS action only if requested by PF/VF.
> 
> Fixes: 967db3529eca ("octeontx2-af: add support for multicast/promisc packet replication feature")
> Signed-off-by: Subbaraya Sundeep <sbhatta@...vell.com>
> ---
>   drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index e5d6156..516adb5 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -415,6 +415,10 @@ static void npc_fixup_vf_rule(struct rvu *rvu, struct npc_mcam *mcam,
>   			return;
>   	}
>   
> +	/* AF modifies given action iff PF/VF has requested for it */
> +	if ((entry->action & 0xFULL) != NIX_RX_ACTION_DEFAULT)

this is a magic constant, even if nice sounding one
and it's equal to NIX_RX_ACTION_DEFAULT BTW, but it's incidental
(perhaps you should have two defines, one for mask, and one for action)

> +		return;
> +
>   	/* copy VF default entry action to the VF mcam entry */
>   	rx_action = npc_get_default_entry_action(rvu, mcam, blkaddr,
>   						 target_func);

Otherwise the patch makes sense