Messages in this thread | | | From | "Volodymyr Mytnyk [C]" <> | Subject | Re: [PATCH net-next v2 4/4] net: marvell: prestera: Offload FLOW_ACTION_POLICE | Date | Tue, 3 Aug 2021 16:19:03 +0000 |
| |
Hi Ido,
Thanks for the review. Pls see the comments inline.
> On Mon, Aug 02, 2021 at 05:08:49PM +0300, Vadym Kochan wrote: > > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c > > index e571ba09ec08..76f30856ac98 100644 > > --- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c > > +++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c > > @@ -5,6 +5,8 @@ > > #include "prestera_acl.h" > > #include "prestera_flower.h" > > > > +#define PRESTERA_HW_TC_NUM 8 > > + > > static int prestera_flower_parse_actions(struct prestera_flow_block *block, > > struct prestera_acl_rule *rule, > > struct flow_action *flow_action, > > @@ -30,6 +32,11 @@ static int prestera_flower_parse_actions(struct prestera_flow_block *block, > > case FLOW_ACTION_TRAP: > > a_entry.id = PRESTERA_ACL_RULE_ACTION_TRAP; > > break; > > + case FLOW_ACTION_POLICE: > > + a_entry.id = PRESTERA_ACL_RULE_ACTION_POLICE; > > + a_entry.police.rate = act->police.rate_bytes_ps; > > + a_entry.police.burst = act->police.burst; > > If packet rate based policing is not supported, an error should be > returned here with extack.
Agree, it makes sense.
> > It seems the implementation assumes that each rule has a different > policer, so an error should be returned in case the same policer is > shared between different rules.
Each rule has a different policer assigned by HW. Do you mean the police.index should be checked here ?
> > > + break; > > default: > > NL_SET_ERR_MSG_MOD(extack, "Unsupported action"); > > pr_err("Unsupported action\n"); > > @@ -110,6 +117,17 @@ static int prestera_flower_parse(struct prestera_flow_block *block, > > return -EOPNOTSUPP; > > } > > > > + if (f->classid) { > > + int hw_tc = __tc_classid_to_hwtc(PRESTERA_HW_TC_NUM, f->classid); > > + > > + if (hw_tc < 0) { > > + NL_SET_ERR_MSG_MOD(f->common.extack, "Unsupported HW TC"); > > + return hw_tc; > > + } > > + > > + prestera_acl_rule_hw_tc_set(rule, hw_tc); > > + } > > Not sure what this is. Can you show a command line example of how this > is used?
This is HW traffic class used for packets that are trapped to CPU port. The usage is as the following:
tc qdisc add dev DEV clsact tc filter add dev DEV ingress flower skip_sw dst_mac 00:AA:AA:AA:AA:00 hw_tc 1 action trap
> > What about visibility regarding number of packets that were dropped by > the policer?
This is not support at this moment by the driver, so it is always zero now.
| |