On Thu, Feb 17, 2022 at 08:28:03AM +0000, Jianbo Liu wrote:
> As more police parameters are passed to flow_offload, driver can check
> them to make sure hardware handles packets in the way indicated by tc.
> The conform-exceed control should be drop/pipe or drop/ok. Besides,
> for drop/ok, the police should be the last action. As hardware can't
> configure peakrate/avrate/overhead, offload should not be supported if
> any of them is configured.
>
> Signed-off-by: Jianbo Liu <jianbol@xxxxxxxxxx>
> Reviewed-by: Roi Dayan <roid@xxxxxxxxxx>
> Reviewed-by: Ido Schimmel <idosch@xxxxxxxxxx>
> ---
Tested-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
But could we cut down on line length a little? Example for sja1105
(messages were also shortened):
diff --git a/drivers/net/dsa/sja1105/sja1105_flower.c b/drivers/net/dsa/sja1105/sja1105_flower.c
index 8a14df8cf91e..54a16369a39e 100644
--- a/drivers/net/dsa/sja1105/sja1105_flower.c
+++ b/drivers/net/dsa/sja1105/sja1105_flower.c
@@ -300,6 +300,46 @@ static int sja1105_flower_parse_key(struct sja1105_private *priv,
return -EOPNOTSUPP;
}
+static int sja1105_policer_validate(const struct flow_action *action,
+ const struct flow_action_entry *act,
+ struct netlink_ext_ack *extack)
+{
+ if (act->police.exceed.act_id != FLOW_ACTION_DROP) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when exceed action is not drop");
+ return -EOPNOTSUPP;
+ }
+
+ if (act->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+ act->police.notexceed.act_id != FLOW_ACTION_ACCEPT) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when conform action is not pipe or ok");
+ return -EOPNOTSUPP;
+ }
+
+ if (act->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+ !flow_action_is_last_entry(action, act)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when conform action is ok, but action is not last");
+ return -EOPNOTSUPP;
+ }
+
+ if (act->police.peakrate_bytes_ps ||
+ act->police.avrate || act->police.overhead) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when peakrate/avrate/overhead is configured");
+ return -EOPNOTSUPP;
+ }
+
+ if (act->police.rate_pkt_ps) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "QoS offload not support packets per second");
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
int sja1105_cls_flower_add(struct dsa_switch *ds, int port,
struct flow_cls_offload *cls, bool ingress)
{
@@ -321,39 +361,10 @@ int sja1105_cls_flower_add(struct dsa_switch *ds, int port,
flow_action_for_each(i, act, &rule->action) {
switch (act->id) {
case FLOW_ACTION_POLICE:
- if (act->police.exceed.act_id != FLOW_ACTION_DROP) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the exceed action is not drop");
- return -EOPNOTSUPP;
- }
-
- if (act->police.notexceed.act_id != FLOW_ACTION_PIPE &&
- act->police.notexceed.act_id != FLOW_ACTION_ACCEPT) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the conform action is not pipe or ok");
- return -EOPNOTSUPP;
- }
-
- if (act->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
- !flow_action_is_last_entry(&rule->action, act)) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when the conform action is ok, but police action is not last");
- return -EOPNOTSUPP;
- }
-
- if (act->police.peakrate_bytes_ps ||
- act->police.avrate || act->police.overhead) {
- NL_SET_ERR_MSG_MOD(extack,
- "Police offload is not supported when peakrate/avrate/overhead is configured");
- return -EOPNOTSUPP;
- }
-
- if (act->police.rate_pkt_ps) {
- NL_SET_ERR_MSG_MOD(extack,
- "QoS offload not support packets per second");
- rc = -EOPNOTSUPP;
+ rc = sja1105_policer_validate(&rule->action, act,
+ extack);
+ if (rc)
goto out;
- }
rc = sja1105_flower_policer(priv, port, extack, cookie,
&key,
Also, if you create a "validate" function for every driver, you'll
remove code duplication for those drivers that support both matchall and
flower policers.
