Re: [PATCH net-next,v2 09/12] flow_dissector: add basic ethtool_rx_flow_spec to flow_rule structure translator
From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-11-20 01:19:32
Mon, Nov 19, 2018 at 01:15:16AM CET, pablo@netfilter.org wrote:
quoted hunk ↗ jump to hunk
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@netfilter.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); + #endifdiff --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