On 20 Jun 2024, at 13:01, Finn, Emma wrote:
>> -----Original Message----- >> From: Eelco Chaudron <[email protected]> >> Sent: Thursday, June 20, 2024 9:48 AM >> To: Finn, Emma <[email protected]> >> Cc: [email protected]; [email protected] >> Subject: Re: [v2] odp-execute: Check IPv4 checksum offload flag in AVX. >> >> On 17 Jun 2024, at 16:08, Emma Finn wrote: >> >>> The AVX implementation for IPv4 action did not check whether the IPv4 >>> checksum offload flag has been set and was incorrectly calculating >>> checksums in software. Adding a check to skip AVX checksum calculation >>> when offload flags are set. >>> >>> Signed-off-by: Emma Finn <[email protected]> >>> Reported-by: Eelco Chaudron <[email protected]> >> >> This is missing a fixes tag. And maybe you can also add which test is >> failing, so >> people reviewing know what to look for. >> >> ‘nsh - triangle PTAP bridge setup with NSH over vxlan-gpe’ >> >>> --- >>> lib/odp-execute-avx512.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c index >>> 569ea789e..54bd556e1 100644 >>> --- a/lib/odp-execute-avx512.c >>> +++ b/lib/odp-execute-avx512.c >>> @@ -473,7 +473,7 @@ action_avx512_ipv4_set_addrs(struct >> dp_packet_batch *batch, >>> * (v_pkt_masked). */ >>> __m256i v_new_hdr = _mm256_or_si256(v_key_shuf, >>> v_pkt_masked); >>> >>> - if (dp_packet_hwol_tx_ip_csum(packet)) { >>> + if (dp_packet_hwol_l3_ipv4(packet)) { >> >> I’m trying to understand why this change is needed. The scaler >> implementation is working fine with this check. Is something not initialized >> correctly in the AVX implementation? > > Sure, I can send v3 with fixes tag and failing test info. > > The test nsh - triangle PTAP bridge setup with NSH over vxlan-gpe, was > failing the autovalidator because the AVX implementation was calculating a > checksum for the outer IPv4 header when it shouldn't have been. > Previously with just checking dp_packet_hwol_tx_ip_csum(), the AVX > implementation was only checking if packet was marked for IPv4 csum offload. > It was never checking if the packet was encapsulated AND the outer layer is > marked for IPv4 checksum offload. The scalar does this check also in > packet_set_ipv4_addr(). You are right I misread the diff :( Let’s wait with the v3 as I know Mike was going to take a look at this also. If he finds nothing, I can make a suggestion for the change and apply. Cheers, Eelco >> >>> dp_packet_ol_reset_ip_csum_good(packet); >>> } else { >>> ovs_be16 old_csum = ~nh->ip_csum; >>> -- >>> 2.34.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
