Hi,

I found a problem the userspace datapath failed to create a 
new datapath flow for the QinQ packet. When getting mask from
nlattrs with the mask include ip,udp,etc, it parsed 802.1Q
header wrongly.

This patch fixes it.  Any ideas?  

Regards
Yunjian

err log:
log_odp_key_attributes[4709]|93701|odp_util(handler18)|: present but not 
expected: ipv4 icmp: 
skb_priority(0),tunnel(ttl=0,flags(0)),skb_mark(0xffffffff),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),recirc_id(0xffffffff),dp_hash(0),in_port(4294967295),eth(src=ff:ff:ff:ff:ff:ff,dst=ff:ff:ff:ff:ff:ff),eth_type(0xffff),vlan(vid=4095,pcp=7),encap(eth_type(0xffff),ipv4(src=255.255.0.0,dst=0.0.0.0,proto=0,tos=0,ttl=0,frag=<error>),icmp(type=0,code=0)

lib/odp-util.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 238 insertions(+), 1 deletion(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 1e8c5f1..7ad8cc6 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -6322,6 +6322,240 @@ parse_ethertype(const struct nlattr 
*attrs[OVS_KEY_ATTR_MAX + 1],
     return true;
}

+static bool
+parse_l2_5(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
+                  uint64_t present_attrs, uint64_t *expected_attrs,
+                  struct flow *flow, const struct flow *src_flow)
+{
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+    bool is_mask = src_flow != flow;
+    const void *check_start = NULL;
+    size_t check_len = 0;
+    enum ovs_key_attr expected_bit = 0xff;
+
+    if (eth_type_mpls(src_flow->dl_type)) {
+        if (!is_mask || present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_MPLS)) {
+            *expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_MPLS);
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_MPLS)) {
+            size_t size = nl_attr_get_size(attrs[OVS_KEY_ATTR_MPLS]);
+            const ovs_be32 *mpls_lse = nl_attr_get(attrs[OVS_KEY_ATTR_MPLS]);
+            int n = size / sizeof(ovs_be32);
+            int i;
+
+            if (!size || size % sizeof(ovs_be32)) {
+                return false;
+            }
+            if (flow->mpls_lse[0] && flow->dl_type != htons(0xffff)) {
+                return false;
+            }
+
+            for (i = 0; i < n && i < FLOW_MAX_MPLS_LABELS; i++) {
+                flow->mpls_lse[i] = mpls_lse[i];
+            }
+            if (n > FLOW_MAX_MPLS_LABELS) {
+                return false;
+            }
+
+            if (!is_mask) {
+                /* BOS may be set only in the innermost label. */
+                for (i = 0; i < n - 1; i++) {
+                    if (flow->mpls_lse[i] & htonl(MPLS_BOS_MASK)) {
+                        return false;
+                    }
+                }
+
+                /* BOS must be set in the innermost label. */
+                if (n < FLOW_MAX_MPLS_LABELS
+                    && !(flow->mpls_lse[n - 1] & htonl(MPLS_BOS_MASK))) {
+                    return false;
+                }
+            }
+        }
+
+        return true;
+    } else if (src_flow->dl_type == htons(ETH_TYPE_IP)) {
+        if (!is_mask) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IPV4;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV4)) {
+            const struct ovs_key_ipv4 *ipv4_key;
+
+            ipv4_key = nl_attr_get(attrs[OVS_KEY_ATTR_IPV4]);
+            put_ipv4_key(ipv4_key, flow, is_mask);
+            if (flow->nw_frag > FLOW_NW_FRAG_MASK) {
+                return false;
+            }
+            if (is_mask) {
+                check_start = ipv4_key;
+                check_len = sizeof *ipv4_key;
+                expected_bit = OVS_KEY_ATTR_IPV4;
+            }
+        }
+    } else if (src_flow->dl_type == htons(ETH_TYPE_IPV6)) {
+        if (!is_mask) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IPV6;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV6)) {
+            const struct ovs_key_ipv6 *ipv6_key;
+
+            ipv6_key = nl_attr_get(attrs[OVS_KEY_ATTR_IPV6]);
+            put_ipv6_key(ipv6_key, flow, is_mask);
+            if (flow->nw_frag > FLOW_NW_FRAG_MASK) {
+                return false;
+            }
+            if (is_mask) {
+                check_start = ipv6_key;
+                check_len = sizeof *ipv6_key;
+                expected_bit = OVS_KEY_ATTR_IPV6;
+            }
+        }
+    } else if (src_flow->dl_type == htons(ETH_TYPE_ARP) ||
+               src_flow->dl_type == htons(ETH_TYPE_RARP)) {
+        if (!is_mask) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ARP;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ARP)) {
+            const struct ovs_key_arp *arp_key;
+
+            arp_key = nl_attr_get(attrs[OVS_KEY_ATTR_ARP]);
+            if (!is_mask && (arp_key->arp_op & htons(0xff00))) {
+                VLOG_ERR_RL(&rl, "unsupported ARP opcode %"PRIu16" in flow "
+                            "key", ntohs(arp_key->arp_op));
+                return false;
+            }
+            put_arp_key(arp_key, flow);
+            if (is_mask) {
+                check_start = arp_key;
+                check_len = sizeof *arp_key;
+                expected_bit = OVS_KEY_ATTR_ARP;
+            }
+        }
+    } else {
+        return true;
+    }
+    if (check_len > 0) { /* Happens only when 'is_mask'. */
+        if (!is_all_zeros(check_start, check_len) &&
+            flow->dl_type != htons(0xffff)) {
+            return false;
+        } else {
+            *expected_attrs |= UINT64_C(1) << expected_bit;
+        }
+    }
+
+    expected_bit = OVS_KEY_ATTR_UNSPEC;
+    if (src_flow->nw_proto == IPPROTO_TCP
+        && (src_flow->dl_type == htons(ETH_TYPE_IP) ||
+            src_flow->dl_type == htons(ETH_TYPE_IPV6))
+        && !(src_flow->nw_frag & FLOW_NW_FRAG_LATER)) {
+        if (!is_mask) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_TCP;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_TCP)) {
+            const union ovs_key_tp *tcp_key;
+
+            tcp_key = nl_attr_get(attrs[OVS_KEY_ATTR_TCP]);
+            put_tp_key(tcp_key, flow);
+            expected_bit = OVS_KEY_ATTR_TCP;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_TCP_FLAGS)) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_TCP_FLAGS;
+            flow->tcp_flags = nl_attr_get_be16(attrs[OVS_KEY_ATTR_TCP_FLAGS]);
+        }
+    } else if (src_flow->nw_proto == IPPROTO_UDP
+               && (src_flow->dl_type == htons(ETH_TYPE_IP) ||
+                   src_flow->dl_type == htons(ETH_TYPE_IPV6))
+               && !(src_flow->nw_frag & FLOW_NW_FRAG_LATER)) {
+        if (!is_mask) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_UDP;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_UDP)) {
+            const union ovs_key_tp *udp_key;
+
+            udp_key = nl_attr_get(attrs[OVS_KEY_ATTR_UDP]);
+            put_tp_key(udp_key, flow);
+            expected_bit = OVS_KEY_ATTR_UDP;
+        }
+    } else if (src_flow->nw_proto == IPPROTO_SCTP
+               && (src_flow->dl_type == htons(ETH_TYPE_IP) ||
+                   src_flow->dl_type == htons(ETH_TYPE_IPV6))
+               && !(src_flow->nw_frag & FLOW_NW_FRAG_LATER)) {
+        if (!is_mask) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_SCTP;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_SCTP)) {
+            const union ovs_key_tp *sctp_key;
+
+            sctp_key = nl_attr_get(attrs[OVS_KEY_ATTR_SCTP]);
+            put_tp_key(sctp_key, flow);
+            expected_bit = OVS_KEY_ATTR_SCTP;
+        }
+    } else if (src_flow->nw_proto == IPPROTO_ICMP
+               && src_flow->dl_type == htons(ETH_TYPE_IP)
+               && !(src_flow->nw_frag & FLOW_NW_FRAG_LATER)) {
+        if (!is_mask) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ICMP;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ICMP)) {
+            const struct ovs_key_icmp *icmp_key;
+
+            icmp_key = nl_attr_get(attrs[OVS_KEY_ATTR_ICMP]);
+            flow->tp_src = htons(icmp_key->icmp_type);
+            flow->tp_dst = htons(icmp_key->icmp_code);
+            expected_bit = OVS_KEY_ATTR_ICMP;
+        }
+    } else if (src_flow->nw_proto == IPPROTO_ICMPV6
+               && src_flow->dl_type == htons(ETH_TYPE_IPV6)
+               && !(src_flow->nw_frag & FLOW_NW_FRAG_LATER)) {
+        if (!is_mask) {
+            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ICMPV6;
+        }
+        if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ICMPV6)) {
+            const struct ovs_key_icmpv6 *icmpv6_key;
+
+            icmpv6_key = nl_attr_get(attrs[OVS_KEY_ATTR_ICMPV6]);
+            flow->tp_src = htons(icmpv6_key->icmpv6_type);
+            flow->tp_dst = htons(icmpv6_key->icmpv6_code);
+            expected_bit = OVS_KEY_ATTR_ICMPV6;
+            if (is_nd(src_flow, NULL)) {
+                if (!is_mask) {
+                    *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
+                }
+                if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ND)) {
+                    const struct ovs_key_nd *nd_key;
+
+                    nd_key = nl_attr_get(attrs[OVS_KEY_ATTR_ND]);
+                    flow->nd_target = nd_key->nd_target;
+                    flow->arp_sha = nd_key->nd_sll;
+                    flow->arp_tha = nd_key->nd_tll;
+                    if (is_mask) {
+                        /* Even though 'tp_src' and 'tp_dst' are 16 bits wide,
+                         * ICMP type and code are 8 bits wide.  Therefore, an
+                         * exact match looks like htons(0xff), not
+                         * htons(0xffff).  See xlate_wc_finish() for details.
+                         * */
+                        if (!is_all_zeros(nd_key, sizeof *nd_key) &&
+                            (flow->tp_src != htons(0xff) ||
+                             flow->tp_dst != htons(0xff))) {
+                            return false;
+                        } else {
+                            *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
+                        }
+                    }
+                }
+            }
+        }
+    }
+    if (is_mask && expected_bit != OVS_KEY_ATTR_UNSPEC) {
+        if ((flow->tp_src || flow->tp_dst) && flow->nw_proto != 0xff) {
+            return false;
+        } else {
+            *expected_attrs |= UINT64_C(1) << expected_bit;
+        }
+    }
+    return true;
+}
+
static enum odp_key_fitness
parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
                   uint64_t present_attrs, int out_of_range_attr,
@@ -6658,7 +6892,10 @@ parse_8021q_onward(const struct nlattr 
*attrs[OVS_KEY_ATTR_MAX + 1],
                              flow, src_flow)) {
             return ODP_FIT_ERROR;
         }
-
+        if (!parse_l2_5(attrs, present_attrs, &expected_attrs,
+                             flow, src_flow)) {
+            return ODP_FIT_ERROR;
+        }
         encaps++;
     }

-- 
1.8.3.1
_______________________________________________
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss

Reply via email to