Re: [PATCH nf 0/2] ipvs: keep templates and cport 0 conns away from packet lookups

[Julian Anastasov <ja@ssi.bg>]

[-- Attachment #1: Type: text/plain, Size: 4487 bytes --]


	Hi Axel,

On Wed, 30 Sep 2026, Axel Mierczuk wrote:

> Hi Julian,
> 
> Sounds good. I will take a look as well and get back to you.
> 
> On Tue, Sep 29, 2026 at 2:29 PM Julian Anastasov <ja@ssi.bg> wrote:
> 
>               Hello,
> 
>       On Fri, 25 Sep 2026, Axel Mierczuk wrote:
> 
>       > A template received over the unauthenticated sync protocol with a
>       > nonzero cport is matched by data packets in ip_vs_conn_in_get() and
>       > the protocol state machine runs on it. Patch 1 rejects sync records
>       > with inconsistent cport and IP_VS_CONN_F_NO_CPORT combinations on
>       > the backup.
>       >
>       > Patch 2 requires a nonzero cp->cport in ip_vs_conn_out_get() so that
>       > templates and connections still waiting for their cport cannot be
>       > matched through the out hook.
>       >
>       > The companion patch "ipvs: filter some flags received in the backup
>       > server" rejects template records carrying IP_VS_CONN_F_NO_CPORT.
>       >
>       > Axel Mierczuk (2):
>       >   ipvs: validate cport in received sync records
>       >   ipvs: skip cport 0 connections in ip_vs_conn_out_get()
>       >
>       >  net/netfilter/ipvs/ip_vs_conn.c |  3 ++-
>       >  net/netfilter/ipvs/ip_vs_sync.c | 19 +++++++++++++++++++
>       >  2 files changed, 21 insertions(+), 1 deletion(-)
> 
>               Axel, this Sashiko review is shattering:
> 
>       https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com
> 
>               I'll be back with analyze how should we fix the
>       problems...

	Here is what we have for the Sashiko review:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com

Patch 1:

Q. Should the template branch also reject IP_VS_CONN_F_NO_CPORT?
A. Fixed by "ipvs: filter some flags received in the backup server"
which is already in the trees

Q. Can the master itself briefly send a record with a nonzero cport and
IP_VS_CONN_F_NO_CPORT still set?
A. Harmless

	So, nothing that needs to change the patch.

Patch 2:

Q1. Does this make the "update or create" lookup in ip_vs_ftp_out() in
net/netfilter/ipvs/ip_vs_ftp.c unreachable?
A. Looks like we need the same check in ip_vs_conn_out_get:

We should use:
	(!p->vport ^ (!(cp->flags & IP_VS_CONN_F_NO_CPORT)))

instead of the wrong cp->cport check.

	Using p->vport and not re-reading cp->cport above due to
tricks in ip_vs_conn_fill_cport(), see Q4 below.

Q2. Can this lead to a new connection being created for every packet of a
flow started by a real server towards client port 0?
A. I can provide patch to ignore packets with port 0.

Q3. The commit message assumes that only templates and NO_CPORT entries have
cport 0. Does that hold for ordinary MASQ connections scheduled for a
client that uses source port 0?
A. I can provide patch to ignore packets with port 0.

Q4. Can the two separate plain reads of cp->cport in ip_vs_conn_out_get()
see different values?
A. We can check p->vport, see Q1

Q5. TEMPLATE and NO_CPORT
A. Fixed by "ipvs: filter some flags received in the backup server"
which is already in the trees

	There is the option to create 3th patch "validate vport and dport 
in received sync records" in your patchset, so that such change can be 
propagated together:

	} else if (!param->vport) {
		...
	} else if (!dport &&
		   (flags & IP_VS_CONN_F_FWD_MASK) == IP_VS_CONN_F_MASQ) {
		...
	}

	Because it depends on your "ipvs: validate cport in received sync 
records" patch. Such change will prevent sync messages to create
connections with port 0 that can be later hit by packets with
port 0. But may be such change is not needed.

	I can provide a separate patch that will disallow connections
with 0 in cp->cport/vport created from packets. It will not conflict
with your changes.

	After correcting the check in ip_vs_conn_out_get() lookup misses
caused by client port 0 can lead to scheduling for every packet and 
creating duplicate connections. The main thing is to isolate cport 0
which for the backup server is already in your patch. The remaining part 
is to filter it in the scheduler. OTOH, the vport/dport checks are
just nice to have. The rule here is, if we do not match cport 0 when 
NO_CPORT is not set, we should also not create connections with cport 0.
As for the zero vport/dport, I'm not sure, they look harmless.
What do you think?

Regards

--
Julian Anastasov <ja@ssi.bg>