On 2022-04-06, at 05:12:44 +0200, Florian Westphal wrote: > Jeremy Sowden <jeremy@xxxxxxxxxx> wrote: > > It wouldn't be straightforward. Expression udata might make more sense > > than adding a new bitwise attribute, but that doesn't currently exist. > > Would it be worth adding? I seem to recall considering something along > > those lines for passing type information with expressions as a way to > > implement casting. > > Had not thought of casting, good point. > Given bitwise needs to be touched anyway to get the second register > operations I think the proposed patch isn't too bad. Cool. > For casts and other use cases (including bitlen), I think its > not needed to add special udata for expressions, as userspace can't > zap them selectively. > > We already do something similar for sets (to embed 'typeof' info > for key and data). > > Probably extend nftnl_udata_rule_types in libnftnl to add a > NFTNL_UDATA_RULE_EXPR_INFO. > > NFTNL_UDATA_RULE_EXPR_INFO would be nested and contain > expression specific (nested) attributes. > > i.e., if you have something like > > meta mark -> reg 1 > binop reg1 &= 0x0000ffff > ct mark -> reg 2 > binop and reg1 &= reg2 // ulen 16 > > Then rule udata would have: > NFTNL_UDATA_RULE_EXPR_INFO (nested) > type 4 (nested, 4 refers to the last expression above, > type '0' is reserved). > type 1 // nla_u32 -> for binop, 1 is 'len', it would be > defined privately in src/bitwise.c > END > > because only expression 4 needs annotations, so we don't waste > space for expressions that do not need extra data. > > We could reserve a few nested numbers for things that might make sense > for all (or many) expresssions, e.g. 'cast to type x'. > > We could of course place expr specific structs in there too but so > far we managed to avoid this and it would be not-so-nice to break > nft userspace when listing a ruleset added by an older version. > > Probably could extend struct netlink_linearize_ctx with a memory > blob pointer that netlink_linearize_rule()/netlink_gen_stmt() can use > to add extra data. > > My problem is that its a lot of (userspace) code for something that can > easily be done by a small kernel patch such as this one and so far we > don't have a real need for something like this. Yes, quite. If you're happy passing the bit-length through the kernel, how about the rest of the series? I'm pretty sure there's some debatable choices in there too. :) J.
