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: <20221020074104.qmow2fc66v4is2rk@wse-c0155>
Date:   Thu, 20 Oct 2022 09:41:04 +0200
From:   Casper Andersson <casper.casan@...il.com>
To:     Steen Hegelund <steen.hegelund@...rochip.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, UNGLinuxDriver@...rochip.com,
        Randy Dunlap <rdunlap@...radead.org>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Wan Jiabing <wanjiabing@...o.com>,
        Nathan Huckleberry <nhuck@...gle.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH net-next v2 6/9] net: microchip: sparx5: Adding basic
 rule management in VCAP API

On 2022-10-19 13:42, Steen Hegelund wrote:
> +/* Write VCAP cache content to the VCAP HW instance */
> +static int vcap_write_rule(struct vcap_rule_internal *ri)
> +{
> +	struct vcap_admin *admin = ri->admin;
> +	int sw_idx, ent_idx = 0, act_idx = 0;
> +	u32 addr = ri->addr;
> +
> +	if (!ri->size || !ri->keyset_sw_regs || !ri->actionset_sw_regs) {
> +		pr_err("%s:%d: rule is empty\n", __func__, __LINE__);
> +		return -EINVAL;
> +	}
> +	/* Use the values in the streams to write the VCAP cache */
> +	for (sw_idx = 0; sw_idx < ri->size; sw_idx++, addr++) {
> +		ri->vctrl->ops->cache_write(ri->ndev, admin,
> +					VCAP_SEL_ENTRY, ent_idx,
> +					ri->keyset_sw_regs);
> +		ri->vctrl->ops->cache_write(ri->ndev, admin,
> +					VCAP_SEL_ACTION, act_idx,
> +					ri->actionset_sw_regs);
> +		ri->vctrl->ops->update(ri->ndev, admin, VCAP_CMD_WRITE,
> +				   VCAP_SEL_ALL, addr);

Arguments not aligned with opening parenthesis.

>  /* Validate a rule with respect to available port keys */
>  int vcap_val_rule(struct vcap_rule *rule, u16 l3_proto)
>  {
>  	struct vcap_rule_internal *ri = to_intrule(rule);
> +	enum vcap_keyfield_set keysets[10];
> +	struct vcap_keyset_list kslist;
> +	int ret;
>  
>  	/* This validation will be much expanded later */
> +	ret = vcap_api_check(ri->vctrl);
> +	if (ret)
> +		return ret;
>  	if (!ri->admin) {
>  		ri->data.exterr = VCAP_ERR_NO_ADMIN;
>  		return -EINVAL;
> @@ -113,14 +304,41 @@ int vcap_val_rule(struct vcap_rule *rule, u16 l3_proto)
>  		ri->data.exterr = VCAP_ERR_NO_KEYSET_MATCH;
>  		return -EINVAL;
>  	}
> +	/* prepare for keyset validation */
> +	keysets[0] = ri->data.keyset;
> +	kslist.keysets = keysets;
> +	kslist.cnt = 1;
> +	/* Pick a keyset that is supported in the port lookups */
> +	ret = ri->vctrl->ops->validate_keyset(ri->ndev, ri->admin, rule, &kslist,
> +					      l3_proto);
> +	if (ret < 0) {
> +		pr_err("%s:%d: keyset validation failed: %d\n",
> +		       __func__, __LINE__, ret);
> +		ri->data.exterr = VCAP_ERR_NO_PORT_KEYSET_MATCH;
> +		return ret;
> +	}
>  	if (ri->data.actionset == VCAP_AFS_NO_VALUE) {
>  		ri->data.exterr = VCAP_ERR_NO_ACTIONSET_MATCH;
>  		return -EINVAL;
>  	}
> -	return 0;
> +	vcap_add_type_keyfield(rule);
> +	/* Add default fields to this rule */
> +	ri->vctrl->ops->add_default_fields(ri->ndev, ri->admin, rule);
> +
> +	/* Rule size is the maximum of the entry and action subword count */
> +	ri->size = max(ri->keyset_sw, ri->actionset_sw);
> +
> +	/* Finally check if there is room for the rule in the VCAP */
> +	return vcap_rule_space(ri->admin, ri->size);
>  }
>  EXPORT_SYMBOL_GPL(vcap_val_rule);

Validating a rule also modifies it. I think validation and modification
should generally be kept apart. But it looks like it might be hard with
the current design since you need to add the fields to then check the
space it takes, and the rule sizes can depend on the hardware.

Tested on Microchip PCB135 switch.

Tested-by: Casper Andersson <casper.casan@...il.com>
Reviewed-by: Casper Andersson <casper.casan@...il.com>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ