Hi, I send this email once, no people replied. I am trying to refresh this email for a reply. This discontiguous bit mask issue arises in the following case:
I installed two rules, ovs-ofctl add-flow br0 in_port=1,dl_type=0x0800,nw_src=10.2.10.0/24,nw_proto=6,tcp_dst=0x0050,priority=1,actions=drop ovs-ofctl add-flow br0 in_port=1,dl_type=0x0800,nw_src=10.2.0.0/16,nw_proto=6,tcp_dst=0x1f90,priority=1,actions=drop and test the rules with packet (nw_src = 10.2.2.1 and tcp_dst = 8080). I got the nw_src mask (in network byte order) 0x8ffff. Even you change it to host byte order the bit mask still has discontiguous 1-bits 0xFFFF08 = 1111 1111 1111 1111 0000 1000. I checked the code and find the problem lies in the *miniflow_and_mask_matches_flow_wc*. /* If we have narrowed down to a single rule already, check whether * that rule matches. Either way, we're done. * * (Rare) hash collisions may cause us to miss the opportunity for this * optimization. */if (!cmap_node_next(inode)) {conststruct cls_match *head; ASSIGN_CONTAINER(head, inode - i, index_nodes); if (miniflow_and_mask_matches_flow_wc(&head->flow, &subtable->mask, flow, wc)) {/* Return highest priority rule that is visible. */CLS_MATCH_FOR_EACH (rule, head) {if (OVS_LIKELY(cls_match_visible_in_version(rule, version))) {return rule; }}} This function is called in the find_match_wc, that when doing staged lookup if the current hash table contains only one rule, the find_match_wc will directly check if the packet matches the rule. *miniflow_and_mask_matches_flow_wc* still needs to calculate the wildcards mask. In the implementation, if the packet does not match the rule, the function only set the first different bit in the mask. Here is the code in the function: staticinlineboolminiflow_and_mask_matches_flow_wc(conststruct miniflow *flow, conststruct minimask *mask, conststruct flow *target, struct flow_wildcards *wc) {const uint64_t *flowp = miniflow_get_values(flow); const uint64_t *maskp = miniflow_get_values(&mask->masks); int idx; MAP_FOR_EACH_INDEX(idx, mask->masks.map) { uint64_t mask = *maskp++; uint64_t diff = (*flowp++ ^ flow_u64_value(target, idx)) & mask; if (diff) {/* Only unwildcard if none of the differing bits is already * exact-matched. */if (!(flow_u64_value(&wc->masks, idx) & diff)) {/* Keep one bit of the difference. The selected bit may be * different in big-endian v.s. little-endian systems. */ *flow_u64_lvalue(&wc->masks, idx) |= rightmost_1bit(diff); }returnfalse; }/* Fill in the bits that were looked at. */ *flow_u64_lvalue(&wc->masks, idx) |= mask; }returntrue; } Consider this case, two rules reside in two hash tables with different masks. The first tuple contains the rule (10.2.10.0/24, 80), which mismatches the packet (10.2.2.1, 8080), however, the first unmatched bit is at the position of 21, the mask is therefore 0x80000. The second tuple contains (10.2.0.0/16, 8080), which matches the packet, this generates the mask with the first 16 bits set, e.g. 0x8FFFF. However there are four bits unset in this case, and the mask should be 0xF8FFFF (in network byte order), that is the prefix 10.2.0.0/21 that should be put in the megaflow. Hope I am clear and please check the code. Thanks. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev