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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181119115623.GA2223@nanopsycho.orion>
Date:   Mon, 19 Nov 2018 12:56:23 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        thomas.lendacky@....com, f.fainelli@...il.com,
        ariel.elior@...ium.com, michael.chan@...adcom.com,
        santosh@...lsio.com, madalin.bucur@....com,
        yisen.zhuang@...wei.com, salil.mehta@...wei.com,
        jeffrey.t.kirsher@...el.com, tariqt@...lanox.com,
        saeedm@...lanox.com, jiri@...lanox.com, idosch@...lanox.com,
        jakub.kicinski@...ronome.com, peppe.cavallaro@...com,
        grygorii.strashko@...com, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, alexandre.torgue@...com,
        joabreu@...opsys.com, linux-net-drivers@...arflare.com,
        ganeshgr@...lsio.com, ogerlitz@...lanox.com
Subject: Re: [PATCH net-next,v2 03/12] flow_dissector: add flow action
 infrastructure

Mon, Nov 19, 2018 at 01:15:10AM CET, pablo@...filter.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@...filter.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 {

Why "key"? Why not just "flow_action_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 {

Please be consistent in naming: "act" vs "action"


>+	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 {

And here "struct flow_action"


>+	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 {

And here "struct flow_actions"


>+	int			num_keys;

unsigned int;


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