Re: [PATCH] netfilter: nfnetlink: always ACK batch end if requested

[Fernando Fernandez Mancera <fmancera@suse.de>]

On 10/1/25 11:15 PM, Nikolaos Gkarlis wrote:
> Before ACKs were introduced for batch begin and batch end messages,
> userspace expected to receive the same number of ACKs as it sent,
> unless a fatal error occurred.
> 
> To preserve this deterministic behavior, send an ACK for batch end
> messages even when an error happens in the middle of the batch,
> similar to how ACKs are handled for command messages.
> 
> Signed-off-by: Nikolaos Gkarlis <nickgarlis@gmail.com>
> ---
> Hi,
> 
> I recently came across the issue introduced by bf2ac490d28c and
> while trying to find a way to handle it by adding an ACK on batch
> begin, end messages, I spotted what looks like an inconsistency?
> 
> I have tested this change with my userspace application and it
> seems to resolve the "problem". However, I am not sure if there
> is a suitable place to add a regression test, since AFAIK nft
> userspace does not currently use this feature. I would be happy
> to contribute a test if you could point me to the right place.
> 
> I may be missing some context, so feedback on whether this is the
> right approach would be very welcome.
> 
>   net/netfilter/nfnetlink.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
> index 811d02b4c4f7..0342087ead06 100644
> --- a/net/netfilter/nfnetlink.c
> +++ b/net/netfilter/nfnetlink.c
> @@ -600,6 +600,11 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
>   			status |= NFNL_BATCH_FAILURE;
>   			goto replay_abort;
>   		}
> +
> +		if (nlh->nlmsg_flags & NLM_F_ACK) {
> +			memset(&extack, 0, sizeof(extack));
> +			nfnl_err_add(&err_list, nlh, 0, &extack);
> +		}
>   	}

Right, if BATCH_END message is reached and has the NLM_F_ACK, nfnetlink 
should send the corresponding ACK. Currently if BATCH_END message is 
missing, this would send an extra wrong ACK if the previous message was 
using NLM_F_ACK.

e.g for a batch formatted like (BATCH_BEGIN|NFT_MSG_NEWRULE + NLM_F_ACK) 
- nfnetlink would send two ACKs while it should be only one. Granted it 
won't configure anything but it would be still misleading.

What about this?

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index b8d0fad1ed10..ecf85346d883 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -601,7 +601,7 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, 
struct nlmsghdr *nlh,
                         goto replay_abort;
                 }

-               if (nlh->nlmsg_flags & NLM_F_ACK) {
+               if (nlh->nlmsg_flags & NLM_F_ACK && status & 
NFNL_BATCH_DONE) {
                         memset(&extack, 0, sizeof(extack));
                         nfnl_err_add(&err_list, nlh, 0, &extack);
                 }

>   
>   	nfnl_err_deliver(&err_list, oskb);