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:   "Volodymyr Mytnyk [C]" <vmytnyk@marvell.com>
To:     Ido Schimmel <idosch@idosch.org>,
        Vadym Kochan <vadym.kochan@plvision.eu>
CC:     "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Vladimir Oltean <olteanv@gmail.com>,
        Serhiy Boiko <serhiy.boiko@plvision.eu>,
        Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>,
        Taras Chornyi <taras.chornyi@plvision.eu>,
        "Taras Chornyi [C]" <tchornyi@marvell.com>,
        Mickey Rachamim <mickeyr@marvell.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Vadym Kochan [C]" <vkochan@marvell.com>
Subject: Re: [PATCH net-next v2 4/4] net: marvell: prestera: Offload
 FLOW_ACTION_POLICE
Thread-Topic: [PATCH net-next v2 4/4] net: marvell: prestera: Offload
 FLOW_ACTION_POLICE
Thread-Index: AQHXiINGT+iNhw1SOE6nI3my9wPbFQ==
Date:   Tue, 3 Aug 2021 16:19:03 +0000
Message-ID: <SJ0PR18MB4009243B42CB9A03C2C35647B2F09@SJ0PR18MB4009.namprd18.prod.outlook.com>
References: <20210802140849.2050-1-vadym.kochan@plvision.eu>
 <20210802140849.2050-5-vadym.kochan@plvision.eu>,<YQgN1djql6wOk8dc@shredder>
In-Reply-To: <YQgN1djql6wOk8dc@shredder>
Accept-Language: en-GB, uk-UA, en-US
Content-Language: en-GB
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?yZwtFNdKtBHFef5eBPKMvOVveNgEE6BMtPTCuVxxVGV0vYsoMgJNGxQe4m?=
 =?iso-8859-1?Q?xtQ6HEU9RVhCAGemtH8oQDorDGnh1I9GcWZeLkJGxsNZkqT7JmCCPns31/?=
 =?iso-8859-1?Q?jzMBF13JK/xnmiI0zAG9EfaiiYqqQxX3wWgDK+DLrxqUF6OEtFnWBTXffl?=
 =?iso-8859-1?Q?GHig/P6PoBJBxJHseotjX2e2OTcojJQdXEw7OMB96U13RU/JJt6DxYGcj2?=
 =?iso-8859-1?Q?KxE+y9y6cXhyXQ+H5dqQZ62DYuBx/LxLHt0eqYOc2tlvXL5a3omzDSoBPi?=
 =?iso-8859-1?Q?a3f53/soB0zvoDLDFx/IJZKABACgPDucF6De9SMy5LsTYhWtlj6ZiJYTYD?=
 =?iso-8859-1?Q?rYRahyyfciHhNXW3P+nlQtiL6iPTdKAvznIr+fwy8W2AuO84PrE2JtTFBN?=
 =?iso-8859-1?Q?bln+o0uxUPPMhh3CNsy68jgvBW5d1JJy/fA/VRSkbr4rxwLQCBNiApluJp?=
 =?iso-8859-1?Q?vQAzcmP8p2VVRauoIMp8GqbJ8ATRV3LWOzozHwXwRsH98m7m4RE+27ZA6T?=
 =?iso-8859-1?Q?zPB1I5pze+OwEIfhM/urhC/KAl14RSH1SXRsEIZAdAsUPQE9TxQGrcX+p0?=
 =?iso-8859-1?Q?Za7V9mPJRfoYr8xuWwacGt/bN+lFRDO/8AG4Ws6lJkf5lGinv0zch5/e00?=
 =?iso-8859-1?Q?EIeRBiTlZfLsOmg8zzk+2HIilZFyABmAL3p4kjnrPnASIiNINqiXFvdO/1?=
 =?iso-8859-1?Q?ILYGOtm98UkFkXOZcnmIKUwSVRk5+wMVWy1eXSpzeDcSG/ueTowomq9z52?=
 =?iso-8859-1?Q?7bHsYe+5D2NBahz4gwOw/BxrgrbxEcM0OOIhS9aax7JWIlapNKV6Lh7fY4?=
 =?iso-8859-1?Q?IMPz8kLvbyZ1GvXLyKwKtaEyJ3Xs1MV9ctcGbbzawcJMtgGxVnmiNfR9rJ?=
 =?iso-8859-1?Q?hwdjbNNeawB81NY5k+H9TrMVIpDIrRj9Q6Ioes5OYMBeWAOfzbZCB7jDo2?=
 =?iso-8859-1?Q?1+12rgrQ8+PFT+gM9/zrM+HfeVf78xQHkT2PIF0UoLUOn7116wbrLHW+6H?=
 =?iso-8859-1?Q?D6Pn/U+FTwskg6IjrmP9aDmFVTQOWcaoKvW9KyZRdE0Tg7nKuX6Dy70TTz?=
 =?iso-8859-1?Q?ICq61xOYAo+YuJy7Gl42gxr+sJ4N8TLHKGt8YklcgSzgnkRPvRDn4lsMoD?=
 =?iso-8859-1?Q?+ffKCS/wRnOBnMdf36H62mArNoz+gtuOGF/W3G0iXMc5FQViAcmnO/iT08?=
 =?iso-8859-1?Q?BtxMgxnLDZ1XRIcK5PfuLFF3D4pAJGe+XTAYRs+PDg73fletCflsrud1HQ?=
 =?iso-8859-1?Q?+Nmr+y+5lDgLBKMwAptjv52GYTgalRS6CAGRR/o18=3D?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR18MB4009.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0fa38c0b-f896-4698-7699-08d9569a6930
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Aug 2021 16:19:03.7938
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: QiGFy7MbXpLwtssQiNha37g/wrp4+eFojLDFqzeDCwN+atkXAxZWdvUpaDfty3kxtFpTQIbyC6t/FhUHeCrglw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR18MB3297
X-Proofpoint-ORIG-GUID: 2M6-GAX0CdHQ_iWG-yDdZh4XlxCcnS9c
X-Proofpoint-GUID: 2M6-GAX0CdHQ_iWG-yDdZh4XlxCcnS9c
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790
 definitions=2021-08-03_05:2021-08-03,2021-08-03 signatures=0
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Ido,=0A=
=0A=
Thanks for the review. Pls see the comments inline.=0A=
=0A=
> On Mon, Aug 02, 2021 at 05:08:49PM +0300, Vadym Kochan wrote:=0A=
> > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/=
drivers/net/ethernet/marvell/prestera/prestera_flower.c=0A=
> > index e571ba09ec08..76f30856ac98 100644=0A=
> > --- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c=0A=
> > +++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c=0A=
> > @@ -5,6 +5,8 @@=0A=
> > =A0#include "prestera_acl.h"=0A=
> > =A0#include "prestera_flower.h"=0A=
> >=0A=
> > +#define PRESTERA_HW_TC_NUM =A0 8=0A=
> > +=0A=
> > =A0static int prestera_flower_parse_actions(struct prestera_flow_block =
*block,=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 struct prestera_acl_rule *rule,=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 struct flow_action *flow_action,=0A=
> > @@ -30,6 +32,11 @@ static int prestera_flower_parse_actions(struct pres=
tera_flow_block *block,=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case FLOW_ACTION_TRAP:=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0a_entry.id =3D PRESTERA_=
ACL_RULE_ACTION_TRAP;=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 case FLOW_ACTION_POLICE:=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 a_entry.id =3D PRESTERA_ACL_R=
ULE_ACTION_POLICE;=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 a_entry.police.rate =3D act->=
police.rate_bytes_ps;=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 a_entry.police.burst =3D act-=
>police.burst;=0A=
>=0A=
> If packet rate based policing is not supported, an error should be=0A=
> returned here with extack.=0A=
=0A=
Agree, it makes sense.=0A=
=0A=
>=0A=
> It seems the implementation assumes that each rule has a different=0A=
> policer, so an error should be returned in case the same policer is=0A=
> shared between different rules.=0A=
=0A=
Each rule has a different policer assigned by HW. Do you mean the police.in=
dex should be checked here ?=0A=
=0A=
>=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0default:=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0NL_SET_ERR_MSG_MOD(extac=
k, "Unsupported action");=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pr_err("Unsupported acti=
on\n");=0A=
> > @@ -110,6 +117,17 @@ static int prestera_flower_parse(struct prestera_f=
low_block *block,=0A=
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EOPNOTSUPP;=0A=
> > =A0 =A0 =A0 =A0}=0A=
> >=0A=
> > + =A0 =A0 if (f->classid) {=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 int hw_tc =3D __tc_classid_to_hwtc(PRESTERA_H=
W_TC_NUM, f->classid);=0A=
> > +=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 if (hw_tc < 0) {=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 NL_SET_ERR_MSG_MOD(f->common.=
extack, "Unsupported HW TC");=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return hw_tc;=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 }=0A=
> > +=0A=
> > + =A0 =A0 =A0 =A0 =A0 =A0 prestera_acl_rule_hw_tc_set(rule, hw_tc);=0A=
> > + =A0 =A0 }=0A=
>=0A=
> Not sure what this is. Can you show a command line example of how this=0A=
> is used?=0A=
=0A=
This is HW traffic class used for packets that are trapped to CPU port. The=
 usage is as the following:=0A=
=0A=
tc qdisc add dev DEV clsact=0A=
tc filter add dev DEV ingress flower skip_sw dst_mac 00:AA:AA:AA:AA:00 hw_t=
c 1 action trap=0A=
=0A=
>=0A=
> What about visibility regarding number of packets that were dropped by=0A=
> the policer?=0A=
=0A=
This is not support at this moment by the driver, so it is always zero now.=
=0A=