From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3868A443AA6 for ; Wed, 19 Aug 2026 10:14:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787134453; cv=none; b=VaJOUGQHXN1mtQcqaMwgqyKEaFrNtsvdyKcUZ8O3GoFWsac9pmklP3IO91fH2y0rVfKxCP1TSeYgoeLnXWxXaia9JYOfPdGY7C/lnw7FfO3/0rhPPIGNSr8A8fTn9KYeht12A/cj/HLctCweuJ9lRSHlIQYKLHV2OmUu6boldQU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787134453; c=relaxed/simple; bh=mJ4TkWkCmsmvOVV58OS1T7y6PlXzGj1rJS8dfNbodo8=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=RW3tkrrAexD4hNCLaEB1cvi4xeqaePYOcODKExm/pQs/7tbAwV5uS0Ws6aMYs4njGKUlNTlb/9Z38FUMPRZjTLBp61AtwRrQDCTFbhGv4zCWTZJEVP5DjeAapIIfV8ag9p3c/a+mFtPyTriKiKJATPfQNIxMh17f187CiF6bTVA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=blackwall.org; spf=none smtp.mailfrom=blackwall.org; dkim=pass (2048-bit key) header.d=blackwall.org header.i=@blackwall.org header.b=dNkxR8E2; arc=none smtp.client-ip=209.85.128.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=blackwall.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=blackwall.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=blackwall.org header.i=@blackwall.org header.b="dNkxR8E2" Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-4954dff6536so6957095e9.0 for ; Wed, 19 Aug 2026 03:14:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=blackwall.org; s=google; t=1787134450; x=1787739250; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:references:cc:to :from:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to:content-type; bh=UAK1rtPDpHr45I7pYyTZ7WdNt1ScKed4iT4J+fZ9Gyg=; b=dNkxR8E2lCV+ovzSTiP0UNBiOmov909t9fmmh9m6M9pBMiQpi8T73vWQittlmGxwWv ts8XlYRMI8MucqDv5FUPL1Lghx14DvDYkkcARap7mC1e7MkHDydJLrGtS4NJDBBzaZCg ETWuNjdwdD1zV7nEk6FynCmymgJUV12Ck1/uUSvjWkDTyNJ7+5C9xi1gVtw8SoqYmAVu ZpIli+bXuNDzGCZNXcqCtb6zN6dcyiYAkK7CyNn7GUtodVr2tKiXxv5gmnCYzqJ0L0IG RA6feuCWjchY7c2eEXppAZRKFVs/iIY+/EgmepEAF/IFPgms77X7mEA/29djkdEHMZna FV2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787134450; x=1787739250; h=content-transfer-encoding:content-type:in-reply-to:references:cc:to :from:content-language:subject:user-agent:mime-version:date :message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=UAK1rtPDpHr45I7pYyTZ7WdNt1ScKed4iT4J+fZ9Gyg=; b=TMZxuh35nbaN3+HyXKY7xfFCgaD8j0JUSfn4bTzXF2E2k1YJqpf0aTVebDUFATA1Nz sBTBeXNpgQ+QRBfG0+JpLpEtAlyQ3zDs4UOiiCSP6G/jp8WraLq65BIIuSuJRwXmGwPz lHRuFDaurhSfcyk45tN22K+KVqsg1nsLCa5Cuo0jFOjofNpUGhPKJ0SozGM83oenIS3l v448KSIP3cpgOVtHQo0FyHqEUhv9HHxvwJvNUrBR2mLb2caDJC4E324p6iF0E+EB8BNs jJDwWpo6pDAkZiwNwz9oHLg7jpFvkxea4N4kDfQeCX4/o2r7r2Bj6ZCAEr46lxCEZd0L wIWw== X-Forwarded-Encrypted: i=1; AHgh+Rp+6Q2XejSOrCMwwZ5dx5xTse/KpIqym7L7OuFMd9JSRyDwkG5nEcPhNWbJFbyteq/p57jHH21yL1giUaM=@vger.kernel.org X-Gm-Message-State: AOJu0Yz6Kz+Sr4fKWFQ7kLd+RqSiqTcdIrmOQpLyBVR6qSPtbN3zx3Qg +dJyU+DBixbzQRmJQ/KVHN/xl60tGxUvnVm2rn2avSgwgTspCBluBGBjTDlQZ2Vk7Gs= X-Gm-Gg: AR+sD12SPFnpTwxdgVcE4uSKcRJtytJRQl+ovrAlESanoz2y9WVlgwJI9jt+OvbyzYS xmkQr/uyqYUEywzwFRjwOAo3qUL6Ois8OlNLAV3Zqf5pz2XzStCL0mOAA9fdroyqFwW0bfgjyun GbwNJp94bMuwMpNV4vOYtgxCspeCyhZ+/H+Z5ZmTwypIMWXIQYysoF8clvfFUmQcx8mBIQLQQYl 3VVseqAhZB6HaeuaZ8x/B6jyXljkJSk9LjVQY6AEEz1sSUk4Hly8Z0kRjyLNYNKkly7sHxXm6sF z0hPLbtPQ6lvZlyIYHmwQKC0pUjNyiVXNqYVjAVqVojjEohcOunpBBJwKjInLapsOEZT0B2yp1d qrjOB+29b1s2RdgF3imhJ2ck7uhaAulfXsz/fiVxe8HGTPfW8JsNv9d2ssdHPoqHSZue+glwmq4 Wz/QwwQJtXyiUD1LGj5zHXYx6bWltODocnASeADBIY9QAmbaHMDn9L2vSez87PF66a36ooYOlh6 uyKDxpudIm9hN4/49A= X-Received: by 2002:a05:600c:34ca:b0:499:858e:de42 with SMTP id 5b1f17b1804b1-499aa172300mr70859005e9.6.1787134450214; Wed, 19 Aug 2026 03:14:10 -0700 (PDT) Received: from [192.168.0.161] (78-154-15-182.ip.btc-net.bg. [78.154.15.182]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-499aa0dd620sm46012225e9.13.2026.08.19.03.14.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Aug 2026 03:14:09 -0700 (PDT) Message-ID: Date: Wed, 19 Aug 2026 13:14:08 +0300 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net v3 2/2] bonding: fix u32 overflow in compute_gap() Content-Language: en-US, bg From: Nikolay Aleksandrov To: Hangbin Liu Cc: Jay Vosburgh , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Hangbin Liu References: <20260818-bond_overflow-v3-0-e05d4dbc2fd8@kylinos.cn> <20260818-bond_overflow-v3-2-e05d4dbc2fd8@kylinos.cn> <80d704a8-aca6-44f8-8933-eb0cf14ecf3b@blackwall.org> <1ebd9c8a-5b7e-4ebb-9c7d-5b2b2fe4a675@blackwall.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 19/08/2026 13:02, Nikolay Aleksandrov wrote: > On 19/08/2026 12:51, Hangbin Liu wrote: >> On Wed, Aug 19, 2026 at 11:35:29AM +0300, Nikolay Aleksandrov wrote: >>> hmm why don't you change the way the reset is done? *untested* but in theory >>> you could just record the values at a reset "moment" in reset unbalanced and >>> just use the delta, so it becomes a reader and there is only 1 writer left (tx). >>> Keep the counters only increasing (important), only record a snapshot at a reset >>> moment, count current total bytes (sum all per-cpu data), decrement the previous >>> total from it and use that as the "interval bytes" to div. >> >> Oh, you mean add another variable to track the total unbalanced load? e.g. >> > > right, but without any locking because... > one more minor nit below >> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c >> index 659a77323444..a65be54049d3 100644 >> --- a/drivers/net/bonding/bond_alb.c >> +++ b/drivers/net/bonding/bond_alb.c >> @@ -1546,10 +1546,10 @@ netdev_tx_t bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) >>       return bond_do_alb_xmit(skb, bond, tx_slave); >>   } >> -static u64 reset_unbalanced_load(struct alb_bond_info *bond_info) >> +static u64 reset_unbalanced_load(struct bonding *bond, struct alb_bond_info *bond_info) >>   { >>       struct unbalanced_load_stats *p; >> -    u64 tx_bytes, total_bytes = 0; >> +    u64 delta, tx_bytes, total_bytes = 0; >>       unsigned int start; >>       int i; >> @@ -1560,14 +1560,15 @@ static u64 reset_unbalanced_load(struct alb_bond_info *bond_info) >>               tx_bytes = u64_stats_read(&p->tx_bytes); >>           } while (u64_stats_fetch_retry(&p->syncp, start)); >> -        u64_stats_update_begin(&p->syncp); >> -        u64_stats_set(&p->tx_bytes, 0); >> -        u64_stats_update_end(&p->syncp); >> - >>           total_bytes += tx_bytes; >>       } >> -    return div_u64(total_bytes, BOND_TLB_REBALANCE_INTERVAL); >> +    spin_lock_bh(&bond->mode_lock); >> +    delta = total_bytes - bond_info->total_unbalanced; >> +    bond_info->total_unbalanced = total_bytes; >> +    spin_unlock_bh(&bond->mode_lock); >> + > > ... there should be only 1 alb monitor running, no need to lock to keep it up-to-date >     also this is its only user, so remove the spinlock > >> +    return div_u64(delta, BOND_TLB_REBALANCE_INTERVAL); >>   } >>   void bond_alb_monitor(struct work_struct *work) >> @@ -1612,7 +1613,7 @@ void bond_alb_monitor(struct work_struct *work) >>           bond_for_each_slave_rcu(bond, slave, iter) { >>               tlb_clear_slave(bond, slave, 1); >>               if (slave == rcu_access_pointer(bond->curr_active_slave)) >> -                SLAVE_TLB_INFO(slave).load = reset_unbalanced_load(bond_info); >> +                SLAVE_TLB_INFO(slave).load = reset_unbalanced_load(bond, bond_info); >>           } >>           atomic_set(&bond_info->tx_rebalance_counter, 0); >>       } >> diff --git a/include/net/bond_alb.h b/include/net/bond_alb.h >> index 51c083c76115..9d3877644286 100644 >> --- a/include/net/bond_alb.h >> +++ b/include/net/bond_alb.h >> @@ -131,6 +131,7 @@ struct unbalanced_load_stats { >>   struct alb_bond_info { >>       struct tlb_client_info    *tx_hashtbl; /* Dynamically allocated */ >>       struct unbalanced_load_stats __percpu    *unbalanced_load; >> +    u64            total_unbalanced; I'd name this prev_total_unbalanced or something similar since it is the previous recorded value >>       atomic_t        tx_rebalance_counter; >>       int            lp_counter; >>       /* -------- rlb parameters -------- */ >> >> This looks like an easy update :) Hope I didn't miss anything. >> >> Thanks >> Hangbin >