New options were introduced to FLOW_ACTION_POLICE after struct dsa_mall_policer_tc_entry was added. The following commands will succeed on DSA ports: tc qdisc add dev lan1 handle ffff: ingress tc filter add dev lan1 ingress matchall skip_sw action police \ pkts_rate 80000 pkts_burst 100 mtu 1000 conform-exceed ok resulting 1. burst_pkt, rate_pkt_ps, etc. being ignored; 2. burst and rate_bytes_per_sec set to 0 without any error. Instead of making decisions for drivers, extend struct dsa_mall_policer_tc_entry to all options for FLOW_ACTION_POLICE in favor of full functionalities, such as packet rate mode. Drivers must reject unsupported combinations. Signed-off-by: David Yang --- include/net/dsa.h | 11 +++++++++++ net/dsa/user.c | 7 +++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/net/dsa.h b/include/net/dsa.h index 6b2b5ed64ea4..4c177b168ec8 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -21,6 +21,7 @@ #include #include #include +#include struct dsa_8021q_context; struct tc_action; @@ -220,6 +221,16 @@ struct dsa_mall_mirror_tc_entry { struct dsa_mall_policer_tc_entry { u32 burst; u64 rate_bytes_per_sec; + u64 peakrate_bytes_ps; + u32 avrate; + u16 overhead; + u64 burst_pkt; + u64 rate_pkt_ps; + u32 mtu; + struct { + enum flow_action_id act_id; + u32 extval; + } exceed, notexceed; }; /* TC matchall entry */ diff --git a/net/dsa/user.c b/net/dsa/user.c index f59d66f0975d..4c9cff629d5c 100644 --- a/net/dsa/user.c +++ b/net/dsa/user.c @@ -1497,8 +1497,11 @@ dsa_user_add_cls_matchall_police(struct net_device *dev, mall_tc_entry->cookie = cls->cookie; mall_tc_entry->type = DSA_PORT_MALL_POLICER; policer = &mall_tc_entry->policer; - policer->rate_bytes_per_sec = act->police.rate_bytes_ps; - policer->burst = act->police.burst; + /* so sad, flow_offload.h did not export the type of act->police, and + * it's a nightmare to copy it field by field + */ + static_assert(sizeof(act->police) == sizeof(*policer)); + memcpy(policer, &act->police, sizeof(*policer)); err = ds->ops->port_policer_add(ds, dp->index, policer); if (err) { -- 2.51.0 It has shown FLOW_ACTION_POLICE is not stable, and it's very likely that options were added at drivers' request. However, that also means .port_policer_add() would have to check the parameters themselves. Giving that struct dsa_mall_policer_tc_entry is not stable, adjusting the interface between the framework and drivers would be a pain. Introduce a helper to maximize forward compatibility. Drivers may use it to recognize the pattern of tc entry reliably, and ensure all newly-added options are set to 0 / their default values and do not break the semantics. Signed-off-by: David Yang --- include/net/dsa.h | 14 ++++++++++++++ net/dsa/dsa.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/include/net/dsa.h b/include/net/dsa.h index 4c177b168ec8..957439846fe5 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -1410,4 +1410,18 @@ netdev_tx_t dsa_enqueue_skb(struct sk_buff *skb, struct net_device *dev); void dsa_port_phylink_mac_change(struct dsa_switch *ds, int port, bool up); bool dsa_supports_eee(struct dsa_switch *ds, int port); +enum dsa_mall_policer_tc_type { + /* Pattern recognized, no surprising fields exist */ + DSA_MALL_POLICER_TC_KNOWN = BIT(0), + /* Unset: .burst and .rate_bytes_per_sec valid + * Set: .burst_pkt and .rate_pkt_ps valid + */ + DSA_MALL_POLICER_TC_PKT_MODE = BIT(1), + /* .mtu valid */ + DSA_MALL_POLICER_TC_MTU = BIT(2), +}; + +unsigned long +dsa_mall_policer_tc_entry_type(struct dsa_mall_policer_tc_entry *entry); + #endif diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c index 35ce3941fae3..c8102cea5ab3 100644 --- a/net/dsa/dsa.c +++ b/net/dsa/dsa.c @@ -1834,6 +1834,39 @@ int dsa_port_simple_hsr_leave(struct dsa_switch *ds, int port, } EXPORT_SYMBOL_GPL(dsa_port_simple_hsr_leave); +/* dsa_mall_policer_tc_entry_type - map tc_entry to some "known" types + * @entry: the tc entry + * + * A helper to check dsa_mall_policer_tc_entry against some known patterns, + * without having to know the exact struct layout. + * + * Returns: ORs of enum dsa_mall_policer_tc_type with DSA_MALL_POLICER_TC_KNOWN + * set if recognized, otherwise 0 + */ +unsigned long +dsa_mall_policer_tc_entry_type(struct dsa_mall_policer_tc_entry *entry) +{ + bool byte_mode = (entry->burst || entry->rate_bytes_per_sec); + bool pkt_mode = (entry->burst_pkt || entry->rate_pkt_ps); + unsigned long flags = DSA_MALL_POLICER_TC_KNOWN; + + if (byte_mode == pkt_mode) + return 0; + if (entry->peakrate_bytes_ps || entry->avrate || entry->overhead) + return 0; + if (entry->exceed.act_id != FLOW_ACTION_DROP || + entry->notexceed.act_id != FLOW_ACTION_ACCEPT) + return 0; + + if (pkt_mode) + flags |= DSA_MALL_POLICER_TC_PKT_MODE; + if (entry->mtu) + flags |= DSA_MALL_POLICER_TC_MTU; + + return flags; +} +EXPORT_SYMBOL_GPL(dsa_mall_policer_tc_entry_type); + static const struct dsa_stubs __dsa_stubs = { .conduit_hwtstamp_validate = __dsa_conduit_hwtstamp_validate, }; -- 2.51.0 rate_bytes_per_sec might be 0, check for it. Signed-off-by: David Yang --- drivers/net/dsa/ocelot/felix.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index 9e5ede932b42..fb2d02ff0fe7 100644 --- a/drivers/net/dsa/ocelot/felix.c +++ b/drivers/net/dsa/ocelot/felix.c @@ -2011,6 +2011,10 @@ static int felix_port_policer_add(struct dsa_switch *ds, int port, .burst = policer->burst, }; + if (dsa_mall_policer_tc_entry_type(policer) != + DSA_MALL_POLICER_TC_KNOWN) + return -EOPNOTSUPP; + return ocelot_port_policer_add(ocelot, port, &pol); } -- 2.51.0