-----Original Message-----
From: Ido Schimmel <redacted>
Sent: Tuesday, September 12, 2023 4:27 PM
To: Drewek, Wojciech <redacted>
Cc: Jakub Kicinski <kuba@kernel.org>; intel-wired-lan@lists.osuosl.org;
netdev@vger.kernel.org; Kitszel, Przemyslaw [off-list ref];
idosch@nvidia.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:

quoted

quoted

-----Original Message-----
From: Ido Schimmel <redacted>
Sent: środa, 30 sierpnia 2023 17:17
To: Drewek, Wojciech <redacted>
Cc: Jakub Kicinski <kuba@kernel.org>; intel-wired-lan@lists.osuosl.org;
netdev@vger.kernel.org; Kitszel, Przemyslaw

[off-list ref];

quoted

quoted

idosch@nvidia.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:

quoted

In some cases users are trying to use media with power exceeding max

allowed value.

quoted

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.

quoted

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

quoted

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).

quoted

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 :)

quoted

quoted

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.