diff mbox series

[net-next,v2,2/2] flow_offload: reject offload for all drivers with invalid police parameters

Message ID 20220217082803.3881-3-jianbol@nvidia.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series flow_offload: add tc police parameters | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1919 this patch: 1919
netdev/cc_maintainers success CCed 26 of 26 maintainers
netdev/build_clang success Errors and warnings before: 206 this patch: 206
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 2074 this patch: 2074
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jianbo Liu Feb. 17, 2022, 8:28 a.m. UTC 
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@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/dsa/sja1105/sja1105_flower.c      | 27 +++++++++
 .../chelsio/cxgb4/cxgb4_tc_matchall.c         | 55 +++++++++++++++++++
 .../net/ethernet/freescale/enetc/enetc_qos.c  | 31 +++++++++++
 .../ethernet/marvell/octeontx2/nic/otx2_tc.c  | 54 ++++++++++++++++++
 .../net/ethernet/mellanox/mlx5/core/en_tc.c   | 27 +++++++++
 .../ethernet/mellanox/mlxsw/spectrum_flower.c | 27 +++++++++
 drivers/net/ethernet/mscc/ocelot_flower.c     | 28 ++++++++++
 drivers/net/ethernet/mscc/ocelot_net.c        | 27 +++++++++
 .../ethernet/netronome/nfp/flower/qos_conf.c  | 28 ++++++++++
 include/net/flow_offload.h                    |  6 ++
 10 files changed, 310 insertions(+)

Comments

Vladimir Oltean Feb. 17, 2022, 12:49 p.m. UTC | #1 
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@nvidia.com>
> Reviewed-by: Roi Dayan <roid@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---

Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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.
Ido Schimmel Feb. 17, 2022, 1:57 p.m. UTC | #2 
On Thu, Feb 17, 2022 at 02:49:35PM +0200, Vladimir Oltean wrote:
> Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Thanks for testing

> 
> But could we cut down on line length a little? Example for sja1105
> (messages were also shortened):

No problem

[...]

> 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.

Will do
Jianbo Liu Feb. 22, 2022, 1:58 a.m. UTC | #3 
On Thu, 2022-02-17 at 14:49 +0200, Vladimir Oltean wrote:
> 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@nvidia.com>
> > Reviewed-by: Roi Dayan <roid@nvidia.com>
> > Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> > ---
> 
> Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> 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.

Hi Vladimir,

I'd love to hear your suggestion regarding where this validate function
to be placed for drivers/net/ethernet/mscc, as it will be used by both
ocelot_net.c and ocelot_flower.c. 

Thanks!
Jianbo
Vladimir Oltean Feb. 22, 2022, 10:09 a.m. UTC | #4 
Hi Jianbo,

On Tue, Feb 22, 2022 at 01:58:23AM +0000, Jianbo Liu wrote:
> Hi Vladimir,
> 
> I'd love to hear your suggestion regarding where this validate function
> to be placed for drivers/net/ethernet/mscc, as it will be used by both
> ocelot_net.c and ocelot_flower.c. 
> 
> Thanks!
> Jianbo

Try the attached patch on top of yours.
Jianbo Liu Feb. 22, 2022, 10:27 a.m. UTC | #5 
On Tue, 2022-02-22 at 10:09 +0000, Vladimir Oltean wrote:
> Hi Jianbo,
> 
> On Tue, Feb 22, 2022 at 01:58:23AM +0000, Jianbo Liu wrote:
> > Hi Vladimir,
> > 
> > I'd love to hear your suggestion regarding where this validate
> > function
> > to be placed for drivers/net/ethernet/mscc, as it will be used by
> > both
> > ocelot_net.c and ocelot_flower.c. 
> > 
> > Thanks!
> > Jianbo
> 
> Try the attached patch on top of yours.

OK, thanks!
Baowen Zheng Feb. 22, 2022, 10:29 a.m. UTC | #6 
Since almost all the drivers that support to offload police action make the similar validation, if it make sense to add the validation in the file of flow_offload.h or flow_offload.c?
Then the other drivers do not need to make the similar validation.
WDYT?

On Tuesday, February 22, 2022 6:10 PM, Vladimir wrote:
>On Tue, Feb 22, 2022 at 01:58:23AM +0000, Jianbo Liu wrote:
>> Hi Vladimir,
>>
>> I'd love to hear your suggestion regarding where this validate
>> function to be placed for drivers/net/ethernet/mscc, as it will be
>> used by both ocelot_net.c and ocelot_flower.c.
>>
>> Thanks!
>> Jianbo
>
>Try the attached patch on top of yours.
Ido Schimmel Feb. 22, 2022, 4:31 p.m. UTC | #7 
On Tue, Feb 22, 2022 at 10:29:57AM +0000, Baowen Zheng wrote:
> Since almost all the drivers that support to offload police action make the similar validation, if it make sense to add the validation in the file of flow_offload.h or flow_offload.c?
> Then the other drivers do not need to make the similar validation.
> WDYT?

But not all the drivers need the same validation. For example, nfp is
one of the few drivers that supports policing based on packet rate. The
octeontx2 driver has different restrictions based on whether the policer
is attached to matchall or flower.

We can put the restrictions that are common between all the drivers
somewhere, but it's not that much and it will also change over time,
resulting in needless churn where checks are moved to individual
drivers.
diff mbox series

Patch

diff --git a/drivers/net/dsa/sja1105/sja1105_flower.c b/drivers/net/dsa/sja1105/sja1105_flower.c
index 7dcdd784aea4..8a14df8cf91e 100644
--- a/drivers/net/dsa/sja1105/sja1105_flower.c
+++ b/drivers/net/dsa/sja1105/sja1105_flower.c
@@ -321,6 +321,33 @@  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");
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
index 28fd2de9e4cf..124e65116b2d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
@@ -48,6 +48,33 @@  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.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 (entry->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+			    entry->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 (entry->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+			    !flow_action_is_last_entry(actions, entry)) {
+				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 (entry->police.peakrate_bytes_ps ||
+			    entry->police.avrate || entry->police.overhead) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "Police offload is not supported when peakrate/avrate/overhead is configured");
+				return -EOPNOTSUPP;
+			}
+
 			if (entry->police.rate_pkt_ps) {
 				NL_SET_ERR_MSG_MOD(extack,
 						   "QoS offload not support packets per second");
@@ -150,6 +177,34 @@  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.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 (entry->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+	    entry->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 (entry->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+	    !flow_action_is_last_entry(&cls->rule->action, entry)) {
+		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 (entry->police.peakrate_bytes_ps ||
+	    entry->police.avrate || entry->police.overhead) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Police offload is not supported when peakrate/avrate/overhead is configured");
+		return -EOPNOTSUPP;
+	}
+
 	if (entry->police.rate_pkt_ps) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "QoS offload not support packets per second");
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index 3555c12edb45..c992d9e86467 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -1230,6 +1230,37 @@  static int enetc_psfp_parse_clsflower(struct enetc_ndev_priv *priv,
 
 	/* Flow meter and max frame size */
 	if (entryp) {
+		if (entryp->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");
+			err = -EOPNOTSUPP;
+			goto free_sfi;
+		}
+
+		if (entryp->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+		    entryp->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");
+			err = -EOPNOTSUPP;
+			goto free_sfi;
+		}
+
+		if (entryp->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+		    !flow_action_is_last_entry(&rule->action, entryp)) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Police offload is not supported when the conform action is ok, but police action is not last");
+			err = -EOPNOTSUPP;
+			goto free_sfi;
+		}
+
+		if (entryp->police.peakrate_bytes_ps ||
+		    entryp->police.avrate || entryp->police.overhead) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Police offload is not supported when peakrate/avrate/overhead is configured");
+			err = -EOPNOTSUPP;
+			goto free_sfi;
+		}
+
 		if (entryp->police.rate_pkt_ps) {
 			NL_SET_ERR_MSG_MOD(extack, "QoS offload not support packets per second");
 			err = -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
index 626961a41089..4b6e17765b2f 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
@@ -212,6 +212,33 @@  static int otx2_tc_egress_matchall_install(struct otx2_nic *nic,
 	entry = &cls->rule->action.entries[0];
 	switch (entry->id) {
 	case FLOW_ACTION_POLICE:
+		if (entry->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 (entry->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+		    entry->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 (entry->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+		    !flow_action_is_last_entry(&cls->rule->action, entry)) {
+			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 (entry->police.peakrate_bytes_ps ||
+		    entry->police.avrate || entry->police.overhead) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Police offload is not supported when peakrate/avrate/overhead is configured");
+			return -EOPNOTSUPP;
+		}
+
 		if (entry->police.rate_pkt_ps) {
 			NL_SET_ERR_MSG_MOD(extack, "QoS offload not support packets per second");
 			return -EOPNOTSUPP;
@@ -355,6 +382,33 @@  static int otx2_tc_parse_actions(struct otx2_nic *nic,
 				return -EOPNOTSUPP;
 			}
 
+			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(flow_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_bytes_ps > 0) {
 				rate = act->police.rate_bytes_ps * 8;
 				burst = act->police.burst;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 1287193a019b..7c160961c92e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -4197,6 +4197,33 @@  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.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(flow_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");
 				return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index bb417db773b9..b33c3da4daf8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -191,6 +191,33 @@  static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
 				return -EOPNOTSUPP;
 			}
 
+			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(flow_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");
 				return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c
index 949858891973..eb478ddb0f90 100644
--- a/drivers/net/ethernet/mscc/ocelot_flower.c
+++ b/drivers/net/ethernet/mscc/ocelot_flower.c
@@ -296,6 +296,34 @@  static int ocelot_flower_parse_action(struct ocelot *ocelot, int port,
 						   "Last action must be GOTO");
 				return -EOPNOTSUPP;
 			}
+
+			if (a->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 (a->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+			    a->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 (a->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+			    !flow_action_is_last_entry(&f->rule->action, a)) {
+				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 (a->police.peakrate_bytes_ps ||
+			    a->police.avrate || a->police.overhead) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "Police offload is not supported when peakrate/avrate/overhead is configured");
+				return -EOPNOTSUPP;
+			}
+
 			if (a->police.rate_pkt_ps) {
 				NL_SET_ERR_MSG_MOD(extack,
 						   "QoS offload not support packets per second");
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index e271b6225b72..69afb560ab98 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -258,6 +258,33 @@  static int ocelot_setup_tc_cls_matchall(struct ocelot_port_private *priv,
 			return -EEXIST;
 		}
 
+		if (action->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 (action->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+		    action->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 (action->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+		    !flow_action_is_last_entry(&f->rule->action, action)) {
+			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 (action->police.peakrate_bytes_ps ||
+		    action->police.avrate || action->police.overhead) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Police offload is not supported when peakrate/avrate/overhead is configured");
+			return -EOPNOTSUPP;
+		}
+
 		if (action->police.rate_pkt_ps) {
 			NL_SET_ERR_MSG_MOD(extack,
 					   "QoS offload not support packets per second");
diff --git a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
index 784c6dbf8bc4..247984d50b49 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
@@ -132,6 +132,34 @@  nfp_flower_install_rate_limiter(struct nfp_app *app, struct net_device *netdev,
 					   "unsupported offload: qos rate limit offload requires police action");
 			return -EOPNOTSUPP;
 		}
+
+		if (action->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 (action->police.notexceed.act_id != FLOW_ACTION_PIPE &&
+		    action->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 (action->police.notexceed.act_id == FLOW_ACTION_ACCEPT &&
+		    !flow_action_is_last_entry(&flow->rule->action, action)) {
+			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 (action->police.peakrate_bytes_ps ||
+		    action->police.avrate || action->police.overhead) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Police offload is not supported when peakrate/avrate/overhead is configured");
+			return -EOPNOTSUPP;
+		}
+
 		if (action->police.rate_bytes_ps > 0) {
 			if (bps_num++) {
 				NL_SET_ERR_MSG_MOD(extack,
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 94cde6bbc8a5..2b9890177aa1 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -315,6 +315,12 @@  static inline bool flow_offload_has_one_action(const struct flow_action *action)
 	return action->num_entries == 1;
 }
 
+static inline bool flow_action_is_last_entry(const struct flow_action *action,
+					     const struct flow_action_entry *entry)
+{
+	return entry == &action->entries[action->num_entries - 1];
+}
+
 #define flow_action_for_each(__i, __act, __actions)			\
         for (__i = 0, __act = &(__actions)->entries[0];			\
 	     __i < (__actions)->num_entries;				\