All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Julian Anastasov <ja@ssi.bg>
Cc: Simon Horman <horms@verge.net.au>,
	Florian Westphal <fw@strlen.de>,
	lvs-devel@vger.kernel.org, netfilter-devel@vger.kernel.org
Subject: Re: [PATCH nf v2] ipvs: do not create conns from packets with port 0
Date: Thu, 5 Nov 2026 13:50:44 +0100	[thread overview]
Message-ID: <aux7pEgj-lz9eq4c@chamomile> (raw)
In-Reply-To: <20261104201530.147292-1-ja@ssi.bg>

On Wed, Nov 04, 2026 at 10:15:30PM +0200, Julian Anastasov wrote:
> We should disallow normal connections with cport 0 to be
> created from packets by the scheduler (from clients) or
> by ip_vs_new_conn_out (from real servers) because later
> such traffic can not hit the connection due to the
> NO_CPORT requirement. As result, it can lead to duplicate
> connection for every packet. Do the same for vport 0.

I think a follow up is a better option for patches that are not
clearly wrong, because I prefer to drop patches that need more work,
otherwise we risk entering a loop of N iterations in a PR.

I think safest approach is to drop anything that is controversial.

Just for the next time, if it helps, if patch is good but not perfect
(as it seems to be to me in this case), a follow up is better IMO.

So let's find a slot for this patch in the next PR chance, I think it
is better to skip this new v2 at this stage.

Apologies and thanks.

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103140308.591690-3-pablo%40netfilter.org
> Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925141115.16126-1-axel.mierczuk%401password.com
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> ---
>  Changes in v2:
>  * Drop the packets with port 0 instead of passing them to the local
>    server
> 
>  net/netfilter/ipvs/ip_vs_core.c       | 3 ++-
>  net/netfilter/ipvs/ip_vs_proto_sctp.c | 4 +++-
>  net/netfilter/ipvs/ip_vs_proto_tcp.c  | 4 +++-
>  net/netfilter/ipvs/ip_vs_proto_udp.c  | 3 ++-
>  4 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index fd503f0efb57..0ed5f6c95ad8 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1305,6 +1305,7 @@ static inline bool is_new_conn_expected(const struct ip_vs_conn *cp,
>   *    In fwmark-VS actual vaddr and vport are unknown to IPVS
>   * 2) Real Server and Virtual Service were NOT configured without port:
>   *    This is to allow match of different VS to the same RS ip-addr
> + * 3) Client port is not 0
>   */
>  struct ip_vs_conn *ip_vs_new_conn_out(struct ip_vs_service *svc,
>  				      struct ip_vs_dest *dest,
> @@ -1328,7 +1329,7 @@ struct ip_vs_conn *ip_vs_new_conn_out(struct ip_vs_service *svc,
>  	/* check pre-requisites are satisfied */
>  	if (svc->fwmark)
>  		return NULL;
> -	if (!vport || !dport)
> +	if (!vport || !dport || !cport)
>  		return NULL;
>  
>  	/* for persistent service first create connection template */
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index fb8af6b15a39..31431094073d 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -30,6 +30,7 @@ sctp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
>  			sch = skb_header_pointer(skb, iph->len + sizeof(_sctph),
>  						 sizeof(_schunkh), &_schunkh);
>  			if (sch) {
> +				/* Accept packet without scheduling? */
>  				if (sch->type == SCTP_CID_ABORT ||
>  				    !(sysctl_sloppy_sctp(ipvs) ||
>  				      sch->type == SCTP_CID_INIT))
> @@ -42,7 +43,8 @@ sctp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
>  			skb, iph->len, sizeof(_ports), &_ports);
>  	}
>  
> -	if (!ports) {
> +	/* Do not create connection for port 0 */
> +	if (!ports || !ports[0] || !ports[1]) {
>  		*verdict = NF_DROP;
>  		return 0;
>  	}
> diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> index fec0e8b47b71..6f1cc9165e0c 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> @@ -48,6 +48,7 @@ tcp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
>  	if (likely(!ip_vs_iph_icmp(iph))) {
>  		th = skb_header_pointer(skb, iph->len, sizeof(_tcph), &_tcph);
>  		if (th) {
> +			/* Accept packet without scheduling? */
>  			if (th->rst || !(sysctl_sloppy_tcp(ipvs) || th->syn))
>  				return 1;
>  			ports = &th->source;
> @@ -57,7 +58,8 @@ tcp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
>  			skb, iph->len, sizeof(_ports), &_ports);
>  	}
>  
> -	if (!ports) {
> +	/* Do not create connection for port 0 */
> +	if (!ports || !ports[0] || !ports[1]) {
>  		*verdict = NF_DROP;
>  		return 0;
>  	}
> diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
> index f32785682402..0e7b4427d258 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_udp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
> @@ -47,7 +47,8 @@ udp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
>  			skb, iph->len, sizeof(_ports), &_ports);
>  	}
>  
> -	if (!ports) {
> +	/* Do not create connection for port 0 */
> +	if (!ports || !ports[0] || !ports[1]) {
>  		*verdict = NF_DROP;
>  		return 0;
>  	}
> -- 
> 2.56.0
> 
> 
> 

      parent reply	other threads:[~2026-11-05 12:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04 20:15 [PATCH nf v2] ipvs: do not create conns from packets with port 0 Julian Anastasov
2026-11-05  4:59 ` Julian Anastasov
2026-11-05 12:50 ` Pablo Neira Ayuso [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aux7pEgj-lz9eq4c@chamomile \
    --to=pablo@netfilter.org \
    --cc=fw@strlen.de \
    --cc=horms@verge.net.au \
    --cc=ja@ssi.bg \
    --cc=lvs-devel@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.