On 2/21/18 9:22 AM, Ido Schimmel wrote:
>> +u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
>> + const struct sk_buff *skb)
>> {
>> struct flow_keys hash_keys;
>> u32 mhash;
>>
>> - memset(&hash_keys, 0, sizeof(hash_keys));
>> - hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
>> - if (skb) {
>> - ip6_multipath_l3_keys(skb, &hash_keys);
>> - } else {
>> - hash_keys.addrs.v6addrs.src = fl6->saddr;
>> - hash_keys.addrs.v6addrs.dst = fl6->daddr;
>> - hash_keys.tags.flow_label = (__force u32)fl6->flowlabel;
>> - hash_keys.basic.ip_proto = fl6->flowi6_proto;
>> + switch (net->ipv6.sysctl.multipath_hash_policy) {
>> + case 0:
>> + memset(&hash_keys, 0, sizeof(hash_keys));
>> + hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
>> + if (skb) {
>> + ip6_multipath_l3_keys(skb, &hash_keys);
>> + } else {
>> + hash_keys.addrs.v6addrs.src = fl6->saddr;
>> + hash_keys.addrs.v6addrs.dst = fl6->daddr;
>> + hash_keys.tags.flow_label = (__force u32)fl6->flowlabel;
>> + hash_keys.basic.ip_proto = fl6->flowi6_proto;
>> + }
>> + break;
>> + case 1:
>> + if (skb) {
>> + unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP;
>> + struct flow_keys keys;
>> +
>> + /* short-circuit if we already have L4 hash present */
>> + if (skb->l4_hash)
>> + return skb_get_hash_raw(skb) >> 1;
>> +
>> + memset(&hash_keys, 0, sizeof(hash_keys));
>> +
>> + skb_flow_dissect_flow_keys(skb, &keys, flag);
>> +
>> + hash_keys.addrs.v6addrs.src = keys.addrs.v6addrs.src;
>> + hash_keys.addrs.v6addrs.dst = keys.addrs.v6addrs.dst;
>
> Shouldn't you add:
>
> hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
>
> ?
>
> Otherwise flow_hash_from_keys() will not consistentify the ports and the
> addresses and it will also not use the addresses for the hash
> computation.
>
> It's missing from fib_multipath_hash() as well, so I might be missing
> something.
Yes, I think you are correct. The oversight here is based on the missing
set in the ipv4 variant.
>
>> + hash_keys.ports.src = keys.ports.src;
>> + hash_keys.ports.dst = keys.ports.dst;
>> + hash_keys.tags.flow_label = keys.tags.flow_label;
>
> Why are you using the flow label?
will remove. It is supposed to be an L4 hash.
