Harsha Sharma <harshasharmai...@gmail.com> wrote:
> +ctnl_timeout_parse_policy(void *timeouts,
> +                       const struct nf_conntrack_l4proto *l4proto,
> +                       struct net *net, const struct nlattr *attr)
> +{
> +     int ret = 0;
> +     struct nlattr **tb;
> +
> +     if (!l4proto->ctnl_timeout.nlattr_to_obj)
> +             return 0;

You'll need to warp this with
#if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)

else this fails to build with CONFIG_NF_CT_NETLINK_TIMEOUT=n.

> +static void nft_ct_timeout_obj_eval(struct nft_object *obj,
> +                                 struct nft_regs *regs,
> +                                 const struct nft_pktinfo *pkt)
> +{
> +     const struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
> +     struct ctnl_timeout *to_assign = NULL;
> +     struct nf_conn_timeout *timeout_ext;
> +     struct sk_buff *skb = pkt->skb;
> +
> +     if (!priv->tmpl ||

Better let nft_ct_timeout_obj_init() fail so priv->tmpl is always set.

> +         nf_ct_is_confirmed(priv->tmpl))
> +             return;

Uncessery, the template cannot be confirmed (else bug).

> +     to_assign = priv->timeout;
> +
> +     nf_ct_set(skb, priv->tmpl, IP_CT_NEW);

You will need to check that no previous conntrack exists here.

> +     timeout_ext = nf_ct_timeout_ext_add(priv->tmpl, priv->timeout, 
> GFP_ATOMIC);

This looks strange.  The timeout extension either has to be attached to
the template (but then this has to happen in init, not here).

> +     if (timeout_ext) {
> +             rcu_assign_pointer(timeout_ext->timeout, to_assign);
> +             __set_bit(IPS_CONFIRMED_BIT, &priv->tmpl->status);

Hmm, no, CONFIRMED means conntrack was inserted into the hash table,
this must never happen for templates.

Unrelated to your patch: I think timeout handling is braindead
in current conntrack, we should revisit this.

Right now there is a bizarre mix of getter (l4proto->get_timeout()),
which is then passed to l4proto->new() (but only used by gre), and a
conntrack extension, to pass timeouts from raw table to nf_conntrack_in.

It seems saner to revisit this design, remove l4proto->get_timeout(),
and have the l4 trackers get the timeout from the conntrack extension
(if present) or the pernetns data themselves.

This would also simplify this patch: you'd only have to fetch the
conntrack, check if its unconfirmed, and then call
nf_ct_timeout_ext_add() on it.  No template needed.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to