Hi Phil, On Sat, Dec 16, 2017 at 05:06:51PM +0100, Phil Sutter wrote: > Hi Pablo, > > On Sun, Dec 10, 2017 at 10:55:40PM +0100, Pablo Neira Ayuso wrote: > > On Thu, Dec 07, 2017 at 12:34:31PM +0100, Phil Sutter wrote: > > > On Thu, Dec 07, 2017 at 01:05:45AM +0100, Pablo Neira Ayuso wrote: > > > > On Tue, Dec 05, 2017 at 02:43:17PM +0100, Phil Sutter wrote: > > > [...] > > > > > After tweaking the parser a bit, I can use it now to parse just a > > > > > set_list_member_expr and use the struct expr it returns. This made it > > > > > possible to create the desired struct cmd in above function without > > > > > having to invoke the parser there. > > > > > > > > > > Exercising this refining consequently should allow to reach arbitrary > > > > > levels of granularity. For instance, one could stop at statement level, > > > > > i.e. statements are created using a string representation. Or one could > > > > > go down to expression level, and statements are created using one or two > > > > > expressions (depending on whether it is relational or not). Of course > > > > > this means the library will eventually become as complicated as the > > > > > parser itself, not necessarily a good thing. > > > > > > > > Yes, and we'll expose all internal representation details, that we > > > > will need to maintain forever if we don't want to break backward. > > > > > > Not necessarily. I had this in mind when declaring 'struct nft_table' > > > instead of reusing 'struct table'. :) > > > > > > The parser defines the grammar, the library would just follow it. So if > > > a given internal change complies with the old grammar, it should comply > > > with the library as well. Though this is quite theoretical, of course. > > > > > > Let's take relational expressions as simple example: In bison, we define > > > 'expr op rhs_expr'. An equivalent library function could be: > > > > > > | struct nft_expr *nft_relational_new(struct nft_expr *, > > > | enum rel_ops, > > > | struct nft_expr *); > > > > Then that means you would like to expose an API that allows you to > > build the abstract syntax tree. > > That was the idea I had when I thought about how to transition from > fully text-based simple API to an extended one which allows to work with > objects instead. We could start simple and refine further if > required/sensible. At the basic level, adding a new rule could be > something like: > > | myrule = nft_rule_create("tcp dport 22 accept"); > > If required, one could implement rule building based on statements: > > | stmt1 = nft_stmt_create("tcp dport 22"); > | stmt2 = nft_stmt_create("accept"); > | myrule = nft_rule_create(); > | nft_rule_add_stmt(myrule, stmt1); > | nft_rule_add_stmt(myrule, stmt2); This is mixing parsing and abstract syntax tree creation. If you want to expose the syntax tree, then I would the parsing layer entirely and expose the syntax tree, which is what the json representation for the high level library will be doing. To support new protocol, we will need a new library version too, when the abstraction to represent a payload is already well-defined, ie. [ base, offset, length ], which is pretty much everywhere the same, not only in nftables. I wonder if firewalld could generate high level json representation instead, so it becomes a compiler/translator from its own representation to nftables abstract syntax tree. As I said, the json representation is mapping to the abstract syntax tree we have in nft. I'm refering to the high level json representation that doesn't exist yet, not the low level one for libnftnl. > [...] > > > Yes, that sounds good. I had something like this in mind: > > > > > > | struct nft_stmt *nft_meta_match_immediate(enum nft_meta_type, void *data); > > > | int nft_rule_append_stmt(struct nft_rule *r, struct nft_stmt *stmt); > > > > > > The obvious problem is that at the time that meta match is created, > > > there is no context information. So the second function would have to > > > do that. > > > > > > I am not sure if this kind of context evaluation works in any case. E.g. > > > set elements are interpreted depending on the set they are added to. To > > > my surprise, that wasn't really an issue - the parser interprets it as > > > constant symbol, when evaluating the expression as part of adding it to > > > the set it is resolved properly. This might not work in any case, > > > though. > > > > Not sure I follow, what's the problem with the "missing context"? > > The parser always has the full context available. E.g. if you pass it a > string such as this: > > | add rule ip t c tcp dport 22 accept > > By the time it parses the individual statements, it already knows e.g. > the table family and that might make a difference. As far as I can tell > from my hacking, it is possible to tweak the parser so that one could > use it to parse just a statement ('tcp dport 22'). You could just postpone evaluation once you have the full rule. > > > > A list of use-cases, for the third party application, would be good to > > > > have to design this API. > > > > > > OK, I'll take firewalld as an example and come up with a number of > > > use-cases which would help there. > > > > Thanks, those use-cases would be very useful to design this. > > OK, we've collected a bit: > > Ability to verify ruleset state > ------------------------------- > I want to find out whether a given element (table, chain, rule, set, set > element) previously added to the ruleset still exists. We need to add handle number for every object, I think that would suffice for this. > Possibility to remove previously added ruleset components > --------------------------------------------------------- > After making an addition to the ruleset, it should be possible to remove > the added items again. For tables, chains, sets, set elements and > stateful objects, the data used when adding the item is sufficient for > later removal (as removal happens by value). For rules though, the > corresponding handle is required. > XXX: This is inconsistent regarding items removed and added back > again by another instance: As the rule's handle changes, it is not > found anymore afterwards. Then it's time to add unique handles for everything. > Create/delete user chains > ------------------------- > Firewalld makes heavy use of custom named chains to organize rules. It > needs to be able to create/delete them easily. > > Sets > ---- > I want to be able to freely read/modify sets of IPv4/IPv6/MAC addresses > and have them apply to existing rules. These two, I think we already support. > Transactions > ------------ > I want to be able to add many custom chains and rules in > batch/transaction mode to reduce round trips. Ideally failure of a > transaction would return something that can indicate the invalid piece > of the transaction. I need to find the time to finish the fine grain netlink error reporting for nftables. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html