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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20181119130523.GE2223@nanopsycho.orion>
Date:   Mon, 19 Nov 2018 14:05: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:35:48PM CET, pablo@...filter.org wrote:
>On Mon, Nov 19, 2018 at 12:56:23PM +0100, Jiri Pirko wrote:
>> 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"
>
>Sure, will rename this.
>
>> >+	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,
>
>I assume I should remove _KEY_ from this enum definitions too.

Sure.

>
>> >+};
>> >+
>> >+/* 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"
>
>OK.
>
>> >+	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"
>
>OK.
>
>> >+	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;
>
>OK.