netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.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>,
	"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: Sun, 11 Apr 2021 19:10:36 -0700	[thread overview]
Message-ID: <84a27330-73cb-3ebe-a0a9-eb6afec55902@gmail.com> (raw)
In-Reply-To: <20210411235358.vpql2mppobjhknfg@skbuf>



On 4/11/2021 4:53 PM, Vladimir Oltean wrote:
> On Sun, Apr 11, 2021 at 09:50:17PM +0300, Vladimir Oltean 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).
> 
> Just one more small comment, because I got so carried away with
> describing what I already had in mind, that I forgot to completely
> address your idea.
> 
> I think that DSA should provide the means to do what you want but not
> the policy.

Could not agree more, this point is what has historically prevented any
multi-CPU port patch series from landing because what everyone seems to
have wanted so far is along these lines:

- assign LAN ports 0-3 to CPU port #0
- assign WAN port 4 to CPU port #1

and do that from Device Tree, problem solved? Not entirely unfortunately.

Being able to change the mapping via iproute2 is definitively an
improvement, and to echo to your comment on the iproute2 change proper
we can try to agree on a more specialized syntax.

> Meaning that you can always write a user space program that
> monitors the NETLINK_ROUTE rtnetlink through a socket and listens for
> link state change events on it with poll(), then does whatever (like
> moves the static user-to-CPU port mapping in the way that is adequate to
> your network's requirements). The link up/down events are already
> emitted, and the patch set here gives user space the rope to hang itself.

That seems like an entirely reasonable approach to me, and solving how
to map a given user-port to a particular CPU port definitively belongs
in user-space, within the constraints expressed by what the switch
driver can do of course.

> 
> If you need inspiration, one user of the rtnetlink socket that I know of
> is ptp4l:
> https://github.com/richardcochran/linuxptp/blob/master/rtnl.c
> 

-- 
Florian

  reply	other threads:[~2021-04-12  2:10 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 [this message]
2021-04-12  5:04     ` Ansuel Smith
2021-04-12 12:46     ` Tobias Waldekranz
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=84a27330-73cb-3ebe-a0a9-eb6afec55902@gmail.com \
    --to=f.fainelli@gmail.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=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).