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 >