> -----Original Message-----
> From: Pai G, Sunil <[email protected]>
> Sent: Tuesday 12 July 2022 11:48
> To: Finn, Emma <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; Van Haaren, Harry
> <[email protected]>; Amber, Kumar <[email protected]>
> Subject: RE: [ovs-dev] [v8 00/10] Actions Infrastructure + Optimizations
>
> Hi Emma,
> Thanks for the series.
> As discussed internally, I looked over the patches and have few comments. Hope
> you can address them.
>
>
> > -----Original Message-----
> > From: dev <[email protected]> On Behalf Of Emma Finn
> > Sent: Thursday, July 7, 2022 9:09 PM
> > To: [email protected]; [email protected]; Van Haaren, Harry
> > <[email protected]>; Amber, Kumar <[email protected]>
> > Cc: [email protected]
> > Subject: [ovs-dev] [v8 00/10] Actions Infrastructure + Optimizations
> >
> > This patchset introduces actions infrastructure changes which allows the
> > user to choose between different action implementations based on CPU ISA
> > by using different commands. The infrastructure also provides a way to
> > check the correctness of the ISA optimized action version against the
> > scalar version.
> >
> > This series also introduces optimized versions of the following
> > actions:
> > - push_vlan
> > - pop_vlan
> > - set_masked eth
> > - set_masked ipv4
>
>
> I ran the series with ubsan and asan enabled, it reported couple of errors.
>
> configure command:
> ./configure CFLAGS="-march=native -msse4.2 -O1 -fno-omit-frame-pointer -
> fno-common -g -fsanitize=address,undefined -fno-sanitize-recover=all" \
> --with-dpdk=static CC=clang --enable-autovalidator --enable-mfex-default-
> autovalidator --enable-dpif-default-avx512 --enable-actions-default-
> autovalidator
>
> and ran the testcases with:
> make check TESTSUITEFLAGS="-j32"
>
> resulted in two test cases failing :
>
> 2481: mcast - check multicasts to trunk ports are not duplicated FAILED
> (mcast-
> snooping.at:64)
>
> lib/odp-execute-avx512.c:220:45: runtime error: left shift of 48390 by 16
> places
> cannot be represented in type 'int'
> #0 0x10652e3 in action_avx512_push_vlan
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute-avx512.c:220:45
> #1 0xc7bf56 in action_autoval_generic
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute-private.c:194:9
> #2 0xc72739 in odp_execute_actions
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute.c:1015:13
> #3 0xb9692e in dp_netdev_execute_actions
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:9115:5
> #4 0xb9692e in handle_packet_upcall
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:8303:5
> #5 0xb9540a in fast_path_processing
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:8399:25
> #6 0xb6e580 in dp_netdev_input__ /usr/local/home/spaig1/expt/ovs/lib/dpif-
> netdev.c:8488:9
> #7 0xb8c122 in dp_netdev_input /usr/local/home/spaig1/expt/ovs/lib/dpif-
> netdev.c:8526:5
> #8 0xb8c122 in dp_netdev_process_rxq_port
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:5339:13
> #9 0xb6ff7d in dpif_netdev_run /usr/local/home/spaig1/expt/ovs/lib/dpif-
> netdev.c:6672:25
> #10 0xbb3134 in dpif_run /usr/local/home/spaig1/expt/ovs/lib/dpif.c:467:16
> #11 0xa58611 in type_run /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-
> dpif.c:366:9
> #12 0xa23e3b in ofproto_type_run
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto.c:1822:31
> #13 0x9eb95f in bridge_run__
> /usr/local/home/spaig1/expt/ovs/vswitchd/bridge.c:3245:9
> #14 0x9e948e in bridge_run
> /usr/local/home/spaig1/expt/ovs/vswitchd/bridge.c:3310:5
> #15 0xa0c478 in main /usr/local/home/spaig1/expt/ovs/vswitchd/ovs-
> vswitchd.c:129:9
> #16 0x7f518432c082 in __libc_start_main /build/glibc-SzIz7B/glibc-
> 2.31/csu/../csu/libc-start.c:308:16
> #17 0x5ab4bd in _start
> (/usr/local/home/spaig1/expt/networking.dataplane.ovs.ovs/vswitchd/ovs-
> vswitchd+0x5ab4bd)
>
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior lib/odp-execute-
> avx512.c:220:45 in
>
>
> Seems the following lines are causing problem in odp-execute-avx512.c:
>
> /* Build up the VLAN TCI/TPID in a single uint32_t. */
> const uint16_t tci_proc = tci & htons(~VLAN_CFI);
> const uint32_t tpid_tci = (tci_proc << 16) | tpid;
>
> We should consider changing tci_proc from uint16_t to something bigger -
> uint32_t perhaps.
>
>
>
> And ..
>
> 1116: 1116: ofproto-dpif - dec_ttl FAILED
> (ofproto-dpif.at:1322)
>
> =================================================================
> ==2956174==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x604000058c7d at pc 0x000001066756 bp 0x7ffcc83ce170 sp 0x7ffcc83ce168
> READ of size 32 at 0x604000058c7d thread T0
> #0 0x1066755 in action_avx512_ipv4_set_addrs
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute-avx512.c:438:28
> #1 0x10653e5 in action_avx512_set_masked
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute-avx512.c:497:9
> #2 0xc7bf56 in action_autoval_generic
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute-private.c:194:9
> #3 0xc72739 in odp_execute_actions
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute.c:1015:13
> #4 0xb7501e in dp_netdev_execute_actions
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:9115:5
> #5 0xb7501e in dpif_netdev_execute
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:4567:5
> #6 0xb7501e in dpif_netdev_operate
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:4616:25
> #7 0xbb7071 in dpif_operate
> /usr/local/home/spaig1/expt/ovs/lib/dpif.c:1372:13
> #8 0xbb8569 in dpif_execute
> /usr/local/home/spaig1/expt/ovs/lib/dpif.c:1326:9
> #9 0xaad2db in execute_actions_except_outputs
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-dpif-trace.c:652:17
> #10 0xaad2db in ofproto_trace__
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-dpif-trace.c:787:13
> #11 0xaabf96 in ofproto_trace
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-dpif-trace.c:815:5
> #12 0xaad909 in ofproto_unixctl_trace
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-dpif-trace.c:485:9
> #13 0xe3a9ef in process_command
> /usr/local/home/spaig1/expt/ovs/lib/unixctl.c:310:13
> #14 0xe3a9ef in run_connection
> /usr/local/home/spaig1/expt/ovs/lib/unixctl.c:344:17
> #15 0xe3a9ef in unixctl_server_run
> /usr/local/home/spaig1/expt/ovs/lib/unixctl.c:395:21
> #16 0xa0c490 in main /usr/local/home/spaig1/expt/ovs/vswitchd/ovs-
> vswitchd.c:130:9
> #17 0x7fc3724ca082 in __libc_start_main /build/glibc-SzIz7B/glibc-
> 2.31/csu/../csu/libc-start.c:308:16
> #18 0x5ab4bd in _start
> (/usr/local/home/spaig1/expt/networking.dataplane.ovs.ovs/vswitchd/ovs-
> vswitchd+0x5ab4bd)
>
> 0x604000058c7d is located 11 bytes to the right of 34-byte region
> [0x604000058c50,0x604000058c72)
> allocated by thread T0 here:
> #0 0x627c6d in malloc
> (/usr/local/home/spaig1/expt/networking.dataplane.ovs.ovs/vswitchd/ovs-
> vswitchd+0x627c6d)
> #1 0xe3c2bc in xmalloc__ /usr/local/home/spaig1/expt/ovs/lib/util.c:137:15
> #2 0xe3c2bc in xmalloc /usr/local/home/spaig1/expt/ovs/lib/util.c:172:12
> #3 0xb66d33 in dp_packet_init /usr/local/home/spaig1/expt/ovs/lib/dp-
> packet.c:126:29
> #4 0xb66d33 in dp_packet_new /usr/local/home/spaig1/expt/ovs/lib/dp-
> packet.c:154:5
> #5 0xb66d33 in dp_packet_new_with_headroom
> /usr/local/home/spaig1/expt/ovs/lib/dp-packet.c:163:27
> #6 0xb66d33 in dp_packet_clone_data_with_headroom
> /usr/local/home/spaig1/expt/ovs/lib/dp-packet.c:222:27
> #7 0xb669fd in dp_packet_clone_with_headroom
> /usr/local/home/spaig1/expt/ovs/lib/dp-packet.c:186:18
> #8 0xc7cb6a in dp_packet_batch_clone
> /usr/local/home/spaig1/expt/ovs/./lib/dp-packet.h:863:22
> #9 0xc7bf07 in action_autoval_generic
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute-private.c:193:9
> #10 0xc72739 in odp_execute_actions
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute.c:1015:13
> #11 0xb7501e in dp_netdev_execute_actions
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:9115:5
> #12 0xb7501e in dpif_netdev_execute
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:4567:5
> #13 0xb7501e in dpif_netdev_operate
> /usr/local/home/spaig1/expt/ovs/lib/dpif-netdev.c:4616:25
> #14 0xbb7071 in dpif_operate
> /usr/local/home/spaig1/expt/ovs/lib/dpif.c:1372:13
> #15 0xbb8569 in dpif_execute
> /usr/local/home/spaig1/expt/ovs/lib/dpif.c:1326:9
> #16 0xaad2db in execute_actions_except_outputs
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-dpif-trace.c:652:17
> #17 0xaad2db in ofproto_trace__
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-dpif-trace.c:787:13
> #18 0xaabf96 in ofproto_trace
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-dpif-trace.c:815:5
> #19 0xaad909 in ofproto_unixctl_trace
> /usr/local/home/spaig1/expt/ovs/ofproto/ofproto-dpif-trace.c:485:9
> #20 0xe3a9ef in process_command
> /usr/local/home/spaig1/expt/ovs/lib/unixctl.c:310:13
> #21 0xe3a9ef in run_connection
> /usr/local/home/spaig1/expt/ovs/lib/unixctl.c:344:17
> #22 0xe3a9ef in unixctl_server_run
> /usr/local/home/spaig1/expt/ovs/lib/unixctl.c:395:21
> #23 0xa0c490 in main /usr/local/home/spaig1/expt/ovs/vswitchd/ovs-
> vswitchd.c:130:9
> #24 0x7fc3724ca082 in __libc_start_main /build/glibc-SzIz7B/glibc-
> 2.31/csu/../csu/libc-start.c:308:16
>
> SUMMARY: AddressSanitizer: heap-buffer-overflow
> /usr/local/home/spaig1/expt/ovs/lib/odp-execute-avx512.c:438:28 in
> action_avx512_ipv4_set_addrs
> Shadow bytes around the buggy address:
> 0x0c0880003130: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00
> 0x0c0880003140: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00
> 0x0c0880003150: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00
> 0x0c0880003160: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00
> 0x0c0880003170: fa fa 00 00 00 00 04 fa fa fa 00 00 00 00 02 fa
> =>0x0c0880003180: fa fa 00 00 00 00 02 fa fa fa 00 00 00 00 02[fa]
> 0x0c0880003190: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> 0x0c08800031a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> 0x0c08800031b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> 0x0c08800031c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> 0x0c08800031d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> ==2956174==ABORTING
>
> Seems Asan is not happy with action_avx512_ipv4_set_addrs.
> Would you please consider fixing these ?
>
> Thanks and regards
> Sunil
>
Thanks Sunil, I have root caused and will have these issues resolved in the
next revision.
Thanks,
Emma
>
>
>
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
