Thanks for reviewing and merging the rest of the series!
> On 20 Apr 2015, at 21:04, Ethan Jackson <[email protected]> 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
> <[email protected]> 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 <[email protected]>
>> Signed-off-by: Daniele Di Proietto <[email protected]>
>> ---
>> 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
>> [email protected]
>> 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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
