netdev - Re: [PATCH net-next 1/4] sfc: support TC left-hand-side rules on foreign netdevs

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]

Date: Tue, 3 Oct 2023 14:20:35 +0200
From: Simon Horman <horms@...nel.org>
To: edward.cree@....com
Cc: linux-net-drivers@....com, davem@...emloft.net, kuba@...nel.org,
	edumazet@...gle.com, pabeni@...hat.com,
	Edward Cree <ecree.xilinx@...il.com>, netdev@...r.kernel.org,
	habetsm.xilinx@...il.com, pieter.jansen-van-vuuren@....com
Subject: Re: [PATCH net-next 1/4] sfc: support TC left-hand-side rules on
 foreign netdevs

On Mon, Oct 02, 2023 at 04:44:41PM +0100, edward.cree@....com wrote:
> From: Edward Cree <ecree.xilinx@...il.com>
> 
> Allow a tunnel netdevice (such as a vxlan) to offload conntrack lookups,
>  in much the same way as efx netdevs.
> To ensure this rule does not overlap with other tunnel rules on the same
>  sip,dip,dport tuple, register a pseudo encap match of a new type
>  (EFX_TC_EM_PSEUDO_OR), which unlike PSEUDO_MASK may only be referenced
>  once (because an actual Outer Rule in hardware exists, although its
>  fw_id is not recorded in the encap match entry).
> 
> Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@....com>
> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>

...

> @@ -1354,6 +1450,119 @@ static int efx_tc_incomplete_mangle(struct efx_tc_mangler_state *mung,
>  	return 0;
>  }
>  
> +static int efx_tc_flower_replace_foreign_lhs(struct efx_nic *efx,
> +					     struct flow_cls_offload *tc,
> +					     struct flow_rule *fr,
> +					     struct efx_tc_match *match,
> +					     struct net_device *net_dev)
> +{
> +	struct netlink_ext_ack *extack = tc->common.extack;
> +	struct efx_tc_lhs_rule *rule, *old;
> +	enum efx_encap_type type;
> +	int rc;
> +
> +	if (tc->common.chain_index) {
> +		NL_SET_ERR_MSG_MOD(extack, "LHS rule only allowed in chain 0");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (!efx_tc_match_is_encap(&match->mask)) {
> +		/* This is not a tunnel decap rule, ignore it */
> +		netif_dbg(efx, drv, efx->net_dev, "Ignoring foreign LHS filter without encap match\n");

Hi Edward,

is NL_SET_ERR_MSG_MOD() appropriate here?

> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (efx_tc_flower_flhs_needs_ar(match)) {
> +		NL_SET_ERR_MSG_MOD(extack, "Match keys not available in Outer Rule");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	type = efx_tc_indr_netdev_type(net_dev);
> +	if (type == EFX_ENCAP_TYPE_NONE) {
> +		NL_SET_ERR_MSG_MOD(extack, "Egress encap match on unsupported tunnel device\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	rc = efx_mae_check_encap_type_supported(efx, type);
> +	if (rc) {
> +		NL_SET_ERR_MSG_FMT_MOD(extack,
> +				       "Firmware reports no support for %s encap match",
> +				       efx_tc_encap_type_name(type));
> +		return rc;
> +	}
> +	/* Reserve the outer tuple with a pseudo Encap Match */
> +	rc = efx_tc_flower_record_encap_match(efx, match, type,
> +					      EFX_TC_EM_PSEUDO_OR, 0, 0,
> +					      extack);
> +	if (rc)
> +		return rc;
> +
> +	if (match->mask.ct_state_trk && match->value.ct_state_trk) {
> +		NL_SET_ERR_MSG_MOD(extack, "LHS rule can never match +trk");
> +		rc = -EOPNOTSUPP;
> +		goto release_encap_match;
> +	}
> +	/* LHS rules are always -trk, so we don't need to match on that */
> +	match->mask.ct_state_trk = 0;
> +	match->value.ct_state_trk = 0;
> +
> +	rc = efx_tc_flower_translate_flhs_match(match);
> +	if (rc) {
> +		NL_SET_ERR_MSG_MOD(extack, "LHS rule cannot match on inner fields");
> +		goto release_encap_match;
> +	}
> +
> +	rc = efx_mae_match_check_caps_lhs(efx, &match->mask, extack);
> +	if (rc)
> +		goto release_encap_match;
> +
> +	rule = kzalloc(sizeof(*rule), GFP_USER);
> +	if (!rule) {
> +		rc = -ENOMEM;
> +		goto release_encap_match;
> +	}
> +	rule->cookie = tc->cookie;
> +	old = rhashtable_lookup_get_insert_fast(&efx->tc->lhs_rule_ht,
> +						&rule->linkage,
> +						efx_tc_lhs_rule_ht_params);
> +	if (old) {
> +		netif_dbg(efx, drv, efx->net_dev,
> +			  "Already offloaded rule (cookie %lx)\n", tc->cookie);
> +		rc = -EEXIST;
> +		NL_SET_ERR_MSG_MOD(extack, "Rule already offloaded");
> +		goto release;
> +	}
> +
> +	/* Parse actions */
> +	rc = efx_tc_flower_handle_lhs_actions(efx, tc, fr, net_dev, rule);
> +	if (rc)
> +		goto release;
> +
> +	rule->match = *match;
> +	rule->lhs_act.tun_type = type;
> +
> +	rc = efx_mae_insert_lhs_rule(efx, rule, EFX_TC_PRIO_TC);
> +	if (rc) {
> +		NL_SET_ERR_MSG_MOD(extack, "Failed to insert rule in hw");
> +		goto release;
> +	}
> +	netif_dbg(efx, drv, efx->net_dev,
> +		  "Successfully parsed lhs rule (cookie %lx)\n",
> +		  tc->cookie);
> +	return 0;
> +
> +release:
> +	efx_tc_flower_release_lhs_actions(efx, &rule->lhs_act);
> +	if (!old)
> +		rhashtable_remove_fast(&efx->tc->lhs_rule_ht, &rule->linkage,
> +				       efx_tc_lhs_rule_ht_params);
> +	kfree(rule);
> +release_encap_match:
> +	if (match->encap)
> +		efx_tc_flower_release_encap_match(efx, match->encap);
> +	return rc;
> +}

...