netdev - RE: [PATCH iwl-next v2] ice: Disable Cage Max Power override

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 15 Sep 2023 13:15:01 +0000
From: "Drewek, Wojciech" <wojciech.drewek@...el.com>
To: Ido Schimmel <idosch@...sch.org>
CC: Jakub Kicinski <kuba@...nel.org>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, "idosch@...dia.com" <idosch@...dia.com>
Subject: RE: [PATCH iwl-next v2] ice: Disable Cage Max Power override



> -----Original Message-----
> From: Ido Schimmel <idosch@...sch.org>
> Sent: Tuesday, September 12, 2023 4:27 PM
> To: Drewek, Wojciech <wojciech.drewek@...el.com>
> Cc: Jakub Kicinski <kuba@...nel.org>; intel-wired-lan@...ts.osuosl.org;
> netdev@...r.kernel.org; Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>;
> idosch@...dia.com
> Subject: Re: [PATCH iwl-next v2] ice: Disable Cage Max Power override
> 
> On Fri, Sep 01, 2023 at 01:34:04PM +0000, Drewek, Wojciech wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ido Schimmel <idosch@...sch.org>
> > > Sent: środa, 30 sierpnia 2023 17:17
> > > To: Drewek, Wojciech <wojciech.drewek@...el.com>
> > > Cc: Jakub Kicinski <kuba@...nel.org>; intel-wired-lan@...ts.osuosl.org;
> > > netdev@...r.kernel.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@...el.com>;
> > > idosch@...dia.com
> > > Subject: Re: [PATCH iwl-next v2] ice: Disable Cage Max Power override
> > >
> > > On Tue, Aug 29, 2023 at 09:12:22AM +0000, Drewek, Wojciech wrote:
> > > > In some cases users are trying to use media with power exceeding max
> > > allowed value.
> > > > Port split require system reboot so it feels natural to me to restore default
> > > settings.
> > >
> > > I don't believe it's the kernel's responsibility to undo changes done by
> > > external tools. Given that the tool is able to change this setting, I
> > > assume it can also restore it back to default.
> >
> > I agree with that, but we can end up with no link if we don't restore
> > default settings. Let me explain how.
> >
> > >
> > > Moreover, it doesn't sound like port split won't work without this
> > > change, so placing this change there only because we assume that a
> > > reboot will follow seems random.
> >
> > After port split, we might end up with no link in one of the ports.
> > In dual port card if we increase max pwr on the 1st cage the 2nd one
> > will have max pwr decreased automatically. This might be useful if we have port
> > option with count 1, the second cage is not used in this case. If we then split and
> > use two ports now, the second port will use second cage which has decreased
> max pwr, default module
> > used there will not work.
> 
> Not sure I understand how it's related to port split. You have a dual
> port card with two netdevs (e.g., eth0 and eth1) and two cages. You used
> some tool to increase the max power on the first cage which means that
> the second cage will have its max power decreased. Now you split the
> first port:
> 
> # devlink port split eth0 count 2
> 
> eth0s0 and eth0s1 correspond to the first cage. Why are they affected by
> the second cage?
> 
> I have a feeling we mean different things by "port split". As far as I'm
> concerned, you split a port in order to connect a splitter cable to the
> cage. For example:
> https://network.nvidia.com/related-docs/prod_cables/PB_MCP7H50-
> Vxxxyzz_200GbE_QSFP56_to_2x100GbE_QSFP56_DAC.pdf

In ice driver port split works per device not per port. According to
/Documentation/networking/devlink/ice.rst, section "Port split":
	The "ice" driver supports port splitting only for port 0, as the FW has
	a predefined set of available port split options for the whole device.
And if you look at available port options (same file) you'll see that in case of "Split count" 1
only quad 1 is working. And in case of "Split count" 2 the second quad might be used. So, if we
increase max_pwr in the first quad in case of "Split count" 1 and then switch to "Split count" 2,
the second quad might end up with no link (because it will have decreased max_pwr).

> 
> >
> > So, should we leave the restoration of the default settings to the user?
> 
> Let's first clear up the above. BTW, if a port doesn't come up because
> of power issues you can try communicating it to user space using
> 'ETHTOOL_LINK_EXT_STATE_POWER_BUDGET_EXCEEDED'.

Thanks for suggestion.

PS I'll be on holidays for two weeks so sorry for lack of reply in advance :)

> 
> >
> > >
> > > I think the best way forward is to extend ethtool as was already
> > > suggested. It should allow you to avoid the split brain situation where
> > > the hardware is configured by both the kernel and an external tool.
> >
> > I'll try to follow up with the ethtool extension.