On 6/23/22 17:39, David Marchand wrote:
> On Thu, Jun 23, 2022 at 3:33 PM Van Haaren, Harry
> <[email protected]> wrote:
>>
>>> -----Original Message-----
>>> From: dev <[email protected]> On Behalf Of Ales Musil
>>> Sent: Thursday, June 23, 2022 1:41 PM
>>> To: [email protected]
>>> Cc: [email protected]
>>> Subject: [ovs-dev] [PATCH] cpu.c: Build cpu.c without any assumptions about
>>> CPU flags
>>>
>>> The cpu.c was built with multiple feature flags that might
>>> not be available on all hosts. It manifested itself as
>>> Illegal instruction with crashing on "shlx" instruction
>>> which is part of the bmi2 extension, that might not be available
>>> on all CPU that openvswitch runs on.
>>>
>>> Move the cpu.c and cpu.h to lib_libopenvswitch_la_SOURCES which
>>> should build it without any assumptions about CPU capabilities.
>>
>> Although this seems a good solution, the CI shows a linking failure;
>> https://mail.openvswitch.org/pipermail/ovs-build/2022-June/022795.html
> 
> From the link error, the "normal" ovs archive seems to be passed when
> linking binaries.
> 
> libtool: link: gcc -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare
> -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum
> -Wunused-parameter -Wbad-function-cast -Wcast-align
> -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes
> -Wmissing-field-initializers -fno-strict-aliasing -Wswitch-bool
> -Wlogical-not-parentheses -Wsizeof-array-argument -Wbool-compare
> -Wshift-negative-value -Wduplicated-cond -Wshadow
> -Wmultistatement-macros -Wcast-align=strict -mssse3
> -I/home/dmarchan/git/pub/dpdk.org/v21.11/install/include -include
> rte_config.h -I/usr/usr/include -Werror -Werror -D_FILE_OFFSET_BITS=64
> -g -DOPENSSL_API_COMPAT=0x10100000L -march=x86-64-v2 -o
> utilities/ovs-appctl utilities/ovs-appctl.o -Wl,--as-needed
> lib/.libs/libopenvswitch.a
> -L/home/dmarchan/git/pub/dpdk.org/v21.11/install/lib64 -lssl -lcrypto
> -lbpf /home/dmarchan/git/pub/ovs/master/lib/.libs/libopenvswitchavx512.a
> -lrte_node -lrte_graph -lrte_flow_classify -lrte_pipeline -lrte_table
> -lrte_pdump -lrte_port -lrte_fib -lrte_ipsec -lrte_vhost -lrte_stack
> -lrte_security -lrte_sched -lrte_reorder -lrte_rib -lrte_dmadev
> -lrte_regexdev -lrte_rawdev -lrte_power -lrte_pcapng -lrte_member
> -lrte_lpm -lrte_latencystats -lrte_kni -lrte_jobstats -lrte_ip_frag
> -lrte_gso -lrte_gro -lrte_gpudev -lrte_eventdev -lrte_efd
> -lrte_distributor -lrte_cryptodev -lrte_compressdev -lrte_cfgfile
> -lrte_bpf -lrte_bitratestats -lrte_bbdev -lrte_acl -lrte_timer
> -lrte_hash -lrte_metrics -lrte_cmdline -lrte_pci -lrte_ethdev
> -lrte_meter -lrte_net -lrte_mbuf -lrte_mempool -lrte_rcu -lrte_ring
> -lrte_eal -lrte_telemetry -lrte_kvargs -lbsd -lmlx4 -libverbs -lmlx5
> -lpcap -lnuma -latomic -lm
> 
> For this error on cpu_has_isa to happen, the linker probably judged no
> symbol from cpu.c could be used and decided to drop the whole .o.
> So we need something to make sure those won't be dropped.
> 
> I just saw Ilya proposal before clicking Send, but it does not seem to
> work (or maybe I did something wrong on my side..).
> 

Hmm, yes, you're right.  The issue is a bit larger, and I think
I already brought that up at some point.  Basically all the
functions that use cpu_has_isa should be moved to a generic
libopenvswitch library out of libopenvswitchavx512, because they
are supposed to check for CPU capabilities, but are built with
all the avx512 flags enabled, hence may contain illegal instructions.

So, the correct solution should be, along with moving cpu.{c,h}, to
also move:
  dp_netdev_input_outer_avx512_probe to lib/dpif-netdev-private-dpif.c
  mfex_avx512_probe to lib/dpif-netdev-private-extract.c
  dpcls_subtable_avx512_gather_probe to lib/dpif-netdev-lookup.c

The last one is a bit tricky, because CHECK_LOOKUP_FUNCTION() and
DECLARE_OPTIMIZED_LOOKUP_FUNCTION() will have to be correctly
exposed, i.e. DECLARE_OPTIMIZED_LOOKUP_FUNCTION macro will need
to declare non-static functions and their prototypes should be
declared in the lib/dpif-netdev-lookup.h.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to