On Wed, May 22, 2019 at 01:39:57PM +0200, Stéphane Veyret wrote:
[...]
> Le mer. 22 mai 2019 à 10:46, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> a écrit :
> > I think we should set a maximum number of expectations to be created,
> > as a mandatory field, eg.
> >
> > size 10;
>
> I feel it would be complicated to set, as it would require to keep
> track of all expectations set using this definition, and moreover,
> check if those expectations are still alive, or deleted because
> already used or timed out.
You can use the 'expecting[0]' counter in the ct helper extension to
limit the number of expectations per conntrack entry:
struct nf_conn_help {
[...]
/* Current number of expected connections */
u8 expecting[NF_CT_MAX_EXPECT_CLASSES];
};
You have to check if the ct helper area exists in first place.
> > > + priv->l3num = ctx->family;
> >
> > priv->l3num is only set and never used, remove it. You'll also have to
>
> priv->l3num is used for setting expectation, in function
> nft_ct_expect_obj_eval (see the call to nf_ct_expect_init).
OK, thanks for explaining.
Still this new expectation extension won't work with NFPROTO_INET
tables though, since the expectation infrastructure does not know what
to do with NFPROTO_INET.
If NFPROTO_INET is specified, you could just fetch the l3proto from
the ct object, from the packet path by when you call
nf_ct_expect_init().
> > > + nf_ct_helper_ext_add(ct, GFP_ATOMIC);
> >
> > I think you don't need nf_ct_helper_ext_add(...);
>
> Actually, I had to add this instruction. While testing the feature, i
> saw that, even if no helper is really set on the connection,
> expectation functions require NF_CT_EXT_HELPER to be set on master
> connection. Without it, there would be some null pointer exception,
> which fortunately is checked at expectation creation by
> __nf_ct_expect_check.
Thanks again for explaining.
You still have to check if the conntrack already has a helper
extensions, otherwise I'm afraid this resets it for this conntrack.
