On 12 Jul 2022, at 13:45, Finn, Emma wrote:
>> -----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.
Is this the v9 that just was sent out by Harry?
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
