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
http://openvswitch.org/mailman/listinfo/dev

Reply via email to