ACK: [SRU][Jammy][Kinetic][PATCH 1/1] s390/qeth: cache link_info for ethtool
Philip Cox
philip.cox at canonical.com
Mon Sep 12 18:09:11 UTC 2022
Acked-by: Philip Cox <philip.cox at canonical.com>
On Mon, Sep 12, 2022 at 12:28 PM Joseph Salisbury <
joseph.salisbury at canonical.com> wrote:
> From: Alexandra Winter <wintera at linux.ibm.com>
>
> BugLink: https://bugs.launchpad.net/bugs/1984103
>
> Since
> commit e6e771b3d897 ("s390/qeth: detach netdevice while card is offline")
> there was a timing window during recovery, that qeth_query_card_info could
> be sent to the card, even before it was ready for it, leading to a failing
> card recovery. There is evidence that this window was hit, as not all
> callers of get_link_ksettings() check for netif_device_present.
>
> Use cached values in qeth_get_link_ksettings(), instead of calling
> qeth_query_card_info() and falling back to default values in case it
> fails. Link info is already updated when the card goes online, e.g. after
> STARTLAN (physical link up). Set the link info to default values, when the
> card goes offline or at STOPLAN (physical link down). A follow-on patch
> will improve values reported for link down.
>
> Fixes: e6e771b3d897 ("s390/qeth: detach netdevice while card is offline")
> Signed-off-by: Alexandra Winter <wintera at linux.ibm.com>
> Reviewed-by: Thorsten Winkler <twinkler at linux.ibm.com>
> Link:
> https://lore.kernel.org/r/20220805155714.59609-1-wintera@linux.ibm.com
> Signed-off-by: Jakub Kicinski <kuba at kernel.org>
> (cherry picked from commit 7a07a29e4f6713b224f3bcde5f835e777301bdb8)
> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>
> ---
> drivers/s390/net/qeth_core_main.c | 168 +++++++++---------------------
> drivers/s390/net/qeth_ethtool.c | 12 +--
> 2 files changed, 48 insertions(+), 132 deletions(-)
>
> diff --git a/drivers/s390/net/qeth_core_main.c
> b/drivers/s390/net/qeth_core_main.c
> index 35d4b398c197..8bd9fd51208c 100644
> --- a/drivers/s390/net/qeth_core_main.c
> +++ b/drivers/s390/net/qeth_core_main.c
> @@ -763,6 +763,49 @@ static void qeth_issue_ipa_msg(struct qeth_ipa_cmd
> *cmd, int rc,
> ipa_name, com, CARD_DEVID(card));
> }
>
> +static void qeth_default_link_info(struct qeth_card *card)
> +{
> + struct qeth_link_info *link_info = &card->info.link_info;
> +
> + QETH_CARD_TEXT(card, 2, "dftlinfo");
> + link_info->duplex = DUPLEX_FULL;
> +
> + if (IS_IQD(card) || IS_VM_NIC(card)) {
> + link_info->speed = SPEED_10000;
> + link_info->port = PORT_FIBRE;
> + link_info->link_mode = QETH_LINK_MODE_FIBRE_SHORT;
> + } else {
> + switch (card->info.link_type) {
> + case QETH_LINK_TYPE_FAST_ETH:
> + case QETH_LINK_TYPE_LANE_ETH100:
> + link_info->speed = SPEED_100;
> + link_info->port = PORT_TP;
> + break;
> + case QETH_LINK_TYPE_GBIT_ETH:
> + case QETH_LINK_TYPE_LANE_ETH1000:
> + link_info->speed = SPEED_1000;
> + link_info->port = PORT_FIBRE;
> + break;
> + case QETH_LINK_TYPE_10GBIT_ETH:
> + link_info->speed = SPEED_10000;
> + link_info->port = PORT_FIBRE;
> + break;
> + case QETH_LINK_TYPE_25GBIT_ETH:
> + link_info->speed = SPEED_25000;
> + link_info->port = PORT_FIBRE;
> + break;
> + default:
> + dev_info(&card->gdev->dev,
> + "Unknown link type %x\n",
> + card->info.link_type);
> + link_info->speed = SPEED_UNKNOWN;
> + link_info->port = PORT_OTHER;
> + }
> +
> + link_info->link_mode = QETH_LINK_MODE_UNKNOWN;
> + }
> +}
> +
> static struct qeth_ipa_cmd *qeth_check_ipa_data(struct qeth_card *card,
> struct qeth_ipa_cmd *cmd)
> {
> @@ -790,6 +833,7 @@ static struct qeth_ipa_cmd *qeth_check_ipa_data(struct
> qeth_card *card,
> netdev_name(card->dev), card->info.chpid);
> qeth_issue_ipa_msg(cmd, cmd->hdr.return_code,
> card);
> netif_carrier_off(card->dev);
> + qeth_default_link_info(card);
> }
> return NULL;
> case IPA_CMD_STARTLAN:
> @@ -4744,92 +4788,6 @@ static int qeth_query_oat_command(struct qeth_card
> *card, char __user *udata)
> return rc;
> }
>
> -static int qeth_query_card_info_cb(struct qeth_card *card,
> - struct qeth_reply *reply, unsigned long
> data)
> -{
> - struct qeth_ipa_cmd *cmd = (struct qeth_ipa_cmd *)data;
> - struct qeth_link_info *link_info = reply->param;
> - struct qeth_query_card_info *card_info;
> -
> - QETH_CARD_TEXT(card, 2, "qcrdincb");
> - if (qeth_setadpparms_inspect_rc(cmd))
> - return -EIO;
> -
> - card_info = &cmd->data.setadapterparms.data.card_info;
> - netdev_dbg(card->dev,
> - "card info: card_type=0x%02x, port_mode=0x%04x,
> port_speed=0x%08x\n",
> - card_info->card_type, card_info->port_mode,
> - card_info->port_speed);
> -
> - switch (card_info->port_mode) {
> - case CARD_INFO_PORTM_FULLDUPLEX:
> - link_info->duplex = DUPLEX_FULL;
> - break;
> - case CARD_INFO_PORTM_HALFDUPLEX:
> - link_info->duplex = DUPLEX_HALF;
> - break;
> - default:
> - link_info->duplex = DUPLEX_UNKNOWN;
> - }
> -
> - switch (card_info->card_type) {
> - case CARD_INFO_TYPE_1G_COPPER_A:
> - case CARD_INFO_TYPE_1G_COPPER_B:
> - link_info->speed = SPEED_1000;
> - link_info->port = PORT_TP;
> - break;
> - case CARD_INFO_TYPE_1G_FIBRE_A:
> - case CARD_INFO_TYPE_1G_FIBRE_B:
> - link_info->speed = SPEED_1000;
> - link_info->port = PORT_FIBRE;
> - break;
> - case CARD_INFO_TYPE_10G_FIBRE_A:
> - case CARD_INFO_TYPE_10G_FIBRE_B:
> - link_info->speed = SPEED_10000;
> - link_info->port = PORT_FIBRE;
> - break;
> - default:
> - switch (card_info->port_speed) {
> - case CARD_INFO_PORTS_10M:
> - link_info->speed = SPEED_10;
> - break;
> - case CARD_INFO_PORTS_100M:
> - link_info->speed = SPEED_100;
> - break;
> - case CARD_INFO_PORTS_1G:
> - link_info->speed = SPEED_1000;
> - break;
> - case CARD_INFO_PORTS_10G:
> - link_info->speed = SPEED_10000;
> - break;
> - case CARD_INFO_PORTS_25G:
> - link_info->speed = SPEED_25000;
> - break;
> - default:
> - link_info->speed = SPEED_UNKNOWN;
> - }
> -
> - link_info->port = PORT_OTHER;
> - }
> -
> - return 0;
> -}
> -
> -int qeth_query_card_info(struct qeth_card *card,
> - struct qeth_link_info *link_info)
> -{
> - struct qeth_cmd_buffer *iob;
> -
> - QETH_CARD_TEXT(card, 2, "qcrdinfo");
> - if (!qeth_adp_supported(card, IPA_SETADP_QUERY_CARD_INFO))
> - return -EOPNOTSUPP;
> - iob = qeth_get_adapter_cmd(card, IPA_SETADP_QUERY_CARD_INFO, 0);
> - if (!iob)
> - return -ENOMEM;
> -
> - return qeth_send_ipa_cmd(card, iob, qeth_query_card_info_cb,
> link_info);
> -}
> -
> static int qeth_init_link_info_oat_cb(struct qeth_card *card,
> struct qeth_reply *reply_priv,
> unsigned long data)
> @@ -4839,6 +4797,7 @@ static int qeth_init_link_info_oat_cb(struct
> qeth_card *card,
> struct qeth_query_oat_physical_if *phys_if;
> struct qeth_query_oat_reply *reply;
>
> + QETH_CARD_TEXT(card, 2, "qoatincb");
> if (qeth_setadpparms_inspect_rc(cmd))
> return -EIO;
>
> @@ -4918,41 +4877,7 @@ static int qeth_init_link_info_oat_cb(struct
> qeth_card *card,
>
> static void qeth_init_link_info(struct qeth_card *card)
> {
> - card->info.link_info.duplex = DUPLEX_FULL;
> -
> - if (IS_IQD(card) || IS_VM_NIC(card)) {
> - card->info.link_info.speed = SPEED_10000;
> - card->info.link_info.port = PORT_FIBRE;
> - card->info.link_info.link_mode =
> QETH_LINK_MODE_FIBRE_SHORT;
> - } else {
> - switch (card->info.link_type) {
> - case QETH_LINK_TYPE_FAST_ETH:
> - case QETH_LINK_TYPE_LANE_ETH100:
> - card->info.link_info.speed = SPEED_100;
> - card->info.link_info.port = PORT_TP;
> - break;
> - case QETH_LINK_TYPE_GBIT_ETH:
> - case QETH_LINK_TYPE_LANE_ETH1000:
> - card->info.link_info.speed = SPEED_1000;
> - card->info.link_info.port = PORT_FIBRE;
> - break;
> - case QETH_LINK_TYPE_10GBIT_ETH:
> - card->info.link_info.speed = SPEED_10000;
> - card->info.link_info.port = PORT_FIBRE;
> - break;
> - case QETH_LINK_TYPE_25GBIT_ETH:
> - card->info.link_info.speed = SPEED_25000;
> - card->info.link_info.port = PORT_FIBRE;
> - break;
> - default:
> - dev_info(&card->gdev->dev, "Unknown link type
> %x\n",
> - card->info.link_type);
> - card->info.link_info.speed = SPEED_UNKNOWN;
> - card->info.link_info.port = PORT_OTHER;
> - }
> -
> - card->info.link_info.link_mode = QETH_LINK_MODE_UNKNOWN;
> - }
> + qeth_default_link_info(card);
>
> /* Get more accurate data via QUERY OAT: */
> if (qeth_adp_supported(card, IPA_SETADP_QUERY_OAT)) {
> @@ -5461,6 +5386,7 @@ int qeth_set_offline(struct qeth_card *card, const
> struct qeth_discipline *disc,
> qeth_clear_working_pool_list(card);
> qeth_flush_local_addrs(card);
> card->info.promisc_mode = 0;
> + qeth_default_link_info(card);
>
> rc = qeth_stop_channel(&card->data);
> rc2 = qeth_stop_channel(&card->write);
> diff --git a/drivers/s390/net/qeth_ethtool.c
> b/drivers/s390/net/qeth_ethtool.c
> index b0b36b2132fe..9eba0a32e9f9 100644
> --- a/drivers/s390/net/qeth_ethtool.c
> +++ b/drivers/s390/net/qeth_ethtool.c
> @@ -428,8 +428,8 @@ static int qeth_get_link_ksettings(struct net_device
> *netdev,
> struct ethtool_link_ksettings *cmd)
> {
> struct qeth_card *card = netdev->ml_priv;
> - struct qeth_link_info link_info;
>
> + QETH_CARD_TEXT(card, 4, "ethtglks");
> cmd->base.speed = card->info.link_info.speed;
> cmd->base.duplex = card->info.link_info.duplex;
> cmd->base.port = card->info.link_info.port;
> @@ -439,16 +439,6 @@ static int qeth_get_link_ksettings(struct net_device
> *netdev,
> cmd->base.eth_tp_mdix = ETH_TP_MDI_INVALID;
> cmd->base.eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID;
>
> - /* Check if we can obtain more accurate information. */
> - if (!qeth_query_card_info(card, &link_info)) {
> - if (link_info.speed != SPEED_UNKNOWN)
> - cmd->base.speed = link_info.speed;
> - if (link_info.duplex != DUPLEX_UNKNOWN)
> - cmd->base.duplex = link_info.duplex;
> - if (link_info.port != PORT_OTHER)
> - cmd->base.port = link_info.port;
> - }
> -
> qeth_set_ethtool_link_modes(cmd, card->info.link_info.link_mode);
>
> return 0;
> --
> 2.34.1
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20220912/5f716078/attachment-0001.html>
More information about the kernel-team
mailing list