[PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[Simon Horman]

This series enhances the TC policer action implementation to allow a
policer action instance to enforce a rate-limit based on
packets-per-second, configurable using a packet-per-second rate and burst
parameters.

In the hope of aiding review this is broken up into three patches.

* [PATCH 1/3] flow_offload: add support for packet-per-second policing

  Add support for this feature to the flow_offload API that is used to allow
  programming flows, including TC rules and their actions, into hardware.

* [PATCH 2/3] flow_offload: reject configuration of packet-per-second policing in offload drivers

  Teach all exiting users of the flow_offload API that allow offload of
  policer action instances to reject offload if packet-per-second rate
  limiting is configured: none support it at this time

* [PATCH 3/3] net/sched: act_police: add support for packet-per-second policing

  With the above ground-work in place add the new feature to the TC policer
  action itself

With the above in place the feature may be used.

As follow-ups we plan to provide:
* Corresponding updates to iproute2
* Corresponding self tests (which depend on the iproute2 changes)
* Hardware offload support for the NFP driver

Key changes since v2:
* Added patches 1 and 2, which makes adding patch 3 safe for existing
  hardware offload of the policer action
* Re-worked patch 3 so that a TC policer action instance may be configured
  for packet-per-second or byte-per-second rate limiting, but not both.
* Corrected kdoc usage

Baowen Zheng (2):
  flow_offload: reject configuration of packet-per-second policing in
    offload drivers
  net/sched: act_police: add support for packet-per-second policing

Xingfeng Hu (1):
  flow_offload: add support for packet-per-second policing

 drivers/net/dsa/sja1105/sja1105_flower.c      |  6 ++
 .../chelsio/cxgb4/cxgb4_tc_matchall.c         | 11 ++-
 .../net/ethernet/freescale/enetc/enetc_qos.c  |  5 ++
 .../net/ethernet/mellanox/mlx5/core/en_tc.c   |  4 +
 .../ethernet/mellanox/mlxsw/spectrum_flower.c |  5 ++
 drivers/net/ethernet/mscc/ocelot_flower.c     |  5 ++
 drivers/net/ethernet/mscc/ocelot_net.c        |  6 ++
 .../ethernet/netronome/nfp/flower/qos_conf.c  |  5 ++
 include/net/flow_offload.h                    |  2 +
 include/net/sch_generic.h                     | 14 ++++
 include/net/tc_act/tc_police.h                | 52 +++++++++++++
 include/uapi/linux/pkt_cls.h                  |  2 +
 net/sched/act_police.c                        | 59 +++++++++++++--
 net/sched/cls_api.c                           |  3 +
 net/sched/sch_generic.c                       | 75 ++++++++++++-------
 15 files changed, 221 insertions(+), 33 deletions(-)

-- 
2.20.1

[PATCH v3 net-next 1/3] flow_offload: add support for packet-per-second policing

[Simon Horman]

From: Xingfeng Hu <xingfeng.hu@corigine.com>

Allow flow_offload API to configure packet-per-second policing using rate
and burst parameters.

Dummy implementations of tcf_police_rate_pkt_ps() and
tcf_police_burst_pkt() are supplied which return 0, the unconfigured state.
This is to facilitate splitting the offload, driver, and TC code portion of
this feature into separate patches with the aim of providing a logical flow
for review. And the implementation of these helpers will be filled out by a
follow-up patch.

Signed-off-by: Xingfeng Hu <xingfeng.hu@corigine.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Louis Peens <louis.peens@netronome.com>
---
 include/net/flow_offload.h     |  2 ++
 include/net/tc_act/tc_police.h | 12 ++++++++++++
 net/sched/cls_api.c            |  3 +++
 3 files changed, 17 insertions(+)

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index e6bd8ebf9ac3..fde025c57b4f 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -234,6 +234,8 @@ struct flow_action_entry {
 			u32			index;
 			u32			burst;
 			u64			rate_bytes_ps;
+			u64			burst_pkt;
+			u64			rate_pkt_ps;
 			u32			mtu;
 		} police;
 		struct {				/* FLOW_ACTION_CT */
diff --git a/include/net/tc_act/tc_police.h b/include/net/tc_act/tc_police.h
index 6d1e26b709b5..ae117f7937d5 100644
--- a/include/net/tc_act/tc_police.h
+++ b/include/net/tc_act/tc_police.h
@@ -97,6 +97,18 @@ static inline u32 tcf_police_burst(const struct tc_action *act)
 	return burst;
 }
 
+static inline u64 tcf_police_rate_pkt_ps(const struct tc_action *act)
+{
+	/* Not implemented */
+	return 0;
+}
+
+static inline u32 tcf_police_burst_pkt(const struct tc_action *act)
+{
+	/* Not implemented */
+	return 0;
+}
+
 static inline u32 tcf_police_tcfp_mtu(const struct tc_action *act)
 {
 	struct tcf_police *police = to_police(act);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index e37556cc37ab..ca8e177bf31b 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3661,6 +3661,9 @@ int tc_setup_flow_action(struct flow_action *flow_action,
 			entry->police.burst = tcf_police_burst(act);
 			entry->police.rate_bytes_ps =
 				tcf_police_rate_bytes_ps(act);
+			entry->police.burst_pkt = tcf_police_burst_pkt(act);
+			entry->police.rate_pkt_ps =
+				tcf_police_rate_pkt_ps(act);
 			entry->police.mtu = tcf_police_tcfp_mtu(act);
 			entry->police.index = act->tcfa_index;
 		} else if (is_tcf_ct(act)) {
-- 
2.20.1

[PATCH v3 net-next 2/3] flow_offload: reject configuration of packet-per-second policing in offload drivers

[Simon Horman]

From: Baowen Zheng <baowen.zheng@corigine.com>

A follow-up patch will allow users to configures packet-per-second policing
in the software datapath. In preparation for this, teach all drivers that
support offload of the policer action to reject such configuration as
currently none of them support it.

Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Louis Peens <louis.peens@netronome.com>
---
 drivers/net/dsa/sja1105/sja1105_flower.c              |  6 ++++++
 .../net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c    | 11 ++++++++++-
 drivers/net/ethernet/freescale/enetc/enetc_qos.c      |  5 +++++
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c       |  4 ++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c |  5 +++++
 drivers/net/ethernet/mscc/ocelot_flower.c             |  5 +++++
 drivers/net/ethernet/mscc/ocelot_net.c                |  6 ++++++
 drivers/net/ethernet/netronome/nfp/flower/qos_conf.c  |  5 +++++
 8 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_flower.c b/drivers/net/dsa/sja1105/sja1105_flower.c
index 12e76020bea3..f78b767f86ee 100644
--- a/drivers/net/dsa/sja1105/sja1105_flower.c
+++ b/drivers/net/dsa/sja1105/sja1105_flower.c
@@ -322,6 +322,12 @@ 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.rate_pkt_ps) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "QoS offload not support packets per second");
+				goto out;
+			}
+
 			rc = sja1105_flower_policer(priv, port, extack, cookie,
 						    &key,
 						    act->police.rate_bytes_ps,
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
index 2e309f6673f7..28fd2de9e4cf 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
@@ -48,6 +48,11 @@ static int cxgb4_matchall_egress_validate(struct net_device *dev,
 	flow_action_for_each(i, entry, actions) {
 		switch (entry->id) {
 		case FLOW_ACTION_POLICE:
+			if (entry->police.rate_pkt_ps) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "QoS offload not support packets per second");
+				return -EOPNOTSUPP;
+			}
 			/* Convert bytes per second to bits per second */
 			if (entry->police.rate_bytes_ps * 8 > max_link_rate) {
 				NL_SET_ERR_MSG_MOD(extack,
@@ -145,7 +150,11 @@ static int cxgb4_matchall_alloc_tc(struct net_device *dev,
 	flow_action_for_each(i, entry, &cls->rule->action)
 		if (entry->id == FLOW_ACTION_POLICE)
 			break;
-
+	if (entry->police.rate_pkt_ps) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "QoS offload not support packets per second");
+		return -EOPNOTSUPP;
+	}
 	/* Convert from bytes per second to Kbps */
 	p.u.params.maxrate = div_u64(entry->police.rate_bytes_ps * 8, 1000);
 	p.u.params.channel = pi->tx_chan;
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index a9aee219fb58..cb7fa4bceaf2 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -1221,6 +1221,11 @@ static int enetc_psfp_parse_clsflower(struct enetc_ndev_priv *priv,
 
 	/* Flow meter and max frame size */
 	if (entryp) {
+		if (entryp->police.rate_pkt_ps) {
+			NL_SET_ERR_MSG_MOD(extack, "QoS offload not support packets per second");
+			err = -EOPNOTSUPP;
+			goto free_sfi;
+		}
 		if (entryp->police.burst) {
 			fmi = kzalloc(sizeof(*fmi), GFP_KERNEL);
 			if (!fmi) {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index dc126389291d..1fe745653a53 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -4480,6 +4480,10 @@ static int scan_tc_matchall_fdb_actions(struct mlx5e_priv *priv,
 	flow_action_for_each(i, act, flow_action) {
 		switch (act->id) {
 		case FLOW_ACTION_POLICE:
+			if (act->police.rate_pkt_ps) {
+				NL_SET_ERR_MSG_MOD(extack, "QoS offload not support packets per second");
+				return -EOPNOTSUPP;
+			}
 			err = apply_police_params(priv, act->police.rate_bytes_ps, extack);
 			if (err)
 				return err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 41855e58564b..ea637fa552f5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -190,6 +190,11 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
 				return -EOPNOTSUPP;
 			}
 
+			if (act->police.rate_pkt_ps) {
+				NL_SET_ERR_MSG_MOD(extack, "QoS offload not support packets per second");
+				return -EOPNOTSUPP;
+			}
+
 			/* The kernel might adjust the requested burst size so
 			 * that it is not exactly a power of two. Re-adjust it
 			 * here since the hardware only supports burst sizes
diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c
index a41b458b1b3e..8b843d3c9189 100644
--- a/drivers/net/ethernet/mscc/ocelot_flower.c
+++ b/drivers/net/ethernet/mscc/ocelot_flower.c
@@ -220,6 +220,11 @@ static int ocelot_flower_parse_action(struct ocelot *ocelot, int port,
 						   "Last action must be GOTO");
 				return -EOPNOTSUPP;
 			}
+			if (a->police.rate_pkt_ps) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "QoS offload not support packets per second");
+				return -EOPNOTSUPP;
+			}
 			filter->action.police_ena = true;
 			rate = a->police.rate_bytes_ps;
 			filter->action.pol.rate = div_u64(rate, 1000) * 8;
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 12cb6867a2d0..c08164cd88f4 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -251,6 +251,12 @@ static int ocelot_setup_tc_cls_matchall(struct ocelot_port_private *priv,
 			return -EEXIST;
 		}
 
+		if (action->police.rate_pkt_ps) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "QoS offload not support packets per second");
+			return -EOPNOTSUPP;
+		}
+
 		pol.rate = (u32)div_u64(action->police.rate_bytes_ps, 1000) * 8;
 		pol.burst = action->police.burst;
 
diff --git a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
index d4ce8f9ef3cc..88bea6ad59bc 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
@@ -104,6 +104,11 @@ nfp_flower_install_rate_limiter(struct nfp_app *app, struct net_device *netdev,
 		return -EOPNOTSUPP;
 	}
 
+	if (action->police.rate_pkt_ps) {
+		NL_SET_ERR_MSG_MOD(extack, "unsupported offload: qos rate limit offload not support packets per second");
+		return -EOPNOTSUPP;
+	}
+
 	rate = action->police.rate_bytes_ps;
 	burst = action->police.burst;
 	netdev_port_id = nfp_repr_get_port_id(netdev);
-- 
2.20.1

[PATCH v3 net-next 3/3] net/sched: act_police: add support for packet-per-second policing

[Simon Horman]

From: Baowen Zheng <baowen.zheng@corigine.com>

Allow a policer action to enforce a rate-limit based on packets-per-second,
configurable using a packet-per-second rate and burst parameters.

e.g.
tc filter add dev tap1 parent ffff: u32 match \
        u32 0 0 police pkts_rate 3000 pkts_burst 1000

Testing was unable to uncover a performance impact of this change on
existing features.

Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Louis Peens <louis.peens@netronome.com>
---
 include/net/sch_generic.h      | 14 +++++++
 include/net/tc_act/tc_police.h | 48 ++++++++++++++++++++--
 include/uapi/linux/pkt_cls.h   |  2 +
 net/sched/act_police.c         | 59 ++++++++++++++++++++++----
 net/sched/sch_generic.c        | 75 ++++++++++++++++++++++------------
 5 files changed, 162 insertions(+), 36 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 2d6eb60c58c8..f7a6e14491fb 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -1242,6 +1242,20 @@ static inline void psched_ratecfg_getrate(struct tc_ratespec *res,
 	res->linklayer = (r->linklayer & TC_LINKLAYER_MASK);
 }
 
+struct psched_pktrate {
+	u64	rate_pkts_ps; /* packets per second */
+	u32	mult;
+	u8	shift;
+};
+
+static inline u64 psched_pkt2t_ns(const struct psched_pktrate *r,
+				  unsigned int pkt_num)
+{
+	return ((u64)pkt_num * r->mult) >> r->shift;
+}
+
+void psched_ppscfg_precompute(struct psched_pktrate *r, u64 pktrate64);
+
 /* Mini Qdisc serves for specific needs of ingress/clsact Qdisc.
  * The fast path only needs to access filter list and to update stats
  */
diff --git a/include/net/tc_act/tc_police.h b/include/net/tc_act/tc_police.h
index ae117f7937d5..72649512dcdd 100644
--- a/include/net/tc_act/tc_police.h
+++ b/include/net/tc_act/tc_police.h
@@ -10,10 +10,13 @@ struct tcf_police_params {
 	s64			tcfp_burst;
 	u32			tcfp_mtu;
 	s64			tcfp_mtu_ptoks;
+	s64			tcfp_pkt_burst;
 	struct psched_ratecfg	rate;
 	bool			rate_present;
 	struct psched_ratecfg	peak;
 	bool			peak_present;
+	struct psched_pktrate	ppsrate;
+	bool			pps_present;
 	struct rcu_head rcu;
 };
 
@@ -24,6 +27,7 @@ struct tcf_police {
 	spinlock_t		tcfp_lock ____cacheline_aligned_in_smp;
 	s64			tcfp_toks;
 	s64			tcfp_ptoks;
+	s64			tcfp_pkttoks;
 	s64			tcfp_t_c;
 };
 
@@ -99,14 +103,50 @@ static inline u32 tcf_police_burst(const struct tc_action *act)
 
 static inline u64 tcf_police_rate_pkt_ps(const struct tc_action *act)
 {
-	/* Not implemented */
-	return 0;
+	struct tcf_police *police = to_police(act);
+	struct tcf_police_params *params;
+
+	params = rcu_dereference_protected(police->params,
+					   lockdep_is_held(&police->tcf_lock));
+	return params->ppsrate.rate_pkts_ps;
 }
 
 static inline u32 tcf_police_burst_pkt(const struct tc_action *act)
 {
-	/* Not implemented */
-	return 0;
+	struct tcf_police *police = to_police(act);
+	struct tcf_police_params *params;
+	u32 burst;
+
+	params = rcu_dereference_protected(police->params,
+					   lockdep_is_held(&police->tcf_lock));
+
+	/*
+	 *  "rate" pkts     "burst" nanoseconds
+	 *  ------------ *  -------------------
+	 *    1 second          2^6 ticks
+	 *
+	 * ------------------------------------
+	 *        NSEC_PER_SEC nanoseconds
+	 *        ------------------------
+	 *              2^6 ticks
+	 *
+	 *    "rate" pkts    "burst" nanoseconds            2^6 ticks
+	 *  = ------------ * ------------------- * ------------------------
+	 *      1 second          2^6 ticks        NSEC_PER_SEC nanoseconds
+	 *
+	 *   "rate" * "burst"
+	 * = ---------------- pkts/nanosecond
+	 *    NSEC_PER_SEC^2
+	 *
+	 *
+	 *   "rate" * "burst"
+	 * = ---------------- pkts/second
+	 *     NSEC_PER_SEC
+	 */
+	burst = div_u64(params->tcfp_pkt_burst * params->ppsrate.rate_pkts_ps,
+			NSEC_PER_SEC);
+
+	return burst;
 }
 
 static inline u32 tcf_police_tcfp_mtu(const struct tc_action *act)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 7ea59cfe1fa7..025c40fef93d 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -190,6 +190,8 @@ enum {
 	TCA_POLICE_PAD,
 	TCA_POLICE_RATE64,
 	TCA_POLICE_PEAKRATE64,
+	TCA_POLICE_PKTRATE64,
+	TCA_POLICE_PKTBURST64,
 	__TCA_POLICE_MAX
 #define TCA_POLICE_RESULT TCA_POLICE_RESULT
 };
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 8d8452b1cdd4..0fab8de176d2 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -42,6 +42,8 @@ static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = {
 	[TCA_POLICE_RESULT]	= { .type = NLA_U32 },
 	[TCA_POLICE_RATE64]     = { .type = NLA_U64 },
 	[TCA_POLICE_PEAKRATE64] = { .type = NLA_U64 },
+	[TCA_POLICE_PKTRATE64]  = { .type = NLA_U64, .min = 1 },
+	[TCA_POLICE_PKTBURST64] = { .type = NLA_U64, .min = 1 },
 };
 
 static int tcf_police_init(struct net *net, struct nlattr *nla,
@@ -61,6 +63,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	bool exists = false;
 	u32 index;
 	u64 rate64, prate64;
+	u64 pps, ppsburst;
 
 	if (nla == NULL)
 		return -EINVAL;
@@ -142,6 +145,21 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 		}
 	}
 
+	if ((tb[TCA_POLICE_PKTRATE64] && !tb[TCA_POLICE_PKTBURST64]) ||
+	    (!tb[TCA_POLICE_PKTRATE64] && tb[TCA_POLICE_PKTBURST64])) {
+		NL_SET_ERR_MSG(extack,
+			       "Both or neither packet-per-second burst and rate must be provided");
+		err = -EINVAL;
+		goto failure;
+	}
+
+	if (tb[TCA_POLICE_PKTRATE64] && R_tab) {
+		NL_SET_ERR_MSG(extack,
+			       "packet-per-second and byte-per-second rate limits not allowed in same action");
+		err = -EINVAL;
+		goto failure;
+	}
+
 	new = kzalloc(sizeof(*new), GFP_KERNEL);
 	if (unlikely(!new)) {
 		err = -ENOMEM;
@@ -183,6 +201,14 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	if (tb[TCA_POLICE_AVRATE])
 		new->tcfp_ewma_rate = nla_get_u32(tb[TCA_POLICE_AVRATE]);
 
+	if (tb[TCA_POLICE_PKTRATE64]) {
+		pps = nla_get_u64(tb[TCA_POLICE_PKTRATE64]);
+		ppsburst = nla_get_u64(tb[TCA_POLICE_PKTBURST64]);
+		new->pps_present = true;
+		new->tcfp_pkt_burst = PSCHED_TICKS2NS(ppsburst);
+		psched_ppscfg_precompute(&new->ppsrate, pps);
+	}
+
 	spin_lock_bh(&police->tcf_lock);
 	spin_lock_bh(&police->tcfp_lock);
 	police->tcfp_t_c = ktime_get_ns();
@@ -217,8 +243,8 @@ static int tcf_police_act(struct sk_buff *skb, const struct tc_action *a,
 			  struct tcf_result *res)
 {
 	struct tcf_police *police = to_police(a);
+	s64 now, toks, ppstoks = 0, ptoks = 0;
 	struct tcf_police_params *p;
-	s64 now, toks, ptoks = 0;
 	int ret;
 
 	tcf_lastuse_update(&police->tcf_tm);
@@ -236,7 +262,7 @@ static int tcf_police_act(struct sk_buff *skb, const struct tc_action *a,
 	}
 
 	if (qdisc_pkt_len(skb) <= p->tcfp_mtu) {
-		if (!p->rate_present) {
+		if (!p->rate_present && !p->pps_present) {
 			ret = p->tcfp_result;
 			goto end;
 		}
@@ -251,14 +277,23 @@ static int tcf_police_act(struct sk_buff *skb, const struct tc_action *a,
 			ptoks -= (s64)psched_l2t_ns(&p->peak,
 						    qdisc_pkt_len(skb));
 		}
-		toks += police->tcfp_toks;
-		if (toks > p->tcfp_burst)
-			toks = p->tcfp_burst;
-		toks -= (s64)psched_l2t_ns(&p->rate, qdisc_pkt_len(skb));
-		if ((toks|ptoks) >= 0) {
+		if (p->rate_present) {
+			toks += police->tcfp_toks;
+			if (toks > p->tcfp_burst)
+				toks = p->tcfp_burst;
+			toks -= (s64)psched_l2t_ns(&p->rate, qdisc_pkt_len(skb));
+		} else if (p->pps_present) {
+			ppstoks = min_t(s64, now - police->tcfp_t_c, p->tcfp_pkt_burst);
+			ppstoks += police->tcfp_pkttoks;
+			if (ppstoks > p->tcfp_pkt_burst)
+				ppstoks = p->tcfp_pkt_burst;
+			ppstoks -= (s64)psched_pkt2t_ns(&p->ppsrate, 1);
+		}
+		if ((toks | ptoks | ppstoks) >= 0) {
 			police->tcfp_t_c = now;
 			police->tcfp_toks = toks;
 			police->tcfp_ptoks = ptoks;
+			police->tcfp_pkttoks = ppstoks;
 			spin_unlock_bh(&police->tcfp_lock);
 			ret = p->tcfp_result;
 			goto inc_drops;
@@ -331,6 +366,16 @@ static int tcf_police_dump(struct sk_buff *skb, struct tc_action *a,
 				      TCA_POLICE_PAD))
 			goto nla_put_failure;
 	}
+	if (p->pps_present) {
+		if (nla_put_u64_64bit(skb, TCA_POLICE_PKTRATE64,
+				      police->params->ppsrate.rate_pkts_ps,
+				      TCA_POLICE_PAD))
+			goto nla_put_failure;
+		if (nla_put_u64_64bit(skb, TCA_POLICE_PKTBURST64,
+				      PSCHED_NS2TICKS(p->tcfp_pkt_burst),
+				      TCA_POLICE_PAD))
+			goto nla_put_failure;
+	}
 	if (nla_put(skb, TCA_POLICE_TBF, sizeof(opt), &opt))
 		goto nla_put_failure;
 	if (p->tcfp_result &&
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 49eae93d1489..44991ea726fc 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -1325,6 +1325,48 @@ void dev_shutdown(struct net_device *dev)
 	WARN_ON(timer_pending(&dev->watchdog_timer));
 }
 
+/**
+ * psched_ratecfg_precompute__() - Pre-compute values for reciprocal division
+ * @rate:   Rate to compute reciprocal division values of
+ * @mult:   Multiplier for reciprocal division
+ * @shift:  Shift for reciprocal division
+ *
+ * The multiplier and shift for reciprocal division by rate are stored
+ * in mult and shift.
+ *
+ * The deal here is to replace a divide by a reciprocal one
+ * in fast path (a reciprocal divide is a multiply and a shift)
+ *
+ * Normal formula would be :
+ *  time_in_ns = (NSEC_PER_SEC * len) / rate_bps
+ *
+ * We compute mult/shift to use instead :
+ *  time_in_ns = (len * mult) >> shift;
+ *
+ * We try to get the highest possible mult value for accuracy,
+ * but have to make sure no overflows will ever happen.
+ *
+ * reciprocal_value() is not used here it doesn't handle 64-bit values.
+ */
+static void psched_ratecfg_precompute__(u64 rate, u32 *mult, u8 *shift)
+{
+	u64 factor = NSEC_PER_SEC;
+
+	*mult = 1;
+	*shift = 0;
+
+	if (rate <= 0)
+		return;
+
+	for (;;) {
+		*mult = div64_u64(factor, rate);
+		if (*mult & (1U << 31) || factor & (1ULL << 63))
+			break;
+		factor <<= 1;
+		(*shift)++;
+	}
+}
+
 void psched_ratecfg_precompute(struct psched_ratecfg *r,
 			       const struct tc_ratespec *conf,
 			       u64 rate64)
@@ -1333,34 +1375,17 @@ void psched_ratecfg_precompute(struct psched_ratecfg *r,
 	r->overhead = conf->overhead;
 	r->rate_bytes_ps = max_t(u64, conf->rate, rate64);
 	r->linklayer = (conf->linklayer & TC_LINKLAYER_MASK);
-	r->mult = 1;
-	/*
-	 * The deal here is to replace a divide by a reciprocal one
-	 * in fast path (a reciprocal divide is a multiply and a shift)
-	 *
-	 * Normal formula would be :
-	 *  time_in_ns = (NSEC_PER_SEC * len) / rate_bps
-	 *
-	 * We compute mult/shift to use instead :
-	 *  time_in_ns = (len * mult) >> shift;
-	 *
-	 * We try to get the highest possible mult value for accuracy,
-	 * but have to make sure no overflows will ever happen.
-	 */
-	if (r->rate_bytes_ps > 0) {
-		u64 factor = NSEC_PER_SEC;
-
-		for (;;) {
-			r->mult = div64_u64(factor, r->rate_bytes_ps);
-			if (r->mult & (1U << 31) || factor & (1ULL << 63))
-				break;
-			factor <<= 1;
-			r->shift++;
-		}
-	}
+	psched_ratecfg_precompute__(r->rate_bytes_ps, &r->mult, &r->shift);
 }
 EXPORT_SYMBOL(psched_ratecfg_precompute);
 
+void psched_ppscfg_precompute(struct psched_pktrate *r, u64 pktrate64)
+{
+	r->rate_pkts_ps = pktrate64;
+	psched_ratecfg_precompute__(r->rate_pkts_ps, &r->mult, &r->shift);
+}
+EXPORT_SYMBOL(psched_ppscfg_precompute);
+
 static void mini_qdisc_rcu_func(struct rcu_head *head)
 {
 }
-- 
2.20.1

Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[Simon Horman]

On Fri, Mar 12, 2021 at 03:08:28PM +0100, Simon Horman wrote:
> This series enhances the TC policer action implementation to allow a
> policer action instance to enforce a rate-limit based on
> packets-per-second, configurable using a packet-per-second rate and burst
> parameters.

...

Sorry, I missed CCing a number of interested parties when posting
this patch-set. I've added them to this email.

Ref: https://lore.kernel.org/netdev/20210312140831.23346-1-simon.horman@netronome.com/

Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[Jamal Hadi Salim]

On 2021-03-12 9:22 a.m., Simon Horman wrote:
> On Fri, Mar 12, 2021 at 03:08:28PM +0100, Simon Horman wrote:
>> This series enhances the TC policer action implementation to allow a
>> policer action instance to enforce a rate-limit based on
>> packets-per-second, configurable using a packet-per-second rate and burst
>> parameters.
> 
> ...
> 
> Sorry, I missed CCing a number of interested parties when posting
> this patch-set. I've added them to this email.
> 
> Ref: https://lore.kernel.org/netdev/20210312140831.23346-1-simon.horman@netronome.com/
> 

For 2/3:
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>

If you submit one or two testcases to tools/testing/selftests/tc-testing
you'll be both a hero and someone committing an act of kindness.

cheers,
jamal

Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[Simon Horman]

On Tue, Mar 16, 2021 at 09:53:49AM -0400, Jamal Hadi Salim wrote:
> On 2021-03-12 9:22 a.m., Simon Horman wrote:
> > On Fri, Mar 12, 2021 at 03:08:28PM +0100, Simon Horman wrote:
> > > This series enhances the TC policer action implementation to allow a
> > > policer action instance to enforce a rate-limit based on
> > > packets-per-second, configurable using a packet-per-second rate and burst
> > > parameters.
> > 
> > ...
> > 
> > Sorry, I missed CCing a number of interested parties when posting
> > this patch-set. I've added them to this email.
> > 
> > Ref: https://lore.kernel.org/netdev/20210312140831.23346-1-simon.horman@netronome.com/
> > 
> 
> For 2/3:
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> If you submit one or two testcases to tools/testing/selftests/tc-testing
> you'll be both a hero and someone committing an act of kindness.

Thanks Jamal,

my team are heroes and are working on testcases.

Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 12 Mar 2021 15:08:28 +0100 you wrote:
> This series enhances the TC policer action implementation to allow a
> policer action instance to enforce a rate-limit based on
> packets-per-second, configurable using a packet-per-second rate and burst
> parameters.
> 
> In the hope of aiding review this is broken up into three patches.
> 
> [...]

Here is the summary with links:
  - [v3,net-next,1/3] flow_offload: add support for packet-per-second policing
    https://git.kernel.org/netdev/net-next/c/25660156f4cc
  - [v3,net-next,2/3] flow_offload: reject configuration of packet-per-second policing in offload drivers
    https://git.kernel.org/netdev/net-next/c/6a56e19902af
  - [v3,net-next,3/3] net/sched: act_police: add support for packet-per-second policing
    https://git.kernel.org/netdev/net-next/c/2ffe0395288a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[Ido Schimmel]

On Fri, Mar 12, 2021 at 03:08:28PM +0100, Simon Horman wrote:
> This series enhances the TC policer action implementation to allow a
> policer action instance to enforce a rate-limit based on
> packets-per-second, configurable using a packet-per-second rate and burst
> parameters.
> 
> In the hope of aiding review this is broken up into three patches.
> 
> * [PATCH 1/3] flow_offload: add support for packet-per-second policing
> 
>   Add support for this feature to the flow_offload API that is used to allow
>   programming flows, including TC rules and their actions, into hardware.
> 
> * [PATCH 2/3] flow_offload: reject configuration of packet-per-second policing in offload drivers
> 
>   Teach all exiting users of the flow_offload API that allow offload of
>   policer action instances to reject offload if packet-per-second rate
>   limiting is configured: none support it at this time
> 
> * [PATCH 3/3] net/sched: act_police: add support for packet-per-second policing
> 
>   With the above ground-work in place add the new feature to the TC policer
>   action itself
> 
> With the above in place the feature may be used.
> 
> As follow-ups we plan to provide:
> * Corresponding updates to iproute2
> * Corresponding self tests (which depend on the iproute2 changes)

I was about to ask :)

FYI, there is this selftest:
tools/testing/selftests/net/forwarding/tc_police.sh

Which can be extended to also test packet rate policing

> * Hardware offload support for the NFP driver
> 
> Key changes since v2:
> * Added patches 1 and 2, which makes adding patch 3 safe for existing
>   hardware offload of the policer action
> * Re-worked patch 3 so that a TC policer action instance may be configured
>   for packet-per-second or byte-per-second rate limiting, but not both.
> * Corrected kdoc usage

Thanks!

Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[Simon Horman]

On Sun, Mar 14, 2021 at 10:17:37AM +0200, Ido Schimmel wrote:
> On Fri, Mar 12, 2021 at 03:08:28PM +0100, Simon Horman wrote:
> > This series enhances the TC policer action implementation to allow a
> > policer action instance to enforce a rate-limit based on
> > packets-per-second, configurable using a packet-per-second rate and burst
> > parameters.
> > 
> > In the hope of aiding review this is broken up into three patches.
> > 
> > * [PATCH 1/3] flow_offload: add support for packet-per-second policing
> > 
> >   Add support for this feature to the flow_offload API that is used to allow
> >   programming flows, including TC rules and their actions, into hardware.
> > 
> > * [PATCH 2/3] flow_offload: reject configuration of packet-per-second policing in offload drivers
> > 
> >   Teach all exiting users of the flow_offload API that allow offload of
> >   policer action instances to reject offload if packet-per-second rate
> >   limiting is configured: none support it at this time
> > 
> > * [PATCH 3/3] net/sched: act_police: add support for packet-per-second policing
> > 
> >   With the above ground-work in place add the new feature to the TC policer
> >   action itself
> > 
> > With the above in place the feature may be used.
> > 
> > As follow-ups we plan to provide:
> > * Corresponding updates to iproute2
> > * Corresponding self tests (which depend on the iproute2 changes)
> 
> I was about to ask :)
> 
> FYI, there is this selftest:
> tools/testing/selftests/net/forwarding/tc_police.sh
> 
> Which can be extended to also test packet rate policing

Thanks Ido,

The approach we have taken is to add tests to
tools/testing/selftests/tc-testing/tc-tests/actions/police.json

Do you think adding a test to tc_police.sh is also worthwhile? Or should be
done instead of updating police.json?

Lastly, my assumption is that the tests should be posted once iproute2
changes they depend on have been accepted. Is this correct in your opinion?

In any case, I'll get moving on posting the iproute2 changes.

> > * Hardware offload support for the NFP driver
> > 
> > Key changes since v2:
> > * Added patches 1 and 2, which makes adding patch 3 safe for existing
> >   hardware offload of the policer action
> > * Re-worked patch 3 so that a TC policer action instance may be configured
> >   for packet-per-second or byte-per-second rate limiting, but not both.
> > * Corrected kdoc usage
> 
> Thanks!

Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[Ido Schimmel]

Sorry for the delay. Was AFK yesterday

On Mon, Mar 15, 2021 at 03:41:56PM +0100, Simon Horman wrote:
> On Sun, Mar 14, 2021 at 10:17:37AM +0200, Ido Schimmel wrote:
> > On Fri, Mar 12, 2021 at 03:08:28PM +0100, Simon Horman wrote:
> > > This series enhances the TC policer action implementation to allow a
> > > policer action instance to enforce a rate-limit based on
> > > packets-per-second, configurable using a packet-per-second rate and burst
> > > parameters.
> > > 
> > > In the hope of aiding review this is broken up into three patches.
> > > 
> > > * [PATCH 1/3] flow_offload: add support for packet-per-second policing
> > > 
> > >   Add support for this feature to the flow_offload API that is used to allow
> > >   programming flows, including TC rules and their actions, into hardware.
> > > 
> > > * [PATCH 2/3] flow_offload: reject configuration of packet-per-second policing in offload drivers
> > > 
> > >   Teach all exiting users of the flow_offload API that allow offload of
> > >   policer action instances to reject offload if packet-per-second rate
> > >   limiting is configured: none support it at this time
> > > 
> > > * [PATCH 3/3] net/sched: act_police: add support for packet-per-second policing
> > > 
> > >   With the above ground-work in place add the new feature to the TC policer
> > >   action itself
> > > 
> > > With the above in place the feature may be used.
> > > 
> > > As follow-ups we plan to provide:
> > > * Corresponding updates to iproute2
> > > * Corresponding self tests (which depend on the iproute2 changes)
> > 
> > I was about to ask :)
> > 
> > FYI, there is this selftest:
> > tools/testing/selftests/net/forwarding/tc_police.sh
> > 
> > Which can be extended to also test packet rate policing
> 
> Thanks Ido,
> 
> The approach we have taken is to add tests to
> tools/testing/selftests/tc-testing/tc-tests/actions/police.json
> 
> Do you think adding a test to tc_police.sh is also worthwhile? Or should be
> done instead of updating police.json?

IIUC, police.json only performs configuration tests. tc_police.sh on the
other hand, configures a topology, injects traffic and validates that
the bandwidth after the police action is according to user
configuration. You can test the software data path by using veth pairs
or the hardware data path by using physical ports looped to each other.

So I think that extending both tests is worthwhile.

> 
> Lastly, my assumption is that the tests should be posted once iproute2
> changes they depend on have been accepted. Is this correct in your opinion?

Personally, I prefer selftests to be posted together with the
implementation, regardless if they depend on new iproute2 functionality.
In the unlikely case that the kernel patches were accepted, but changes
were requested for the command line interface, you can always patch the
selftests later.

Jakub recently added this section:
https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#how-do-i-post-corresponding-changes-to-user-space-components

He writes "User space code exercising kernel features should be posted
alongside kernel patches."

And you can see that in the example the last patch is a selftest:

```
[PATCH net-next 0/3] net: some feature cover letter
 └─ [PATCH net-next 1/3] net: some feature prep
 └─ [PATCH net-next 2/3] net: some feature do it
 └─ [PATCH net-next 3/3] selftest: net: some feature

[PATCH iproute2-next] ip: add support for some feature
```

> 
> In any case, I'll get moving on posting the iproute2 changes.

Thanks!

> 
> > > * Hardware offload support for the NFP driver
> > > 
> > > Key changes since v2:
> > > * Added patches 1 and 2, which makes adding patch 3 safe for existing
> > >   hardware offload of the policer action
> > > * Re-worked patch 3 so that a TC policer action instance may be configured
> > >   for packet-per-second or byte-per-second rate limiting, but not both.
> > > * Corrected kdoc usage
> > 
> > Thanks!

Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

[Simon Horman]

Hi Ido,

On Tue, Mar 16, 2021 at 10:35:35AM +0200, Ido Schimmel wrote:
> Sorry for the delay. Was AFK yesterday

No problem at all.

...

> > > > As follow-ups we plan to provide:
> > > > * Corresponding updates to iproute2
> > > > * Corresponding self tests (which depend on the iproute2 changes)
> > > 
> > > I was about to ask :)
> > > 
> > > FYI, there is this selftest:
> > > tools/testing/selftests/net/forwarding/tc_police.sh
> > > 
> > > Which can be extended to also test packet rate policing
> > 
> > Thanks Ido,
> > 
> > The approach we have taken is to add tests to
> > tools/testing/selftests/tc-testing/tc-tests/actions/police.json
> > 
> > Do you think adding a test to tc_police.sh is also worthwhile? Or should be
> > done instead of updating police.json?
> 
> IIUC, police.json only performs configuration tests. tc_police.sh on the
> other hand, configures a topology, injects traffic and validates that
> the bandwidth after the police action is according to user
> configuration. You can test the software data path by using veth pairs
> or the hardware data path by using physical ports looped to each other.
> 
> So I think that extending both tests is worthwhile.

Thanks, we'll see about making it so.

> > Lastly, my assumption is that the tests should be posted once iproute2
> > changes they depend on have been accepted. Is this correct in your opinion?
> 
> Personally, I prefer selftests to be posted together with the
> implementation, regardless if they depend on new iproute2 functionality.
> In the unlikely case that the kernel patches were accepted, but changes
> were requested for the command line interface, you can always patch the
> selftests later.
> 
> Jakub recently added this section:
> https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#how-do-i-post-corresponding-changes-to-user-space-components
> 
> He writes "User space code exercising kernel features should be posted
> alongside kernel patches."
> 
> And you can see that in the example the last patch is a selftest:
> 
> ```
> [PATCH net-next 0/3] net: some feature cover letter
>  └─ [PATCH net-next 1/3] net: some feature prep
>  └─ [PATCH net-next 2/3] net: some feature do it
>  └─ [PATCH net-next 3/3] selftest: net: some feature
> 
> [PATCH iproute2-next] ip: add support for some feature
> ```

Thanks, we'll try to follow this in our next feature submission.

...