| Message ID | 20241030142722.2901744-1-sdf@fomichev.me (mailing list archive) |
|---|---|
| Headers | show |
| Series | selftests: ncdevmem: Add ncdevmem to ksft | expand |
On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > The goal of the series is to simplify and make it possible to use > ncdevmem in an automated way from the ksft python wrapper. > > ncdevmem is slowly mutated into a state where it uses stdout > to print the payload and the python wrapper is added to > make sure the arrived payload matches the expected one. > > v6: > - fix compilation issue in 'Unify error handling' patch (Jakub) > Since I saw a compilation failures on a couple of iterations I cherry-picked this locally and tested compilation. I'm seeing this: sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw TARGETS=ncdevmem 2>&1 make: Entering directory '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' CC ncdevmem In file included from ncdevmem.c:63: /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43: warning: ‘enum ethtool_header_flags’ declared inside parameter list will not be visible outside of this definition or declaration 23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value); | ^~~~~~~~~~~~~~~~~~~~ /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41: warning: ‘enum ethtool_module_fw_flash_status’ declared inside parameter list will not be visible outside of this definition or declaration 25 | ethtool_module_fw_flash_status_str(enum ethtool_module_fw_flash_status value); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45: error: field ‘status’ has incomplete type 6766 | enum ethtool_module_fw_flash_status status; | ^~~~~~ ncdevmem.c: In function ‘do_server’: ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known 517 | struct dmabuf_token token; | ^~~~~ ncdevmem.c:542:47: error: ‘SCM_DEVMEM_DMABUF’ undeclared (first use in this function) 542 | (cm->cmsg_type != SCM_DEVMEM_DMABUF && | ^~~~~~~~~~~~~~~~~ ncdevmem.c:542:47: note: each undeclared identifier is reported only once for each function it appears in ncdevmem.c:543:47: error: ‘SCM_DEVMEM_LINEAR’ undeclared (first use in this function) 543 | cm->cmsg_type != SCM_DEVMEM_LINEAR)) { | ^~~~~~~~~~~~~~~~~ ncdevmem.c:557:52: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 557 | dmabuf_cmsg->frag_size); | ^~ ncdevmem.c:562:56: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 562 | token.token_start = dmabuf_cmsg->frag_token; | ^~ ncdevmem.c:566:42: error: ‘SO_DEVMEM_DONTNEED’ undeclared (first use in this function) 566 | SO_DEVMEM_DONTNEED, &token, | ^~~~~~~~~~~~~~~~~~ ncdevmem.c:573:56: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 573 | token.token_start = dmabuf_cmsg->frag_token; | ^~ ncdevmem.c:576:54: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 576 | total_received += dmabuf_cmsg->frag_size; | ^~ ncdevmem.c:579:44: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 579 | dmabuf_cmsg->frag_offset >> PAGE_SHIFT, | ^~ ncdevmem.c:580:44: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 580 | dmabuf_cmsg->frag_offset % getpagesize(), | ^~ ncdevmem.c:581:44: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 581 | dmabuf_cmsg->frag_offset, | ^~ ncdevmem.c:582:44: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 582 | dmabuf_cmsg->frag_size, dmabuf_cmsg->frag_token, | ^~ ncdevmem.c:582:68: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 582 | dmabuf_cmsg->frag_size, dmabuf_cmsg->frag_token, | ^~ ncdevmem.c:583:60: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 583 | total_received, dmabuf_cmsg->dmabuf_id); | ^~ ncdevmem.c:585:40: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 585 | if (dmabuf_cmsg->dmabuf_id != dmabuf_id) | ^~ ncdevmem.c:589:40: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 589 | if (dmabuf_cmsg->frag_size % getpagesize()) | ^~ ncdevmem.c:595:65: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 595 | dmabuf_cmsg->frag_offset, | ^~ ncdevmem.c:596:65: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 596 | dmabuf_cmsg->frag_size); | ^~ ncdevmem.c:601:60: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 601 | dmabuf_cmsg->frag_offset, | ^~ ncdevmem.c:602:52: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 602 | dmabuf_cmsg->frag_size); | ^~ ncdevmem.c:604:73: error: invalid use of undefined type ‘struct dmabuf_cmsg’ 604 | print_nonzero_bytes(tmp_mem, dmabuf_cmsg->frag_size); | ^~ make: *** [../../../lib.mk:222: /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw/ncdevmem] Error 1 make: Leaving directory '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' The errors are still there even without the CFLAGS="-static". Can you take a look before merge?
On 10/30, Mina Almasry wrote: > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > The goal of the series is to simplify and make it possible to use > > ncdevmem in an automated way from the ksft python wrapper. > > > > ncdevmem is slowly mutated into a state where it uses stdout > > to print the payload and the python wrapper is added to > > make sure the arrived payload matches the expected one. > > > > v6: > > - fix compilation issue in 'Unify error handling' patch (Jakub) > > > > Since I saw a compilation failures on a couple of iterations I > cherry-picked this locally and tested compilation. I'm seeing this: Are you cherry picking the whole series or just this patch? It looks too broken. > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw > TARGETS=ncdevmem 2>&1 > make: Entering directory > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > CC ncdevmem > In file included from ncdevmem.c:63: > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43: > warning: ‘enum ethtool_header_flags’ declared inside parameter list > will not be visible outside of this definition or declaration > 23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value); > | ^~~~~~~~~~~~~~~~~~~~ > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41: > warning: ‘enum ethtool_module_fw_flash_status’ declared inside > parameter list will not be visible outside of this definition or > declaration > 25 | ethtool_module_fw_flash_status_str(enum > ethtool_module_fw_flash_status value); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45: > error: field ‘status’ has incomplete type > 6766 | enum ethtool_module_fw_flash_status status; > | ^~~~~~ This has been fixed via '#include <linux/ethtool_netlink.h>' > ncdevmem.c: In function ‘do_server’: > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known > 517 | struct dmabuf_token token; And this, and the rest, don't make sense at all? I'll double check on my side.
On Wed, Oct 30, 2024 at 8:13 AM Stanislav Fomichev <stfomichev@gmail.com> wrote: > > On 10/30, Mina Almasry wrote: > > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > The goal of the series is to simplify and make it possible to use > > > ncdevmem in an automated way from the ksft python wrapper. > > > > > > ncdevmem is slowly mutated into a state where it uses stdout > > > to print the payload and the python wrapper is added to > > > make sure the arrived payload matches the expected one. > > > > > > v6: > > > - fix compilation issue in 'Unify error handling' patch (Jakub) > > > > > > > Since I saw a compilation failures on a couple of iterations I > > cherry-picked this locally and tested compilation. I'm seeing this: > > Are you cherry picking the whole series or just this patch? It looks > too broken. > > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw > > TARGETS=ncdevmem 2>&1 > > make: Entering directory > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > > CC ncdevmem > > In file included from ncdevmem.c:63: > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43: > > warning: ‘enum ethtool_header_flags’ declared inside parameter list > > will not be visible outside of this definition or declaration > > 23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value); > > | ^~~~~~~~~~~~~~~~~~~~ > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41: > > warning: ‘enum ethtool_module_fw_flash_status’ declared inside > > parameter list will not be visible outside of this definition or > > declaration > > 25 | ethtool_module_fw_flash_status_str(enum > > ethtool_module_fw_flash_status value); > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45: > > error: field ‘status’ has incomplete type > > 6766 | enum ethtool_module_fw_flash_status status; > > | ^~~~~~ > > This has been fixed via '#include <linux/ethtool_netlink.h>' > > > ncdevmem.c: In function ‘do_server’: > > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known > > 517 | struct dmabuf_token token; > > And this, and the rest, don't make sense at all? > > I'll double check on my side. Oh, whoops, I forgot to headers_install first. This works for me: ➜ cos-kernel git:(tcpdevmem-fixes-1) ✗ sudo make headers_install && sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw TARGETS=ncdevmem 2>&1 INSTALL ./usr/include make: Entering directory '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' make: Nothing to be done for 'all'. make: Leaving directory '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' ➜ cos-kernel git:(tcpdevmem-fixes-1) ✗ find . -iname ncdevmem ./tools/testing/selftests/drivers/net/hw/ncdevmem Sorry for the noise :D
On 10/30, Mina Almasry wrote: > On Wed, Oct 30, 2024 at 8:13 AM Stanislav Fomichev <stfomichev@gmail.com> wrote: > > > > On 10/30, Mina Almasry wrote: > > > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > > > The goal of the series is to simplify and make it possible to use > > > > ncdevmem in an automated way from the ksft python wrapper. > > > > > > > > ncdevmem is slowly mutated into a state where it uses stdout > > > > to print the payload and the python wrapper is added to > > > > make sure the arrived payload matches the expected one. > > > > > > > > v6: > > > > - fix compilation issue in 'Unify error handling' patch (Jakub) > > > > > > > > > > Since I saw a compilation failures on a couple of iterations I > > > cherry-picked this locally and tested compilation. I'm seeing this: > > > > Are you cherry picking the whole series or just this patch? It looks > > too broken. > > > > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw > > > TARGETS=ncdevmem 2>&1 > > > make: Entering directory > > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > > > CC ncdevmem > > > In file included from ncdevmem.c:63: > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43: > > > warning: ‘enum ethtool_header_flags’ declared inside parameter list > > > will not be visible outside of this definition or declaration > > > 23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value); > > > | ^~~~~~~~~~~~~~~~~~~~ > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41: > > > warning: ‘enum ethtool_module_fw_flash_status’ declared inside > > > parameter list will not be visible outside of this definition or > > > declaration > > > 25 | ethtool_module_fw_flash_status_str(enum > > > ethtool_module_fw_flash_status value); > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45: > > > error: field ‘status’ has incomplete type > > > 6766 | enum ethtool_module_fw_flash_status status; > > > | ^~~~~~ > > > > This has been fixed via '#include <linux/ethtool_netlink.h>' > > > > > ncdevmem.c: In function ‘do_server’: > > > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known > > > 517 | struct dmabuf_token token; > > > > And this, and the rest, don't make sense at all? > > > > I'll double check on my side. > > Oh, whoops, I forgot to headers_install first. This works for me: > > ➜ cos-kernel git:(tcpdevmem-fixes-1) ✗ sudo make headers_install && > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw > TARGETS=ncdevmem 2>&1 > INSTALL ./usr/include > make: Entering directory > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > make: Nothing to be done for 'all'. > make: Leaving directory > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > ➜ cos-kernel git:(tcpdevmem-fixes-1) ✗ find . -iname ncdevmem > ./tools/testing/selftests/drivers/net/hw/ncdevmem > > Sorry for the noise :D Whew, thanks and no worries!
On Wed, Oct 30, 2024 at 8:37 AM Stanislav Fomichev <stfomichev@gmail.com> wrote: > > On 10/30, Mina Almasry wrote: > > On Wed, Oct 30, 2024 at 8:13 AM Stanislav Fomichev <stfomichev@gmail.com> wrote: > > > > > > On 10/30, Mina Almasry wrote: > > > > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > > > > > The goal of the series is to simplify and make it possible to use > > > > > ncdevmem in an automated way from the ksft python wrapper. > > > > > > > > > > ncdevmem is slowly mutated into a state where it uses stdout > > > > > to print the payload and the python wrapper is added to > > > > > make sure the arrived payload matches the expected one. > > > > > > > > > > v6: > > > > > - fix compilation issue in 'Unify error handling' patch (Jakub) > > > > > > > > > > > > > Since I saw a compilation failures on a couple of iterations I > > > > cherry-picked this locally and tested compilation. I'm seeing this: > > > > > > Are you cherry picking the whole series or just this patch? It looks > > > too broken. > > > > > > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw > > > > TARGETS=ncdevmem 2>&1 > > > > make: Entering directory > > > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > > > > CC ncdevmem > > > > In file included from ncdevmem.c:63: > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43: > > > > warning: ‘enum ethtool_header_flags’ declared inside parameter list > > > > will not be visible outside of this definition or declaration > > > > 23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value); > > > > | ^~~~~~~~~~~~~~~~~~~~ > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41: > > > > warning: ‘enum ethtool_module_fw_flash_status’ declared inside > > > > parameter list will not be visible outside of this definition or > > > > declaration > > > > 25 | ethtool_module_fw_flash_status_str(enum > > > > ethtool_module_fw_flash_status value); > > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45: > > > > error: field ‘status’ has incomplete type > > > > 6766 | enum ethtool_module_fw_flash_status status; > > > > | ^~~~~~ > > > > > > This has been fixed via '#include <linux/ethtool_netlink.h>' > > > > > > > ncdevmem.c: In function ‘do_server’: > > > > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known > > > > 517 | struct dmabuf_token token; > > > > > > And this, and the rest, don't make sense at all? > > > > > > I'll double check on my side. > > > > Oh, whoops, I forgot to headers_install first. This works for me: > > > > ➜ cos-kernel git:(tcpdevmem-fixes-1) ✗ sudo make headers_install && > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw > > TARGETS=ncdevmem 2>&1 > > INSTALL ./usr/include > > make: Entering directory > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > > make: Nothing to be done for 'all'. > > make: Leaving directory > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > > ➜ cos-kernel git:(tcpdevmem-fixes-1) ✗ find . -iname ncdevmem > > ./tools/testing/selftests/drivers/net/hw/ncdevmem > > > > Sorry for the noise :D > > Whew, thanks and no worries! Sorry, 2 issues testing this series: 1. ipv4 addresses seem broken, or maybe i'm using them wrong. Client command: yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | tr \\n \\0 | head -c 1G | nc 192.168.1.4 5224 -p 5224 Server command and logs: mina-1 /home/almasrymina # ./ncdevmem -s 192.168.1.4 -c 192.168.1.5 -l -p 5224 -v 7 -f eth1 here: ynl.c:887:ynl_req_trampoline using queues 15..16 Running: sudo ethtool -K eth1 ntuple off >&2 Running: sudo ethtool -K eth1 ntuple on >&2 Running: sudo ethtool -n eth1 | grep 'Filter:' | awk '{print $2}' | xargs -n1 ethtool -N eth1 delete >&2 ethtool: bad command line argument(s) For more information run ethtool -h here: ynl.c:887:ynl_req_trampoline TCP header split: on Running: sudo ethtool -X eth1 equal 15 >&2 Running: sudo ethtool -N eth1 flow-type tcp6 src-ip 192.168.1.5 dst-ip 192.168.1.4 src-port 5224 dst-port 5224 queue 15 >&2 Invalid src-ip value[192.168.1.5] ethtool: bad command line argument(s) For more information run ethtool -h ./ncdevmem: Failed to configure flow steering The ethtool command to configure flow steering is not working for me. It thinks it's in v6 mode, when the ip address is a v4 address. (notice `-s 192.168.1.4 -c 192.168.1.5`). flow-type should be tcp in this case. Reverting patch 9e2da4faeccf ("Revert "selftests: ncdevmem: Switch to AF_INET6"") resolves this issue. Leading to the second issue: 2. Validation is now broken: Client command: yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | tr \\n \\0 | head -c 1G | nc 192.168.1.4 5224 -p 5224 Server command and logs: mina-1 /home/almasrymina # ./ncdevmem -s 192.168.1.4 -c 192.168.1.5 -l -p 5224 -v 7 -f eth1 here: ynl.c:887:ynl_req_trampoline using queues 15..16 Running: sudo ethtool -K eth1 ntuple off >&2 Running: sudo ethtool -K eth1 ntuple on >&2 Running: sudo ethtool -n eth1 | grep 'Filter:' | awk '{print $2}' | xargs -n1 ethtool -N eth1 delete >&2 ethtool: bad command line argument(s) For more information run ethtool -h here: ynl.c:887:ynl_req_trampoline TCP header split: on Running: sudo ethtool -X eth1 equal 15 >&2 Running: sudo ethtool -N eth1 flow-type tcp4 src-ip 192.168.1.5 dst-ip 192.168.1.4 src-port 5224 dst-port 5224 queue 15 >&2 Added rule with ID 19999 here: ynl.c:887:ynl_req_trampoline got dmabuf id=1 binding to address 192.168.1.4:5224 Waiting or connection on 192.168.1.4:5224 Got connection from 192.168.1.5:5224 recvmsg ret=8192 received frag_page=15997, in_page_offset=0, frag_offset=65523712, frag_size=4096, token=1, total_received=4096, dmabuf_id=1 Failed validation: expected=1, actual=0, index=0 Failed validation: expected=2, actual=0, index=1 Failed validation: expected=3, actual=0, index=2 Failed validation: expected=4, actual=0, index=3 Failed validation: expected=5, actual=0, index=4 Failed validation: expected=6, actual=0, index=5 Failed validation: expected=1, actual=0, index=7 Failed validation: expected=2, actual=0, index=8 Failed validation: expected=3, actual=0, index=9 Failed validation: expected=4, actual=0, index=10 Failed validation: expected=5, actual=0, index=11 Failed validation: expected=6, actual=0, index=12 Failed validation: expected=1, actual=0, index=14 Failed validation: expected=2, actual=0, index=15 Failed validation: expected=3, actual=0, index=16 Failed validation: expected=4, actual=0, index=17 Failed validation: expected=5, actual=0, index=18 Failed validation: expected=6, actual=0, index=19 Failed validation: expected=1, actual=0, index=21 Failed validation: expected=2, actual=0, index=22 Failed validation: expected=3, actual=0, index=23 ./ncdevmem: validation failed. I haven't debugged issue #2 yet, but both need to be resolved before merge. I'm happy to provide more details if you can't repro. My setup: mina-1 /home/almasrymina # cat /boot/config-6.12.0-rc4 | grep -i ipv6 CONFIG_IPV6=y mina-1 /home/almasrymina # cat /proc/sys/net/ipv6/bindv6only 0
On Thu, Oct 31, 2024 at 9:45 AM Mina Almasry <almasrymina@google.com> wrote: > ... > > Sorry, 2 issues testing this series: > ... > > 2. Validation is now broken: > Validation is re-fixed with this diff: diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index 692c189bb5cc..494ae66d8abf 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -568,8 +568,7 @@ int do_server(struct memory_buffer *mem) if (do_validation) validate_buffer( - ((unsigned char *)tmp_mem) + - dmabuf_cmsg->frag_offset, + ((unsigned char *)tmp_mem), dmabuf_cmsg->frag_size); else print_nonzero_bytes(tmp_mem, dmabuf_cmsg->frag_size); Since memcpy_from_device copies to the beginning of tmp_mem, then the beginning tmp_mem should be passed to the validation.
On 10/31, Mina Almasry wrote: > On Wed, Oct 30, 2024 at 8:37 AM Stanislav Fomichev <stfomichev@gmail.com> wrote: > > > > On 10/30, Mina Almasry wrote: > > > On Wed, Oct 30, 2024 at 8:13 AM Stanislav Fomichev <stfomichev@gmail.com> wrote: > > > > > > > > On 10/30, Mina Almasry wrote: > > > > > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > > > > > > > The goal of the series is to simplify and make it possible to use > > > > > > ncdevmem in an automated way from the ksft python wrapper. > > > > > > > > > > > > ncdevmem is slowly mutated into a state where it uses stdout > > > > > > to print the payload and the python wrapper is added to > > > > > > make sure the arrived payload matches the expected one. > > > > > > > > > > > > v6: > > > > > > - fix compilation issue in 'Unify error handling' patch (Jakub) > > > > > > > > > > > > > > > > Since I saw a compilation failures on a couple of iterations I > > > > > cherry-picked this locally and tested compilation. I'm seeing this: > > > > > > > > Are you cherry picking the whole series or just this patch? It looks > > > > too broken. > > > > > > > > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw > > > > > TARGETS=ncdevmem 2>&1 > > > > > make: Entering directory > > > > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > > > > > CC ncdevmem > > > > > In file included from ncdevmem.c:63: > > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43: > > > > > warning: ‘enum ethtool_header_flags’ declared inside parameter list > > > > > will not be visible outside of this definition or declaration > > > > > 23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value); > > > > > | ^~~~~~~~~~~~~~~~~~~~ > > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41: > > > > > warning: ‘enum ethtool_module_fw_flash_status’ declared inside > > > > > parameter list will not be visible outside of this definition or > > > > > declaration > > > > > 25 | ethtool_module_fw_flash_status_str(enum > > > > > ethtool_module_fw_flash_status value); > > > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45: > > > > > error: field ‘status’ has incomplete type > > > > > 6766 | enum ethtool_module_fw_flash_status status; > > > > > | ^~~~~~ > > > > > > > > This has been fixed via '#include <linux/ethtool_netlink.h>' > > > > > > > > > ncdevmem.c: In function ‘do_server’: > > > > > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known > > > > > 517 | struct dmabuf_token token; > > > > > > > > And this, and the rest, don't make sense at all? > > > > > > > > I'll double check on my side. > > > > > > Oh, whoops, I forgot to headers_install first. This works for me: > > > > > > ➜ cos-kernel git:(tcpdevmem-fixes-1) ✗ sudo make headers_install && > > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw > > > TARGETS=ncdevmem 2>&1 > > > INSTALL ./usr/include > > > make: Entering directory > > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > > > make: Nothing to be done for 'all'. > > > make: Leaving directory > > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw' > > > ➜ cos-kernel git:(tcpdevmem-fixes-1) ✗ find . -iname ncdevmem > > > ./tools/testing/selftests/drivers/net/hw/ncdevmem > > > > > > Sorry for the noise :D > > > > Whew, thanks and no worries! > > Sorry, 2 issues testing this series: Thank you for testing! > 1. ipv4 addresses seem broken, or maybe i'm using them wrong. > > Client command: > yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | tr \\n \\0 | head -c > 1G | nc 192.168.1.4 5224 -p 5224 > > Server command and logs: > mina-1 /home/almasrymina # ./ncdevmem -s 192.168.1.4 -c 192.168.1.5 -l > -p 5224 -v 7 -f eth1 > here: ynl.c:887:ynl_req_trampoline > using queues 15..16 > Running: sudo ethtool -K eth1 ntuple off >&2 > Running: sudo ethtool -K eth1 ntuple on >&2 > Running: sudo ethtool -n eth1 | grep 'Filter:' | awk '{print $2}' | > xargs -n1 ethtool -N eth1 delete >&2 > ethtool: bad command line argument(s) > For more information run ethtool -h > here: ynl.c:887:ynl_req_trampoline > TCP header split: on > Running: sudo ethtool -X eth1 equal 15 >&2 > Running: sudo ethtool -N eth1 flow-type tcp6 src-ip 192.168.1.5 dst-ip > 192.168.1.4 src-port 5224 dst-port 5224 queue 15 >&2 > Invalid src-ip value[192.168.1.5] > ethtool: bad command line argument(s) > For more information run ethtool -h > ./ncdevmem: Failed to configure flow steering > > The ethtool command to configure flow steering is not working for me. > It thinks it's in v6 mode, when the ip address is a v4 address. > (notice `-s 192.168.1.4 -c 192.168.1.5`). flow-type should be tcp in > this case. > > Reverting patch 9e2da4faeccf ("Revert "selftests: ncdevmem: Switch to > AF_INET6"") resolves this issue. Leading to the second issue: For IPv4, you have to use IPv4-mapped-IPv6, so your invocation needs to be: ./ncdevmem -s ::ffff:192.168.1.4 -c ::ffff:192.168.1.5 ... Can you try that? I actually never tested that with non-ffff-prefixed addresses, maybe that needs some error handling or something. Will double-check on my side. > 2. Validation is now broken: > > Client command: > yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | tr \\n \\0 | head -c > 1G | nc 192.168.1.4 5224 -p 5224 > > Server command and logs: mina-1 /home/almasrymina # ./ncdevmem -s > 192.168.1.4 -c 192.168.1.5 -l -p 5224 -v 7 -f eth1 > here: ynl.c:887:ynl_req_trampoline > using queues 15..16 > Running: sudo ethtool -K eth1 ntuple off >&2 > Running: sudo ethtool -K eth1 ntuple on >&2 > Running: sudo ethtool -n eth1 | grep 'Filter:' | awk '{print $2}' | > xargs -n1 ethtool -N eth1 delete >&2 > ethtool: bad command line argument(s) > For more information run ethtool -h > here: ynl.c:887:ynl_req_trampoline > TCP header split: on > Running: sudo ethtool -X eth1 equal 15 >&2 > Running: sudo ethtool -N eth1 flow-type tcp4 src-ip 192.168.1.5 dst-ip > 192.168.1.4 src-port 5224 dst-port 5224 queue 15 >&2 > Added rule with ID 19999 > here: ynl.c:887:ynl_req_trampoline > got dmabuf id=1 > binding to address 192.168.1.4:5224 > Waiting or connection on 192.168.1.4:5224 > Got connection from 192.168.1.5:5224 > recvmsg ret=8192 > received frag_page=15997, in_page_offset=0, frag_offset=65523712, > frag_size=4096, token=1, total_received=4096, dmabuf_id=1 > Failed validation: expected=1, actual=0, index=0 > Failed validation: expected=2, actual=0, index=1 > Failed validation: expected=3, actual=0, index=2 > Failed validation: expected=4, actual=0, index=3 > Failed validation: expected=5, actual=0, index=4 > Failed validation: expected=6, actual=0, index=5 > Failed validation: expected=1, actual=0, index=7 > Failed validation: expected=2, actual=0, index=8 > Failed validation: expected=3, actual=0, index=9 > Failed validation: expected=4, actual=0, index=10 > Failed validation: expected=5, actual=0, index=11 > Failed validation: expected=6, actual=0, index=12 > Failed validation: expected=1, actual=0, index=14 > Failed validation: expected=2, actual=0, index=15 > Failed validation: expected=3, actual=0, index=16 > Failed validation: expected=4, actual=0, index=17 > Failed validation: expected=5, actual=0, index=18 > Failed validation: expected=6, actual=0, index=19 > Failed validation: expected=1, actual=0, index=21 > Failed validation: expected=2, actual=0, index=22 > Failed validation: expected=3, actual=0, index=23 > ./ncdevmem: validation failed. > > I haven't debugged issue #2 yet, but both need to be resolved before > merge. I'm happy to provide more details if you can't repro. My setup: > > mina-1 /home/almasrymina # cat /boot/config-6.12.0-rc4 | grep -i ipv6 > CONFIG_IPV6=y > mina-1 /home/almasrymina # cat /proc/sys/net/ipv6/bindv6only > 0 I see you've fixed it out already in a follow up, will pull in the fix! --- pw-bot: cr
The goal of the series is to simplify and make it possible to use ncdevmem in an automated way from the ksft python wrapper. ncdevmem is slowly mutated into a state where it uses stdout to print the payload and the python wrapper is added to make sure the arrived payload matches the expected one. v6: - fix compilation issue in 'Unify error handling' patch (Jakub) v5: - properly handle errors from inet_pton() and socket() (Paolo) - remove unneeded import from python selftest (Paolo) v4: - keep usage example with validation (Mina) - fix compilation issue in one patch (s/start_queues/start_queue/) v3: - keep and refine the comment about ncdevmem invocation (Mina) - add the comment about not enforcing exit status for ntuple reset (Mina) - make configure_headersplit more robust (Mina) - use num_queues/2 in selftest and let the users override it (Mina) - remove memory_provider.memcpy_to_device (Mina) - keep ksft as is (don't use -v validate flags): we are gonna need a --debug-disable flag to make it less chatty; otherwise it times out when sending too much data; so leaving it as a separate follow up v2: - don't remove validation (Mina) - keep 5-tuple flow steering but use it only when -c is provided (Mina) - remove separate flag for probing (Mina) - move ncdevmem under drivers/net/hw, not drivers/net (Jakub) Cc: Mina Almasry <almasrymina@google.com> Stanislav Fomichev (12): selftests: ncdevmem: Redirect all non-payload output to stderr selftests: ncdevmem: Separate out dmabuf provider selftests: ncdevmem: Unify error handling selftests: ncdevmem: Make client_ip optional selftests: ncdevmem: Remove default arguments selftests: ncdevmem: Switch to AF_INET6 selftests: ncdevmem: Properly reset flow steering selftests: ncdevmem: Use YNL to enable TCP header split selftests: ncdevmem: Remove hard-coded queue numbers selftests: ncdevmem: Run selftest when none of the -s or -c has been provided selftests: ncdevmem: Move ncdevmem under drivers/net/hw selftests: ncdevmem: Add automated test .../selftests/drivers/net/hw/.gitignore | 1 + .../testing/selftests/drivers/net/hw/Makefile | 9 + .../selftests/drivers/net/hw/devmem.py | 45 + .../selftests/drivers/net/hw/ncdevmem.c | 773 ++++++++++++++++++ tools/testing/selftests/net/.gitignore | 1 - tools/testing/selftests/net/Makefile | 8 - tools/testing/selftests/net/ncdevmem.c | 570 ------------- 7 files changed, 828 insertions(+), 579 deletions(-) create mode 100644 tools/testing/selftests/drivers/net/hw/.gitignore create mode 100755 tools/testing/selftests/drivers/net/hw/devmem.py create mode 100644 tools/testing/selftests/drivers/net/hw/ncdevmem.c delete mode 100644 tools/testing/selftests/net/ncdevmem.c