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