Re: [PATCH net-next,v2 03/12] flow_dissector: add flow action infrastructure

Jiri Pirko
2018-11-20T00:33:47Z

Mon, Nov 19, 2018 at 01:15:10AM CET, pablo@netfilter.org wrote:
>This new infrastructure defines the nic actions that you can perform
>from existing network drivers. This infrastructure allows us to avoid a
>direct dependency with the native software TC action representation.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>---
>v2: no changes.
>
> include/net/flow_dissector.h | 70 ++++++++++++++++++++++++++++++++++++++++++++
> net/core/flow_dissector.c | 18 ++++++++++++
> 2 files changed, 88 insertions(+)
>
>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index 965a82b8d881..925c208816f1 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -402,8 +402,78 @@ void flow_rule_match_enc_keyid(const struct flow_rule *rule,
> void flow_rule_match_enc_opts(const struct flow_rule *rule,
> struct flow_match_enc_opts *out);
>
>+enum flow_action_key_id {
>+ FLOW_ACTION_KEY_ACCEPT = 0,
>+ FLOW_ACTION_KEY_DROP,
>+ FLOW_ACTION_KEY_TRAP,
>+ FLOW_ACTION_KEY_GOTO,
>+ FLOW_ACTION_KEY_REDIRECT,
>+ FLOW_ACTION_KEY_MIRRED,
>+ FLOW_ACTION_KEY_VLAN_PUSH,
>+ FLOW_ACTION_KEY_VLAN_POP,
>+ FLOW_ACTION_KEY_VLAN_MANGLE,
>+ FLOW_ACTION_KEY_TUNNEL_ENCAP,
>+ FLOW_ACTION_KEY_TUNNEL_DECAP,
>+ FLOW_ACTION_KEY_MANGLE,
>+ FLOW_ACTION_KEY_ADD,
>+ FLOW_ACTION_KEY_CSUM,
>+ FLOW_ACTION_KEY_MARK,
>+};
>+
>+/* This is mirroring enum pedit_header_type definition for easy mapping between
>+ * tc pedit action. Legacy TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK is mapped to
>+ * FLOW_ACT_MANGLE_UNSPEC, which is supported by no driver.
>+ */
>+enum flow_act_mangle_base {
>+ FLOW_ACT_MANGLE_UNSPEC = 0,
>+ FLOW_ACT_MANGLE_HDR_TYPE_ETH,
>+ FLOW_ACT_MANGLE_HDR_TYPE_IP4,
>+ FLOW_ACT_MANGLE_HDR_TYPE_IP6,
>+ FLOW_ACT_MANGLE_HDR_TYPE_TCP,
>+ FLOW_ACT_MANGLE_HDR_TYPE_UDP,
>+};
>+
>+struct flow_action_key {
>+ enum flow_action_key_id id;
>+ union {
>+ u32 chain_index; /* FLOW_ACTION_KEY_GOTO */
>+ struct net_device *dev; /* FLOW_ACTION_KEY_REDIRECT */
>+ struct { /* FLOW_ACTION_KEY_VLAN */
>+ u16 vid;
>+ __be16 proto;
>+ u8 prio;
>+ } vlan;
>+ struct { /* FLOW_ACTION_KEY_PACKET_EDIT */
>+ enum flow_act_mangle_base htype;
>+ u32 offset;
>+ u32 mask;
>+ u32 val;
>+ } mangle;
>+ const struct ip_tunnel_info *tunnel; /* FLOW_ACTION_KEY_TUNNEL_ENCAP */
>+ u32 csum_flags; /* FLOW_ACTION_KEY_CSUM */
>+ u32 mark; /* FLOW_ACTION_KEY_MARK */
>+ };
>+};
>+
>+struct flow_action {

Hmm, thinking about it a bit more, none of this is is related to flow
dissector so it is misleading to put the code in flow_dissector.[hc].

Maybe you can push this and related stuff into new files include/net/flow.h
and net/core/flow.c.

>+ int num_keys;
>+ struct flow_action_key *keys;
>+};
>+
>+int flow_action_init(struct flow_action *flow_action, int num_acts);
>+void flow_action_free(struct flow_action *flow_action);
>+
>+static inline bool flow_action_has_keys(const struct flow_action *action)
>+{
>+ return action->num_keys;
>+}
>+
>+#define flow_action_for_each(__i, __act, __actions) \
>+ for (__i = 0, __act = &(__actions)->keys[0]; __i < (__actions)->num_keys; __act = &(__actions)->keys[++__i])
>+
> struct flow_rule {
> struct flow_match match;
>+ struct flow_action action;
> };
>
> static inline bool flow_rule_match_key(const struct flow_rule *rule,
>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>index 186089b8d852..b9368349f0f7 100644
>--- a/net/core/flow_dissector.c
>+++ b/net/core/flow_dissector.c
>@@ -258,6 +258,24 @@ void flow_rule_match_enc_opts(const struct flow_rule *rule,
> }
> EXPORT_SYMBOL(flow_rule_match_enc_opts);
>
>+int flow_action_init(struct flow_action *flow_action, int num_acts)
>+{
>+ flow_action->keys = kmalloc(sizeof(struct flow_action_key) * num_acts,
>+ GFP_KERNEL);
>+ if (!flow_action->keys)
>+ return -ENOMEM;
>+
>+ flow_action->num_keys = num_acts;
>+ return 0;
>+}
>+EXPORT_SYMBOL(flow_action_init);
>+
>+void flow_action_free(struct flow_action *flow_action)
>+{
>+ kfree(flow_action->keys);
>+}
>+EXPORT_SYMBOL(flow_action_free);
>+
> /**
> * __skb_flow_get_ports - extract the upper layer ports and return them
> * @skb: sk_buff to extract the ports from
>--
>2.11.0
>