linux-kernel - Re: [PATCH v1 net-next 5/5] net: dsa: felix: add police action for tc flower offload

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: <20201020233857.ifa4zjehupbvv5vs@skbuf>
Date:   Tue, 20 Oct 2020 23:38:58 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Xiaoliang Yang <xiaoliang.yang_1@....com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "allan.nielsen@...rochip.com" <allan.nielsen@...rochip.com>,
        "joergen.andreasen@...rochip.com" <joergen.andreasen@...rochip.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        "vinicius.gomes@...el.com" <vinicius.gomes@...el.com>,
        "michael.chan@...adcom.com" <michael.chan@...adcom.com>,
        "vishal@...lsio.com" <vishal@...lsio.com>,
        "saeedm@...lanox.com" <saeedm@...lanox.com>,
        "jiri@...lanox.com" <jiri@...lanox.com>,
        "idosch@...lanox.com" <idosch@...lanox.com>,
        "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
        "kuba@...nel.org" <kuba@...nel.org>, Po Liu <po.liu@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        Leo Li <leoyang.li@....com>, Mingkai Hu <mingkai.hu@....com>
Subject: Re: [PATCH v1 net-next 5/5] net: dsa: felix: add police action for tc
 flower offload

On Tue, Oct 20, 2020 at 03:23:21PM +0800, Xiaoliang Yang wrote:
> This patch add police action to set flow meter table which is defined
> in IEEE802.1Qci. Flow metering is two rates two buckets and three color
> marker to policing the frames, we only enable one rate one bucket in
> this patch.
> 
> Flow metering shares a same policer pool with VCAP policers, it calls
> ocelot_vcap_policer_add() and ocelot_vcap_policer_del() to set flow
> meter table.
> 
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@....com>
> ---

It would be good to provide a functional example in
tools/testing/selftests/drivers/net/ocelot/ which sets a policer up and
explains what is the practical difference between a PSFP policer and a
regular VCAP policer, except for the fact that the PSFP policer can only
match on {DMAC, VLAN} and the regular one can match on a lot more (L2,
L3 and L4 headers).
Specifically, why would the user ever want to use a PSFP policer.

>  drivers/net/dsa/ocelot/felix_flower.c | 32 +++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/net/dsa/ocelot/felix_flower.c b/drivers/net/dsa/ocelot/felix_flower.c
> index 71894dcc0af2..d58a2357bab1 100644
> --- a/drivers/net/dsa/ocelot/felix_flower.c
> +++ b/drivers/net/dsa/ocelot/felix_flower.c
> @@ -363,6 +363,8 @@ static void felix_list_stream_filter_del(struct ocelot *ocelot, u32 index)
>  		if (tmp->index == index) {
>  			if (tmp->sg_valid)
>  				felix_list_gate_del(ocelot, tmp->sgid);
> +			if (tmp->fm_valid)
> +				ocelot_vcap_policer_del(ocelot, tmp->fmid);
>  
>  			z = refcount_dec_and_test(&tmp->refcount);
>  			if (z) {
> @@ -466,6 +468,8 @@ static int felix_psfp_set(struct ocelot *ocelot,
>  	if (ret) {
>  		if (sfi->sg_valid)
>  			felix_list_gate_del(ocelot, sfi->sgid);
> +		if (sfi->fm_valid)
> +			ocelot_vcap_policer_del(ocelot, sfi->fmid);
>  		return ret;
>  	}
>  
> @@ -559,7 +563,9 @@ int felix_flower_stream_replace(struct ocelot *ocelot, int port,
>  	struct felix_streamid stream = {0};
>  	struct felix_stream_gate_conf *sgi;
>  	const struct flow_action_entry *a;
> +	struct ocelot_policer pol;
>  	int ret, size, i;
> +	u64 rate, burst;
>  	u32 index;
>  
>  	ret = felix_flower_parse_key(f, &stream);
> @@ -595,6 +601,32 @@ int felix_flower_stream_replace(struct ocelot *ocelot, int port,
>  			stream.sfid_valid = 1;
>  			kfree(sgi);
>  			break;
> +		case FLOW_ACTION_POLICE:
> +			if (f->common.chain_index != OCELOT_PSFP_CHAIN) {
> +				NL_SET_ERR_MSG_MOD(extack,
> +						   "Police action only be offloaded to PSFP chain");

This message is confusing and outright incorrect. The policing action
can be offloaded to VCAP IS2 too.

> +				return -EOPNOTSUPP;
> +			}
> +
> +			index = a->police.index + FELIX_POLICER_PSFP_BASE;
> +			if (index > FELIX_POLICER_PSFP_MAX)
> +				return -EINVAL;
> +
> +			rate = a->police.rate_bytes_ps;
> +			burst = rate * PSCHED_NS2TICKS(a->police.burst);
> +			pol = (struct ocelot_policer) {
> +				.burst = div_u64(burst, PSCHED_TICKS_PER_SEC),
> +				.rate = div_u64(rate, 1000) * 8,
> +			};
> +			ret = ocelot_vcap_policer_add(ocelot, index, &pol);
> +			if (ret)
> +				return ret;
> +
> +			sfi.fm_valid = 1;
> +			sfi.fmid = index;
> +			sfi.maxsdu = a->police.mtu;
> +			stream.sfid_valid = 1;
> +			break;
>  		default:
>  			return -EOPNOTSUPP;
>  		}
> -- 
> 2.17.1
>