Message ID | 20190816144241.11469-1-fw@strlen.de |
---|---|
Headers | show |
Series | add typeof keyword | expand |
Hi Florian, On Fri, Aug 16, 2019 at 04:42:33PM +0200, Florian Westphal wrote: > This patch series adds the typeof keyword. > > The only dependency is a small change to libnftnl to add two new > UDATA_SET_TYPEOF enum values. Thanks for working on this. > named set can be configured as follows: > > set os { > type typeof(osf name) > elements = { "Linux", "Windows" } > } > > or > nft add set ip filter allowed "{ type typeof(ip daddr) . typeof(tcp dport); }" I know I sent a RFC using typeof(), I wonder if you could just use the selector instead, it's a bit of a lot of type typeof() . typeof() probably. So this is left as this: type osf name in concatenations, like this: nft add set ip filter allowed "{ type ip daddr . tcp dport; }" Probably I would ask my sysadmin friends what they think. I spent too much time on coding, so all these typeof() look natural to me, but it might be a bit too much syntactic sugar for someone that is more in network operations, not sure. P.S: patch 1/8 and 2/8 are related to this patchset? After quick glance, not obvious to me or if they are again related to multiple nft_ctx_new() calls.
On Sat, Aug 17, 2019 at 12:23:51PM +0200, Pablo Neira Ayuso wrote: [...] > On Fri, Aug 16, 2019 at 04:42:33PM +0200, Florian Westphal wrote: [..] > P.S: patch 1/8 and 2/8 are related to this patchset? After quick > glance, not obvious to me or if they are again related to multiple > nft_ctx_new() calls. I found it, it is in patch 6/8, it making a call to set_make_key to get the datatype to translate the selector to datatype using the parser API.
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Sat, Aug 17, 2019 at 12:23:51PM +0200, Pablo Neira Ayuso wrote: > [...] > > On Fri, Aug 16, 2019 at 04:42:33PM +0200, Florian Westphal wrote: > [..] > > P.S: patch 1/8 and 2/8 are related to this patchset? After quick > > glance, not obvious to me or if they are again related to multiple > > nft_ctx_new() calls. > > I found it, it is in patch 6/8, it making a call to set_make_key to > get the datatype to translate the selector to datatype using the > parser API. Yes, I could push patch #1 independently though.
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > I know I sent a RFC using typeof(), I wonder if you could just use the > selector instead, it's a bit of a lot of type typeof() . typeof() > probably. > > So this is left as this: > > type osf name > > in concatenations, like this: > > nft add set ip filter allowed "{ type ip daddr . tcp dport; }" > > Probably I would ask my sysadmin friends what they think. Yes, please do, it would be good to get a non-developer perspective. I'm very used to things like sizeof(), so typeof() felt natural to me. Might be very un-intuitive for non-developers though, so it would be good to get outside perspective. Thanks!
On 8/17/19 10:55 PM, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: >> I know I sent a RFC using typeof(), I wonder if you could just use the >> selector instead, it's a bit of a lot of type typeof() . typeof() >> probably. >> >> So this is left as this: >> >> type osf name >> >> in concatenations, like this: >> >> nft add set ip filter allowed "{ type ip daddr . tcp dport; }" >> >> Probably I would ask my sysadmin friends what they think. > > Yes, please do, it would be good to get a non-developer perspective. > > I'm very used to things like sizeof(), so typeof() felt natural to me. > > Might be very un-intuitive for non-developers though, so it would be > good to get outside perspective. > From my point of view, this is a rather advanced operation. As long as it is properly documented, I don't see any problem with `typeof()`. Also, just `typeof` would work of course. Up to you. Thanks for working on this!
Hi Florian, On Sun, Aug 18, 2019 at 04:33:15PM +0200, Arturo Borrero Gonzalez wrote: > On 8/17/19 10:55 PM, Florian Westphal wrote: > > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > >> I know I sent a RFC using typeof(), I wonder if you could just use the > >> selector instead, it's a bit of a lot of type typeof() . typeof() > >> probably. > >> > >> So this is left as this: > >> > >> type osf name > >> > >> in concatenations, like this: > >> > >> nft add set ip filter allowed "{ type ip daddr . tcp dport; }" > >> > >> Probably I would ask my sysadmin friends what they think. > > > > Yes, please do, it would be good to get a non-developer perspective. > > > > I'm very used to things like sizeof(), so typeof() felt natural to me. > > > > Might be very un-intuitive for non-developers though, so it would be > > good to get outside perspective. > > From my point of view, this is a rather advanced operation. As long as it is > properly documented, I don't see any problem with `typeof()`. > > Also, just `typeof` would work of course. Up to you. My suggestion is: typeof ip saddr . tcp dport . meta mark but, not to allow the use of the existing datatypes with typeof. So either the user picks: type ipv4_addr . inet_service . mark or this: typeof ip saddr . tcp dport . meta mark Then, for the notation: integer,bits I would suggest this one can only be used from 'type'. Goal is basically: * Avoiding repetitive typeof() that looks a bit like C programming. * Mixing datatype with inferred datatypes looks a bit sloppy to me from syntax consistency point of view. This approach is flexible enough to cover all use cases this patchset is dealing with I think. If you like this approach, I would suggest you just update the patchset to support this and then push this out, I'll catch up later on to revisit this before the next release, in case I can propose you some refinement for your review. Let me know, thanks.