[ovs-dev] [PATCH] packets: Fix using outdated RSS hash after MPLS decapsulation.

Nitin Katiyar nitin.katiyar at ericsson.com
Wed Aug 28 07:03:23 UTC 2019



> -----Original Message-----
> From: Ilya Maximets [mailto:i.maximets at samsung.com]
> Sent: Tuesday, August 27, 2019 5:37 PM
> To: Nitin Katiyar <nitin.katiyar at ericsson.com>; ovs-dev at openvswitch.org
> Cc: Stokes, Ian <ian.stokes at intel.com>
> Subject: Re: [ovs-dev] [PATCH] packets: Fix using outdated RSS hash after
> MPLS decapsulation.
> 
> Hi.
> 
> Since this is not a first version of a patch, the subject prefix should be [PATCH
> v2]. For the next version, please, use [PATCH v3].
> This way it's much easier to recognize/track patches in a list or mailbox.
Hi,
Since the title got of the patch got changed so I didn't make it V2. I wasn't sure to use V2 as with new title it becomes new thread. Anyway, I will be sending the new patch with V3.

> 
> Hint: It's generally speeds up review process if relevant people are
>       in CC list while sending the patch. Not everyone subscribed for
>       all the messages in a list.
Thanks for suggestion. I will follow it in future.

> 
> The code looks good in general.
> Few comments inline.
> 
> Best regards, Ilya Maximets.
> 
> On 16.08.2019 21:54, 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 MPLS encapsulated packet is received, the MPLS header is popped
> > and the packet is recirculated. Since the RSS hash has not been
> > invalidated here, the EMC lookup will hit the same entry as that
> > before recirculation. This degrades performance severely.
> 
> Above sentences are not correct. It's stated that the collision happens with
> the previous pass of the same packet, but it happens with different packets
> from the same MPLS tunnel. This should be fixed.
It will actually collide with the entry of packet before de-capsulation. I have rephrased it as follows:

    When a MPLS encapsulated packet is received, the MPLS header is popped
    and the packet is recirculated. Since the RSS hash has not been
    invalidated here, the EMC lookup for decapsulated packets will hit the
    same entry as that before recirculation (i.e. entry of the packet with
    MPLS encapsulation). This degrades performance severely as different
    inner packets from the same MPLS tunnel would hit the same EMC entry.

Let me know if it is more clear now.

Thanks,
Nitin
> 
> >
> > This patch invalidates RSS hash (by resetting offload flags) after
> > MPLS header is popped.
> >
> > Signed-off-by: Nitin Katiyar <nitin.katiyar at ericsson.com>
> > ---
> >  lib/packets.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/packets.c b/lib/packets.c index ab0b1a3..db3f6d0
> > 100644
> > --- a/lib/packets.c
> > +++ b/lib/packets.c
> > @@ -404,6 +404,10 @@ pop_mpls(struct dp_packet *packet, ovs_be16
> ethtype)
> >          struct mpls_hdr *mh = dp_packet_l2_5(packet);
> >          size_t len = packet->l2_5_ofs;
> >
> > +        /* Invalidate offload flags as they are not valid after
> > +         * decapsulation of MPLS header. */
> > +        dp_packet_reset_offload(packet);
> 
> Maybe it's better to move this code down to 'dp_packet_resize_l2_5' at the
> end of this function to have similar changes closer?
> 
Sure, I will move it to end  of function.
> > +
> >          set_ethertype(packet, ethtype);
> >          if (get_16aligned_be32(&mh->mpls_lse) & htonl(MPLS_BOS_MASK)) {
> >              dp_packet_set_l2_5(packet, NULL);
> >


More information about the dev mailing list