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

[Paolo Abeni <pabeni@redhat.com>]

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@marvell.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