Re: [PATCH nf-next 1/3] netfilter: nftables: add connlabel set support

[Florian Westphal <fw@strlen.de>]

Florian Westphal <fw@strlen.de> wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > @@ -777,6 +778,7 @@ enum nft_ct_attributes {
> > >  	NFTA_CT_KEY,
> > >  	NFTA_CT_DIRECTION,
> > >  	NFTA_CT_SREG,
> > > +	NFTA_CT_LABEL,
> > 
> > We can probably add:
> > 
> >         NFTA_CT_IMM
> 
> Right, we can derive the exact type based on the key.
> 
> > This would be a nested attribute, then inside there we can define the:
> > 
> >         NFTA_CT_LABEL
> 
> Hmm, why a nested attribute?
> What do we gain from that?
> 
> Do you want the nested attr so that we can define policies
> for the "immediate subtypes"...?
> 
> If its the latter, then ok.
> 
> I'd then suggest adding a new enum, for instance:
> 
> NFTA_CT_IMM_LABEL
> 
> That appear as nested attrs inside the NFTA_CT_IMM one.

i.e., this:

  * @NFTA_CT_SREG: source register (NLA_U32)
+ * @NFTA_CT_IMM: immediate value (NLA_NESTED)
  */
 enum nft_ct_attributes {
        NFTA_CT_UNSPEC,
@@ -783,6 +783,17 @@ enum nft_ct_attributes {
 };
 #define NFTA_CT_MAX            (__NFTA_CT_MAX - 1)
 
+/*
+ * enum nft_ct_imm_attributes - nf_tables ct expression immediate attributes
+ *
+ * @NFTA_CT_IMM_LABEL: label bit (NLA_U32)
+ */
+enum nft_ct_imm_attributes {
+       NFT_CT_IMM_LABEL,
+       __NFT_IMM_MAX
+};
+#define NFT_CT_IMM_MAX (__NFT_IMM_MAX - 1)

I still don't see the advantage of using a nested attribute for
this since we cannot have mutliple IMM keys at once.

Or was that something that you were interested in?

In that case we'd have to ignore NFTA_CT_KEY and just
rely on the presence of IMM values to tell us what we should do.