Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > I was thinking on simplifying the codebase one we get more immediates, > we will at least get one more to handle ct helper assignment. > > So from the code we can just check: > > if (nla[NFTA_CT_IMM]) { > err = nla_parse_nested(tb, NFTA_CT_IMM, nla, nft_ct_imm_policy); > if (err < 0) > return err; > > switch (key) { > case NFT_CT_LABEL: > if (!tb[NFTA_CT_IMM_LABEL]) > return -EINVAL; Right, something like this. > BTW, probably we should consider using some generic way to represent > the immediates through enum nft_data_attributes. So we have core code > that that every expression can reuse to handle immediates. I think > the concern here is data length validation, eg. labels are 4 bytes and > helpers are strings limited to 16 bytes. From the libnftnl side, so > far we represent these as _DATA attributes: > > enum { > NFTNL_EXPR_CMP_SREG = NFTNL_EXPR_BASE, > NFTNL_EXPR_CMP_OP, > NFTNL_EXPR_CMP_DATA, > }; > > Otherwise, we'll start seeing code in every expression handling > immediates in their own way (ie. in a not consolidated way). Yes, thats a valid concern. I guess it depends on wheter we want to support multi-action, or if we will just put several actions after one another. set label bla set helper ftp If thats two nft_ct invocations, then generic representation (NTA_IMM_DATA_U32, NFTA_IMM_DATA_STRING, ...) that can be used by any expressions that need to carry their own immediate values is definitely much better. I'll go with using generic IMM_U32 for now, inside a CT_IMM nested attr. Thanks Pablo! -- 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