LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH 1/1] ipvs: preserve conn hash flags when late-binding dest

To: Ren Wei <n05ec@xxxxxxxxxx>
Subject: Re: [PATCH 1/1] ipvs: preserve conn hash flags when late-binding dest
Cc: lvs-devel@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, horms@xxxxxxxxxxxx, pablo@xxxxxxxxxxxxx, fw@xxxxxxxxx, phil@xxxxxx, kaber@xxxxxxxxx, nick@xxxxxxxxxxxxxxxx, yuantan098@xxxxxxxxx, yifanwucs@xxxxxxxxx, tomapufckgml@xxxxxxxxx, bird@xxxxxxxxxx, roxy520tt@xxxxxxxxx
From: Julian Anastasov <ja@xxxxxx>
Date: Sat, 27 Jun 2026 23:46:39 +0300 (EEST)
        Hello,

On Sun, 28 Jun 2026, Ren Wei wrote:

> From: Zhiling Zou <roxy520tt@xxxxxxxxx>
> 
> Synced connections can be created before their destination exists. When
> the destination is later added, ip_vs_try_bind_dest() binds it to the
> existing connection through ip_vs_bind_dest().
> 
> ip_vs_bind_dest() copies destination connection flags into cp->flags.
> For an already hashed connection, changing flags that define conn_tab
> membership breaks the hash table invariants. In particular, adding
> IP_VS_CONN_F_ONE_PACKET after the connection has been hashed can make
> expiry skip unlinking it from conn_tab. Changing the forwarding method
> can also make unlink use a different single or double hash-node layout
> than the one used at insertion time.
> 
> Preserve the flags that define conn_tab hashing when binding a
> destination to an already hashed connection.
> 
> Fixes: 26ec037f9841 ("IPVS: one-packet scheduling")

        The problem with the fix is that we should do it
in the hard way: the backup server should be able to define
its own forwarding methods. Otherwise, we can break existing
setups. For example, master can have localnode for some
dests, this can not be preserved in the backup for the
synced conns.

> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Yuan Tan <yuantan098@xxxxxxxxx>
> Reported-by: Yifan Wu <yifanwucs@xxxxxxxxx>
> Reported-by: Juefei Pu <tomapufckgml@xxxxxxxxx>
> Reported-by: Xin Liu <bird@xxxxxxxxxx>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zhiling Zou <roxy520tt@xxxxxxxxx>
> Signed-off-by: Ren Wei <n05ec@xxxxxxxxxx>
> ---
>  net/netfilter/ipvs/ip_vs_conn.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index cb36641f8d1c..016273906aac 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -998,7 +998,11 @@ static inline int ip_vs_dest_totalconns(struct 
> ip_vs_dest *dest)
>  static inline void
>  ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
>  {
> +     const unsigned int hash_flags = IP_VS_CONN_F_FWD_MASK |
> +                                     IP_VS_CONN_F_NOOUTPUT |
> +                                     IP_VS_CONN_F_ONE_PACKET;
>       unsigned int conn_flags;
> +     __u32 old_flags;
>       __u32 flags;
>  
>       /* if dest is NULL, then return directly */
> @@ -1011,7 +1015,8 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct 
> ip_vs_dest *dest)
>       conn_flags = atomic_read(&dest->conn_flags);
>       if (cp->protocol != IPPROTO_UDP)
>               conn_flags &= ~IP_VS_CONN_F_ONE_PACKET;
> -     flags = cp->flags;
> +     old_flags = cp->flags;
> +     flags = old_flags;
>       /* Bind with the destination and its corresponding transmitter */
>       if (flags & IP_VS_CONN_F_SYNC) {

        We can here unconditionally drop the IP_VS_CONN_F_ONE_PACKET flag:

                conn_flags &= ~IP_VS_CONN_F_ONE_PACKET;

        Because IP_VS_CONN_F_ONE_PACKET conns are not synced.

        And here when (flags & IP_VS_CONN_F_HASHED) and the fwd
        method changes between MASQ and non-MASQ for 
        !IP_VS_CONN_F_TEMPLATE we should call some new func
        that properly hashes/unhashes just the hn1 node.
        I can provide such function with proper locking.

>               /* if the connection is not template and is created
> @@ -1023,6 +1028,13 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct 
> ip_vs_dest *dest)
>               flags &= ~(IP_VS_CONN_F_FWD_MASK | IP_VS_CONN_F_NOOUTPUT);
>       }
>       flags |= conn_flags;
> +
> +     /* Preserve conn_tab hashing invariants after late binding. */
> +     if (old_flags & IP_VS_CONN_F_HASHED) {
> +             flags &= ~hash_flags;
> +             flags |= old_flags & hash_flags;
> +     }
> +
>       cp->flags = flags;
>       cp->dest = dest;

Regards

--
Julian Anastasov <ja@xxxxxx>



<Prev in Thread] Current Thread [Next in Thread>