Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "allan.nielsen@microchip.com" <allan.nielsen@microchip.com>,
        "joergen.andreasen@microchip.com" <joergen.andreasen@microchip.com>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        "vinicius.gomes@intel.com" <vinicius.gomes@intel.com>,
        "michael.chan@broadcom.com" <michael.chan@broadcom.com>,
        "vishal@chelsio.com" <vishal@chelsio.com>,
        "saeedm@mellanox.com" <saeedm@mellanox.com>,
        "jiri@mellanox.com" <jiri@mellanox.com>,
        "idosch@mellanox.com" <idosch@mellanox.com>,
        "alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
        "kuba@kernel.org" <kuba@kernel.org>, Po Liu <po.liu@nxp.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandru Marginean <alexandru.marginean@nxp.com>,
        Leo Li <leoyang.li@nxp.com>, Mingkai Hu <mingkai.hu@nxp.com>
Subject: Re: [PATCH v1 net-next 5/5] net: dsa: felix: add police action for tc
 flower offload
Thread-Topic: [PATCH v1 net-next 5/5] net: dsa: felix: add police action for
 tc flower offload
Thread-Index: AQHWprM3+3GICwmxPUy+mJkFGzle6amhJvSA
Date:   Tue, 20 Oct 2020 23:38:58 +0000
Message-ID: <20201020233857.ifa4zjehupbvv5vs@skbuf>
References: <20201020072321.36921-1-xiaoliang.yang_1@nxp.com>
 <20201020072321.36921-6-xiaoliang.yang_1@nxp.com>
In-Reply-To: <20201020072321.36921-6-xiaoliang.yang_1@nxp.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C7F177C6A8CB2A47A4F7684B70FF793C@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 032f978b-53e2-4ade-8945-08d875515147
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Oct 2020 23:38:58.7429
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: lAgmHN2IE+zkiaDzV+r9BvCZHamw/aPxIivSphCqyVXNh5KHuwNi44tDKB/MLfd2j3GbRHbaXkaOXwHy/BLNgg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR04MB4184
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.
>=20
> 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.
>=20
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.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(+)
>=20
> diff --git a/drivers/net/dsa/ocelot/felix_flower.c b/drivers/net/dsa/ocel=
ot/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 ocelo=
t *ocelot, u32 index)
>  		if (tmp->index =3D=3D index) {
>  			if (tmp->sg_valid)
>  				felix_list_gate_del(ocelot, tmp->sgid);
> +			if (tmp->fm_valid)
> +				ocelot_vcap_policer_del(ocelot, tmp->fmid);
> =20
>  			z =3D 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;
>  	}
> =20
> @@ -559,7 +563,9 @@ int felix_flower_stream_replace(struct ocelot *ocelot=
, int port,
>  	struct felix_streamid stream =3D {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;
> =20
>  	ret =3D felix_flower_parse_key(f, &stream);
> @@ -595,6 +601,32 @@ int felix_flower_stream_replace(struct ocelot *ocelo=
t, int port,
>  			stream.sfid_valid =3D 1;
>  			kfree(sgi);
>  			break;
> +		case FLOW_ACTION_POLICE:
> +			if (f->common.chain_index !=3D 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 =3D a->police.index + FELIX_POLICER_PSFP_BASE;
> +			if (index > FELIX_POLICER_PSFP_MAX)
> +				return -EINVAL;
> +
> +			rate =3D a->police.rate_bytes_ps;
> +			burst =3D rate * PSCHED_NS2TICKS(a->police.burst);
> +			pol =3D (struct ocelot_policer) {
> +				.burst =3D div_u64(burst, PSCHED_TICKS_PER_SEC),
> +				.rate =3D div_u64(rate, 1000) * 8,
> +			};
> +			ret =3D ocelot_vcap_policer_add(ocelot, index, &pol);
> +			if (ret)
> +				return ret;
> +
> +			sfi.fm_valid =3D 1;
> +			sfi.fmid =3D index;
> +			sfi.maxsdu =3D a->police.mtu;
> +			stream.sfid_valid =3D 1;
> +			break;
>  		default:
>  			return -EOPNOTSUPP;
>  		}
> --=20
> 2.17.1
> =