On Tue, Feb 01, 2022 at 01:04:54PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > On Tue, Feb 01, 2022 at 10:08:55AM +0700, Pham Thanh Tuyen wrote:
> > > When the conntrack is created, the extension is created before the conntrack
> > > is assigned confirmed and inserted into the hash table. But the function
> > > ctnetlink_setup_nat() causes loss of helper in the mentioned situation. I
> > > mention the template because it's seamless in the
> > > __nf_ct_try_assign_helper() function. Please double check.
> >
> > Conntrack entries that are created via ctnetlink as IPS_CONFIRMED always
> > set on.
> >
> > The helper code is only exercised from the packet path for conntrack
> > entries that are newly created.
>
> I suspect this is the most simple fix, might make sense to also
> update the comment of IPS_HELPER to say that it means 'explicitly
> attached via ctnetlink or ruleset'.
>
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -2313,6 +2313,9 @@ ctnetlink_create_conntrack(struct net *net,
>
> /* not in hash table yet so not strictly necessary */
> RCU_INIT_POINTER(help->helper, helper);
> +
> + /* explicitly attached from userspace */
> + ct->status |= IPS_HELPER;
> }
> } else {
> /* try an implicit helper assignation */
This also LGTM.
I'd suggest you update the .h file to describe that ctnetlink also
sets on this bit.
Thanks
