There's a corner case in ovs_flow_extract() that seems a bit
troublesome.  For an ARP with an opcode other than 1 or 2, we fill in
key->ip.proto with the opcode but we do not adjust key_len to include
the ARP info (so it won't get hashed).  However, in
ovs_flow_to_nlattrs() we do include the opcode.in what we send to
userspace, and in ovs_flow_from_nlattrs() we do include the ARP info
(so it will get hashed).

I think that we need to move the assignment to key_len from where it
is here:
    if (key->ip.proto == ARPOP_REQUEST
        || key->ip.proto == ARPOP_REPLY) {
            memcpy(&key->ipv4.addr.src, arp->ar_sip, 
sizeof(key->ipv4.addr.src));
            memcpy(&key->ipv4.addr.dst, arp->ar_tip, 
sizeof(key->ipv4.addr.dst));
            memcpy(key->ipv4.arp.sha, arp->ar_sha, ETH_ALEN);
            memcpy(key->ipv4.arp.tha, arp->ar_tha, ETH_ALEN);
            key_len = SW_FLOW_KEY_OFFSET(ipv4.arp);
    }
to outside the conditional block.

However, this is by inspection and I haven't tested anything, so I'm
just sending this out as an observation so far.

Comments?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to