Re: [net v2 PATCH 3/5] octeontx2-af: Allow mkex profiles without dmac.

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 26 Jul 2022 19:41:20 +0530 Subbaraya Sundeep wrote:
> From: Suman Ghosh <sumang@marvell.com>
> 
> It is possible to have custom mkex profiles which do not extract
> DMAC into the key to free up space in the key and use it for L3
> or L4 packet fields. Current code bails out if DMAC extraction is
> not present in the key. This patch fixes it by allowing profiles
> without DMAC and also supports installing rules based on L2MB bit
> set by hardware for multicast and broadcast packets.

This sounds half way between a feature and a fix. Can you make it
clearer why it's a fix and not an optimization?

> This patch also adds debugging prints needed to identify profiles
> with wrong configuration.

All those prints make the patch even less acceptable as a fix.
We merge fixes into net-next every week, you can send a minimal
fix and extend the code in net-next soon after that.

> Fixes: 9b179a960a96 ("octeontx2-af: Generate key field bit mask from KEX profile")

> +	/* If DMAC is not extracted in MKEX, rules installed by AF
> +	 * can rely on L2MB bit set by hardware protocol checker for
> +	 * broadcast and multicast addresses.
> +	 */
> +	if (npc_check_field(rvu, blkaddr, NPC_DMAC, req->intf))
> +		goto process_flow;

Merge this condition with the condition below, goto should be avoided
but for error paths.

> +	if (is_pffunc_af(req->hdr.pcifunc) &&
> +	    req->features & BIT_ULL(NPC_DMAC)) {
> +		if (is_unicast_ether_addr(req->packet.dmac)) {
> +			dev_err(rvu->dev,
> +				"%s: mkex profile does not support ucast flow\n",
> +				__func__);
> +			return NPC_FLOW_NOT_SUPPORTED;
> +		}
> +
> +		if (!npc_is_field_present(rvu, NPC_LXMB, req->intf)) {
> +			dev_err(rvu->dev,
> +				"%s: mkex profile does not support bcast/mcast flow",
> +				__func__);
> +			return NPC_FLOW_NOT_SUPPORTED;
> +		}
> +
> +		/* Modify feature to use LXMB instead of DMAC */
> +		req->features &= ~BIT_ULL(NPC_DMAC);
> +		req->features |= BIT_ULL(NPC_LXMB);
> +	}
> +
> +process_flow:
>  	if (from_vf && req->default_rule)
>  		return NPC_FLOW_VF_PERM_DENIED;
>