netdev - Re: [net PATCH] octeontx2-af: Fix installation of PF multicast rules

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <96747b28-1548-4503-838b-e7a994be4647@redhat.com>
Date: Tue, 3 Dec 2024 10:40:38 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Geetha sowjanya <gakula@...vell.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: kuba@...nel.org, davem@...emloft.net, horms@...nel.org,
 andrew+netdev@...n.ch, edumazet@...gle.com, sgoutham@...vell.com,
 sbhatta@...vell.com, hkelam@...vell.com
Subject: Re: [net PATCH] octeontx2-af: Fix installation of PF multicast rules

On 11/27/24 12:48, Geetha sowjanya wrote:
> Due to target variable is being reassigned in npc_install_flow()
> function, PF multicast rules are not getting installed.
> This patch addresses the issue by fixing the "IF" condition
> checks when rules are installed by AF.
> 
> Fixes: 6c40ca957fe5 ("octeontx2-pf: Adds TC offload support").
> Signed-off-by: Geetha sowjanya <gakula@...vell.com>
> ---
>  drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> index da69e454662a..8a2444a8b7d3 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> @@ -1457,14 +1457,14 @@ int rvu_mbox_handler_npc_install_flow(struct rvu *rvu,
>  		target = req->vf;
>  
>  	/* PF installing for its VF */
> -	if (!from_vf && req->vf && !from_rep_dev) {
> +	else if (!from_vf && req->vf && !from_rep_dev) {

This IMHO makes the code quite unreadable and error-prone, as the else
branches are quite separate from the 'if' statement and easy to miss.

It also breaks the kernel style, as you must apply the curly brackets on
all the branches, if one of them is using them.

Please restructure the code a bit:

	if (!req->hdr.pcifunc) {
		/* AF installing for a PF/VF */
		target = req->vf;
	} else if (!from_vf && req->vf && !from_rep_dev) {
		/* PF installing for its VF */
		...

Thanks,

Paolo