netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tobias Waldekranz <tobias@waldekranz.com>
To: Vladimir Oltean <olteanv@gmail.com>, Marek Behun <marek.behun@nic.cz>
Cc: "Ansuel Smith" <ansuelsmth@gmail.com>,
	netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Vivien Didelot" <vivien.didelot@gmail.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andriin@fb.com>,
	"Eric Dumazet" <edumazet@google.com>,
	"Wei Wang" <weiwan@google.com>,
	"Cong Wang" <cong.wang@bytedance.com>,
	"Taehee Yoo" <ap420073@gmail.com>,
	"Björn Töpel" <bjorn@kernel.org>,
	"zhang kai" <zhangkaiheb@126.com>,
	"Weilong Chen" <chenweilong@huawei.com>,
	"Roopa Prabhu" <roopa@cumulusnetworks.com>,
	"Di Zhu" <zhudi21@huawei.com>,
	"Francis Laniel" <laniel_francis@privacyrequired.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC net-next 0/3] Multi-CPU DSA support
Date: Mon, 12 Apr 2021 14:46:11 +0200	[thread overview]
Message-ID: <878s5nllgs.fsf@waldekranz.com> (raw)
In-Reply-To: <20210411185017.3xf7kxzzq2vefpwu@skbuf>

On Sun, Apr 11, 2021 at 21:50, Vladimir Oltean <olteanv@gmail.com> wrote:
> On Sun, Apr 11, 2021 at 08:01:35PM +0200, Marek Behun wrote:
>> On Sat, 10 Apr 2021 15:34:46 +0200
>> Ansuel Smith <ansuelsmth@gmail.com> wrote:
>> 
>> > Hi,
>> > this is a respin of the Marek series in hope that this time we can
>> > finally make some progress with dsa supporting multi-cpu port.
>> > 
>> > This implementation is similar to the Marek series but with some tweaks.
>> > This adds support for multiple-cpu port but leave the driver the
>> > decision of the type of logic to use about assigning a CPU port to the
>> > various port. The driver can also provide no preference and the CPU port
>> > is decided using a round-robin way.
>> 
>> In the last couple of months I have been giving some thought to this
>> problem, and came up with one important thing: if there are multiple
>> upstream ports, it would make a lot of sense to dynamically reallocate
>> them to each user port, based on which user port is actually used, and
>> at what speed.
>> 
>> For example on Turris Omnia we have 2 CPU ports and 5 user ports. All
>> ports support at most 1 Gbps. Round-robin would assign:
>>   CPU port 0 - Port 0
>>   CPU port 1 - Port 1
>>   CPU port 0 - Port 2
>>   CPU port 1 - Port 3
>>   CPU port 0 - Port 4
>> 
>> Now suppose that the user plugs ethernet cables only into ports 0 and 2,
>> with 1, 3 and 4 free:
>>   CPU port 0 - Port 0 (plugged)
>>   CPU port 1 - Port 1 (free)
>>   CPU port 0 - Port 2 (plugged)
>>   CPU port 1 - Port 3 (free)
>>   CPU port 0 - Port 4 (free)
>> 
>> We end up in a situation where ports 0 and 2 share 1 Gbps bandwidth to
>> CPU, and the second CPU port is not used at all.
>> 
>> A mechanism for automatic reassignment of CPU ports would be ideal here.
>> 
>> What do you guys think?
>
> The reason why I don't think this is such a great idea is because the
> CPU port assignment is a major reconfiguration step which should at the
> very least be done while the network is down, to avoid races with the
> data path (something which this series does not appear to handle).
> And if you allow the static user-port-to-CPU-port assignment to change
> every time a link goes up/down, I don't think you really want to force
> the network down through the entire switch basically.
>
> So I'd be tempted to say 'tough luck' if all your ports are not up, and
> the ones that are are assigned statically to the same CPU port. It's a
> compromise between flexibility and simplicity, and I would go for
> simplicity here. That's the most you can achieve with static assignment,
> just put the CPU ports in a LAG if you want better dynamic load balancing
> (for details read on below).

I agree. Unless you only have a few really wideband flows, a LAG will
typically do a great job with balancing. This will happen without the
user having to do any configuration at all. It would also perform well
in "router-on-a-stick"-setups where the incoming and outgoing port is
the same.

...

> But there is something which is even more interesting about Felix with
> the ocelot-8021q tagger. Since Marek posted his RFC and until Ansuel
> posted the follow-up, things have happened, and now both Felix and the
> Marvell driver support LAG offload via the bonding and/or team drivers.
> At least for Felix, when using the ocelot-8021q tagged, it should be
> possible to put the two CPU ports in a hardware LAG, and the two DSA
> masters in a software LAG, and let the bond/team upper of the DSA
> masters be the CPU port.
>
> I would like us to keep the door open for both alternatives, and to have
> a way to switch between static user-to-CPU port assignment, and LAG.
> I think that if there are multiple 'ethernet = ' phandles present in the
> device tree, DSA should populate a list of valid DSA masters, and then
> call into the driver to allow it to select which master it prefers for
> each user port. This is similar to what Ansuel added with 'port_get_preferred_cpu',
> except that I chose "DSA master" and not "CPU port" for a specific reason.
> For LAG, the DSA master would be bond0.

I do not see why we would go through the trouble of creating a
user-visible bond/team for this. As you detail below, it would mean
jumping through a lot of hoops. I am not sure there is that much we can
use from those drivers.

- We know that the CPU ports are statically up, so there is no "active
  transmit set" to manage, it always consists of all ports.

- The LAG members are statically known at boot time via the DT, so we do
  not need (or want, in fact) any management of that from userspace.

We could just let the drivers setup the LAG internally, and then do the
load-balancing in dsa_slave_xmit or provide a generic helper that the
taggers could use.

  parent reply	other threads:[~2021-04-12 12:46 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-10 13:34 [PATCH RFC net-next 0/3] Multi-CPU DSA support Ansuel Smith
2021-04-10 13:34 ` [PATCH RFC net-next 1/3] net: dsa: allow for multiple CPU ports Ansuel Smith
2021-04-12  3:35   ` DENG Qingfang
2021-04-12  4:41     ` Ansuel Smith
2021-04-12 15:30       ` DENG Qingfang
2021-04-12 16:17         ` Frank Wunderlich
2021-04-10 13:34 ` [PATCH RFC net-next 2/3] net: add ndo for setting the iflink property Ansuel Smith
2021-04-10 13:34 ` [PATCH RFC net-next 3/3] net: dsa: implement ndo_set_netlink for chaning port's CPU port Ansuel Smith
2021-04-10 13:34 ` [PATCH RFC iproute2-next] iplink: allow to change iplink value Ansuel Smith
2021-04-11 17:04   ` Stephen Hemminger
2021-04-11 17:09     ` Vladimir Oltean
2021-04-11 18:01 ` [PATCH RFC net-next 0/3] Multi-CPU DSA support Marek Behun
2021-04-11 18:08   ` Ansuel Smith
2021-04-11 18:39   ` Andrew Lunn
2021-04-12  2:07     ` Florian Fainelli
2021-04-12  4:53     ` Ansuel Smith
2021-04-11 18:50   ` Vladimir Oltean
2021-04-11 23:53     ` Vladimir Oltean
2021-04-12  2:10       ` Florian Fainelli
2021-04-12  5:04     ` Ansuel Smith
2021-04-12 12:46     ` Tobias Waldekranz [this message]
2021-04-12 14:35       ` Vladimir Oltean
2021-04-12 21:06         ` Tobias Waldekranz
2021-04-12 19:30       ` Marek Behun
2021-04-12 21:22         ` Tobias Waldekranz
2021-04-12 21:34           ` Vladimir Oltean
2021-04-12 21:49             ` Tobias Waldekranz
2021-04-12 21:56               ` Marek Behun
2021-04-12 22:06               ` Vladimir Oltean
2021-04-12 22:26                 ` Tobias Waldekranz
2021-04-12 22:48                   ` Vladimir Oltean
2021-04-12 23:04                     ` Marek Behun
2021-04-12 21:50           ` Marek Behun
2021-04-12 22:05             ` Tobias Waldekranz
2021-04-12 22:55               ` Marek Behun
2021-04-12 23:09                 ` Tobias Waldekranz
2021-04-12 23:13                   ` Tobias Waldekranz
2021-04-12 23:54                     ` Marek Behun
2021-04-13  0:27                       ` Marek Behun
2021-04-13  0:31                         ` Marek Behun
2021-04-13 14:46                         ` Tobias Waldekranz
2021-04-13 15:14                           ` Marek Behun
2021-04-13 18:16                             ` Tobias Waldekranz
2021-04-14 15:14                               ` Marek Behun
2021-04-14 18:39                                 ` Tobias Waldekranz
2021-04-14 23:39                                   ` Vladimir Oltean
2021-04-15  9:20                                     ` Tobias Waldekranz
2021-04-13 14:40                       ` Tobias Waldekranz
2021-04-12 15:00     ` DENG Qingfang
2021-04-12 16:32       ` Vladimir Oltean
2021-04-12 22:04         ` Marek Behun
2021-04-12 22:17           ` Vladimir Oltean
2021-04-12 22:47             ` Marek Behun
  -- strict thread matches above, loose matches on Subject: below --
2019-08-24  2:42 Marek Behún
2019-08-24 15:24 ` Andrew Lunn
2019-08-24 17:45   ` Marek Behun
2019-08-24 17:54     ` Andrew Lunn
2019-08-25  4:19   ` Marek Behun
2019-08-24 15:40 ` Vladimir Oltean
2019-08-24 15:44   ` Vladimir Oltean
2019-08-24 17:55     ` Marek Behun
2019-08-24 15:56   ` Andrew Lunn
2019-08-24 17:58     ` Marek Behun
2019-08-24 20:04 ` Florian Fainelli
2019-08-24 21:01   ` Marek Behun
2019-08-25  4:08     ` Marek Behun
2019-08-25  7:13   ` Marek Behun
2019-08-25 15:00     ` Florian Fainelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878s5nllgs.fsf@waldekranz.com \
    --to=tobias@waldekranz.com \
    --cc=andrew@lunn.ch \
    --cc=andriin@fb.com \
    --cc=ansuelsmth@gmail.com \
    --cc=ap420073@gmail.com \
    --cc=ast@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=chenweilong@huawei.com \
    --cc=cong.wang@bytedance.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=laniel_francis@privacyrequired.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marek.behun@nic.cz \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=vivien.didelot@gmail.com \
    --cc=weiwan@google.com \
    --cc=zhangkaiheb@126.com \
    --cc=zhudi21@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).