On Tue, Jun 12, 2018 at 03:21:35PM +0200, Florian Westphal wrote:
> Harsha Sharma <harshasharmaiitr@xxxxxxxxx> 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.
That's a short term solution, yes. But we need native interface for
this that integrates with the nft interface, let's talk about this
during the workshop.
> > +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.
Harsha, please address comments from Florian.
Regarding this one below...
> Unrelated to your patch: I think timeout handling is braindead
> in current conntrack, we should revisit this.
By now, I think it's fine as is, I mean using the template, so
Harsha/someone else can have a look at this in a second step.
We'll need to update iptables too if we want to get rid of the
template in any case.
--
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
