From: Tobias Waldekranz <tobias@waldekranz.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Vadym Kochan <vadym.kochan@plvision.eu>,
"David S. Miller" <davem@davemloft.net>,
netdev@vger.kernel.org, Mickey Rachamim <mickeyr@marvell.com>,
linux-kernel@vger.kernel.org,
Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH net-next 5/7] net: marvell: prestera: add LAG support
Date: Tue, 09 Feb 2021 12:56:55 +0100 [thread overview]
Message-ID: <87pn194fp4.fsf@waldekranz.com> (raw)
In-Reply-To: <20210208130557.56b14429@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Mon, Feb 08, 2021 at 13:05, Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 08 Feb 2021 20:54:29 +0100 Tobias Waldekranz wrote:
>> On Thu, Feb 04, 2021 at 21:16, Jakub Kicinski <kuba@kernel.org> wrote:
>> > On Wed, 3 Feb 2021 18:54:56 +0200 Vadym Kochan wrote:
>> >> From: Serhiy Boiko <serhiy.boiko@plvision.eu>
>> >>
>> >> The following features are supported:
>> >>
>> >> - LAG basic operations
>> >> - create/delete LAG
>> >> - add/remove a member to LAG
>> >> - enable/disable member in LAG
>> >> - LAG Bridge support
>> >> - LAG VLAN support
>> >> - LAG FDB support
>> >>
>> >> Limitations:
>> >>
>> >> - Only HASH lag tx type is supported
>> >> - The Hash parameters are not configurable. They are applied
>> >> during the LAG creation stage.
>> >> - Enslaving a port to the LAG device that already has an
>> >> upper device is not supported.
>> >
>> > Tobias, Vladimir, you worked on LAG support recently, would you mind
>> > taking a look at this one?
>>
>> I took a quick look at it, and what I found left me very puzzled. I hope
>> you do not mind me asking a generic question about the policy around
>> switchdev drivers. If someone published a driver using something similar
>> to the following configuration flow:
>>
>> iproute2 daemon(SDK)
>> | ^ |
>> : : : user/kernel boundary
>> v | |
>> netlink | |
>> | | |
>> v | |
>> driver | |
>> | | |
>> '--------' |
>> : kernel/hardware boundary
>> v
>> ASIC
>>
>> My guess is that they would be (rightly IMO) told something along the
>> lines of "we do not accept drivers that are just shims for proprietary
>> SDKs".
>>
>> But it seems like if that same someone has enough area to spare in their
>> ASIC to embed a CPU, it is perfectly fine to run that same SDK on it,
>> call it "firmware", and then push a shim driver into the kernel tree.
>>
>> iproute2
>> |
>> : user/kernel boundary
>> v
>> netlink
>> |
>> v
>> driver
>> |
>> |
>> : kernel/hardware boundary
>> '-------------.
>> v
>> daemon(SDK)
>> |
>> v
>> ASIC
>>
>> What have we, the community, gained by this? In the old world, the
>> vendor usually at least had to ship me the SDK in source form. Having
>> seen the inside of some of those sausage factories, they are not the
>> kinds of code bases that I want at the bottom of my stack; even less so
>> in binary form where I am entirely at the vendor's mercy for bugfixes.
>>
>> We are talking about a pure Ethernet fabric here, so there is no fig
>> leaf of "regulatory requirements" to hide behind, in contrast to WiFi
>> for example.
>>
>> Is it the opinion of the netdev community that it is OK for vendors to
>> use this model?
>
> I ask myself that question pretty much every day. Sadly I have no clear
> answer.
Thank you for your candid answer, really appreciate it. I do not envy
you one bit, making those decisions must be extremely hard.
> Silicon is cheap, you can embed a reasonable ARM or Risc-V core in the
> chip for the area and power draw comparable to one high speed serdes
> lane.
>
> The drivers landing in the kernel are increasingly meaningless. My day
> job is working for a hyperscaler. Even though we have one of the most
> capable kernel teams on the planet most of issues with HW we face
> result in "something is wrong with the FW, let's call the vendor".
Right, and being a hyperscaler probably at least gets you some attention
when you call your vendor. My day job is working for a nanoscaler, so my
experience is that we must be prepared to solve all issues in-house; if
we get any help from the vendor that is just a bonus.
> And even when I say "drivers landing" it is an overstatement.
> If you look at high speed anything these days the drivers cover
> multiple generations of hardware, seems like ~5 years ago most
> NIC vendors reached sufficient FW saturation to cover up differences
> between HW generations.
>
> At the same time some FW is necessary. Certain chip functions, are
> best driven by a micro-controller running a tight control loop.
I agree. But I still do not understand why vendors cling to the source
of these like it was their wallet. That is the beauty of selling
silicon; you can fully leverage OSS and still have a very straight
forward business model.
> The complexity of FW is a spectrum, from basic to Qualcomm.
> The problem is there is no way for us to know what FW is hiding
> by just looking at the driver.
>
> Where do we draw the line?
Yeah it is a very hard problem. In this particular case though, the
vendor explicitly said that what they have done is compiled their
existing SDK to run on the ASIC:
https://lore.kernel.org/netdev/BN6PR18MB1587EB225C6B80BF35A44EBFBA5A0@BN6PR18MB1587.namprd18.prod.outlook.com
So there is no reason that it could not be done as a proper driver.
> Personally I'd really like to see us pushing back stronger.
Hear, hear!
next prev parent reply other threads:[~2021-02-09 11:59 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-03 16:54 [PATCH net-next 0/7] Marvell Prestera Switchdev misc updates Vadym Kochan
2021-02-03 16:54 ` [PATCH net-next 1/7] net: marvell: prestera: bump supported firmware version to 2.5 Vadym Kochan
2021-02-03 16:54 ` [PATCH net-next 2/7] net: marvell: prestera: disable events interrupt while handling Vadym Kochan
2021-02-05 5:10 ` Jakub Kicinski
2021-02-05 11:28 ` Vadym Kochan
2021-02-03 16:54 ` [PATCH net-next 3/7] net: marvell: prestera: add support for AC3X 98DX3265 device Vadym Kochan
2021-02-03 16:54 ` [PATCH net-next 4/7] net: marvell: prestera: move netdev topology validation to prestera_main Vadym Kochan
2021-02-05 14:09 ` Vladimir Oltean
2021-02-03 16:54 ` [PATCH net-next 5/7] net: marvell: prestera: add LAG support Vadym Kochan
2021-02-05 5:16 ` Jakub Kicinski
2021-02-08 19:54 ` Tobias Waldekranz
2021-02-08 21:05 ` Jakub Kicinski
2021-02-08 22:30 ` Andrew Lunn
2021-02-09 12:37 ` Tobias Waldekranz
2021-02-09 11:56 ` Tobias Waldekranz [this message]
2021-02-09 17:48 ` Jakub Kicinski
2021-02-09 13:58 ` Andrew Lunn
2021-02-09 17:35 ` Jakub Kicinski
2021-02-09 20:31 ` [EXT] " Mickey Rachamim
2021-02-09 21:34 ` Tobias Waldekranz
2021-02-10 10:41 ` Mickey Rachamim
2021-02-10 21:44 ` Tobias Waldekranz
2021-02-10 0:28 ` Andrew Lunn
2021-02-10 10:42 ` Mickey Rachamim
2021-02-10 19:25 ` Jakub Kicinski
2021-02-10 20:52 ` Taras Chornyi
2021-02-05 15:24 ` Vladimir Oltean
2021-02-03 16:54 ` [PATCH net-next 6/7] net: marvell: prestera: align flood setting according to latest firmware version Vadym Kochan
2021-02-03 16:54 ` [PATCH net-next 7/7] net: marvell: prestera: fix port event handling on init Vadym Kochan
2021-02-05 5:19 ` Jakub Kicinski
2021-02-05 12:31 ` Vadym Kochan
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=87pn194fp4.fsf@waldekranz.com \
--to=tobias@waldekranz.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mickeyr@marvell.com \
--cc=netdev@vger.kernel.org \
--cc=vadym.kochan@plvision.eu \
--cc=vladimir.oltean@nxp.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).