Mon, Nov 19, 2018 at 01:15:16AM CET, pa...@netfilter.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 <pa...@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);
>+
> #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
>

Reply via email to