| 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
| ||
|
Message-ID: <20181119144900.GK2223@nanopsycho.orion>
Date: Mon, 19 Nov 2018 15:49:01 +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 09/12] flow_dissector: add basic
ethtool_rx_flow_spec to flow_rule structure translator
Mon, Nov 19, 2018 at 01:15:16AM CET, pablo@...filter.org wrote:
>This patch adds a function to translate the ethtool_rx_flow_spec
>structure to the flow_rule representation.
>
>This allows us to reuse code from the driver side given that both flower
>and ethtool_rx_flow interfaces use the same representation.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@...filter.org>
>---
>v2: no changes.
>
> include/net/flow_dissector.h | 5 ++
> net/core/flow_dissector.c | 190 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 195 insertions(+)
>
>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index 7a4683646d5a..ec9036232538 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -485,4 +485,9 @@ static inline bool flow_rule_match_key(const struct flow_rule *rule,
> return dissector_uses_key(rule->match.dissector, key);
> }
>
>+struct ethtool_rx_flow_spec;
>+
>+struct flow_rule *ethtool_rx_flow_rule(const struct ethtool_rx_flow_spec *fs);
>+void ethtool_rx_flow_rule_free(struct flow_rule *rule);
>+
> #endif
>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>index b9368349f0f7..ef5bdb62620c 100644
>--- a/net/core/flow_dissector.c
>+++ b/net/core/flow_dissector.c
>@@ -17,6 +17,7 @@
> #include <linux/dccp.h>
> #include <linux/if_tunnel.h>
> #include <linux/if_pppox.h>
>+#include <uapi/linux/ethtool.h>
> #include <linux/ppp_defs.h>
> #include <linux/stddef.h>
> #include <linux/if_ether.h>
>@@ -276,6 +277,195 @@ void flow_action_free(struct flow_action *flow_action)
> }
> EXPORT_SYMBOL(flow_action_free);
>
>+struct ethtool_rx_flow_key {
>+ struct flow_dissector_key_basic basic;
>+ union {
>+ struct flow_dissector_key_ipv4_addrs ipv4;
>+ struct flow_dissector_key_ipv6_addrs ipv6;
>+ };
>+ struct flow_dissector_key_ports tp;
>+ struct flow_dissector_key_ip ip;
>+} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
>+
>+struct ethtool_rx_flow_match {
>+ struct flow_dissector dissector;
>+ struct ethtool_rx_flow_key key;
>+ struct ethtool_rx_flow_key mask;
>+};
>+
>+struct flow_rule *ethtool_rx_flow_rule(const struct ethtool_rx_flow_spec *fs)
>+{
>+ static struct in6_addr zero_addr = {};
>+ struct ethtool_rx_flow_match *match;
>+ struct flow_action_key *act;
>+ struct flow_rule *rule;
>+
>+ rule = kmalloc(sizeof(struct flow_rule), GFP_KERNEL);
Allocating struct flow_rule manually here seems wrong. There should
be some helpers. Please see below.***
>+ if (!rule)
>+ return NULL;
>+
>+ match = kzalloc(sizeof(struct ethtool_rx_flow_match), GFP_KERNEL);
>+ if (!match)
>+ goto err_match;
>+
>+ rule->match.dissector = &match->dissector;
>+ rule->match.mask = &match->mask;
>+ rule->match.key = &match->key;
>+
>+ match->mask.basic.n_proto = 0xffff;
>+
>+ switch (fs->flow_type & ~FLOW_EXT) {
>+ case TCP_V4_FLOW:
>+ case UDP_V4_FLOW: {
>+ const struct ethtool_tcpip4_spec *v4_spec, *v4_m_spec;
>+
>+ match->key.basic.n_proto = htons(ETH_P_IP);
>+
>+ v4_spec = &fs->h_u.tcp_ip4_spec;
>+ v4_m_spec = &fs->m_u.tcp_ip4_spec;
>+
>+ if (v4_m_spec->ip4src) {
>+ match->key.ipv4.src = v4_spec->ip4src;
>+ match->mask.ipv4.src = v4_m_spec->ip4src;
>+ }
>+ if (v4_m_spec->ip4dst) {
>+ match->key.ipv4.dst = v4_spec->ip4dst;
>+ match->mask.ipv4.dst = v4_m_spec->ip4dst;
>+ }
>+ if (v4_m_spec->ip4src ||
>+ v4_m_spec->ip4dst) {
>+ match->dissector.used_keys |=
>+ FLOW_DISSECTOR_KEY_IPV4_ADDRS;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_IPV4_ADDRS] =
>+ offsetof(struct ethtool_rx_flow_key, ipv4);
>+ }
>+ if (v4_m_spec->psrc) {
>+ match->key.tp.src = v4_spec->psrc;
>+ match->mask.tp.src = v4_m_spec->psrc;
>+ }
>+ if (v4_m_spec->pdst) {
>+ match->key.tp.dst = v4_spec->pdst;
>+ match->mask.tp.dst = v4_m_spec->pdst;
>+ }
>+ if (v4_m_spec->psrc ||
>+ v4_m_spec->pdst) {
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_PORTS;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_PORTS] =
>+ offsetof(struct ethtool_rx_flow_key, tp);
>+ }
>+ if (v4_m_spec->tos) {
>+ match->key.ip.tos = v4_spec->pdst;
>+ match->mask.ip.tos = v4_m_spec->pdst;
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_IP;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_IP] =
>+ offsetof(struct ethtool_rx_flow_key, ip);
>+ }
>+ }
>+ break;
>+ case TCP_V6_FLOW:
>+ case UDP_V6_FLOW: {
>+ const struct ethtool_tcpip6_spec *v6_spec, *v6_m_spec;
>+
>+ match->key.basic.n_proto = htons(ETH_P_IPV6);
>+
>+ v6_spec = &fs->h_u.tcp_ip6_spec;
>+ v6_m_spec = &fs->m_u.tcp_ip6_spec;
>+ if (memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr))) {
>+ memcpy(&match->key.ipv6.src, v6_spec->ip6src,
>+ sizeof(match->key.ipv6.src));
>+ memcpy(&match->mask.ipv6.src, v6_m_spec->ip6src,
>+ sizeof(match->mask.ipv6.src));
>+ }
>+ if (memcmp(v6_m_spec->ip6dst, &zero_addr, sizeof(zero_addr))) {
>+ memcpy(&match->key.ipv6.dst, v6_spec->ip6dst,
>+ sizeof(match->key.ipv6.dst));
>+ memcpy(&match->mask.ipv6.dst, v6_m_spec->ip6dst,
>+ sizeof(match->mask.ipv6.dst));
>+ }
>+ if (memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr)) ||
>+ memcmp(v6_m_spec->ip6src, &zero_addr, sizeof(zero_addr))) {
>+ match->dissector.used_keys |=
>+ FLOW_DISSECTOR_KEY_IPV6_ADDRS;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_IPV6_ADDRS] =
>+ offsetof(struct ethtool_rx_flow_key, ipv6);
>+ }
>+ if (v6_m_spec->psrc) {
>+ match->key.tp.src = v6_spec->psrc;
>+ match->mask.tp.src = v6_m_spec->psrc;
>+ }
>+ if (v6_m_spec->pdst) {
>+ match->key.tp.dst = v6_spec->pdst;
>+ match->mask.tp.dst = v6_m_spec->pdst;
>+ }
>+ if (v6_m_spec->psrc ||
>+ v6_m_spec->pdst) {
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_PORTS;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_PORTS] =
>+ offsetof(struct ethtool_rx_flow_key, tp);
>+ }
>+ if (v6_m_spec->tclass) {
>+ match->key.ip.tos = v6_spec->tclass;
>+ match->mask.ip.tos = v6_m_spec->tclass;
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_IP;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_IP] =
>+ offsetof(struct ethtool_rx_flow_key, ip);
>+ }
>+ }
>+ break;
>+ }
>+
>+ switch (fs->flow_type & ~FLOW_EXT) {
>+ case TCP_V4_FLOW:
>+ case TCP_V6_FLOW:
>+ match->key.basic.ip_proto = IPPROTO_TCP;
>+ break;
>+ case UDP_V4_FLOW:
>+ case UDP_V6_FLOW:
>+ match->key.basic.ip_proto = IPPROTO_UDP;
>+ break;
>+ }
>+ match->mask.basic.ip_proto = 0xff;
>+
>+ match->dissector.used_keys |= FLOW_DISSECTOR_KEY_BASIC;
>+ match->dissector.offset[FLOW_DISSECTOR_KEY_BASIC] =
>+ offsetof(struct ethtool_rx_flow_key, basic);
>+
>+ /* ethtool_rx supports only one single action per rule. */
>+ if (flow_action_init(&rule->action, 1) < 0)
>+ goto err_action;
>+
>+ act = &rule->action.keys[0];
>+ switch (fs->ring_cookie) {
>+ case RX_CLS_FLOW_DISC:
>+ act->id = FLOW_ACTION_KEY_DROP;
>+ break;
>+ case RX_CLS_FLOW_WAKE:
>+ act->id = FLOW_ACTION_KEY_WAKE;
>+ break;
>+ default:
>+ act->id = FLOW_ACTION_KEY_QUEUE;
>+ act->queue_index = fs->ring_cookie;
>+ break;
>+ }
>+
>+ return rule;
>+
>+err_action:
>+ kfree(match);
>+err_match:
>+ kfree(rule);
>+ return NULL;
>+}
>+EXPORT_SYMBOL(ethtool_rx_flow_rule);
>+
>+void ethtool_rx_flow_rule_free(struct flow_rule *rule)
>+{
>+ kfree((struct flow_match *)rule->match.dissector);
Ouch. I wonder if it cannot be stored rather in some rule->priv or
something.
On alloc, you can have a helper to allocate both:
***
struct flow_rule {
...
unsigned long priv[0];
};
struct flow_rule *flow_rule_alloc(size_t priv_size)
{
return kzalloc(sizeof(struct flow_rule) + priv_size, ...);
}
>+ flow_action_free(&rule->action);
>+ kfree(rule);
>+}
>+EXPORT_SYMBOL(ethtool_rx_flow_rule_free);
>+
> /**
> * __skb_flow_get_ports - extract the upper layer ports and return them
> * @skb: sk_buff to extract the ports from
>--
>2.11.0
>
Powered by blists - more mailing lists