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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
