From a7cd39e0c7805a93eaa4256370bcd48c506d46c1 Mon Sep 17 00:00:00 2001 From: Pieter Jansen van Vuuren Date: Fri, 25 Aug 2017 19:31:01 +0200 Subject: [PATCH 1/3] nfp: fix unchecked flow dissector use Previously flow dissectors were referenced without first checking that they are in use and correctly populated by TC. This patch fixes this by checking each flow dissector key before referencing them. Fixes: 5571e8c9f241 ("nfp: extend flower matching capabilities") Signed-off-by: Pieter Jansen van Vuuren Reviewed-by: Jakub Kicinski Reviewed-by: Simon Horman Signed-off-by: David S. Miller --- .../net/ethernet/netronome/nfp/flower/match.c | 133 +++++++++--------- .../ethernet/netronome/nfp/flower/offload.c | 41 +++--- 2 files changed, 93 insertions(+), 81 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c index 0e08404480ef..b36511063a25 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/match.c +++ b/drivers/net/ethernet/netronome/nfp/flower/match.c @@ -45,6 +45,7 @@ nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame, struct flow_dissector_key_vlan *flow_vlan; u16 tmp_tci; + memset(frame, 0, sizeof(struct nfp_flower_meta_two)); /* Populate the metadata frame. */ frame->nfp_flow_key_layer = key_type; frame->mask_id = ~0; @@ -54,21 +55,20 @@ nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame, return; } - flow_vlan = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_VLAN, - flow->key); - - /* Populate the tci field. */ - if (!flow_vlan->vlan_id) { - tmp_tci = 0; - } else { - tmp_tci = FIELD_PREP(NFP_FLOWER_MASK_VLAN_PRIO, - flow_vlan->vlan_priority) | - FIELD_PREP(NFP_FLOWER_MASK_VLAN_VID, - flow_vlan->vlan_id) | - NFP_FLOWER_MASK_VLAN_CFI; + if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_VLAN)) { + flow_vlan = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_VLAN, + flow->key); + /* Populate the tci field. */ + if (flow_vlan->vlan_id) { + tmp_tci = FIELD_PREP(NFP_FLOWER_MASK_VLAN_PRIO, + flow_vlan->vlan_priority) | + FIELD_PREP(NFP_FLOWER_MASK_VLAN_VID, + flow_vlan->vlan_id) | + NFP_FLOWER_MASK_VLAN_CFI; + frame->tci = cpu_to_be16(tmp_tci); + } } - frame->tci = cpu_to_be16(tmp_tci); } static void @@ -99,17 +99,18 @@ nfp_flower_compile_mac(struct nfp_flower_mac_mpls *frame, bool mask_version) { struct fl_flow_key *target = mask_version ? flow->mask : flow->key; - struct flow_dissector_key_eth_addrs *flow_mac; - - flow_mac = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_ETH_ADDRS, - target); + struct flow_dissector_key_eth_addrs *addr; memset(frame, 0, sizeof(struct nfp_flower_mac_mpls)); - /* Populate mac frame. */ - ether_addr_copy(frame->mac_dst, &flow_mac->dst[0]); - ether_addr_copy(frame->mac_src, &flow_mac->src[0]); + if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS)) { + addr = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_ETH_ADDRS, + target); + /* Populate mac frame. */ + ether_addr_copy(frame->mac_dst, &addr->dst[0]); + ether_addr_copy(frame->mac_src, &addr->src[0]); + } if (mask_version) frame->mpls_lse = cpu_to_be32(~0); @@ -121,14 +122,17 @@ nfp_flower_compile_tport(struct nfp_flower_tp_ports *frame, bool mask_version) { struct fl_flow_key *target = mask_version ? flow->mask : flow->key; - struct flow_dissector_key_ports *flow_tp; + struct flow_dissector_key_ports *tp; - flow_tp = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_PORTS, - target); + memset(frame, 0, sizeof(struct nfp_flower_tp_ports)); - frame->port_src = flow_tp->src; - frame->port_dst = flow_tp->dst; + if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_PORTS)) { + tp = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_PORTS, + target); + frame->port_src = tp->src; + frame->port_dst = tp->dst; + } } static void @@ -137,25 +141,27 @@ nfp_flower_compile_ipv4(struct nfp_flower_ipv4 *frame, bool mask_version) { struct fl_flow_key *target = mask_version ? flow->mask : flow->key; - struct flow_dissector_key_ipv4_addrs *flow_ipv4; - struct flow_dissector_key_basic *flow_basic; + struct flow_dissector_key_ipv4_addrs *addr; + struct flow_dissector_key_basic *basic; - flow_ipv4 = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_IPV4_ADDRS, - target); - - flow_basic = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_BASIC, - target); - - /* Populate IPv4 frame. */ - frame->reserved = 0; - frame->ipv4_src = flow_ipv4->src; - frame->ipv4_dst = flow_ipv4->dst; - frame->proto = flow_basic->ip_proto; /* Wildcard TOS/TTL for now. */ - frame->tos = 0; - frame->ttl = 0; + memset(frame, 0, sizeof(struct nfp_flower_ipv4)); + + if (dissector_uses_key(flow->dissector, + FLOW_DISSECTOR_KEY_IPV4_ADDRS)) { + addr = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_IPV4_ADDRS, + target); + frame->ipv4_src = addr->src; + frame->ipv4_dst = addr->dst; + } + + if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_BASIC)) { + basic = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_BASIC, + target); + frame->proto = basic->ip_proto; + } } static void @@ -164,26 +170,27 @@ nfp_flower_compile_ipv6(struct nfp_flower_ipv6 *frame, bool mask_version) { struct fl_flow_key *target = mask_version ? flow->mask : flow->key; - struct flow_dissector_key_ipv6_addrs *flow_ipv6; - struct flow_dissector_key_basic *flow_basic; + struct flow_dissector_key_ipv6_addrs *addr; + struct flow_dissector_key_basic *basic; - flow_ipv6 = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_IPV6_ADDRS, - target); - - flow_basic = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_BASIC, - target); - - /* Populate IPv6 frame. */ - frame->reserved = 0; - frame->ipv6_src = flow_ipv6->src; - frame->ipv6_dst = flow_ipv6->dst; - frame->proto = flow_basic->ip_proto; /* Wildcard LABEL/TOS/TTL for now. */ - frame->ipv6_flow_label_exthdr = 0; - frame->tos = 0; - frame->ttl = 0; + memset(frame, 0, sizeof(struct nfp_flower_ipv6)); + + if (dissector_uses_key(flow->dissector, + FLOW_DISSECTOR_KEY_IPV6_ADDRS)) { + addr = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_IPV6_ADDRS, + target); + frame->ipv6_src = addr->src; + frame->ipv6_dst = addr->dst; + } + + if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_BASIC)) { + basic = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_BASIC, + target); + frame->proto = basic->ip_proto; + } } int nfp_flower_compile_flow_match(struct tc_cls_flower_offload *flow, diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c index 4ad10bd5e139..6c8ecc211568 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/offload.c +++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c @@ -105,35 +105,40 @@ static int nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls, struct tc_cls_flower_offload *flow) { - struct flow_dissector_key_control *mask_enc_ctl; - struct flow_dissector_key_basic *mask_basic; - struct flow_dissector_key_basic *key_basic; + struct flow_dissector_key_basic *mask_basic = NULL; + struct flow_dissector_key_basic *key_basic = NULL; u32 key_layer_two; u8 key_layer; int key_size; - mask_enc_ctl = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_ENC_CONTROL, - flow->mask); + if (dissector_uses_key(flow->dissector, + FLOW_DISSECTOR_KEY_ENC_CONTROL)) { + struct flow_dissector_key_control *mask_enc_ctl = + skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_ENC_CONTROL, + flow->mask); + /* We are expecting a tunnel. For now we ignore offloading. */ + if (mask_enc_ctl->addr_type) + return -EOPNOTSUPP; + } - mask_basic = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_BASIC, - flow->mask); + if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_BASIC)) { + mask_basic = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_BASIC, + flow->mask); + + key_basic = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_BASIC, + flow->key); + } - key_basic = skb_flow_dissector_target(flow->dissector, - FLOW_DISSECTOR_KEY_BASIC, - flow->key); key_layer_two = 0; key_layer = NFP_FLOWER_LAYER_PORT | NFP_FLOWER_LAYER_MAC; key_size = sizeof(struct nfp_flower_meta_one) + sizeof(struct nfp_flower_in_port) + sizeof(struct nfp_flower_mac_mpls); - /* We are expecting a tunnel. For now we ignore offloading. */ - if (mask_enc_ctl->addr_type) - return -EOPNOTSUPP; - - if (mask_basic->n_proto) { + if (mask_basic && mask_basic->n_proto) { /* Ethernet type is present in the key. */ switch (key_basic->n_proto) { case cpu_to_be16(ETH_P_IP): @@ -166,7 +171,7 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls, } } - if (mask_basic->ip_proto) { + if (mask_basic && mask_basic->ip_proto) { /* Ethernet type is present in the key. */ switch (key_basic->ip_proto) { case IPPROTO_TCP: From 74af5975108f54f9443952c0b4d52487031a7569 Mon Sep 17 00:00:00 2001 From: Pieter Jansen van Vuuren Date: Fri, 25 Aug 2017 19:31:02 +0200 Subject: [PATCH 2/3] nfp: fix supported key layers calculation Previously when calculating the supported key layers MPLS, IPv4/6 TTL and TOS were not considered. This patch checks that the TTL and TOS fields are masked out before offloading. Additionally this patch checks that MPLS packets are correctly handled, by not offloading them. Fixes: af9d842c1354 ("nfp: extend flower add flow offload") Signed-off-by: Pieter Jansen van Vuuren Reviewed-by: Jakub Kicinski Reviewed-by: Simon Horman Signed-off-by: David S. Miller --- .../ethernet/netronome/nfp/flower/offload.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c index 6c8ecc211568..74a96d6bb05c 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/offload.c +++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c @@ -107,6 +107,7 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls, { struct flow_dissector_key_basic *mask_basic = NULL; struct flow_dissector_key_basic *key_basic = NULL; + struct flow_dissector_key_ip *mask_ip = NULL; u32 key_layer_two; u8 key_layer; int key_size; @@ -132,6 +133,11 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls, flow->key); } + if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_IP)) + mask_ip = skb_flow_dissector_target(flow->dissector, + FLOW_DISSECTOR_KEY_IP, + flow->mask); + key_layer_two = 0; key_layer = NFP_FLOWER_LAYER_PORT | NFP_FLOWER_LAYER_MAC; key_size = sizeof(struct nfp_flower_meta_one) + @@ -142,11 +148,19 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls, /* Ethernet type is present in the key. */ switch (key_basic->n_proto) { case cpu_to_be16(ETH_P_IP): + if (mask_ip && mask_ip->tos) + return -EOPNOTSUPP; + if (mask_ip && mask_ip->ttl) + return -EOPNOTSUPP; key_layer |= NFP_FLOWER_LAYER_IPV4; key_size += sizeof(struct nfp_flower_ipv4); break; case cpu_to_be16(ETH_P_IPV6): + if (mask_ip && mask_ip->tos) + return -EOPNOTSUPP; + if (mask_ip && mask_ip->ttl) + return -EOPNOTSUPP; key_layer |= NFP_FLOWER_LAYER_IPV6; key_size += sizeof(struct nfp_flower_ipv6); break; @@ -157,6 +171,11 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls, case cpu_to_be16(ETH_P_ARP): return -EOPNOTSUPP; + /* Currently we do not offload MPLS. */ + case cpu_to_be16(ETH_P_MPLS_UC): + case cpu_to_be16(ETH_P_MPLS_MC): + return -EOPNOTSUPP; + /* Will be included in layer 2. */ case cpu_to_be16(ETH_P_8021Q): break; From 6afd33e4384060e692705912337b184c1e159aff Mon Sep 17 00:00:00 2001 From: Pieter Jansen van Vuuren Date: Fri, 25 Aug 2017 19:31:03 +0200 Subject: [PATCH 3/3] nfp: remove incorrect mask check for vlan matching Previously the vlan tci field was incorrectly exact matched. This patch fixes this by using the flow dissector to populate the vlan tci field. Fixes: 5571e8c9f241 ("nfp: extend flower matching capabilities") Signed-off-by: Pieter Jansen van Vuuren Reviewed-by: Jakub Kicinski Reviewed-by: Simon Horman Signed-off-by: David S. Miller --- drivers/net/ethernet/netronome/nfp/flower/match.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c index b36511063a25..d25b5038c3a2 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/match.c +++ b/drivers/net/ethernet/netronome/nfp/flower/match.c @@ -42,6 +42,7 @@ nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame, struct tc_cls_flower_offload *flow, u8 key_type, bool mask_version) { + struct fl_flow_key *target = mask_version ? flow->mask : flow->key; struct flow_dissector_key_vlan *flow_vlan; u16 tmp_tci; @@ -50,15 +51,10 @@ nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame, frame->nfp_flow_key_layer = key_type; frame->mask_id = ~0; - if (mask_version) { - frame->tci = cpu_to_be16(~0); - return; - } - if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_VLAN)) { flow_vlan = skb_flow_dissector_target(flow->dissector, FLOW_DISSECTOR_KEY_VLAN, - flow->key); + target); /* Populate the tci field. */ if (flow_vlan->vlan_id) { tmp_tci = FIELD_PREP(NFP_FLOWER_MASK_VLAN_PRIO,