> -----Original Message-----
> From: Eelco Chaudron <echau...@redhat.com>
> Sent: Wednesday, July 13, 2022 9:56 AM
> To: Finn, Emma <emma.f...@intel.com>
> Cc: Pai G, Sunil <sunil.pa...@intel.com>; d...@openvswitch.org;
> i.maxim...@ovn.org; Van Haaren, Harry <harry.van.haa...@intel.com>; Amber,
> Kumar <kumar.am...@intel.com>
> Subject: Re: [ovs-dev] [v8 00/10] Actions Infrastructure + Optimizations
> 
> 
> 
> On 12 Jul 2022, at 13:45, Finn, Emma wrote:
> 
> >> -----Original Message-----
> >> From: Pai G, Sunil <sunil.pa...@intel.com>
> >> Sent: Tuesday 12 July 2022 11:48
> >> To: Finn, Emma <emma.f...@intel.com>; d...@openvswitch.org
> >> Cc: i.maxim...@ovn.org; echau...@redhat.com; Van Haaren, Harry
> >> <harry.van.haa...@intel.com>; Amber, Kumar <kumar.am...@intel.com>
> >> 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 <ovs-dev-boun...@openvswitch.org> On Behalf Of Emma Finn
> >>> Sent: Thursday, July 7, 2022 9:09 PM
> >>> To: d...@openvswitch.org; echau...@redhat.com; Van Haaren, Harry
> >>> <harry.van.haa...@intel.com>; Amber, Kumar <kumar.am...@intel.com>
> >>> Cc: i.maxim...@ovn.org
> >>> 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

Hi Eelco,

Yes the v9 that I sent yesterday was Emma's rework, and included the fixes for 
this issue,
and the others that Emma said "will fix in v9".

We've seen a CI failure on OSX/AVX512 builds, which we're currently fixing. A 
v10 will
be posted soon including that (and Ilya's documentation feedback on 6/10).

Regards, -Harry



_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to