Wed, Dec 07, 2016 at 10:41:36AM CET, simon.hor...@netronome.com wrote:
>Allow dissection of ICMP(V6) type and code. This should only occur
>if a packet is ICMP(V6) and the dissector has FLOW_DISSECTOR_KEY_ICMP set.
>
>There are currently no users of FLOW_DISSECTOR_KEY_ICMP.
>A follow-up patch will allow FLOW_DISSECTOR_KEY_ICMP to be used by
>the flower classifier.
>---
>v3
>* Add FLOW_DISSECTOR_KEY_ICMP and use separate structure for ICMP, struct
>  flow_dissector_key_icmp, which is a union with struct
>  flow_dissector_key_ports in struct flow_keys.
>* Drop checks for !ICMP before accessing port field
>
>v2
>* Include all helpers in this patch
>---
> include/net/flow_dissector.h | 55 +++++++++++++++++++++++++++++++++++++++++++-
> net/core/flow_dissector.c    | 49 +++++++++++++++++++++++++++++++++------
> 2 files changed, 96 insertions(+), 8 deletions(-)
>
>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>index c4f31666afd2..aefb7a8138a6 100644
>--- a/include/net/flow_dissector.h
>+++ b/include/net/flow_dissector.h
>@@ -2,6 +2,7 @@
> #define _NET_FLOW_DISSECTOR_H
> 
> #include <linux/types.h>
>+#include <linux/in.h>
> #include <linux/in6.h>
> #include <uapi/linux/if_ether.h>
> 
>@@ -104,6 +105,22 @@ struct flow_dissector_key_ports {
>       };
> };
> 
>+/**
>+ * flow_dissector_key_icmp:
>+ *    @ports: type and code of ICMP header
>+ *            icmp: ICMP type (high) and code (low)
>+ *            type: ICMP type
>+ *            code: ICMP code
>+ */
>+struct flow_dissector_key_icmp {
>+      union {
>+              __be16 icmp;
>+              struct {
>+                      u8 type;
>+                      u8 code;
>+              };
>+      };
>+};
> 
> /**
>  * struct flow_dissector_key_eth_addrs:
>@@ -122,6 +139,7 @@ enum flow_dissector_key_id {
>       FLOW_DISSECTOR_KEY_IPV4_ADDRS, /* struct flow_dissector_key_ipv4_addrs 
> */
>       FLOW_DISSECTOR_KEY_IPV6_ADDRS, /* struct flow_dissector_key_ipv6_addrs 
> */
>       FLOW_DISSECTOR_KEY_PORTS, /* struct flow_dissector_key_ports */
>+      FLOW_DISSECTOR_KEY_ICMP, /* struct flow_dissector_key_icmp */
>       FLOW_DISSECTOR_KEY_ETH_ADDRS, /* struct flow_dissector_key_eth_addrs */
>       FLOW_DISSECTOR_KEY_TIPC_ADDRS, /* struct flow_dissector_key_tipc_addrs 
> */
>       FLOW_DISSECTOR_KEY_VLAN, /* struct flow_dissector_key_flow_vlan */
>@@ -160,7 +178,10 @@ struct flow_keys {
>       struct flow_dissector_key_tags tags;
>       struct flow_dissector_key_vlan vlan;
>       struct flow_dissector_key_keyid keyid;
>-      struct flow_dissector_key_ports ports;
>+      union {
>+              struct flow_dissector_key_ports ports;
>+              struct flow_dissector_key_icmp icmp;
>+      };

Why? You don't need this here.


>       struct flow_dissector_key_addrs addrs;
> };
> 
>@@ -188,6 +209,38 @@ struct flow_keys_digest {
> void make_flow_keys_digest(struct flow_keys_digest *digest,
>                          const struct flow_keys *flow);
> 
>+static inline bool flow_protos_are_icmpv4(__be16 n_proto, u8 ip_proto)
>+{
>+      return n_proto == htons(ETH_P_IP) && ip_proto == IPPROTO_ICMP;
>+}
>+
>+static inline bool flow_protos_are_icmpv6(__be16 n_proto, u8 ip_proto)
>+{
>+      return n_proto == htons(ETH_P_IPV6) && ip_proto == IPPROTO_ICMPV6;
>+}
>+
>+static inline bool flow_protos_are_icmp_any(__be16 n_proto, u8 ip_proto)
>+{
>+      return flow_protos_are_icmpv4(n_proto, ip_proto) ||
>+              flow_protos_are_icmpv6(n_proto, ip_proto);
>+}
>+
>+static inline bool flow_basic_key_is_icmpv4(const struct 
>flow_dissector_key_basic *basic)
>+{
>+      return flow_protos_are_icmpv4(basic->n_proto, basic->ip_proto);
>+}
>+
>+static inline bool flow_basic_key_is_icmpv6(const struct 
>flow_dissector_key_basic *basic)
>+{
>+      return flow_protos_are_icmpv6(basic->n_proto, basic->ip_proto);
>+}
>+
>+static inline bool flow_keys_are_icmp_any(const struct flow_keys *keys)
>+{
>+      return flow_protos_are_icmp_any(keys->basic.n_proto,
>+                                      keys->basic.ip_proto);
>+}
>+
> static inline bool flow_keys_have_l4(const struct flow_keys *keys)
> {
>       return (keys->ports.ports || keys->tags.flow_label);
>diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>index 1eb6f949e5b2..e37ff021a19e 100644
>--- a/net/core/flow_dissector.c
>+++ b/net/core/flow_dissector.c
>@@ -58,6 +58,28 @@ void skb_flow_dissector_init(struct flow_dissector 
>*flow_dissector,
> EXPORT_SYMBOL(skb_flow_dissector_init);
> 
> /**
>+ * skb_flow_get_be16 - extract be16 entity
>+ * @skb: sk_buff to extract from
>+ * @poff: offset to extract at
>+ * @data: raw buffer pointer to the packet
>+ * @hlen: packet header length
>+ *
>+ * The function will try to retrieve a be32 entity at
>+ * offset poff
>+ */
>+__be16 skb_flow_get_be16(const struct sk_buff *skb, int poff, void *data,
>+                       int hlen)
>+{
>+      __be16 *u, _u;
>+
>+      u = __skb_header_pointer(skb, poff, sizeof(_u), data, hlen, &_u);
>+      if (u)
>+              return *u;
>+
>+      return 0;
>+}
>+
>+/**
>  * __skb_flow_get_ports - extract the upper layer ports and return them
>  * @skb: sk_buff to extract the ports from
>  * @thoff: transport header offset
>@@ -117,6 +139,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>       struct flow_dissector_key_basic *key_basic;
>       struct flow_dissector_key_addrs *key_addrs;
>       struct flow_dissector_key_ports *key_ports;
>+      struct flow_dissector_key_icmp *key_icmp;
>       struct flow_dissector_key_tags *key_tags;
>       struct flow_dissector_key_vlan *key_vlan;
>       struct flow_dissector_key_keyid *key_keyid;
>@@ -537,13 +560,25 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>               break;
>       }
> 
>-      if (dissector_uses_key(flow_dissector,
>-                             FLOW_DISSECTOR_KEY_PORTS)) {
>-              key_ports = skb_flow_dissector_target(flow_dissector,
>-                                                    FLOW_DISSECTOR_KEY_PORTS,
>-                                                    target_container);
>-              key_ports->ports = __skb_flow_get_ports(skb, nhoff, ip_proto,
>-                                                      data, hlen);
>+      if (flow_protos_are_icmp_any(proto, ip_proto)) {

You don't need this.

Just left FLOW_DISSECTOR_KEY_PORTS untouched, and add:

        if (dissector_uses_key(flow_dissector,
                               FLOW_DISSECTOR_KEY_ICMP)) {
                        ....


>+              if (dissector_uses_key(flow_dissector,
>+                                     FLOW_DISSECTOR_KEY_ICMP)) {
>+                      key_icmp = skb_flow_dissector_target(flow_dissector,
>+                                                           
>FLOW_DISSECTOR_KEY_ICMP,
>+                                                           target_container);
>+                      key_icmp->icmp = skb_flow_get_be16(skb, nhoff, data,
>+                                                         hlen);
>+              }
>+      } else {
>+              if (dissector_uses_key(flow_dissector,
>+                                     FLOW_DISSECTOR_KEY_PORTS)) {
>+                      key_ports = skb_flow_dissector_target(flow_dissector,
>+                                                            
>FLOW_DISSECTOR_KEY_PORTS,
>+                                                            target_container);
>+                      key_ports->ports = __skb_flow_get_ports(skb, nhoff,
>+                                                              ip_proto, data,
>+                                                              hlen);
>+              }
>       }
> 
> out_good:
>-- 
>2.7.0.rc3.207.g0ac5344
>

Reply via email to