netdev - Re: [PATCH net-next 3/5] sfc: add functions to insert encap matches into the MAE

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: <2c0c7f42-9e1d-7b4b-4c8d-bfb1a0ea3187@gmail.com>
Date:   Wed, 15 Mar 2023 13:58:36 +0000
From:   Edward Cree <ecree.xilinx@...il.com>
To:     Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
        edward.cree@....com
Cc:     linux-net-drivers@....com, davem@...emloft.net, kuba@...nel.org,
        pabeni@...hat.com, edumazet@...gle.com, netdev@...r.kernel.org,
        habetsm.xilinx@...il.com
Subject: Re: [PATCH net-next 3/5] sfc: add functions to insert encap matches
 into the MAE

On 15/03/2023 09:23, Michal Swiatkowski wrote:
> On Tue, Mar 14, 2023 at 05:35:23PM +0000, edward.cree@....com wrote:
>> +#ifdef CONFIG_IPV6
>> +	if (encap->src_ip | encap->dst_ip) {
>> +#endif
> Looks strange, in case CONFIG_IPV6 isn't defined You can also check if
> theres is no zero ip.

The idea is that #ifdef CONFIG_IPV6 then we use this to decide whether
 this is an IPv4 or IPv6 filter, otherwise we don't need to check
 anything because there's only IPv4.  What would the alternative be,
 put a WARN_ON_ONCE() and return -EINVAL in the else clause #ifndef
 CONFIG_IPV6?  Is that better?
I agree this does look strange.

> 
>> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_SRC_IP4_BE,
>> +					 encap->src_ip);
>> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_SRC_IP4_BE_MASK,
>> +					 ~(__be32)0);
>> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_DST_IP4_BE,
>> +					 encap->dst_ip);
>> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_DST_IP4_BE_MASK,
>> +					 ~(__be32)0);
>> +		MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_ETHER_TYPE_BE,
>> +					htons(ETH_P_IP));
>> +#ifdef CONFIG_IPV6
>> +	} else {
>> +		memcpy(MCDI_STRUCT_PTR(match_crit, MAE_ENC_FIELD_PAIRS_ENC_SRC_IP6_BE),
>> +		       &encap->src_ip6, sizeof(encap->src_ip6));
>> +		memset(MCDI_STRUCT_PTR(match_crit, MAE_ENC_FIELD_PAIRS_ENC_SRC_IP6_BE_MASK),
>> +		       0xff, sizeof(encap->src_ip6));
>> +		memcpy(MCDI_STRUCT_PTR(match_crit, MAE_ENC_FIELD_PAIRS_ENC_DST_IP6_BE),
>> +		       &encap->dst_ip6, sizeof(encap->dst_ip6));
>> +		memset(MCDI_STRUCT_PTR(match_crit, MAE_ENC_FIELD_PAIRS_ENC_DST_IP6_BE_MASK),
>> +		       0xff, sizeof(encap->dst_ip6));
>> +		MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_ETHER_TYPE_BE,
>> +					htons(ETH_P_IPV6));
>> +	}
>> +#endif
>> +	MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_ETHER_TYPE_BE_MASK,
>> +				~(__be16)0);
>> +	MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_L4_DPORT_BE,
>> +				encap->udp_dport);
>> +	MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_ENC_FIELD_PAIRS_ENC_L4_DPORT_BE_MASK,
>> +				~(__be16)0);
> Question, from tc we can set masks for matching fields. You are setting
> default one, because hardware doesn't support different masks?

See my reply on patch #1 about mask sets and overlap.
The hardware supports masks on some fields in the Outer Rule table,
 although not (in the current version) L4 ports.