onload
onload copied to clipboard
Project does not build with clang, and does not link with GCC 10
Firstly, onload's external headers don't parse correctly with clang, so even if you build onload with GCC and then everything else uses clang, you can't #include
onload from clang without error. Here is our patch:
# openonload headers included by XXX are not compatible with clang, so fix that now
file(GLOB_RECURSE openonloadheaders "${XXX_ROOT_DIR}/thirdparty/openonload/*/debug.h")
foreach(openonloadheader ${openonloadheaders})
file(READ "${openonloadheader}" origcontents)
string(REPLACE "[%s=\"IPX_FMT\"]\"" "[%s=\" IPX_FMT \"]\"" newcontents "${origcontents}")
if(NOT origcontents STREQUAL newcontents)
message(STATUS "NOTE: Fixed illegal C++ in '${openonloadheader}'")
file(WRITE "${openonloadheader}" "${newcontents}")
endif()
endforeach()
Secondly, attempting to build onload with clang fails badly. You appear to be using quite a bit of GCC-specific stuff. It would be great if you fixed this, as we currently have to special case only onload when we are compiling with clang.
Thirdly, onload won't link when built with GCC 10 due to:
ci_bpf_oobpf_elf_sh.o:(.bss+0x0): multiple definition of `citp_signal_data'
ci_bpf_oobpf_sh.o:(.bss+0x0): first defined here
collect2: error: ld returned 1 exit status
make[3]: *** [oobpfintf0.so] Error 1
Also, your configure script can't handle GCC 10:
gcc_major_version() {
gcc --version |head -1 |sed "s/^gcc.*) \([1-9]*\)\..*$/\1/"
}
This doesn't account for GCC 10.
Hi, thanks for trying Onload.
Your second and third issues are no longer relevant to this repository - the offending code was removed/changed some time ago. For support with formal Onload releases please use [email protected]
Your first issue is something we should definitely clean up. To help us prioritise the problem, can you please tell us what the top-level file you originally #included in your source file was?
Good to hear about the second and third issues being fixed. I'll pass that on to the others.
For the first issue, it's a widespread problem best illustrated using https://godbolt.org/z/q65nxG
To fix, you need spaces between all macros and their surrounding string literals, in all your header files. That's what the above cmake patch does for all debug.h
files in all our many versions of openonload.
Hi Niall,
My colleague was interested in knowing which of the external header files you were trying to include from your application to help direct our effort.
You are also most welcome to raise a pull request with the changes your recommend!
Thanks, Andy
We currently use these open onloads:
- onload-7.0.0.176
- onload-7.1.0.265
- openonload-201310-u1
- openonload-201405-u2
- openonload-201502-u3
- openonload-201509
- openonload-201606
- openonload-201606-u1.2
- openonload-201606-u1.3
- openonload-201805
- openonload-201811
- openonload-201811-u1
As a result, we cannot avoid source patching in any case, but I figured it would be nice to let you know of the issue.
In terms of which header files we use, I think these are the approximate set:
- #include <etherfabric/vi.h>
- #include <etherfabric/pd.h>
- #include <etherfabric/memreg.h>
- #include <etherfabric/packedstream.h>
- #include <ci/tools.h>
- #include <ci/tools/ippacket.h>
- #include <ci/net/ipv4.h>
In any case, it doesn't matter: you can't have macros in between string literals in C++ 11 onwards without whitespace separating them. So, for example, you can't have this in valid C++ 11 code: https://github.com/Xilinx-CNS/onload/blob/master/src/include/ci/tools/debug.h#L220 where "where [%s="IPX_FMT"]"
would need to become "where [%s=" IPX_FMT "]"
to become valid C++ 11.
If you search your source code for the case sensitive regex "[^"]+"[A-Z_]+"[^"]+"
, that gives a nice list of candidates to fix.