|
On Sun, Jun 28, 2026 at 4:47 AM Julian Anastasov <ja@xxxxxx> wrote:
>
>
> 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.
>
Thank you for the review.
I agree. Preserving the forwarding method is too conservative and can
break backup setups where the backup server intentionally uses its own
forwarding method.
For v2 I will drop IP_VS_CONN_F_ONE_PACKET from conn_flags
unconditionally, since one-packet connections are not synced.
For the MASQ/non-MASQ transition on already hashed non-template
connections, I agree that the right fix is to update only the hn1 hash
node instead of preserving the old forwarding method. Since this needs
to follow the conn_tab locking rules carefully, I would appreciate the
helper you mentioned and will use it for v2.
Thanks,
Zhiling
> > /* 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>
>
|
