On Thu, Sep 7, 2017 at 5:39 PM, Mahesh Bandewar (महेश बंडेवार) <mahe...@google.com> wrote: > On Thu, Sep 7, 2017 at 4:09 PM, Nikolay Aleksandrov > <niko...@cumulusnetworks.com> wrote: >> On 7.09.2017 01:47, Kosuke Tatsukawa wrote: >>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in >>> balance-alb mode") tried to fix transmit dynamic load balancing in >>> balance-alb mode, which wasn't working after commit 8b426dc54cf4 >>> ("bonding: remove hardcoded value"). >>> >>> It turned out that my previous patch only fixed the case when >>> balance-alb was specified as bonding module parameter, and not when >>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most >>> common usage). In the latter case, tlb_dynamic_lb was set up according >>> to the default mode of the bonding interface, which happens to be >>> balance-rr. >>> >>> This additional patch addresses this issue by setting up tlb_dynamic_lb >>> to 1 if "mode" is set to balance-alb through the sysfs interface. >>> >>> I didn't add code to change tlb_balance_lb back to the default value for >>> other modes, because "mode" is usually set up only once during >>> initialization, and it's not worthwhile to change the static variable >>> bonding_defaults in bond_main.c to a global variable just for this >>> purpose. >>> >>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for >>> balance-tlb mode if it is set up using the sysfs interface. I didn't >>> change that behavior, because the value of tlb_balance_lb can be changed >>> using the sysfs interface for balance-tlb, and I didn't like changing >>> the default value back and forth for balance-tlb. >>> >>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be >>> written to. However, I think balance-alb with tlb_dynamic_lb set to 0 >>> is not an intended usage, so there is little use making it writable at >>> this moment. >>> >>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value") > > This is wrong! I think you are confusing various things here. ALB is > different mode from TLB and TLB-dynamic-lb is *only* a special case of > TLB. Your earlier patch is also wrong for the same reasons. However, > since the default value of tlb_dynamic_lb is set to 0 it's not > causing issues for ALB mode otherwise it would break ALB mode. I take this back. The default value is 1 so ALB is broken because of the referenced change.
> tlb_dynamic_lb has absolutely nothing to do with ALB mode. Please > revert the earlier change (cbf5ecb30560). > > It's not clear to me what you saw as broken, so can't really suggest > what really need to be done.