Commit 060b6381 authored by Edward Cree's avatar Edward Cree Committed by David S. Miller

net: flow_offload: simplify hw stats check handling

Make FLOW_ACTION_HW_STATS_DONT_CARE be all bits, rather than none, so that
 drivers and __flow_action_hw_stats_check can use simple bitwise checks.

Pre-fill all actions with DONT_CARE in flow_rule_alloc(), rather than
 relying on implicit semantics of zero from kzalloc, so that callers which
 don't configure action stats themselves (i.e. netfilter) get the correct
 behaviour by default.

Only the kernel's internal API semantics change; the TC uAPI is unaffected.

v4: move DONT_CARE setting to flow_rule_alloc() for robustness and simplicity.

v3: set DONT_CARE in nft and ct offload.

v2: rebased on net-next, removed RFC tags.
Signed-off-by: default avatarEdward Cree <ecree@solarflare.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent df0651f8
...@@ -30,14 +30,14 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp, ...@@ -30,14 +30,14 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
return -EOPNOTSUPP; return -EOPNOTSUPP;
act = flow_action_first_entry_get(flow_action); act = flow_action_first_entry_get(flow_action);
if (act->hw_stats == FLOW_ACTION_HW_STATS_ANY || if (act->hw_stats & FLOW_ACTION_HW_STATS_DISABLED) {
act->hw_stats == FLOW_ACTION_HW_STATS_IMMEDIATE) { /* Nothing to do */
} else if (act->hw_stats & FLOW_ACTION_HW_STATS_IMMEDIATE) {
/* Count action is inserted first */ /* Count action is inserted first */
err = mlxsw_sp_acl_rulei_act_count(mlxsw_sp, rulei, extack); err = mlxsw_sp_acl_rulei_act_count(mlxsw_sp, rulei, extack);
if (err) if (err)
return err; return err;
} else if (act->hw_stats != FLOW_ACTION_HW_STATS_DISABLED && } else {
act->hw_stats != FLOW_ACTION_HW_STATS_DONT_CARE) {
NL_SET_ERR_MSG_MOD(extack, "Unsupported action HW stats type"); NL_SET_ERR_MSG_MOD(extack, "Unsupported action HW stats type");
return -EOPNOTSUPP; return -EOPNOTSUPP;
} }
......
...@@ -168,10 +168,11 @@ enum flow_action_hw_stats_bit { ...@@ -168,10 +168,11 @@ enum flow_action_hw_stats_bit {
FLOW_ACTION_HW_STATS_IMMEDIATE_BIT, FLOW_ACTION_HW_STATS_IMMEDIATE_BIT,
FLOW_ACTION_HW_STATS_DELAYED_BIT, FLOW_ACTION_HW_STATS_DELAYED_BIT,
FLOW_ACTION_HW_STATS_DISABLED_BIT, FLOW_ACTION_HW_STATS_DISABLED_BIT,
FLOW_ACTION_HW_STATS_NUM_BITS
}; };
enum flow_action_hw_stats { enum flow_action_hw_stats {
FLOW_ACTION_HW_STATS_DONT_CARE = 0,
FLOW_ACTION_HW_STATS_IMMEDIATE = FLOW_ACTION_HW_STATS_IMMEDIATE =
BIT(FLOW_ACTION_HW_STATS_IMMEDIATE_BIT), BIT(FLOW_ACTION_HW_STATS_IMMEDIATE_BIT),
FLOW_ACTION_HW_STATS_DELAYED = BIT(FLOW_ACTION_HW_STATS_DELAYED_BIT), FLOW_ACTION_HW_STATS_DELAYED = BIT(FLOW_ACTION_HW_STATS_DELAYED_BIT),
...@@ -179,6 +180,7 @@ enum flow_action_hw_stats { ...@@ -179,6 +180,7 @@ enum flow_action_hw_stats {
FLOW_ACTION_HW_STATS_DELAYED, FLOW_ACTION_HW_STATS_DELAYED,
FLOW_ACTION_HW_STATS_DISABLED = FLOW_ACTION_HW_STATS_DISABLED =
BIT(FLOW_ACTION_HW_STATS_DISABLED_BIT), BIT(FLOW_ACTION_HW_STATS_DISABLED_BIT),
FLOW_ACTION_HW_STATS_DONT_CARE = BIT(FLOW_ACTION_HW_STATS_NUM_BITS) - 1,
}; };
typedef void (*action_destr)(void *priv); typedef void (*action_destr)(void *priv);
...@@ -340,11 +342,12 @@ __flow_action_hw_stats_check(const struct flow_action *action, ...@@ -340,11 +342,12 @@ __flow_action_hw_stats_check(const struct flow_action *action,
return false; return false;
action_entry = flow_action_first_entry_get(action); action_entry = flow_action_first_entry_get(action);
if (action_entry->hw_stats == FLOW_ACTION_HW_STATS_DONT_CARE)
return true; /* Zero is not a legal value for hw_stats, catch anyone passing it */
WARN_ON_ONCE(!action_entry->hw_stats);
if (!check_allow_bit && if (!check_allow_bit &&
action_entry->hw_stats != FLOW_ACTION_HW_STATS_ANY) { ~action_entry->hw_stats & FLOW_ACTION_HW_STATS_ANY) {
NL_SET_ERR_MSG_MOD(extack, "Driver supports only default HW stats type \"any\""); NL_SET_ERR_MSG_MOD(extack, "Driver supports only default HW stats type \"any\"");
return false; return false;
} else if (check_allow_bit && } else if (check_allow_bit &&
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
struct flow_rule *flow_rule_alloc(unsigned int num_actions) struct flow_rule *flow_rule_alloc(unsigned int num_actions)
{ {
struct flow_rule *rule; struct flow_rule *rule;
int i;
rule = kzalloc(struct_size(rule, action.entries, num_actions), rule = kzalloc(struct_size(rule, action.entries, num_actions),
GFP_KERNEL); GFP_KERNEL);
...@@ -15,6 +16,11 @@ struct flow_rule *flow_rule_alloc(unsigned int num_actions) ...@@ -15,6 +16,11 @@ struct flow_rule *flow_rule_alloc(unsigned int num_actions)
return NULL; return NULL;
rule->action.num_entries = num_actions; rule->action.num_entries = num_actions;
/* Pre-fill each action hw_stats with DONT_CARE.
* Caller can override this if it wants stats for a given action.
*/
for (i = 0; i < num_actions; i++)
rule->action.entries[i].hw_stats = FLOW_ACTION_HW_STATS_DONT_CARE;
return rule; return rule;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment