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

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: <20220727195106.44100254@kernel.org>
Date:   Wed, 27 Jul 2022 19:51:06 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Subbaraya Sundeep <sbhatta@...vell.com>
Cc:     <davem@...emloft.net>, <pabeni@...hat.com>, <edumazet@...gle.com>,
        <sgoutham@...vell.com>, <netdev@...r.kernel.org>,
        Suman Ghosh <sumang@...vell.com>
Subject: Re: [net v2 PATCH 3/5] octeontx2-af: Allow mkex profiles without
 dmac.

On Tue, 26 Jul 2022 19:41:20 +0530 Subbaraya Sundeep wrote:
> From: Suman Ghosh <sumang@...vell.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;
>