| Message ID | 20220409135832.17401-1-fw@strlen.de |
|---|---|
| Headers | show |
| Series | nftables: add support for wildcard string as set keys | expand |
On Sat, Apr 09, 2022 at 03:58:23PM +0200, Florian Westphal wrote: > Allow to match something like > > meta iifname { eth0, ppp* }. This series LGTM, thanks for working on this. > Set ranges or concatenations are not yet supported. > Test passes on x86_64 and s390 (bigendian), however, the test fails dump > validation: > > - iifname { "eth0", "abcdef0" } counter packets 0 bytes 0 > + iifname { "abcdef0", "eth0" } counter packets 0 bytes 0 Hm. Is it reordering the listing? > This happens with other tests as well. Other tests fail > on s390 too but there are no new failures. > > I wil try to get string range support working and will > then ook into concat set support. OK, so then this is a WIP?
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Sat, Apr 09, 2022 at 03:58:23PM +0200, Florian Westphal wrote: > > Allow to match something like > > > > meta iifname { eth0, ppp* }. > > This series LGTM, thanks for working on this. > > > Set ranges or concatenations are not yet supported. > > Test passes on x86_64 and s390 (bigendian), however, the test fails dump > > validation: > > > > - iifname { "eth0", "abcdef0" } counter packets 0 bytes 0 > > + iifname { "abcdef0", "eth0" } counter packets 0 bytes 0 > > Hm. Is it reordering the listing? Yes, but its like this also before my patch, there are several test failures on s390 with nft master. I will have a look, so far I only checked that my patch series does not cause any additional test failures, and the only reason why the new test fails is the output reorder on s390. > > I wil try to get string range support working and will > > then ook into concat set support. > > OK, so then this is a WIP? If you want all at once then yes, but do you think thats needed? I have not looked at EXPR_RANGE or concat-with-wildcard yet and I don't know when I will be able to do so.
On Wed, Apr 13, 2022 at 12:43:35AM +0200, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Sat, Apr 09, 2022 at 03:58:23PM +0200, Florian Westphal wrote: > > > Allow to match something like > > > > > > meta iifname { eth0, ppp* }. > > > > This series LGTM, thanks for working on this. > > > > > Set ranges or concatenations are not yet supported. > > > Test passes on x86_64 and s390 (bigendian), however, the test fails dump > > > validation: > > > > > > - iifname { "eth0", "abcdef0" } counter packets 0 bytes 0 > > > + iifname { "abcdef0", "eth0" } counter packets 0 bytes 0 > > > > Hm. Is it reordering the listing? > > Yes, but its like this also before my patch, there are several > test failures on s390 with nft master. Why is the listing being reordered? > I will have a look, so far I only checked that my patch > series does not cause any additional test failures, and the only > reason why the new test fails is the output reorder on s390. This is also related to the set description patchset that Phil posted, correct? > > > I wil try to get string range support working and will > > > then ook into concat set support. > > > > OK, so then this is a WIP? > > If you want all at once then yes, but do you think thats needed? If you consider that adding remaining features is feasible, incrementally should be fine. > I have not looked at EXPR_RANGE or concat-with-wildcard yet and > I don't know when I will be able to do so.
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Yes, but its like this also before my patch, there are several > > test failures on s390 with nft master. > > Why is the listing being reordered? No idea, I only saw that this reordering happens, i did not have time to investigate so far. > > I will have a look, so far I only checked that my patch > > series does not cause any additional test failures, and the only > > reason why the new test fails is the output reorder on s390. > > This is also related to the set description patchset that Phil posted, > correct? No, I don't even know what patchset you are talking about. Is it because of failing pything tests because the debug output has endianess issues? If so, not related. > If you consider that adding remaining features is feasible, > incrementally should be fine. Hmm, if there is a technical reason as to why it does not work, do you think we should hold it back? It lookes like filter on "{ eth0, ppp* }" works fine as-is. I thought that something like "eth0-eth42" would also be doable, by treating both as 128bit bit-string. Don't see what prevents "ppp* . 80" from working from a technical pov. So, I *think* its fine to add the pure ifname set support now and add the rest incrementally.
On Wed, Apr 13, 2022 at 01:30:23AM +0200, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > Yes, but its like this also before my patch, there are several > > > test failures on s390 with nft master. > > > > Why is the listing being reordered? > > No idea, I only saw that this reordering happens, i did not have time to > investigate so far. > > > > I will have a look, so far I only checked that my patch > > > series does not cause any additional test failures, and the only > > > reason why the new test fails is the output reorder on s390. > > > > This is also related to the set description patchset that Phil posted, > > correct? > > No, I don't even know what patchset you are talking about. > Is it because of failing pything tests because the debug output has > endianess issues? If so, not related. > > > If you consider that adding remaining features is feasible, > > incrementally should be fine. > > Hmm, if there is a technical reason as to why it does not work, > do you think we should hold it back? > > It lookes like filter on "{ eth0, ppp* }" works fine as-is. Then, good. Better than not working. > I thought that something like "eth0-eth42" would also be doable, > by treating both as 128bit bit-string. > > Don't see what prevents "ppp* . 80" from working from a technical pov. > > So, I *think* its fine to add the pure ifname set support now and > add the rest incrementally. OK, then move on. Sorry, I read you cover letter and I thought there were pending issues.
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > OK, then move on. > > Sorry, I read you cover letter and I thought there were pending > issues. Ok great, thanks! Sorry for the confusion. I meant to say that only "key ifname; flags inerval" will work after this patchset, and that thinks like "eth0-eth4" or "key ifname . ipv4_addr" do not. I plan to work on that too at some point. I will let you know when I start to work on it.
Allow to match something like meta iifname { eth0, ppp* }. Set ranges or concatenations are not yet supported. Test passes on x86_64 and s390 (bigendian), however, the test fails dump validation: - iifname { "eth0", "abcdef0" } counter packets 0 bytes 0 + iifname { "abcdef0", "eth0" } counter packets 0 bytes 0 This happens with other tests as well. Other tests fail on s390 too but there are no new failures. I wil try to get string range support working and will then ook into concat set support. Florian Westphal (9): evaluate: make byteorder conversion on string base type a no-op evaluate: keep prefix expression length segtree: split prefix and range creation to a helper function evaluate: string prefix expression must retain original length src: make interval sets work with string datatypes segtree: add string "range" reversal support tests: add testcases for interface names in sets segtree: use correct byte order for 'element get' segtree: add support for get element with sets that contain ifnames src/evaluate.c | 18 +- src/expression.c | 9 +- src/segtree.c | 228 +++++++++++++----- .../sets/dumps/sets_with_ifnames.nft | 28 +++ tests/shell/testcases/sets/sets_with_ifnames | 102 ++++++++ 5 files changed, 315 insertions(+), 70 deletions(-) create mode 100644 tests/shell/testcases/sets/dumps/sets_with_ifnames.nft create mode 100755 tests/shell/testcases/sets/sets_with_ifnames