[conntrack-tools PATCH] nfct: helper: Extend error message for EEXIST

[conntrack-tools PATCH] nfct: helper: Extend error message for EEXIST

[Phil Sutter]

Users may be not aware the user space helpers conflict with kernel space
ones, so add a hint about the possible cause of the EEXIST code returned
by the kernel.

Cc: Yi Chen <yiche@redhat.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/nfct-extensions/helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/nfct-extensions/helper.c b/src/nfct-extensions/helper.c
index 894bf269ad2bb..5592dc1340bea 100644
--- a/src/nfct-extensions/helper.c
+++ b/src/nfct-extensions/helper.c
@@ -229,6 +229,9 @@ static int nfct_cmd_helper_add(struct mnl_socket *nl, int argc, char *argv[])
 	portid = mnl_socket_get_portid(nl);
 	if (nfct_mnl_talk(nl, nlh, seq, portid, NULL, NULL) < 0) {
 		nfct_perror("netlink error");
+		if (errno == EEXIST)
+			fprintf(stderr,
+				"Maybe unload nf_conntrack_%s.ko first?\n", argv[3]);
 		return -1;
 	}
 
-- 
2.49.0

Re: [conntrack-tools PATCH] nfct: helper: Extend error message for EEXIST

[Pablo Neira Ayuso]

On Mon, Aug 18, 2025 at 11:47:08AM +0800, Yi Chen wrote:
> This patch adds a hint when:
> 
> # modprobe nf_conntrack_ftp
> # nfct helper del ftp inet tcp
> # nfct helper add ftp inet tcp
> *nfct v1.4.8: netlink error: File exists*
> 
> or other type of helper.

This patch changes EEXIST by EBUSY:

  https://patchwork.ozlabs.org/project/netfilter-devel/patch/20250818112220.26641-1-phil@nwl.cc/

This userspace patch is not very useful after this.

So maybe a follow up fix to retain EEXIST for nfnetlink_cthelper in
the kernel is needed?

I mean, return EEXIST in nfnetlink_cthelper but EBUSY in case of
insmod, ie. add a bool insmod flag to the helper register/unregister
functions to return EBUSY for insmod and EEXIST for
nfnetlink_cthelper.

Re: [conntrack-tools PATCH] nfct: helper: Extend error message for EEXIST

[Phil Sutter]

On Thu, Aug 28, 2025 at 12:34:15AM +0200, Pablo Neira Ayuso wrote:
> On Mon, Aug 18, 2025 at 11:47:08AM +0800, Yi Chen wrote:
> > This patch adds a hint when:
> > 
> > # modprobe nf_conntrack_ftp
> > # nfct helper del ftp inet tcp
> > # nfct helper add ftp inet tcp
> > *nfct v1.4.8: netlink error: File exists*
> > 
> > or other type of helper.
> 
> This patch changes EEXIST by EBUSY:
> 
>   https://patchwork.ozlabs.org/project/netfilter-devel/patch/20250818112220.26641-1-phil@nwl.cc/
> 
> This userspace patch is not very useful after this.

Oh! I missed that nfnl_cthelper_create() also just passes through the
return code from nf_conntrack_helper_register().

> So maybe a follow up fix to retain EEXIST for nfnetlink_cthelper in
> the kernel is needed?
> 
> I mean, return EEXIST in nfnetlink_cthelper but EBUSY in case of
> insmod, ie. add a bool insmod flag to the helper register/unregister
> functions to return EBUSY for insmod and EEXIST for
> nfnetlink_cthelper.

Do we need to retain the old return code? I would just update the patch
to print the message for EBUSY instead of EEXIST.

Cheers, Phil

Re: [conntrack-tools PATCH] nfct: helper: Extend error message for EEXIST

[Pablo Neira Ayuso]

On Thu, Aug 28, 2025 at 01:04:06PM +0200, Phil Sutter wrote:
> On Thu, Aug 28, 2025 at 12:34:15AM +0200, Pablo Neira Ayuso wrote:
> > On Mon, Aug 18, 2025 at 11:47:08AM +0800, Yi Chen wrote:
> > > This patch adds a hint when:
> > > 
> > > # modprobe nf_conntrack_ftp
> > > # nfct helper del ftp inet tcp
> > > # nfct helper add ftp inet tcp
> > > *nfct v1.4.8: netlink error: File exists*
> > > 
> > > or other type of helper.
> > 
> > This patch changes EEXIST by EBUSY:
> > 
> >   https://patchwork.ozlabs.org/project/netfilter-devel/patch/20250818112220.26641-1-phil@nwl.cc/
> > 
> > This userspace patch is not very useful after this.
> 
> Oh! I missed that nfnl_cthelper_create() also just passes through the
> return code from nf_conntrack_helper_register().
> 
> > So maybe a follow up fix to retain EEXIST for nfnetlink_cthelper in
> > the kernel is needed?
> > 
> > I mean, return EEXIST in nfnetlink_cthelper but EBUSY in case of
> > insmod, ie. add a bool insmod flag to the helper register/unregister
> > functions to return EBUSY for insmod and EEXIST for
> > nfnetlink_cthelper.
> 
> Do we need to retain the old return code?

I have change return codes in the past myself, when I considered error
reported to userspace was misleading, but I heard once it is a good
practise not to change them as a general rule.

> I would just update the patch to print the message for EBUSY instead
> of EEXIST.

It is OK, I could not find any code in conntrackd running in helper
mode than relies on this error code. The only case that I can think of
is combining old kernel with new userspace defeats the purpose of this
patch.

Maybe it is not worth the effort to bother about this, judge yourself.

Re: [conntrack-tools PATCH] nfct: helper: Extend error message for EEXIST

[Phil Sutter]

On Thu, Aug 28, 2025 at 01:56:45PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Aug 28, 2025 at 01:04:06PM +0200, Phil Sutter wrote:
> > On Thu, Aug 28, 2025 at 12:34:15AM +0200, Pablo Neira Ayuso wrote:
> > > On Mon, Aug 18, 2025 at 11:47:08AM +0800, Yi Chen wrote:
> > > > This patch adds a hint when:
> > > > 
> > > > # modprobe nf_conntrack_ftp
> > > > # nfct helper del ftp inet tcp
> > > > # nfct helper add ftp inet tcp
> > > > *nfct v1.4.8: netlink error: File exists*
> > > > 
> > > > or other type of helper.
> > > 
> > > This patch changes EEXIST by EBUSY:
> > > 
> > >   https://patchwork.ozlabs.org/project/netfilter-devel/patch/20250818112220.26641-1-phil@nwl.cc/
> > > 
> > > This userspace patch is not very useful after this.
> > 
> > Oh! I missed that nfnl_cthelper_create() also just passes through the
> > return code from nf_conntrack_helper_register().
> > 
> > > So maybe a follow up fix to retain EEXIST for nfnetlink_cthelper in
> > > the kernel is needed?
> > > 
> > > I mean, return EEXIST in nfnetlink_cthelper but EBUSY in case of
> > > insmod, ie. add a bool insmod flag to the helper register/unregister
> > > functions to return EBUSY for insmod and EEXIST for
> > > nfnetlink_cthelper.
> > 
> > Do we need to retain the old return code?
> 
> I have change return codes in the past myself, when I considered error
> reported to userspace was misleading, but I heard once it is a good
> practise not to change them as a general rule.
> 
> > I would just update the patch to print the message for EBUSY instead
> > of EEXIST.
> 
> It is OK, I could not find any code in conntrackd running in helper
> mode than relies on this error code. The only case that I can think of
> is combining old kernel with new userspace defeats the purpose of this
> patch.

Yes, with old kernels new user space behaviour is same as old user space
(so at least not a degradation).

> Maybe it is not worth the effort to bother about this, judge yourself.

The EBUSY return is consistent with insmod/modprobe return code, so I'd
keep it like this.

Thanks, Phil