After an online discussion I've decided to go with your solution, which appears
to make maintenance easier.
I sent a v2 to the list
Thanks!
> On 21 Apr 2015, at 12:00, Daniele Di Proietto <diproiet...@vmware.com> wrote:
>
> Thanks for reviewing and merging the rest of the series!
>
>> On 20 Apr 2015, at 21:04, Ethan Jackson <et...@nicira.com> wrote:
>>
>> I don't really like this. For one thing, Suppose in a particular
>> stage no changes to the packet are made. There's a good chance you'll
>> recompute the same hash and still collide.
>
> All the recirculation-like actions change the miniflow:
>
> * OVS_ACTION_ATTR_TUNNEL_PUSH and OVS_ACTION_ATTR_TUNNEL_POP change the
> packet and the metadata
> * OVS_ACTION_RECIRC changes the recirc_id in the metadata.
>
>> What if instead, in the emc code if the depth > 0, you folded it into
>> the hash for the lookup? Very simple change that I think addresses
>> these issues.
>
> You mean like commit 28465887 ("datapath: update exact match lookup
> hash value to avoid hash collision")? I've chosen to reset the
> RSS hash and force a recalculation because it seemed to take better
> into account the actual changes to the miniflow.
>
> I'll wait for your final feedback
>
> Thanks,
>
> Daniele
>
>> Ethan
>>
>> On Wed, Apr 15, 2015 at 11:11 AM, Daniele Di Proietto
>> <diproiet...@vmware.com> wrote:
>>> Having the same RSS hash after recirculation can cause unnecessary
>>> collisions in the exact match cache. Setting the RSS hash to 0 forces
>>> the datapath to compute a new value and account for the changes in the
>>> packet or in the metadata.
>>>
>>> Requested-by: Ethan Jackson <et...@nicira.com>
>>> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>>> ---
>>> lib/dpif-netdev.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index 20bb498..28262e6 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -3404,6 +3404,10 @@ dp_execute_cb(void *aux_, struct dp_packet
>>> **packets, int cnt,
>>>
>>> err = push_tnl_action(dp, a, packets, cnt);
>>> if (!err) {
>>> + for (i = 0; i < cnt; i++) {
>>> + dp_packet_set_rss_hash(packets[i], 0);
>>> + }
>>> +
>>> (*depth)++;
>>> dp_netdev_input(pmd, packets, cnt);
>>> (*depth)--;
>>> @@ -3433,6 +3437,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>>> int cnt,
>>>
>>> for (i = 0; i < cnt; i++) {
>>> packets[i]->md.in_port.odp_port = portno;
>>> + dp_packet_set_rss_hash(packets[i], 0);
>>> }
>>>
>>> (*depth)++;
>>> @@ -3491,6 +3496,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>>> int cnt,
>>>
>>> for (i = 0; i < cnt; i++) {
>>> packets[i]->md.recirc_id = nl_attr_get_u32(a);
>>> + dp_packet_set_rss_hash(packets[i], 0);
>>> }
>>>
>>> (*depth)++;
>>> --
>>> 2.1.4
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=cackDRkSgHwFZnZl9QKa8vcw4UtM5jbONZKFShnQP0Y&s=ZR5GoctzN1OjXc7w9q9TmVqH7yRIGwKXm9X14Lwt-Lo&e=
>>>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=sGoNXwbp6lLkuAfuWWlD9Ad_i0eKRaJ1QLnclKyuivU&s=DqBhNsRTdieFJ3nAz6UeZK465sYTpedvVUXFamHbRXk&e=
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
