Wed, Jul 22, 2020 at 05:30:05PM CEST, jacob.e.kel...@intel.com wrote: > > >> -----Original Message----- >> From: netdev-ow...@vger.kernel.org <netdev-ow...@vger.kernel.org> On >> Behalf Of Jiri Pirko >> Sent: Wednesday, July 22, 2020 3:52 AM >> To: Jakub Kicinski <kubak...@wp.pl> >> Cc: Keller, Jacob E <jacob.e.kel...@intel.com>; netdev@vger.kernel.org; Tom >> Herbert <t...@herbertland.com>; Jiri Pirko <j...@mellanox.com>; Jakub >> Kicinski >> <k...@kernel.org>; Jonathan Corbet <cor...@lwn.net>; Michael Chan >> <michael.c...@broadcom.com>; Bin Luo <luob...@huawei.com>; Saeed >> Mahameed <sae...@mellanox.com>; Leon Romanovsky <l...@kernel.org>; >> Ido Schimmel <ido...@mellanox.com>; Danielle Ratson >> <daniel...@mellanox.com> >> Subject: Re: [RFC PATCH net-next v2 6/6] devlink: add overwrite mode to flash >> update >> >> Tue, Jul 21, 2020 at 07:04:06PM CEST, kubak...@wp.pl wrote: >> >On Tue, 21 Jul 2020 15:53:56 +0200 Jiri Pirko wrote: >> >> Mon, Jul 20, 2020 at 05:51:59PM CEST, kubak...@wp.pl wrote: >> >> >On Mon, 20 Jul 2020 12:09:53 +0200 Jiri Pirko wrote: >> >> >> This looks odd. You have a single image yet you somehow divide it >> >> >> into "program" and "config" areas. We already have infra in place to >> >> >> take care of this. See DEVLINK_ATTR_FLASH_UPDATE_COMPONENT. >> >> >> You should have 2 components: >> >> >> 1) "program" >> >> >> 2) "config" >> >> >> >> >> >> Then it is up to the user what he decides to flash. >> >> > >> >> >99.9% of the time users want to flash "all". To achieve "don't flash >> >> >config" with current infra users would have to flash each component >> >> >> >> Well you can have multiple component what would overlap: >> >> 1) "program" + "config" (default) >> >> 2) "program" >> >> 3) "config" >> > >> >Say I have FW component and UNDI driver. Now I'll have 4 components? >> >fw.prog, fw.config, undi.prog etc? Are those extra ones visible or just >> >> Visible in which sense? We don't show components anywhere if I'm not >> mistaken. They are currently very rarely used. Basically we just ported >> it from ethtool without much thinking. >> > >Component names are used in devlink info and displayed to end users along with >versions, plus they're names passed by the user in devlink flash update. As >far as documented, we shouldn't add new components without associated versions >in the info report.
Okay. So it is loosely coupled. I think it would be nice to tight those 2 togeter so it is not up to the driver how he decides to implement it. > >> >> >"implied"? If they are visible what version does the config have? >> >> Good question. we don't have per-component version so far. I think it >> would be good to have it alonside with the listing. >> >> >> > >> >Also (3) - flashing config from one firmware version and program from >> >another - makes a very limited amount of sense to me. >> > >> >> >one by one and then omit the one(s) which is config (guessing which >> >> >one that is based on the name). >> >> > >> >> >Wouldn't this be quite inconvenient? >> >> >> >> I see it as an extra knob that is actually somehow provides degradation >> >> of components. >> > >> >Hm. We have the exact opposite view on the matter. To me components >> >currently correspond to separate fw/hw entities, that's a very clear >> >meaning. PHY firmware, management FW, UNDI. Now we would add a >> >completely orthogonal meaning to the same API. >> >> I understand. My concern is, we would have a component with some >> "subparts". Now it is some fuzzy vagely defined "config part", >> in the future it might be something else. That is what I'm concerned >> about. Components have clear api. >> >> So perhaps we can introduce something like "component mask", which would >> allow to flash only part of the component. That is basically what Jacob >> has, I would just like to have it well defined. >> >> > >So, we could make this selection a series of masked bits instead of a single >enumeration value. Yeah.
