[PATCH net-next 05/10] s390/qeth: clean up error handling for isolation mode cmds

Julian Wiedmann
2020-07-14T14:23:25Z

As the cmd IO path has learned to propagate errnos back to its callers,
let them deal with errors instead of trying to restore their previous
configuration from within the IO error path.

Also translate the HW error to a meaningful errno, instead of returning
-EIO for all cases (and don't map this to -EOPNOTSUPP later on...).

While at it, add a READ_ONCE() / WRITE_ONCE() pair to ensure that the
data path always sees a valid isolation mode during reconfiguration.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core.h | 5 +-
drivers/s390/net/qeth_core_main.c | 85 ++++++++++---------------------
drivers/s390/net/qeth_core_sys.c | 20 ++++----
3 files changed, 40 insertions(+), 70 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 51ea56b73a97..c77a87105ea8 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -721,7 +721,6 @@ struct qeth_card_options {
struct qeth_vnicc_info vnicc; /* VNICC options */
enum qeth_discipline_id layer;
enum qeth_ipa_isolation_modes isolation;
- enum qeth_ipa_isolation_modes prev_isolation;
int sniffer;
enum qeth_cq cq;
char hsuid[9];
@@ -1071,6 +1070,9 @@ int qeth_query_switch_attributes(struct qeth_card *card,
struct qeth_switch_info *sw_info);
int qeth_query_card_info(struct qeth_card *card,
struct carrier_info *carrier_info);
+int qeth_setadpparms_set_access_ctrl(struct qeth_card *card,
+ enum qeth_ipa_isolation_modes mode);
+
unsigned int qeth_count_elements(struct sk_buff *skb, unsigned int data_offset);
int qeth_do_send_packet(struct qeth_card *card, struct qeth_qdio_out_q *queue,
struct sk_buff *skb, struct qeth_hdr *hdr,
@@ -1078,7 +1080,6 @@ int qeth_do_send_packet(struct qeth_card *card, struct qeth_qdio_out_q *queue,
int elements_needed);
int qeth_do_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
void qeth_dbf_longtext(debug_info_t *id, int level, char *text, ...);
-int qeth_set_access_ctrl_online(struct qeth_card *card, int fallback);
int qeth_configure_cq(struct qeth_card *, enum qeth_cq);
int qeth_hw_trap(struct qeth_card *, enum qeth_diags_trap_action);
void qeth_trace_features(struct qeth_card *);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 782a5128ac04..bd6489d87ede 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4557,7 +4557,6 @@ static int qeth_setadpparms_set_access_ctrl_cb(struct qeth_card *card,
{
struct qeth_ipa_cmd *cmd = (struct qeth_ipa_cmd *) data;
struct qeth_set_access_ctrl *access_ctrl_req;
- int fallback = *(int *)reply->param;

QETH_CARD_TEXT(card, 4, "setaccb");

@@ -4571,70 +4570,54 @@ static int qeth_setadpparms_set_access_ctrl_cb(struct qeth_card *card,
cmd->data.setadapterparms.hdr.return_code);
switch (qeth_setadpparms_inspect_rc(cmd)) {
case SET_ACCESS_CTRL_RC_SUCCESS:
- if (card->options.isolation == ISOLATION_MODE_NONE) {
+ if (access_ctrl_req->subcmd_code == ISOLATION_MODE_NONE)
dev_info(&card->gdev->dev,
"QDIO data connection isolation is deactivated\n");
- } else {
+ else
dev_info(&card->gdev->dev,
"QDIO data connection isolation is activated\n");
- }
- break;
+ return 0;
case SET_ACCESS_CTRL_RC_ALREADY_NOT_ISOLATED:
QETH_DBF_MESSAGE(2, "QDIO data connection isolation on device %x already deactivated\n",
CARD_DEVID(card));
- if (fallback)
- card->options.isolation = card->options.prev_isolation;
- break;
+ return 0;
case SET_ACCESS_CTRL_RC_ALREADY_ISOLATED:
QETH_DBF_MESSAGE(2, "QDIO data connection isolation on device %x already activated\n",
CARD_DEVID(card));
- if (fallback)
- card->options.isolation = card->options.prev_isolation;
- break;
+ return 0;
case SET_ACCESS_CTRL_RC_NOT_SUPPORTED:
dev_err(&card->gdev->dev, "Adapter does not "
"support QDIO data connection isolation\n");
- break;
+ return -EOPNOTSUPP;
case SET_ACCESS_CTRL_RC_NONE_SHARED_ADAPTER:
dev_err(&card->gdev->dev,
"Adapter is dedicated. "
"QDIO data connection isolation not supported\n");
- if (fallback)
- card->options.isolation = card->options.prev_isolation;
- break;
+ return -EOPNOTSUPP;
case SET_ACCESS_CTRL_RC_ACTIVE_CHECKSUM_OFF:
dev_err(&card->gdev->dev,
"TSO does not permit QDIO data connection isolation\n");
- if (fallback)
- card->options.isolation = card->options.prev_isolation;
- break;
+ return -EPERM;
case SET_ACCESS_CTRL_RC_REFLREL_UNSUPPORTED:
dev_err(&card->gdev->dev, "The adjacent switch port does not "
"support reflective relay mode\n");
- if (fallback)
- card->options.isolation = card->options.prev_isolation;
- break;
+ return -EOPNOTSUPP;
case SET_ACCESS_CTRL_RC_REFLREL_FAILED:
dev_err(&card->gdev->dev, "The reflective relay mode cannot be "
"enabled at the adjacent switch port");
- if (fallback)
- card->options.isolation = card->options.prev_isolation;
- break;
+ return -EREMOTEIO;
case SET_ACCESS_CTRL_RC_REFLREL_DEACT_FAILED:
dev_warn(&card->gdev->dev, "Turning off reflective relay mode "
"at the adjacent switch failed\n");
- break;
+ /* benign error while disabling ISOLATION_MODE_FWD */
+ return 0;
default:
- /* this should never happen */
- if (fallback)
- card->options.isolation = card->options.prev_isolation;
- break;
+ return -EIO;
}
- return (cmd->hdr.return_code) ? -EIO : 0;
}

-static int qeth_setadpparms_set_access_ctrl(struct qeth_card *card,
- enum qeth_ipa_isolation_modes isolation, int fallback)
+int qeth_setadpparms_set_access_ctrl(struct qeth_card *card,
+ enum qeth_ipa_isolation_modes mode)
{
int rc;
struct qeth_cmd_buffer *iob;
@@ -4643,41 +4626,28 @@ static int qeth_setadpparms_set_access_ctrl(struct qeth_card *card,

QETH_CARD_TEXT(card, 4, "setacctl");

+ if (!qeth_adp_supported(card, IPA_SETADP_SET_ACCESS_CONTROL)) {
+ dev_err(&card->gdev->dev,
+ "Adapter does not support QDIO data connection isolation\n");
+ return -EOPNOTSUPP;
+ }
+
iob = qeth_get_adapter_cmd(card, IPA_SETADP_SET_ACCESS_CONTROL,
SETADP_DATA_SIZEOF(set_access_ctrl));
if (!iob)
return -ENOMEM;
cmd = __ipa_cmd(iob);
access_ctrl_req = &cmd->data.setadapterparms.data.set_access_ctrl;
- access_ctrl_req->subcmd_code = isolation;
+ access_ctrl_req->subcmd_code = mode;

rc = qeth_send_ipa_cmd(card, iob, qeth_setadpparms_set_access_ctrl_cb,
- &fallback);
- QETH_CARD_TEXT_(card, 2, "rc=%d", rc);
- return rc;
-}
-
-int qeth_set_access_ctrl_online(struct qeth_card *card, int fallback)
-{
- int rc = 0;
-
- QETH_CARD_TEXT(card, 4, "setactlo");
-
- if (!qeth_adp_supported(card, IPA_SETADP_SET_ACCESS_CONTROL)) {
- dev_err(&card->gdev->dev, "Adapter does not "
- "support QDIO data connection isolation\n");
- if (fallback)
- card->options.isolation = card->options.prev_isolation;
- return -EOPNOTSUPP;
- }
-
- rc = qeth_setadpparms_set_access_ctrl(card, card->options.isolation,
- fallback);
+ NULL);
if (rc) {
+ QETH_CARD_TEXT_(card, 2, "rc=%d", rc);
QETH_DBF_MESSAGE(3, "IPA(SET_ACCESS_CTRL(%d) on device %x: sent failed\n",
rc, CARD_DEVID(card));
- rc = -EOPNOTSUPP;
}
+
return rc;
}

@@ -5347,7 +5317,8 @@ int qeth_core_hardsetup_card(struct qeth_card *card, bool *carrier_ok)
card->info.hwtrap = 0;

if (card->options.isolation != ISOLATION_MODE_NONE) {
- rc = qeth_set_access_ctrl_online(card, 0);
+ rc = qeth_setadpparms_set_access_ctrl(card,
+ card->options.isolation);
if (rc)
goto out;
}
@@ -6858,7 +6829,7 @@ netdev_features_t qeth_features_check(struct sk_buff *skb,

/* Traffic with local next-hop is not eligible for some offloads: */
if (skb->ip_summed == CHECKSUM_PARTIAL &&
- card->options.isolation != ISOLATION_MODE_FWD) {
+ READ_ONCE(card->options.isolation) != ISOLATION_MODE_FWD) {
netdev_features_t restricted = 0;

if (skb_is_gso(skb) && !netif_needs_gso(skb, features))
diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
index c901c942fed7..8def82336f53 100644
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -448,19 +448,17 @@ static ssize_t qeth_dev_isolation_store(struct device *dev,
rc = -EINVAL;
goto out;
}
- rc = count;
-
- /* defer IP assist if device is offline (until discipline->set_online)*/
- card->options.prev_isolation = card->options.isolation;
- card->options.isolation = isolation;
- if (qeth_card_hw_is_reachable(card)) {
- int ipa_rc = qeth_set_access_ctrl_online(card, 1);
- if (ipa_rc != 0)
- rc = ipa_rc;
- }
+
+ if (qeth_card_hw_is_reachable(card))
+ rc = qeth_setadpparms_set_access_ctrl(card, isolation);
+
+ if (!rc)
+ WRITE_ONCE(card->options.isolation, isolation);
+
out:
mutex_unlock(&card->conf_mutex);
- return rc;
+
+ return rc ? rc : count;
}

static DEVICE_ATTR(isolation, 0644, qeth_dev_isolation_show,
--
2.17.1