Use poll instead of pselect6

[3u13r]

pselect6 is limited to FD_SETSIZE, which is 1024 in most cases. When a application holds many fds, this can be reached easily, resulting in a panic when the fd is added to the fd set. Instead of pselect6 use poll, which doesn't have such a limitation.

[nickgarlis]

@3u13r This makes sense. I wasn’t aware of the FD_SETSIZE limit here. The original idea was to mirror what nft does, but the use cases are a bit different. nft likely never ends up with a large number of open FDs, whereas with this library the parent process could own quite a few more.

I assume you didn’t use ppoll because the signal mask isn’t being used and the timeout is immediate?

[3u13r]

whereas with this library the parent process could own quite a few more.

Yeah, we run into this because we have a lot of tests, each in their own network namespace testing different nftables things. Also those run concurrently.

I assume you didn’t use ppoll because the signal mask isn’t being used and the timeout is immediate?

Now that you're mentioning it, ppoll would probably be the more exact translation. Though I didn't look that deep into it since, as you said, we return immediately anyway.
Do you think this is subject to change? Do you prefer ppoll?

[nickgarlis]

Now that you're mentioning it, ppoll would probably be the more exact translation. Though I didn't look that deep into it since, as you said, we return immediately anyway.
Do you think this is subject to change? Do you prefer ppoll?

I don’t have a strong preference here. I’m not a maintainer, but using poll seems fine as long as we’re confident there’s no difference compared to ppoll with these inputs. I also don’t see any reason to use a signal mask in this case.

[3u13r]

Thanks for the fast review(s). I've guarded the test with the run_system_tests flag. Not sure if you prefer those the new test as part of the integration tests or in the run_system_tests guarded normal tests. Those the second option for now.

Sadly I get testing errors on the main branch when executing sudo ./nftables.test -test.v -run_system_tests locally

nftables_test.go:8080: conn.Flush() failed: receive: NFT_MSG_NEWTABLE: netlink receive: operation not supported
--- FAIL: TestTableOwnership/owned_persistent_table (0.01s)
(...)
nftables_test.go:8200: expected operation not permitted error, got receive: NFT_MSG_NEWTABLE: netlink receive: operation not supported
--- FAIL: TestSocketDrainingOnErrors/non_short_circuited_error (0.01s)

I guess it's due to my kernel version (6.8.0).
But those are the only failures in the integration and normal tests both on main and this branch.

[nickgarlis]

Sadly I get testing errors on the main branch when executing sudo ./nftables.test -test.v -run_system_tests locally
I guess it's due to my kernel version (6.8.0).

When it comes to the test failures that you're seeing locally, I think you're right to suspect your kernel version as both of these tests use NFT_TABLE_F_PERSIST which is not present in that version.