Yes, you are correct. I am misled by the fact that the bit mask starts with two
0s from right to left (has discontiguous 1-bits). This can be
explained by the byte order.
However, there are still some cases that byte order cannot explain. 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.
At 2016-03-05 07:36:25, "Ben Pfaff" <b...@ovn.org> wrote:
>On Fri, Mar 04, 2016 at 02:51:42PM +0800, Peng He wrote:
>> Hi,
>> I installed some rulesets generated by ClassBench (a ruleset benchmark
>> for generating ACL and FW rules) in Open vSwitch, and I find many megaflow's
>> mask has
>> discontiguous bits. Like mask = 0xfc in network byte order. After some code
>> investigation I find the problem is in the function *mask_set_prefix_bits*:
>>
>>
>>
>>
>> staticvoidmask_set_prefix_bits(struct flow_wildcards *wc, uint8_t be32ofs,
>> unsignedint n_bits)
>> {
>> ovs_be32 *mask = &((ovs_be32 *)&wc->masks)[be32ofs];
>> unsignedint i;
>>
>> for (i = 0; i < n_bits / 32; i++) {
>> mask[i] = OVS_BE32_MAX;
>> }if (n_bits % 32) {
>> mask[i] |= htonl(~0u << (32 - n_bits % 32));
>> }}
>> For example, if the input n_bits = 6, the mask should be 0x2f, as there is 6
>> contiguous bits. However, the current code will generate 0xfc:
>> (0xFFFFFFFF << (32 - 6) ) = 0xFC000000
>> and htonl (0xFC000000) = 0xFC.
>> I am writing this to make sure that I am correct. The OVS code I use is 2.4
>> release version.
>
>0xffffffff << (32 - 6) == 0xfc000000, which yields the bytes "fc 00 00
>00" in hex, or "11111100 00000000 00000000 00000000" in binary, when
>stored into memory in network byte order.
>
>I don't see the problem.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
