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

[Phil Sutter <phil@nwl.cc>]

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