Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753106AbdIHBzS (ORCPT ); Thu, 7 Sep 2017 21:55:18 -0400 Received: from mail-yw0-f178.google.com ([209.85.161.178]:35717 "EHLO mail-yw0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750852AbdIHBzQ (ORCPT ); Thu, 7 Sep 2017 21:55:16 -0400 X-Google-Smtp-Source: ADKCNb5iA5pN7vhmfZA8iKbj4nwYkMDXVCCLhv5X3/laXeUwyaMNECo+0VdYZ6E6csqSQcBUqA8ESLeWr5WHn9g6UGk= MIME-Version: 1.0 In-Reply-To: References: <17EC94B0A072C34B8DCF0D30AD16044A02985CB5@BPXM09GP.gisp.nec.co.jp> From: =?UTF-8?B?TWFoZXNoIEJhbmRld2FyICjgpK7gpLngpYfgpLYg4KSs4KSC4KSh4KWH4KS14KS+4KSwKQ==?= Date: Thu, 7 Sep 2017 18:54:54 -0700 Message-ID: Subject: Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs To: Nikolay Aleksandrov Cc: Kosuke Tatsukawa , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Reinis Rozitis Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v881tTBb020731 Content-Length: 2855 Lines: 55 On Thu, Sep 7, 2017 at 5:47 PM, Mahesh Bandewar (महेश बंडेवार) wrote: > On Thu, Sep 7, 2017 at 5:39 PM, Mahesh Bandewar (महेश बंडेवार) > wrote: >> On Thu, Sep 7, 2017 at 4:09 PM, Nikolay Aleksandrov >> 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. Please scratch all I said.