LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH] ipvs: use explicitly signed chars

To: Julian Anastasov <ja@xxxxxx>
Subject: Re: [PATCH] ipvs: use explicitly signed chars
Cc: netdev@xxxxxxxxxxxxxxx, lvs-devel@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>, Simon Horman <horms@xxxxxxxxxxxx>, stable@xxxxxxxxxxxxxxx
From: "Jason A. Donenfeld" <Jason@xxxxxxxxx>
Date: Wed, 26 Oct 2022 16:30:17 +0200
On Wed, Oct 26, 2022 at 05:20:03PM +0300, Julian Anastasov wrote:
> 
>       Hello,
> 
> On Wed, 26 Oct 2022, Jason A. Donenfeld wrote:
> 
> > The `char` type with no explicit sign is sometimes signed and sometimes
> > unsigned. This code will break on platforms such as arm, where char is
> > unsigned. So mark it here as explicitly signed, so that the
> > todrop_counter decrement and subsequent comparison is correct.
> > 
> > Cc: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> > Cc: Julian Anastasov <ja@xxxxxx>
> > Cc: Simon Horman <horms@xxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
> 
>       Looks good to me for -next, thanks!

This is actually net.git material, not net-next.git material,
considering it fixes a bug on arm and many other archs, and is marked
with a stable@ tag.

> 
> Acked-by: Julian Anastasov <ja@xxxxxx>
> 
> > ---
> >  net/netfilter/ipvs/ip_vs_conn.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/netfilter/ipvs/ip_vs_conn.c 
> > b/net/netfilter/ipvs/ip_vs_conn.c
> > index 8c04bb57dd6f..7c4866c04343 100644
> > --- a/net/netfilter/ipvs/ip_vs_conn.c
> > +++ b/net/netfilter/ipvs/ip_vs_conn.c
> > @@ -1249,40 +1249,40 @@ static const struct seq_operations 
> > ip_vs_conn_sync_seq_ops = {
> >     .next  = ip_vs_conn_seq_next,
> >     .stop  = ip_vs_conn_seq_stop,
> >     .show  = ip_vs_conn_sync_seq_show,
> >  };
> >  #endif
> >  
> >  
> >  /* Randomly drop connection entries before running out of memory
> >   * Can be used for DATA and CTL conns. For TPL conns there are exceptions:
> >   * - traffic for services in OPS mode increases ct->in_pkts, so it is 
> > supported
> >   * - traffic for services not in OPS mode does not increase ct->in_pkts in
> >   * all cases, so it is not supported
> >   */
> >  static inline int todrop_entry(struct ip_vs_conn *cp)
> >  {
> >     /*
> >      * The drop rate array needs tuning for real environments.
> >      * Called from timer bh only => no locking
> >      */
> > -   static const char todrop_rate[9] = {0, 1, 2, 3, 4, 5, 6, 7, 8};
> > -   static char todrop_counter[9] = {0};
> > +   static const signed char todrop_rate[9] = {0, 1, 2, 3, 4, 5, 6, 7, 8};
> > +   static signed char todrop_counter[9] = {0};
> >     int i;
> >  
> >     /* if the conn entry hasn't lasted for 60 seconds, don't drop it.
> >        This will leave enough time for normal connection to get
> >        through. */
> >     if (time_before(cp->timeout + jiffies, cp->timer.expires + 60*HZ))
> >             return 0;
> >  
> >     /* Don't drop the entry if its number of incoming packets is not
> >        located in [0, 8] */
> >     i = atomic_read(&cp->in_pkts);
> >     if (i > 8 || i < 0) return 0;
> >  
> >     if (!todrop_rate[i]) return 0;
> >     if (--todrop_counter[i] > 0) return 0;
> >  
> >     todrop_counter[i] = todrop_rate[i];
> >     return 1;
> >  }
> > -- 
> > 2.38.1
> 
> Regards
> 
> --
> Julian Anastasov <ja@xxxxxx>
> 

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