Re: [PATCH net] s390/qeth: allow bridgeport queries despite OS_MISMATCH

On 06/09/26 1:29 AM, netdev-bot+sashiko@kernel.org wrote:

Thank you for the careful review. Addressing each point below.

[Severity: Medium]
Should this test also be qualified with IS_IQD(card)?

    IPA_RC_SBP_IQD_OS_MISMATCH = 0x0010,
    IPA_RC_SBP_OSA_OS_MISMATCH = 0x2B10,

On an OSA (non-IQD) card, a reply carrying ipa_rc == 0 and sbp_rc ==
0x0010 is not an OS mismatch in the OSA numbering, yet the new shortcut
turns it into success...

OSA devices cannot reach qeth_bridgeport_query_ports_cb(). Commit
cdf8df5b42e7 ("s390/qeth: don't offer .ndo_bridge_* ops for OSA
devices") removed ndo_bridge_getlink and ndo_bridge_setlink from the
OSA netdev ops entirely. Its commit message states:
"qeth_l2_bridge_getlink() and qeth_l2_bridge_setlink() will always
return -EOPNOTSUPP on OSA devices."

qeth_l2_detect_dev2br_support() also confirms this: it returns
immediately for non-IQD cards (if (!IS_IQD(card)) return;) and never
sets brport_hw_features on OSA.

There is therefore no call path by which an OSA card can invoke
qeth_bridgeport_query_ports() or reach its callback. The IS_IQD(card)
guard is not needed.

[Severity: High]
Can this discard the user's configured bridge port role?

...qeth_bridgeport_setrole() still fails with -EPERM under mismatch,
and the following qeth_bridgeport_query_ports(card,
&card->options.sbp.role, NULL, NULL) now succeeds and overwrites the
field with "none"...so once the field has been zeroed, no later
set-online or recovery retries qeth_bridgeport_setrole(), even after
the other OS family releases the port.

IPA_RC_SBP_IQD_OS_MISMATCH reflects a deliberate configuration:
bridge port ownership on the VCHID is assigned to a different OS
family via z/VM Virtual Switch configuration. This is not a transient
error — it is a permanent topology decision. If the other OS family
releases the bridge port, that is equally a deliberate administrative
action, after which bridge_role would be explicitly reconfigured.

In the deployment this patch fixes, the Linux device observes bridge
port status for visibility but does not own the bridge port.
card->options.sbp.role is NONE before the query. Firmware returns
role=NONE under OS_MISMATCH, so the query writes NONE into NONE: no
user-configured value is clobbered. Confirmed on hardware: cat
bridge_role returns "none (OS family mismatch)" with no impact on
device functionality.

The scenario of a device that previously held an active bridge role
losing it to another OS family, then expecting automatic role
re-application, requires the driver to act as a persistent intent
store across an administrative topology change. That is not the
contract qeth bridge port configuration provides.

[Severity: Medium]
Can the value emitted here be written back to the same attribute?

...a read-modify-write or a save-and-restore of bridge_role by the
zdev tooling named in the commit message (chzdev save/restore) would
get -EINVAL...

"none (OS family mismatch)" is only emitted when the card is online
and hardware-reachable. A write-back attempt fails immediately at the
parse step in qeth_bridge_port_role_store() before reaching setrole().
Confirmed on hardware:

  # echo "none (OS family mismatch)" > bridge_role
  -bash: echo: write error: Invalid argument

No configuration is corrupted or discarded.

The patch is correct as submitted. No code changes are needed in
response to these review points. Tested on a HiperSockets IQD device
under OS_MISMATCH: bridge_role reads "none (OS family mismatch)" and
bridge_state is readable, while write attempts to bridge_role are
correctly rejected. The change carries Reviewed-by from Alexandra Winter.

Regards,
Nagamani