Xilinx-CNS / onload

OpenOnload high performance user-level network stack
Other
580 stars 95 forks source link

EF_SIGNALS_NOPOSTPONE might be handled incorrectly due to wrong type in "1<<(signum-1)" #29

Closed dmitriisilin closed 3 years ago

dmitriisilin commented 3 years ago

src/lib/transport/ip/signal.c

if (our_info && our_info->inside_lib && (CITP_OPTS.signals_no_postpone & (1 << (signum-1))) == 0)

4 bytes integer instead of 8 bytes is getting shifted

rhughes-xilinx commented 3 years ago

I agree that this is suboptimal, but it's ultimately harmless: there's no reason to pass any of the realtime signals to EF_SIGNALS_NOPOSTPONE because they already have they behaviour anyway.

If you'd like to propose a pull request to clean it up, I'd happily accept it.

dmitriisilin commented 3 years ago

I was trying to build project, to make sure my changes will pass unit tests before opening a merge request. However I'm experiencing build issues. Looks as kernel_compat.h needs some explicit casts. I'll try to workaround failures and see how it goes. In case, I discover more issues with build system, is there a "pre-integration pipeline" for the project?, In other words, can I propose a fix for the original problem without testing?

make[3]: Entering directory `/usr/src/kernels/3.10.0-229.el7.x86_64' CC /home/dsilin/ws/github_onload/onload/build/x86_64_linux-3.10.0-229.el7.x86_64/lib/cplane/mib.o CC /home/dsilin/ws/github_onload/onload/build/x86_64_linux-3.10.0-229.el7.x86_64/lib/cplane/mib_fwd.o CC /home/dsilin/ws/github_onload/onload/build/x86_64_linux-3.10.0-229.el7.x86_64/lib/cplane/services.o CC /home/dsilin/ws/github_onload/onload/build/x86_64_linux-3.10.0-229.el7.x86_64/lib/cplane/onload.o In file included from /home/dsilin/ws/github_onload/onload/build/x86_64_linux-3.10.0-229.el7.x86_64/lib/cplane/onload.c:8:0: /home/dsilin/ws/github_onload/onload/src/include/ci/driver/kernel_compat.h: In function \u2018pin_user_pages\u2019: /home/dsilin/ws/github_onload/onload/src/include/ci/driver/kernel_compat.h:163:42: error: passing argument 1 of \u2018get_user_pages\u2019 makes pointer from integer without a cast [-Werror] pages, vmas); ^ In file included from include/linux/scatterlist.h:6:0, from include/linux/dmaengine.h:27, from include/linux/skbuff.h:31, from include/linux/icmpv6.h:4, from include/linux/ipv6.h:59, from /home/dsilin/ws/github_onload/onload/src/include/ci/tools/platform/linux_kernel.h:54, from /home/dsilin/ws/github_onload/onload/src/include/ci/tools/sysdep.h:56, from /home/dsilin/ws/github_onload/onload/src/include/cplane/mib.h:25, from /home/dsilin/ws/github_onload/onload/src/include/cplane/cplane.h:21, from /home/dsilin/ws/github_onload/onload/build/x86_64_linux-3.10.0-229.el7.x86_64/lib/cplane/onload.c:4: include/linux/mm.h:1159:6: note: expected \u2018struct task_struct \u2019 but argument is of type \u2018long unsigned int\u2019 long get_user_pages(struct task_struct tsk, struct mm_struct *mm,

rhughes-xilinx commented 3 years ago

We no longer support RHEL7.1 (and Red Hat stopped supporting it 4 years ago).

ol-alexandra commented 3 years ago

I've created internal bug ON-13290. I'll fix it in the master branch and for upcoming onload-7.1.2.

@rhughes-xilinx

there's no reason to pass any of the realtime signals to EF_SIGNALS_NOPOSTPONE because they already have they behaviour anyway

Can you explain, please?

rhughes-xilinx commented 3 years ago

there's no reason to pass any of the realtime signals to EF_SIGNALS_NOPOSTPONE because they already have they behaviour anyway

Can you explain, please?

No, I can't: I misread the code.

ol-alexandra commented 3 years ago

Fixed in master and onload-7.1. If we ever release an update for EOL6, it will be included there as well.