> -----Original Message-----
> From: Ilya Maximets <[email protected]>
> Sent: Wednesday 7 December 2022 19:09
> To: Finn, Emma <[email protected]>; [email protected]
> Cc: [email protected]; Van Haaren, Harry <[email protected]>;
> Stokes, Ian <[email protected]>; Eelco Chaudron
> <[email protected]>
> Subject: Re: [PATCH] odp-execute: Fix ipv4 missing clearing of connection
> tracking fields.
> 
> On 12/6/22 15:18, Emma Finn wrote:
> > This patch add clearing of connection tracking fields to the
> > avx512 implementation of the ipv4 action. This patch also extends the
> > actions autovalidator to include a compare for packet metadata.
> >
> > Signed-off-by: Emma Finn <[email protected]>
> > ---
> >  lib/odp-execute-avx512.c  |  2 ++
> >  lib/odp-execute-private.c | 12 ++++++++++++
> >  2 files changed, 14 insertions(+)
> 
> Thanks!  That fixes the issue with the conntrack for me.
> 
> The patch is missing the Fixes tag.  I added one.  With that, applied and
> backported to 3.0.
> 
> 
> However, there is one more difference I spotted between avx512 and the
> scalar code.  It's the 'l4_size' check before writing to the L4 header.
> 
> In the packet_set_ipv4_addr() we have:
> 
>     if (nh->ip_proto == IPPROTO_TCP && l4_size >= TCP_HEADER_LEN) {
>         ...
>     } else if (nh->ip_proto == IPPROTO_UDP && l4_size >= UDP_HEADER_LEN )
> {
>         ...
>     }
> 
> But we don't have these 'l4_size' checks in avx512 implementations.
> 
> These checks are there to catch any truncated packets that do not really have
> a full L4 header in them, so they supposed to prevent writes outside of the
> actual packet.
> 
> I'm not sure how to trigger the issue in tests, but since we're trying to keep
> the implementations functionally identical, we need to add these checks to
> action_avx512_ipv4_set_addrs().
> 
> Or am I missing something?
> 
Sure, I will add those checks and send another patch for this ipv4 fix. 

Thanks, 
Emma 

> Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to