[2/3] common/sfc_efx/base: add MAE VLAN presence match bits

Message ID 20210428094926.22185-2-ivan.malov@oktetlabs.ru (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [1/3] common/sfc_efx/base: update MCDI headers |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Ivan Malov April 28, 2021, 9:49 a.m. UTC
  Introduce necessary infrastructure for these fields to
be set, validated and compared during class comparison.
Enumeration and mappings envisaged are MCDI-compatible.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/common/sfc_efx/base/efx.h      |  18 ++
 drivers/common/sfc_efx/base/efx_impl.h |   3 +-
 drivers/common/sfc_efx/base/efx_mae.c  | 235 ++++++++++++++++++++++++-
 drivers/common/sfc_efx/version.map     |   1 +
 4 files changed, 254 insertions(+), 3 deletions(-)
  

Comments

Ray Kinsella April 28, 2021, 10:08 a.m. UTC | #1
On 28/04/2021 10:49, Ivan Malov wrote:
> Introduce necessary infrastructure for these fields to
> be set, validated and compared during class comparison.
> Enumeration and mappings envisaged are MCDI-compatible.
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
>  drivers/common/sfc_efx/base/efx.h      |  18 ++
>  drivers/common/sfc_efx/base/efx_impl.h |   3 +-
>  drivers/common/sfc_efx/base/efx_mae.c  | 235 ++++++++++++++++++++++++-
>  drivers/common/sfc_efx/version.map     |   1 +
>  4 files changed, 254 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
> index 771fe5a17..8e13075b0 100644
> --- a/drivers/common/sfc_efx/base/efx.h
> +++ b/drivers/common/sfc_efx/base/efx.h
> @@ -4103,6 +4103,10 @@ efx_mae_match_spec_fini(
>  	__in				efx_mae_match_spec_t *spec);
>  
>  typedef enum efx_mae_field_id_e {
> +	/*
> +	 * Fields which can be set by efx_mae_match_spec_field_set()
> +	 * or by using dedicated field-specific helper APIs.
> +	 */
>  	EFX_MAE_FIELD_INGRESS_MPORT_SELECTOR = 0,
>  	EFX_MAE_FIELD_ETHER_TYPE_BE,
>  	EFX_MAE_FIELD_ETH_SADDR_BE,
> @@ -4140,6 +4144,12 @@ typedef enum efx_mae_field_id_e {
>  	EFX_MAE_FIELD_ENC_VNET_ID_BE,
>  	EFX_MAE_FIELD_OUTER_RULE_ID,
>  
> +	/* Single bits which can be set by efx_mae_match_spec_bit_set(). */
> +	EFX_MAE_FIELD_HAS_OVLAN,
> +	EFX_MAE_FIELD_HAS_IVLAN,
> +	EFX_MAE_FIELD_ENC_HAS_OVLAN,
> +	EFX_MAE_FIELD_ENC_HAS_IVLAN,
> +
>  	EFX_MAE_FIELD_NIDS
>  } efx_mae_field_id_t;
>  
> @@ -4198,6 +4208,14 @@ efx_mae_match_spec_field_set(
>  	__in				size_t mask_size,
>  	__in_bcount(mask_size)		const uint8_t *mask);
>  
> +/* The corresponding mask will be set to B_TRUE. */
> +LIBEFX_API
> +extern	__checkReturn			efx_rc_t

Missing rte_internal on the forward declaration?

> +efx_mae_match_spec_bit_set(
> +	__in				efx_mae_match_spec_t *spec,
> +	__in				efx_mae_field_id_t field_id,
> +	__in				boolean_t value);
> +
>  /* If the mask argument is NULL, the API will use full mask by default. */
>  LIBEFX_API
>  extern	__checkReturn			efx_rc_t
> diff --git a/drivers/common/sfc_efx/base/efx_impl.h b/drivers/common/sfc_efx/base/efx_impl.h
> index 4a513171a..8b63cfb37 100644
> --- a/drivers/common/sfc_efx/base/efx_impl.h
> +++ b/drivers/common/sfc_efx/base/efx_impl.h
> @@ -1720,7 +1720,8 @@ struct efx_mae_match_spec_s {
>  	efx_mae_rule_type_t		emms_type;
>  	uint32_t			emms_prio;
>  	union emms_mask_value_pairs {
> -		uint8_t			action[MAE_FIELD_MASK_VALUE_PAIRS_LEN];
> +		uint8_t			action[
> +					    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN];
>  		uint8_t			outer[MAE_ENC_FIELD_PAIRS_LEN];
>  	} emms_mask_value_pairs;
>  };
> diff --git a/drivers/common/sfc_efx/base/efx_mae.c b/drivers/common/sfc_efx/base/efx_mae.c
> index 80fe155d0..33ba31389 100644
> --- a/drivers/common/sfc_efx/base/efx_mae.c
> +++ b/drivers/common/sfc_efx/base/efx_mae.c
> @@ -451,6 +451,10 @@ typedef enum efx_mae_field_cap_id_e {
>  	EFX_MAE_FIELD_ID_ENC_L4_DPORT_BE = MAE_FIELD_ENC_L4_DPORT,
>  	EFX_MAE_FIELD_ID_ENC_VNET_ID_BE = MAE_FIELD_ENC_VNET_ID,
>  	EFX_MAE_FIELD_ID_OUTER_RULE_ID = MAE_FIELD_OUTER_RULE_ID,
> +	EFX_MAE_FIELD_ID_HAS_OVLAN = MAE_FIELD_HAS_OVLAN,
> +	EFX_MAE_FIELD_ID_HAS_IVLAN = MAE_FIELD_HAS_IVLAN,
> +	EFX_MAE_FIELD_ID_ENC_HAS_OVLAN = MAE_FIELD_ENC_HAS_OVLAN,
> +	EFX_MAE_FIELD_ID_ENC_HAS_IVLAN = MAE_FIELD_ENC_HAS_IVLAN,
>  
>  	EFX_MAE_FIELD_CAP_NIDS
>  } efx_mae_field_cap_id_t;
> @@ -577,6 +581,65 @@ static const efx_mae_mv_desc_t __efx_mae_outer_rule_mv_desc_set[] = {
>  
>  #undef EFX_MAE_MV_DESC_ALT
>  #undef EFX_MAE_MV_DESC
> +};
> +
> +/*
> + * The following structure is a means to describe an MAE bit.
> + * The information in it is meant to be used internally by
> + * APIs for addressing a given flag in a mask-value pairs
> + * structure and for validation purposes.
> + */
> +typedef struct efx_mae_mv_bit_desc_s {
> +	/*
> +	 * Arrays using this struct are indexed by field IDs.
> +	 * Fields which aren't meant to be referenced by these
> +	 * arrays comprise gaps (invalid entries). Below field
> +	 * helps to identify such entries.
> +	 */
> +	boolean_t			emmbd_entry_is_valid;
> +	efx_mae_field_cap_id_t		emmbd_bit_cap_id;
> +	size_t				emmbd_value_ofst;
> +	unsigned int			emmbd_value_lbn;
> +	size_t				emmbd_mask_ofst;
> +	unsigned int			emmbd_mask_lbn;
> +} efx_mae_mv_bit_desc_t;
> +
> +static const efx_mae_mv_bit_desc_t __efx_mae_outer_rule_mv_bit_desc_set[] = {
> +#define	EFX_MAE_MV_BIT_DESC(_name)					\
> +	[EFX_MAE_FIELD_##_name] =					\
> +	{								\
> +		B_TRUE,							\
> +		EFX_MAE_FIELD_ID_##_name,				\
> +		MAE_ENC_FIELD_PAIRS_##_name##_OFST,			\
> +		MAE_ENC_FIELD_PAIRS_##_name##_LBN,			\
> +		MAE_ENC_FIELD_PAIRS_##_name##_MASK_OFST,		\
> +		MAE_ENC_FIELD_PAIRS_##_name##_MASK_LBN,			\
> +	}
> +
> +	EFX_MAE_MV_BIT_DESC(ENC_HAS_OVLAN),
> +	EFX_MAE_MV_BIT_DESC(ENC_HAS_IVLAN),
> +
> +#undef EFX_MAE_MV_BIT_DESC
> +};
> +
> +static const efx_mae_mv_bit_desc_t __efx_mae_action_rule_mv_bit_desc_set[] = {
> +#define	EFX_MAE_MV_BIT_DESC(_name)					\
> +	[EFX_MAE_FIELD_##_name] =					\
> +	{								\
> +		B_TRUE,							\
> +		EFX_MAE_FIELD_ID_##_name,				\
> +		MAE_FIELD_MASK_VALUE_PAIRS_V2_FLAGS_OFST,		\
> +		MAE_FIELD_MASK_VALUE_PAIRS_V2_##_name##_LBN,		\
> +		MAE_FIELD_MASK_VALUE_PAIRS_V2_FLAGS_MASK_OFST,		\
> +		MAE_FIELD_MASK_VALUE_PAIRS_V2_##_name##_LBN,		\
> +	}
> +
> +	EFX_MAE_MV_BIT_DESC(HAS_OVLAN),
> +	EFX_MAE_MV_BIT_DESC(HAS_IVLAN),
> +	EFX_MAE_MV_BIT_DESC(ENC_HAS_OVLAN),
> +	EFX_MAE_MV_BIT_DESC(ENC_HAS_IVLAN),
> +
> +#undef EFX_MAE_MV_BIT_DESC
>  };
>  
>  	__checkReturn			efx_rc_t
> @@ -775,6 +838,70 @@ efx_mae_match_spec_field_set(
>  	EFSYS_PROBE(fail5);
>  fail4:
>  	EFSYS_PROBE(fail4);
> +fail3:
> +	EFSYS_PROBE(fail3);
> +fail2:
> +	EFSYS_PROBE(fail2);
> +fail1:
> +	EFSYS_PROBE1(fail1, efx_rc_t, rc);
> +	return (rc);
> +}
> +
> +	__checkReturn			efx_rc_t
> +efx_mae_match_spec_bit_set(
> +	__in				efx_mae_match_spec_t *spec,
> +	__in				efx_mae_field_id_t field_id,
> +	__in				boolean_t value)
> +{
> +	const efx_mae_mv_bit_desc_t *bit_descp;
> +	unsigned int bit_desc_set_nentries;
> +	unsigned int byte_idx;
> +	unsigned int bit_idx;
> +	uint8_t *mvp;
> +	efx_rc_t rc;
> +
> +	switch (spec->emms_type) {
> +	case EFX_MAE_RULE_OUTER:
> +		bit_desc_set_nentries =
> +		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
> +		bit_descp = &__efx_mae_outer_rule_mv_bit_desc_set[field_id];
> +		mvp = spec->emms_mask_value_pairs.outer;
> +		break;
> +	case EFX_MAE_RULE_ACTION:
> +		bit_desc_set_nentries =
> +		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
> +		bit_descp = &__efx_mae_action_rule_mv_bit_desc_set[field_id];
> +		mvp = spec->emms_mask_value_pairs.action;
> +		break;
> +	default:
> +		rc = ENOTSUP;
> +		goto fail1;
> +	}
> +
> +	if ((unsigned int)field_id >= bit_desc_set_nentries) {
> +		rc = EINVAL;
> +		goto fail2;
> +	}
> +
> +	if (bit_descp->emmbd_entry_is_valid == B_FALSE) {
> +		rc = EINVAL;
> +		goto fail3;
> +	}
> +
> +	byte_idx = bit_descp->emmbd_value_ofst + bit_descp->emmbd_value_lbn / 8;
> +	bit_idx = bit_descp->emmbd_value_lbn % 8;
> +
> +	if (value != B_FALSE)
> +		mvp[byte_idx] |= (1U << bit_idx);
> +	else
> +		mvp[byte_idx] &= ~(1U << bit_idx);
> +
> +	byte_idx = bit_descp->emmbd_mask_ofst + bit_descp->emmbd_mask_lbn / 8;
> +	bit_idx = bit_descp->emmbd_mask_lbn % 8;
> +	mvp[byte_idx] |= (1U << bit_idx);
> +
> +	return (0);
> +
>  fail3:
>  	EFSYS_PROBE(fail3);
>  fail2:
> @@ -891,6 +1018,8 @@ efx_mae_match_spec_is_valid(
>  	const efx_mae_field_cap_t *field_caps;
>  	const efx_mae_mv_desc_t *desc_setp;
>  	unsigned int desc_set_nentries;
> +	const efx_mae_mv_bit_desc_t *bit_desc_setp;
> +	unsigned int bit_desc_set_nentries;
>  	boolean_t is_valid = B_TRUE;
>  	efx_mae_field_id_t field_id;
>  	const uint8_t *mvp;
> @@ -901,6 +1030,9 @@ efx_mae_match_spec_is_valid(
>  		desc_setp = __efx_mae_outer_rule_mv_desc_set;
>  		desc_set_nentries =
>  		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_desc_set);
> +		bit_desc_setp = __efx_mae_outer_rule_mv_bit_desc_set;
> +		bit_desc_set_nentries =
> +		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
>  		mvp = spec->emms_mask_value_pairs.outer;
>  		break;
>  	case EFX_MAE_RULE_ACTION:
> @@ -908,6 +1040,9 @@ efx_mae_match_spec_is_valid(
>  		desc_setp = __efx_mae_action_rule_mv_desc_set;
>  		desc_set_nentries =
>  		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_desc_set);
> +		bit_desc_setp = __efx_mae_action_rule_mv_bit_desc_set;
> +		bit_desc_set_nentries =
> +		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
>  		mvp = spec->emms_mask_value_pairs.action;
>  		break;
>  	default:
> @@ -975,6 +1110,48 @@ efx_mae_match_spec_is_valid(
>  			break;
>  		}
>  
> +		if (is_valid == B_FALSE)
> +			return (B_FALSE);
> +	}
> +
> +	for (field_id = 0; (unsigned int)field_id < bit_desc_set_nentries;
> +	     ++field_id) {
> +		const efx_mae_mv_bit_desc_t *bit_descp =
> +		    &bit_desc_setp[field_id];
> +		unsigned int byte_idx =
> +		    bit_descp->emmbd_mask_ofst +
> +		    bit_descp->emmbd_mask_lbn / 8;
> +		unsigned int bit_idx =
> +		    bit_descp->emmbd_mask_lbn % 8;
> +		efx_mae_field_cap_id_t bit_cap_id =
> +		    bit_descp->emmbd_bit_cap_id;
> +
> +		if (bit_descp->emmbd_entry_is_valid == B_FALSE)
> +			continue; /* Skip array gap */
> +
> +		if ((unsigned int)bit_cap_id >= field_ncaps) {
> +			/* No capability for this bit = unsupported. */
> +			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) == 0);
> +			if (is_valid == B_FALSE)
> +				break;
> +			else
> +				continue;
> +		}
> +
> +		switch (field_caps[bit_cap_id].emfc_support) {
> +		case MAE_FIELD_SUPPORTED_MATCH_OPTIONAL:
> +			is_valid = B_TRUE;
> +			break;
> +		case MAE_FIELD_SUPPORTED_MATCH_ALWAYS:
> +			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) != 0);
> +			break;
> +		case MAE_FIELD_SUPPORTED_MATCH_NEVER:
> +		case MAE_FIELD_UNSUPPORTED:
> +		default:
> +			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) == 0);
> +			break;
> +		}
> +
>  		if (is_valid == B_FALSE)
>  			break;
>  	}
> @@ -1528,6 +1705,8 @@ efx_mae_match_specs_class_cmp(
>  	const efx_mae_field_cap_t *field_caps;
>  	const efx_mae_mv_desc_t *desc_setp;
>  	unsigned int desc_set_nentries;
> +	const efx_mae_mv_bit_desc_t *bit_desc_setp;
> +	unsigned int bit_desc_set_nentries;
>  	boolean_t have_same_class = B_TRUE;
>  	efx_mae_field_id_t field_id;
>  	const uint8_t *mvpl;
> @@ -1540,6 +1719,9 @@ efx_mae_match_specs_class_cmp(
>  		desc_setp = __efx_mae_outer_rule_mv_desc_set;
>  		desc_set_nentries =
>  		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_desc_set);
> +		bit_desc_setp = __efx_mae_outer_rule_mv_bit_desc_set;
> +		bit_desc_set_nentries =
> +		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
>  		mvpl = left->emms_mask_value_pairs.outer;
>  		mvpr = right->emms_mask_value_pairs.outer;
>  		break;
> @@ -1548,6 +1730,9 @@ efx_mae_match_specs_class_cmp(
>  		desc_setp = __efx_mae_action_rule_mv_desc_set;
>  		desc_set_nentries =
>  		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_desc_set);
> +		bit_desc_setp = __efx_mae_action_rule_mv_bit_desc_set;
> +		bit_desc_set_nentries =
> +		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
>  		mvpl = left->emms_mask_value_pairs.action;
>  		mvpr = right->emms_mask_value_pairs.action;
>  		break;
> @@ -1620,6 +1805,52 @@ efx_mae_match_specs_class_cmp(
>  		}
>  	}
>  
> +	if (have_same_class == B_FALSE)
> +		goto done;
> +
> +	for (field_id = 0; (unsigned int)field_id < bit_desc_set_nentries;
> +	     ++field_id) {
> +		const efx_mae_mv_bit_desc_t *bit_descp =
> +		    &bit_desc_setp[field_id];
> +		efx_mae_field_cap_id_t bit_cap_id =
> +		    bit_descp->emmbd_bit_cap_id;
> +		unsigned int byte_idx;
> +		unsigned int bit_idx;
> +
> +		if (bit_descp->emmbd_entry_is_valid == B_FALSE)
> +			continue; /* Skip array gap */
> +
> +		if ((unsigned int)bit_cap_id >= field_ncaps)
> +			break;
> +
> +		byte_idx =
> +		    bit_descp->emmbd_mask_ofst +
> +		    bit_descp->emmbd_mask_lbn / 8;
> +		bit_idx =
> +		    bit_descp->emmbd_mask_lbn % 8;
> +
> +		if (field_caps[bit_cap_id].emfc_mask_affects_class &&
> +		    (mvpl[byte_idx] & (1U << bit_idx)) !=
> +		    (mvpr[byte_idx] & (1U << bit_idx))) {
> +			have_same_class = B_FALSE;
> +			break;
> +		}
> +
> +		byte_idx =
> +		    bit_descp->emmbd_value_ofst +
> +		    bit_descp->emmbd_value_lbn / 8;
> +		bit_idx =
> +		    bit_descp->emmbd_value_lbn % 8;
> +
> +		if (field_caps[bit_cap_id].emfc_match_affects_class &&
> +		    (mvpl[byte_idx] & (1U << bit_idx)) !=
> +		    (mvpr[byte_idx] & (1U << bit_idx))) {
> +			have_same_class = B_FALSE;
> +			break;
> +		}
> +	}
> +
> +done:
>  	*have_same_classp = have_same_class;
>  
>  	return (0);
> @@ -2254,10 +2485,10 @@ efx_mae_action_rule_insert(
>  	 * MCDI request and are thus safe to be copied directly to the buffer.
>  	 */
>  	EFX_STATIC_ASSERT(sizeof (spec->emms_mask_value_pairs.action) >=
> -	    MAE_FIELD_MASK_VALUE_PAIRS_LEN);
> +	    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN);
>  	offset = MC_CMD_MAE_ACTION_RULE_INSERT_IN_MATCH_CRITERIA_OFST;
>  	memcpy(payload + offset, spec->emms_mask_value_pairs.action,
> -	    MAE_FIELD_MASK_VALUE_PAIRS_LEN);
> +	    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN);
>  
>  	efx_mcdi_execute(enp, &req);
>  
> diff --git a/drivers/common/sfc_efx/version.map b/drivers/common/sfc_efx/version.map
> index 5e724fd10..75da5aa5c 100644
> --- a/drivers/common/sfc_efx/version.map
> +++ b/drivers/common/sfc_efx/version.map
> @@ -106,6 +106,7 @@ INTERNAL {
>  	efx_mae_fini;
>  	efx_mae_get_limits;
>  	efx_mae_init;
> +	efx_mae_match_spec_bit_set;
>  	efx_mae_match_spec_field_set;
>  	efx_mae_match_spec_fini;
>  	efx_mae_match_spec_init;
>
  
Ivan Malov April 28, 2021, 11:09 a.m. UTC | #2
Hi,

On 28/04/2021 13:08, Kinsella, Ray wrote:
> 
> 
> On 28/04/2021 10:49, Ivan Malov wrote:
>> Introduce necessary infrastructure for these fields to
>> be set, validated and compared during class comparison.
>> Enumeration and mappings envisaged are MCDI-compatible.
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>> ---
>>   drivers/common/sfc_efx/base/efx.h      |  18 ++
>>   drivers/common/sfc_efx/base/efx_impl.h |   3 +-
>>   drivers/common/sfc_efx/base/efx_mae.c  | 235 ++++++++++++++++++++++++-
>>   drivers/common/sfc_efx/version.map     |   1 +
>>   4 files changed, 254 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
>> index 771fe5a17..8e13075b0 100644
>> --- a/drivers/common/sfc_efx/base/efx.h
>> +++ b/drivers/common/sfc_efx/base/efx.h
>> @@ -4103,6 +4103,10 @@ efx_mae_match_spec_fini(
>>   	__in				efx_mae_match_spec_t *spec);
>>   
>>   typedef enum efx_mae_field_id_e {
>> +	/*
>> +	 * Fields which can be set by efx_mae_match_spec_field_set()
>> +	 * or by using dedicated field-specific helper APIs.
>> +	 */
>>   	EFX_MAE_FIELD_INGRESS_MPORT_SELECTOR = 0,
>>   	EFX_MAE_FIELD_ETHER_TYPE_BE,
>>   	EFX_MAE_FIELD_ETH_SADDR_BE,
>> @@ -4140,6 +4144,12 @@ typedef enum efx_mae_field_id_e {
>>   	EFX_MAE_FIELD_ENC_VNET_ID_BE,
>>   	EFX_MAE_FIELD_OUTER_RULE_ID,
>>   
>> +	/* Single bits which can be set by efx_mae_match_spec_bit_set(). */
>> +	EFX_MAE_FIELD_HAS_OVLAN,
>> +	EFX_MAE_FIELD_HAS_IVLAN,
>> +	EFX_MAE_FIELD_ENC_HAS_OVLAN,
>> +	EFX_MAE_FIELD_ENC_HAS_IVLAN,
>> +
>>   	EFX_MAE_FIELD_NIDS
>>   } efx_mae_field_id_t;
>>   
>> @@ -4198,6 +4208,14 @@ efx_mae_match_spec_field_set(
>>   	__in				size_t mask_size,
>>   	__in_bcount(mask_size)		const uint8_t *mask);
>>   
>> +/* The corresponding mask will be set to B_TRUE. */
>> +LIBEFX_API
>> +extern	__checkReturn			efx_rc_t
> 
> Missing rte_internal on the forward declaration?

Not really. It has LIBEFX_API in place. And LIBEFX_API, in turn, is 
defined as __rte_internal in drivers/common/sfc_efx/efsys.h . Thank you.

> 
>> +efx_mae_match_spec_bit_set(
>> +	__in				efx_mae_match_spec_t *spec,
>> +	__in				efx_mae_field_id_t field_id,
>> +	__in				boolean_t value);
>> +
>>   /* If the mask argument is NULL, the API will use full mask by default. */
>>   LIBEFX_API
>>   extern	__checkReturn			efx_rc_t
>> diff --git a/drivers/common/sfc_efx/base/efx_impl.h b/drivers/common/sfc_efx/base/efx_impl.h
>> index 4a513171a..8b63cfb37 100644
>> --- a/drivers/common/sfc_efx/base/efx_impl.h
>> +++ b/drivers/common/sfc_efx/base/efx_impl.h
>> @@ -1720,7 +1720,8 @@ struct efx_mae_match_spec_s {
>>   	efx_mae_rule_type_t		emms_type;
>>   	uint32_t			emms_prio;
>>   	union emms_mask_value_pairs {
>> -		uint8_t			action[MAE_FIELD_MASK_VALUE_PAIRS_LEN];
>> +		uint8_t			action[
>> +					    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN];
>>   		uint8_t			outer[MAE_ENC_FIELD_PAIRS_LEN];
>>   	} emms_mask_value_pairs;
>>   };
>> diff --git a/drivers/common/sfc_efx/base/efx_mae.c b/drivers/common/sfc_efx/base/efx_mae.c
>> index 80fe155d0..33ba31389 100644
>> --- a/drivers/common/sfc_efx/base/efx_mae.c
>> +++ b/drivers/common/sfc_efx/base/efx_mae.c
>> @@ -451,6 +451,10 @@ typedef enum efx_mae_field_cap_id_e {
>>   	EFX_MAE_FIELD_ID_ENC_L4_DPORT_BE = MAE_FIELD_ENC_L4_DPORT,
>>   	EFX_MAE_FIELD_ID_ENC_VNET_ID_BE = MAE_FIELD_ENC_VNET_ID,
>>   	EFX_MAE_FIELD_ID_OUTER_RULE_ID = MAE_FIELD_OUTER_RULE_ID,
>> +	EFX_MAE_FIELD_ID_HAS_OVLAN = MAE_FIELD_HAS_OVLAN,
>> +	EFX_MAE_FIELD_ID_HAS_IVLAN = MAE_FIELD_HAS_IVLAN,
>> +	EFX_MAE_FIELD_ID_ENC_HAS_OVLAN = MAE_FIELD_ENC_HAS_OVLAN,
>> +	EFX_MAE_FIELD_ID_ENC_HAS_IVLAN = MAE_FIELD_ENC_HAS_IVLAN,
>>   
>>   	EFX_MAE_FIELD_CAP_NIDS
>>   } efx_mae_field_cap_id_t;
>> @@ -577,6 +581,65 @@ static const efx_mae_mv_desc_t __efx_mae_outer_rule_mv_desc_set[] = {
>>   
>>   #undef EFX_MAE_MV_DESC_ALT
>>   #undef EFX_MAE_MV_DESC
>> +};
>> +
>> +/*
>> + * The following structure is a means to describe an MAE bit.
>> + * The information in it is meant to be used internally by
>> + * APIs for addressing a given flag in a mask-value pairs
>> + * structure and for validation purposes.
>> + */
>> +typedef struct efx_mae_mv_bit_desc_s {
>> +	/*
>> +	 * Arrays using this struct are indexed by field IDs.
>> +	 * Fields which aren't meant to be referenced by these
>> +	 * arrays comprise gaps (invalid entries). Below field
>> +	 * helps to identify such entries.
>> +	 */
>> +	boolean_t			emmbd_entry_is_valid;
>> +	efx_mae_field_cap_id_t		emmbd_bit_cap_id;
>> +	size_t				emmbd_value_ofst;
>> +	unsigned int			emmbd_value_lbn;
>> +	size_t				emmbd_mask_ofst;
>> +	unsigned int			emmbd_mask_lbn;
>> +} efx_mae_mv_bit_desc_t;
>> +
>> +static const efx_mae_mv_bit_desc_t __efx_mae_outer_rule_mv_bit_desc_set[] = {
>> +#define	EFX_MAE_MV_BIT_DESC(_name)					\
>> +	[EFX_MAE_FIELD_##_name] =					\
>> +	{								\
>> +		B_TRUE,							\
>> +		EFX_MAE_FIELD_ID_##_name,				\
>> +		MAE_ENC_FIELD_PAIRS_##_name##_OFST,			\
>> +		MAE_ENC_FIELD_PAIRS_##_name##_LBN,			\
>> +		MAE_ENC_FIELD_PAIRS_##_name##_MASK_OFST,		\
>> +		MAE_ENC_FIELD_PAIRS_##_name##_MASK_LBN,			\
>> +	}
>> +
>> +	EFX_MAE_MV_BIT_DESC(ENC_HAS_OVLAN),
>> +	EFX_MAE_MV_BIT_DESC(ENC_HAS_IVLAN),
>> +
>> +#undef EFX_MAE_MV_BIT_DESC
>> +};
>> +
>> +static const efx_mae_mv_bit_desc_t __efx_mae_action_rule_mv_bit_desc_set[] = {
>> +#define	EFX_MAE_MV_BIT_DESC(_name)					\
>> +	[EFX_MAE_FIELD_##_name] =					\
>> +	{								\
>> +		B_TRUE,							\
>> +		EFX_MAE_FIELD_ID_##_name,				\
>> +		MAE_FIELD_MASK_VALUE_PAIRS_V2_FLAGS_OFST,		\
>> +		MAE_FIELD_MASK_VALUE_PAIRS_V2_##_name##_LBN,		\
>> +		MAE_FIELD_MASK_VALUE_PAIRS_V2_FLAGS_MASK_OFST,		\
>> +		MAE_FIELD_MASK_VALUE_PAIRS_V2_##_name##_LBN,		\
>> +	}
>> +
>> +	EFX_MAE_MV_BIT_DESC(HAS_OVLAN),
>> +	EFX_MAE_MV_BIT_DESC(HAS_IVLAN),
>> +	EFX_MAE_MV_BIT_DESC(ENC_HAS_OVLAN),
>> +	EFX_MAE_MV_BIT_DESC(ENC_HAS_IVLAN),
>> +
>> +#undef EFX_MAE_MV_BIT_DESC
>>   };
>>   
>>   	__checkReturn			efx_rc_t
>> @@ -775,6 +838,70 @@ efx_mae_match_spec_field_set(
>>   	EFSYS_PROBE(fail5);
>>   fail4:
>>   	EFSYS_PROBE(fail4);
>> +fail3:
>> +	EFSYS_PROBE(fail3);
>> +fail2:
>> +	EFSYS_PROBE(fail2);
>> +fail1:
>> +	EFSYS_PROBE1(fail1, efx_rc_t, rc);
>> +	return (rc);
>> +}
>> +
>> +	__checkReturn			efx_rc_t
>> +efx_mae_match_spec_bit_set(
>> +	__in				efx_mae_match_spec_t *spec,
>> +	__in				efx_mae_field_id_t field_id,
>> +	__in				boolean_t value)
>> +{
>> +	const efx_mae_mv_bit_desc_t *bit_descp;
>> +	unsigned int bit_desc_set_nentries;
>> +	unsigned int byte_idx;
>> +	unsigned int bit_idx;
>> +	uint8_t *mvp;
>> +	efx_rc_t rc;
>> +
>> +	switch (spec->emms_type) {
>> +	case EFX_MAE_RULE_OUTER:
>> +		bit_desc_set_nentries =
>> +		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
>> +		bit_descp = &__efx_mae_outer_rule_mv_bit_desc_set[field_id];
>> +		mvp = spec->emms_mask_value_pairs.outer;
>> +		break;
>> +	case EFX_MAE_RULE_ACTION:
>> +		bit_desc_set_nentries =
>> +		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
>> +		bit_descp = &__efx_mae_action_rule_mv_bit_desc_set[field_id];
>> +		mvp = spec->emms_mask_value_pairs.action;
>> +		break;
>> +	default:
>> +		rc = ENOTSUP;
>> +		goto fail1;
>> +	}
>> +
>> +	if ((unsigned int)field_id >= bit_desc_set_nentries) {
>> +		rc = EINVAL;
>> +		goto fail2;
>> +	}
>> +
>> +	if (bit_descp->emmbd_entry_is_valid == B_FALSE) {
>> +		rc = EINVAL;
>> +		goto fail3;
>> +	}
>> +
>> +	byte_idx = bit_descp->emmbd_value_ofst + bit_descp->emmbd_value_lbn / 8;
>> +	bit_idx = bit_descp->emmbd_value_lbn % 8;
>> +
>> +	if (value != B_FALSE)
>> +		mvp[byte_idx] |= (1U << bit_idx);
>> +	else
>> +		mvp[byte_idx] &= ~(1U << bit_idx);
>> +
>> +	byte_idx = bit_descp->emmbd_mask_ofst + bit_descp->emmbd_mask_lbn / 8;
>> +	bit_idx = bit_descp->emmbd_mask_lbn % 8;
>> +	mvp[byte_idx] |= (1U << bit_idx);
>> +
>> +	return (0);
>> +
>>   fail3:
>>   	EFSYS_PROBE(fail3);
>>   fail2:
>> @@ -891,6 +1018,8 @@ efx_mae_match_spec_is_valid(
>>   	const efx_mae_field_cap_t *field_caps;
>>   	const efx_mae_mv_desc_t *desc_setp;
>>   	unsigned int desc_set_nentries;
>> +	const efx_mae_mv_bit_desc_t *bit_desc_setp;
>> +	unsigned int bit_desc_set_nentries;
>>   	boolean_t is_valid = B_TRUE;
>>   	efx_mae_field_id_t field_id;
>>   	const uint8_t *mvp;
>> @@ -901,6 +1030,9 @@ efx_mae_match_spec_is_valid(
>>   		desc_setp = __efx_mae_outer_rule_mv_desc_set;
>>   		desc_set_nentries =
>>   		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_desc_set);
>> +		bit_desc_setp = __efx_mae_outer_rule_mv_bit_desc_set;
>> +		bit_desc_set_nentries =
>> +		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
>>   		mvp = spec->emms_mask_value_pairs.outer;
>>   		break;
>>   	case EFX_MAE_RULE_ACTION:
>> @@ -908,6 +1040,9 @@ efx_mae_match_spec_is_valid(
>>   		desc_setp = __efx_mae_action_rule_mv_desc_set;
>>   		desc_set_nentries =
>>   		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_desc_set);
>> +		bit_desc_setp = __efx_mae_action_rule_mv_bit_desc_set;
>> +		bit_desc_set_nentries =
>> +		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
>>   		mvp = spec->emms_mask_value_pairs.action;
>>   		break;
>>   	default:
>> @@ -975,6 +1110,48 @@ efx_mae_match_spec_is_valid(
>>   			break;
>>   		}
>>   
>> +		if (is_valid == B_FALSE)
>> +			return (B_FALSE);
>> +	}
>> +
>> +	for (field_id = 0; (unsigned int)field_id < bit_desc_set_nentries;
>> +	     ++field_id) {
>> +		const efx_mae_mv_bit_desc_t *bit_descp =
>> +		    &bit_desc_setp[field_id];
>> +		unsigned int byte_idx =
>> +		    bit_descp->emmbd_mask_ofst +
>> +		    bit_descp->emmbd_mask_lbn / 8;
>> +		unsigned int bit_idx =
>> +		    bit_descp->emmbd_mask_lbn % 8;
>> +		efx_mae_field_cap_id_t bit_cap_id =
>> +		    bit_descp->emmbd_bit_cap_id;
>> +
>> +		if (bit_descp->emmbd_entry_is_valid == B_FALSE)
>> +			continue; /* Skip array gap */
>> +
>> +		if ((unsigned int)bit_cap_id >= field_ncaps) {
>> +			/* No capability for this bit = unsupported. */
>> +			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) == 0);
>> +			if (is_valid == B_FALSE)
>> +				break;
>> +			else
>> +				continue;
>> +		}
>> +
>> +		switch (field_caps[bit_cap_id].emfc_support) {
>> +		case MAE_FIELD_SUPPORTED_MATCH_OPTIONAL:
>> +			is_valid = B_TRUE;
>> +			break;
>> +		case MAE_FIELD_SUPPORTED_MATCH_ALWAYS:
>> +			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) != 0);
>> +			break;
>> +		case MAE_FIELD_SUPPORTED_MATCH_NEVER:
>> +		case MAE_FIELD_UNSUPPORTED:
>> +		default:
>> +			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) == 0);
>> +			break;
>> +		}
>> +
>>   		if (is_valid == B_FALSE)
>>   			break;
>>   	}
>> @@ -1528,6 +1705,8 @@ efx_mae_match_specs_class_cmp(
>>   	const efx_mae_field_cap_t *field_caps;
>>   	const efx_mae_mv_desc_t *desc_setp;
>>   	unsigned int desc_set_nentries;
>> +	const efx_mae_mv_bit_desc_t *bit_desc_setp;
>> +	unsigned int bit_desc_set_nentries;
>>   	boolean_t have_same_class = B_TRUE;
>>   	efx_mae_field_id_t field_id;
>>   	const uint8_t *mvpl;
>> @@ -1540,6 +1719,9 @@ efx_mae_match_specs_class_cmp(
>>   		desc_setp = __efx_mae_outer_rule_mv_desc_set;
>>   		desc_set_nentries =
>>   		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_desc_set);
>> +		bit_desc_setp = __efx_mae_outer_rule_mv_bit_desc_set;
>> +		bit_desc_set_nentries =
>> +		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
>>   		mvpl = left->emms_mask_value_pairs.outer;
>>   		mvpr = right->emms_mask_value_pairs.outer;
>>   		break;
>> @@ -1548,6 +1730,9 @@ efx_mae_match_specs_class_cmp(
>>   		desc_setp = __efx_mae_action_rule_mv_desc_set;
>>   		desc_set_nentries =
>>   		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_desc_set);
>> +		bit_desc_setp = __efx_mae_action_rule_mv_bit_desc_set;
>> +		bit_desc_set_nentries =
>> +		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
>>   		mvpl = left->emms_mask_value_pairs.action;
>>   		mvpr = right->emms_mask_value_pairs.action;
>>   		break;
>> @@ -1620,6 +1805,52 @@ efx_mae_match_specs_class_cmp(
>>   		}
>>   	}
>>   
>> +	if (have_same_class == B_FALSE)
>> +		goto done;
>> +
>> +	for (field_id = 0; (unsigned int)field_id < bit_desc_set_nentries;
>> +	     ++field_id) {
>> +		const efx_mae_mv_bit_desc_t *bit_descp =
>> +		    &bit_desc_setp[field_id];
>> +		efx_mae_field_cap_id_t bit_cap_id =
>> +		    bit_descp->emmbd_bit_cap_id;
>> +		unsigned int byte_idx;
>> +		unsigned int bit_idx;
>> +
>> +		if (bit_descp->emmbd_entry_is_valid == B_FALSE)
>> +			continue; /* Skip array gap */
>> +
>> +		if ((unsigned int)bit_cap_id >= field_ncaps)
>> +			break;
>> +
>> +		byte_idx =
>> +		    bit_descp->emmbd_mask_ofst +
>> +		    bit_descp->emmbd_mask_lbn / 8;
>> +		bit_idx =
>> +		    bit_descp->emmbd_mask_lbn % 8;
>> +
>> +		if (field_caps[bit_cap_id].emfc_mask_affects_class &&
>> +		    (mvpl[byte_idx] & (1U << bit_idx)) !=
>> +		    (mvpr[byte_idx] & (1U << bit_idx))) {
>> +			have_same_class = B_FALSE;
>> +			break;
>> +		}
>> +
>> +		byte_idx =
>> +		    bit_descp->emmbd_value_ofst +
>> +		    bit_descp->emmbd_value_lbn / 8;
>> +		bit_idx =
>> +		    bit_descp->emmbd_value_lbn % 8;
>> +
>> +		if (field_caps[bit_cap_id].emfc_match_affects_class &&
>> +		    (mvpl[byte_idx] & (1U << bit_idx)) !=
>> +		    (mvpr[byte_idx] & (1U << bit_idx))) {
>> +			have_same_class = B_FALSE;
>> +			break;
>> +		}
>> +	}
>> +
>> +done:
>>   	*have_same_classp = have_same_class;
>>   
>>   	return (0);
>> @@ -2254,10 +2485,10 @@ efx_mae_action_rule_insert(
>>   	 * MCDI request and are thus safe to be copied directly to the buffer.
>>   	 */
>>   	EFX_STATIC_ASSERT(sizeof (spec->emms_mask_value_pairs.action) >=
>> -	    MAE_FIELD_MASK_VALUE_PAIRS_LEN);
>> +	    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN);
>>   	offset = MC_CMD_MAE_ACTION_RULE_INSERT_IN_MATCH_CRITERIA_OFST;
>>   	memcpy(payload + offset, spec->emms_mask_value_pairs.action,
>> -	    MAE_FIELD_MASK_VALUE_PAIRS_LEN);
>> +	    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN);
>>   
>>   	efx_mcdi_execute(enp, &req);
>>   
>> diff --git a/drivers/common/sfc_efx/version.map b/drivers/common/sfc_efx/version.map
>> index 5e724fd10..75da5aa5c 100644
>> --- a/drivers/common/sfc_efx/version.map
>> +++ b/drivers/common/sfc_efx/version.map
>> @@ -106,6 +106,7 @@ INTERNAL {
>>   	efx_mae_fini;
>>   	efx_mae_get_limits;
>>   	efx_mae_init;
>> +	efx_mae_match_spec_bit_set;
>>   	efx_mae_match_spec_field_set;
>>   	efx_mae_match_spec_fini;
>>   	efx_mae_match_spec_init;
>>
  
Ray Kinsella April 30, 2021, 2:49 p.m. UTC | #3
On 28/04/2021 10:49, Ivan Malov wrote:
> Introduce necessary infrastructure for these fields to
> be set, validated and compared during class comparison.
> Enumeration and mappings envisaged are MCDI-compatible.
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
> ---
>  drivers/common/sfc_efx/base/efx.h      |  18 ++
>  drivers/common/sfc_efx/base/efx_impl.h |   3 +-
>  drivers/common/sfc_efx/base/efx_mae.c  | 235 ++++++++++++++++++++++++-
>  drivers/common/sfc_efx/version.map     |   1 +
>  4 files changed, 254 insertions(+), 3 deletions(-)
> 
Acked-by: Ray Kinsella <mdr@ashroe.eu>
  

Patch

diff --git a/drivers/common/sfc_efx/base/efx.h b/drivers/common/sfc_efx/base/efx.h
index 771fe5a17..8e13075b0 100644
--- a/drivers/common/sfc_efx/base/efx.h
+++ b/drivers/common/sfc_efx/base/efx.h
@@ -4103,6 +4103,10 @@  efx_mae_match_spec_fini(
 	__in				efx_mae_match_spec_t *spec);
 
 typedef enum efx_mae_field_id_e {
+	/*
+	 * Fields which can be set by efx_mae_match_spec_field_set()
+	 * or by using dedicated field-specific helper APIs.
+	 */
 	EFX_MAE_FIELD_INGRESS_MPORT_SELECTOR = 0,
 	EFX_MAE_FIELD_ETHER_TYPE_BE,
 	EFX_MAE_FIELD_ETH_SADDR_BE,
@@ -4140,6 +4144,12 @@  typedef enum efx_mae_field_id_e {
 	EFX_MAE_FIELD_ENC_VNET_ID_BE,
 	EFX_MAE_FIELD_OUTER_RULE_ID,
 
+	/* Single bits which can be set by efx_mae_match_spec_bit_set(). */
+	EFX_MAE_FIELD_HAS_OVLAN,
+	EFX_MAE_FIELD_HAS_IVLAN,
+	EFX_MAE_FIELD_ENC_HAS_OVLAN,
+	EFX_MAE_FIELD_ENC_HAS_IVLAN,
+
 	EFX_MAE_FIELD_NIDS
 } efx_mae_field_id_t;
 
@@ -4198,6 +4208,14 @@  efx_mae_match_spec_field_set(
 	__in				size_t mask_size,
 	__in_bcount(mask_size)		const uint8_t *mask);
 
+/* The corresponding mask will be set to B_TRUE. */
+LIBEFX_API
+extern	__checkReturn			efx_rc_t
+efx_mae_match_spec_bit_set(
+	__in				efx_mae_match_spec_t *spec,
+	__in				efx_mae_field_id_t field_id,
+	__in				boolean_t value);
+
 /* If the mask argument is NULL, the API will use full mask by default. */
 LIBEFX_API
 extern	__checkReturn			efx_rc_t
diff --git a/drivers/common/sfc_efx/base/efx_impl.h b/drivers/common/sfc_efx/base/efx_impl.h
index 4a513171a..8b63cfb37 100644
--- a/drivers/common/sfc_efx/base/efx_impl.h
+++ b/drivers/common/sfc_efx/base/efx_impl.h
@@ -1720,7 +1720,8 @@  struct efx_mae_match_spec_s {
 	efx_mae_rule_type_t		emms_type;
 	uint32_t			emms_prio;
 	union emms_mask_value_pairs {
-		uint8_t			action[MAE_FIELD_MASK_VALUE_PAIRS_LEN];
+		uint8_t			action[
+					    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN];
 		uint8_t			outer[MAE_ENC_FIELD_PAIRS_LEN];
 	} emms_mask_value_pairs;
 };
diff --git a/drivers/common/sfc_efx/base/efx_mae.c b/drivers/common/sfc_efx/base/efx_mae.c
index 80fe155d0..33ba31389 100644
--- a/drivers/common/sfc_efx/base/efx_mae.c
+++ b/drivers/common/sfc_efx/base/efx_mae.c
@@ -451,6 +451,10 @@  typedef enum efx_mae_field_cap_id_e {
 	EFX_MAE_FIELD_ID_ENC_L4_DPORT_BE = MAE_FIELD_ENC_L4_DPORT,
 	EFX_MAE_FIELD_ID_ENC_VNET_ID_BE = MAE_FIELD_ENC_VNET_ID,
 	EFX_MAE_FIELD_ID_OUTER_RULE_ID = MAE_FIELD_OUTER_RULE_ID,
+	EFX_MAE_FIELD_ID_HAS_OVLAN = MAE_FIELD_HAS_OVLAN,
+	EFX_MAE_FIELD_ID_HAS_IVLAN = MAE_FIELD_HAS_IVLAN,
+	EFX_MAE_FIELD_ID_ENC_HAS_OVLAN = MAE_FIELD_ENC_HAS_OVLAN,
+	EFX_MAE_FIELD_ID_ENC_HAS_IVLAN = MAE_FIELD_ENC_HAS_IVLAN,
 
 	EFX_MAE_FIELD_CAP_NIDS
 } efx_mae_field_cap_id_t;
@@ -577,6 +581,65 @@  static const efx_mae_mv_desc_t __efx_mae_outer_rule_mv_desc_set[] = {
 
 #undef EFX_MAE_MV_DESC_ALT
 #undef EFX_MAE_MV_DESC
+};
+
+/*
+ * The following structure is a means to describe an MAE bit.
+ * The information in it is meant to be used internally by
+ * APIs for addressing a given flag in a mask-value pairs
+ * structure and for validation purposes.
+ */
+typedef struct efx_mae_mv_bit_desc_s {
+	/*
+	 * Arrays using this struct are indexed by field IDs.
+	 * Fields which aren't meant to be referenced by these
+	 * arrays comprise gaps (invalid entries). Below field
+	 * helps to identify such entries.
+	 */
+	boolean_t			emmbd_entry_is_valid;
+	efx_mae_field_cap_id_t		emmbd_bit_cap_id;
+	size_t				emmbd_value_ofst;
+	unsigned int			emmbd_value_lbn;
+	size_t				emmbd_mask_ofst;
+	unsigned int			emmbd_mask_lbn;
+} efx_mae_mv_bit_desc_t;
+
+static const efx_mae_mv_bit_desc_t __efx_mae_outer_rule_mv_bit_desc_set[] = {
+#define	EFX_MAE_MV_BIT_DESC(_name)					\
+	[EFX_MAE_FIELD_##_name] =					\
+	{								\
+		B_TRUE,							\
+		EFX_MAE_FIELD_ID_##_name,				\
+		MAE_ENC_FIELD_PAIRS_##_name##_OFST,			\
+		MAE_ENC_FIELD_PAIRS_##_name##_LBN,			\
+		MAE_ENC_FIELD_PAIRS_##_name##_MASK_OFST,		\
+		MAE_ENC_FIELD_PAIRS_##_name##_MASK_LBN,			\
+	}
+
+	EFX_MAE_MV_BIT_DESC(ENC_HAS_OVLAN),
+	EFX_MAE_MV_BIT_DESC(ENC_HAS_IVLAN),
+
+#undef EFX_MAE_MV_BIT_DESC
+};
+
+static const efx_mae_mv_bit_desc_t __efx_mae_action_rule_mv_bit_desc_set[] = {
+#define	EFX_MAE_MV_BIT_DESC(_name)					\
+	[EFX_MAE_FIELD_##_name] =					\
+	{								\
+		B_TRUE,							\
+		EFX_MAE_FIELD_ID_##_name,				\
+		MAE_FIELD_MASK_VALUE_PAIRS_V2_FLAGS_OFST,		\
+		MAE_FIELD_MASK_VALUE_PAIRS_V2_##_name##_LBN,		\
+		MAE_FIELD_MASK_VALUE_PAIRS_V2_FLAGS_MASK_OFST,		\
+		MAE_FIELD_MASK_VALUE_PAIRS_V2_##_name##_LBN,		\
+	}
+
+	EFX_MAE_MV_BIT_DESC(HAS_OVLAN),
+	EFX_MAE_MV_BIT_DESC(HAS_IVLAN),
+	EFX_MAE_MV_BIT_DESC(ENC_HAS_OVLAN),
+	EFX_MAE_MV_BIT_DESC(ENC_HAS_IVLAN),
+
+#undef EFX_MAE_MV_BIT_DESC
 };
 
 	__checkReturn			efx_rc_t
@@ -775,6 +838,70 @@  efx_mae_match_spec_field_set(
 	EFSYS_PROBE(fail5);
 fail4:
 	EFSYS_PROBE(fail4);
+fail3:
+	EFSYS_PROBE(fail3);
+fail2:
+	EFSYS_PROBE(fail2);
+fail1:
+	EFSYS_PROBE1(fail1, efx_rc_t, rc);
+	return (rc);
+}
+
+	__checkReturn			efx_rc_t
+efx_mae_match_spec_bit_set(
+	__in				efx_mae_match_spec_t *spec,
+	__in				efx_mae_field_id_t field_id,
+	__in				boolean_t value)
+{
+	const efx_mae_mv_bit_desc_t *bit_descp;
+	unsigned int bit_desc_set_nentries;
+	unsigned int byte_idx;
+	unsigned int bit_idx;
+	uint8_t *mvp;
+	efx_rc_t rc;
+
+	switch (spec->emms_type) {
+	case EFX_MAE_RULE_OUTER:
+		bit_desc_set_nentries =
+		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
+		bit_descp = &__efx_mae_outer_rule_mv_bit_desc_set[field_id];
+		mvp = spec->emms_mask_value_pairs.outer;
+		break;
+	case EFX_MAE_RULE_ACTION:
+		bit_desc_set_nentries =
+		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
+		bit_descp = &__efx_mae_action_rule_mv_bit_desc_set[field_id];
+		mvp = spec->emms_mask_value_pairs.action;
+		break;
+	default:
+		rc = ENOTSUP;
+		goto fail1;
+	}
+
+	if ((unsigned int)field_id >= bit_desc_set_nentries) {
+		rc = EINVAL;
+		goto fail2;
+	}
+
+	if (bit_descp->emmbd_entry_is_valid == B_FALSE) {
+		rc = EINVAL;
+		goto fail3;
+	}
+
+	byte_idx = bit_descp->emmbd_value_ofst + bit_descp->emmbd_value_lbn / 8;
+	bit_idx = bit_descp->emmbd_value_lbn % 8;
+
+	if (value != B_FALSE)
+		mvp[byte_idx] |= (1U << bit_idx);
+	else
+		mvp[byte_idx] &= ~(1U << bit_idx);
+
+	byte_idx = bit_descp->emmbd_mask_ofst + bit_descp->emmbd_mask_lbn / 8;
+	bit_idx = bit_descp->emmbd_mask_lbn % 8;
+	mvp[byte_idx] |= (1U << bit_idx);
+
+	return (0);
+
 fail3:
 	EFSYS_PROBE(fail3);
 fail2:
@@ -891,6 +1018,8 @@  efx_mae_match_spec_is_valid(
 	const efx_mae_field_cap_t *field_caps;
 	const efx_mae_mv_desc_t *desc_setp;
 	unsigned int desc_set_nentries;
+	const efx_mae_mv_bit_desc_t *bit_desc_setp;
+	unsigned int bit_desc_set_nentries;
 	boolean_t is_valid = B_TRUE;
 	efx_mae_field_id_t field_id;
 	const uint8_t *mvp;
@@ -901,6 +1030,9 @@  efx_mae_match_spec_is_valid(
 		desc_setp = __efx_mae_outer_rule_mv_desc_set;
 		desc_set_nentries =
 		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_desc_set);
+		bit_desc_setp = __efx_mae_outer_rule_mv_bit_desc_set;
+		bit_desc_set_nentries =
+		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
 		mvp = spec->emms_mask_value_pairs.outer;
 		break;
 	case EFX_MAE_RULE_ACTION:
@@ -908,6 +1040,9 @@  efx_mae_match_spec_is_valid(
 		desc_setp = __efx_mae_action_rule_mv_desc_set;
 		desc_set_nentries =
 		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_desc_set);
+		bit_desc_setp = __efx_mae_action_rule_mv_bit_desc_set;
+		bit_desc_set_nentries =
+		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
 		mvp = spec->emms_mask_value_pairs.action;
 		break;
 	default:
@@ -975,6 +1110,48 @@  efx_mae_match_spec_is_valid(
 			break;
 		}
 
+		if (is_valid == B_FALSE)
+			return (B_FALSE);
+	}
+
+	for (field_id = 0; (unsigned int)field_id < bit_desc_set_nentries;
+	     ++field_id) {
+		const efx_mae_mv_bit_desc_t *bit_descp =
+		    &bit_desc_setp[field_id];
+		unsigned int byte_idx =
+		    bit_descp->emmbd_mask_ofst +
+		    bit_descp->emmbd_mask_lbn / 8;
+		unsigned int bit_idx =
+		    bit_descp->emmbd_mask_lbn % 8;
+		efx_mae_field_cap_id_t bit_cap_id =
+		    bit_descp->emmbd_bit_cap_id;
+
+		if (bit_descp->emmbd_entry_is_valid == B_FALSE)
+			continue; /* Skip array gap */
+
+		if ((unsigned int)bit_cap_id >= field_ncaps) {
+			/* No capability for this bit = unsupported. */
+			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) == 0);
+			if (is_valid == B_FALSE)
+				break;
+			else
+				continue;
+		}
+
+		switch (field_caps[bit_cap_id].emfc_support) {
+		case MAE_FIELD_SUPPORTED_MATCH_OPTIONAL:
+			is_valid = B_TRUE;
+			break;
+		case MAE_FIELD_SUPPORTED_MATCH_ALWAYS:
+			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) != 0);
+			break;
+		case MAE_FIELD_SUPPORTED_MATCH_NEVER:
+		case MAE_FIELD_UNSUPPORTED:
+		default:
+			is_valid = ((mvp[byte_idx] & (1U << bit_idx)) == 0);
+			break;
+		}
+
 		if (is_valid == B_FALSE)
 			break;
 	}
@@ -1528,6 +1705,8 @@  efx_mae_match_specs_class_cmp(
 	const efx_mae_field_cap_t *field_caps;
 	const efx_mae_mv_desc_t *desc_setp;
 	unsigned int desc_set_nentries;
+	const efx_mae_mv_bit_desc_t *bit_desc_setp;
+	unsigned int bit_desc_set_nentries;
 	boolean_t have_same_class = B_TRUE;
 	efx_mae_field_id_t field_id;
 	const uint8_t *mvpl;
@@ -1540,6 +1719,9 @@  efx_mae_match_specs_class_cmp(
 		desc_setp = __efx_mae_outer_rule_mv_desc_set;
 		desc_set_nentries =
 		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_desc_set);
+		bit_desc_setp = __efx_mae_outer_rule_mv_bit_desc_set;
+		bit_desc_set_nentries =
+		    EFX_ARRAY_SIZE(__efx_mae_outer_rule_mv_bit_desc_set);
 		mvpl = left->emms_mask_value_pairs.outer;
 		mvpr = right->emms_mask_value_pairs.outer;
 		break;
@@ -1548,6 +1730,9 @@  efx_mae_match_specs_class_cmp(
 		desc_setp = __efx_mae_action_rule_mv_desc_set;
 		desc_set_nentries =
 		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_desc_set);
+		bit_desc_setp = __efx_mae_action_rule_mv_bit_desc_set;
+		bit_desc_set_nentries =
+		    EFX_ARRAY_SIZE(__efx_mae_action_rule_mv_bit_desc_set);
 		mvpl = left->emms_mask_value_pairs.action;
 		mvpr = right->emms_mask_value_pairs.action;
 		break;
@@ -1620,6 +1805,52 @@  efx_mae_match_specs_class_cmp(
 		}
 	}
 
+	if (have_same_class == B_FALSE)
+		goto done;
+
+	for (field_id = 0; (unsigned int)field_id < bit_desc_set_nentries;
+	     ++field_id) {
+		const efx_mae_mv_bit_desc_t *bit_descp =
+		    &bit_desc_setp[field_id];
+		efx_mae_field_cap_id_t bit_cap_id =
+		    bit_descp->emmbd_bit_cap_id;
+		unsigned int byte_idx;
+		unsigned int bit_idx;
+
+		if (bit_descp->emmbd_entry_is_valid == B_FALSE)
+			continue; /* Skip array gap */
+
+		if ((unsigned int)bit_cap_id >= field_ncaps)
+			break;
+
+		byte_idx =
+		    bit_descp->emmbd_mask_ofst +
+		    bit_descp->emmbd_mask_lbn / 8;
+		bit_idx =
+		    bit_descp->emmbd_mask_lbn % 8;
+
+		if (field_caps[bit_cap_id].emfc_mask_affects_class &&
+		    (mvpl[byte_idx] & (1U << bit_idx)) !=
+		    (mvpr[byte_idx] & (1U << bit_idx))) {
+			have_same_class = B_FALSE;
+			break;
+		}
+
+		byte_idx =
+		    bit_descp->emmbd_value_ofst +
+		    bit_descp->emmbd_value_lbn / 8;
+		bit_idx =
+		    bit_descp->emmbd_value_lbn % 8;
+
+		if (field_caps[bit_cap_id].emfc_match_affects_class &&
+		    (mvpl[byte_idx] & (1U << bit_idx)) !=
+		    (mvpr[byte_idx] & (1U << bit_idx))) {
+			have_same_class = B_FALSE;
+			break;
+		}
+	}
+
+done:
 	*have_same_classp = have_same_class;
 
 	return (0);
@@ -2254,10 +2485,10 @@  efx_mae_action_rule_insert(
 	 * MCDI request and are thus safe to be copied directly to the buffer.
 	 */
 	EFX_STATIC_ASSERT(sizeof (spec->emms_mask_value_pairs.action) >=
-	    MAE_FIELD_MASK_VALUE_PAIRS_LEN);
+	    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN);
 	offset = MC_CMD_MAE_ACTION_RULE_INSERT_IN_MATCH_CRITERIA_OFST;
 	memcpy(payload + offset, spec->emms_mask_value_pairs.action,
-	    MAE_FIELD_MASK_VALUE_PAIRS_LEN);
+	    MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN);
 
 	efx_mcdi_execute(enp, &req);
 
diff --git a/drivers/common/sfc_efx/version.map b/drivers/common/sfc_efx/version.map
index 5e724fd10..75da5aa5c 100644
--- a/drivers/common/sfc_efx/version.map
+++ b/drivers/common/sfc_efx/version.map
@@ -106,6 +106,7 @@  INTERNAL {
 	efx_mae_fini;
 	efx_mae_get_limits;
 	efx_mae_init;
+	efx_mae_match_spec_bit_set;
 	efx_mae_match_spec_field_set;
 	efx_mae_match_spec_fini;
 	efx_mae_match_spec_init;