On 14.08.2019 11:33, Nitin Katiyar wrote: > > >> -----Original Message----- >> From: Ilya Maximets [mailto:i.maxim...@samsung.com] >> Sent: Wednesday, August 14, 2019 1:42 PM >> To: Nitin Katiyar <nitin.kati...@ericsson.com>; ovs-dev@openvswitch.org >> Cc: Stokes, Ian <ian.sto...@intel.com> >> Subject: Re: [ovs-dev] [PATCH] Improve MPLSoGRE performance by reducing >> EMC hash collisions. >> >> On 13.08.2019 20:49, Nitin Katiyar wrote: >>> When a packet is received, the RSS hash is calculated if it is not >>> already available. The Exact Match Cache (EMC) entry is then looked up >>> using this RSS hash. >>> >>> When a MPLSoGRE encapsulated packet is received, the GRE header is >>> popped, the RSS hash is invalidated and the packet is recirculated for >>> further processing. When the recirculated packet is processed, the >>> MPLS header is popped and the packet is recirculated again. Since the >>> RSS hash has not been invalidated here, the EMC lookup will hit the >>> same entry as that after the first recirculation. This degrades >>> performance severely. >> > Hi Ilya, >> Hmm. Function 'dpif_netdev_packet_get_rss_hash()' accounts recirculation >> depth into the hash to avoid such cases. Why this doesn't work for you? > This will not very for multiple inner flows. If there are multiple flows are > sent across same tunnel (with same MPLS label) then they will all lead to > same hash as external tuple and recirculation id remains same for them. Let > me know if I am wrong.
OK. I see. Collision is not with the previous pass of this packet but with other packets from the same MPLS tunnel. This needs to be made clear in the commit message. And, IIUC, this issue is not related to MPLSoGRE specifically, it's just an issue with any MPLS. I think, you need to re-write the commit message to be more general and, probably, rename the patch to something like: "packets: Invalidate offloads after MPLS decapsulation." or "packets: Fix using outdated RSS hash after MPLS decapsulation." etc. Another thing: It looks like we need to do the same for NSH decap. What do you think? Note: This change will force hash re-calculation in case of output to balanced bonding. Also, It's better to not mention EMC in a common 'lib/packets.c' file. I think that it's enough to just say that we need to invalidate offloads since they are not valid anymore after decapsulation. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
