All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/9] s390/qeth: updates 2020-09-23
@ 2020-09-23  8:36 Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 1/9] s390/qeth: don't init refcount twice for mcast IPs Julian Wiedmann
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

Hi Dave & Jakub,

please apply the following patch series for qeth to netdev's net-next tree.

This brings all sorts of cleanups. Highlights are more code sharing in
the init/teardown paths, and more fine-grained rollback on errors during
initialization (instead of a full-blown teardown).

Thanks,
Julian

Julian Wiedmann (9):
  s390/qeth: don't init refcount twice for mcast IPs
  s390/qeth: relax locking for ipato config data
  s390/qeth: clean up string ops in qeth_l3_parse_ipatoe()
  s390/qeth: replace deprecated simple_stroul()
  s390/qeth: tighten ucast IP locking
  s390/qeth: cancel cmds earlier during teardown
  s390/qeth: consolidate online code
  s390/qeth: consolidate teardown code
  s390/qeth: remove forward declarations in L2 code

 drivers/s390/net/qeth_core.h      |  22 +-
 drivers/s390/net/qeth_core_main.c |  71 +++--
 drivers/s390/net/qeth_core_sys.c  |  65 ++---
 drivers/s390/net/qeth_l2.h        |   7 +
 drivers/s390/net/qeth_l2_main.c   | 412 +++++++++++++-----------------
 drivers/s390/net/qeth_l3.h        |   4 +-
 drivers/s390/net/qeth_l3_main.c   |  88 ++-----
 drivers/s390/net/qeth_l3_sys.c    |  64 ++---
 8 files changed, 338 insertions(+), 395 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH net-next 1/9] s390/qeth: don't init refcount twice for mcast IPs
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
@ 2020-09-23  8:36 ` Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 2/9] s390/qeth: relax locking for ipato config data Julian Wiedmann
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

mcast IP objects are allocated within qeth_l3_add_mcast_rtnl(),
with .ref_counter already set to 1 via qeth_l3_init_ipaddr().

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_l3_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 33fdad1a6887..903d7b6f511f 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1235,7 +1235,6 @@ static void qeth_l3_rx_mode_work(struct work_struct *work)
 					kfree(addr);
 					break;
 				}
-				addr->ref_counter = 1;
 				fallthrough;
 			default:
 				/* for next call to set_rx_mode(): */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH net-next 2/9] s390/qeth: relax locking for ipato config data
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 1/9] s390/qeth: don't init refcount twice for mcast IPs Julian Wiedmann
@ 2020-09-23  8:36 ` Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 3/9] s390/qeth: clean up string ops in qeth_l3_parse_ipatoe() Julian Wiedmann
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

card->ipato is currently protected by the conf_mutex. But most users
also hold the ip_lock - in particular qeth_l3_add_ip().

So slightly expand the sections under ip_lock in a few places (to
effectively cover a few error & no-op cases), and then drop the
conf_mutex where it's no longer needed.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h    |  7 +++++--
 drivers/s390/net/qeth_l3_main.c |  4 ----
 drivers/s390/net/qeth_l3_sys.c  | 22 +++++++++++-----------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 2c14012ca35d..1b3fe384dcc9 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -814,12 +814,16 @@ struct qeth_card {
 	struct workqueue_struct *event_wq;
 	struct workqueue_struct *cmd_wq;
 	wait_queue_head_t wait_q;
+
+	struct mutex ip_lock;
+	/* protected by ip_lock: */
 	DECLARE_HASHTABLE(ip_htable, 4);
+	struct qeth_ipato ipato;
+
 	DECLARE_HASHTABLE(local_addrs4, 4);
 	DECLARE_HASHTABLE(local_addrs6, 4);
 	spinlock_t local_addrs4_lock;
 	spinlock_t local_addrs6_lock;
-	struct mutex ip_lock;
 	DECLARE_HASHTABLE(rx_mode_addrs, 4);
 	struct work_struct rx_mode_work;
 	struct work_struct kernel_thread_starter;
@@ -827,7 +831,6 @@ struct qeth_card {
 	unsigned long thread_start_mask;
 	unsigned long thread_allowed_mask;
 	unsigned long thread_running_mask;
-	struct qeth_ipato ipato;
 	struct list_head cmd_waiter_list;
 	/* QDIO buffer handling */
 	struct qeth_qdio_info qdio;
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 903d7b6f511f..810d65fd48b4 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -536,7 +536,6 @@ int qeth_l3_add_ipato_entry(struct qeth_card *card,
 
 	QETH_CARD_TEXT(card, 2, "addipato");
 
-	mutex_lock(&card->conf_mutex);
 	mutex_lock(&card->ip_lock);
 
 	list_for_each_entry(ipatoe, &card->ipato.entries, entry) {
@@ -556,7 +555,6 @@ int qeth_l3_add_ipato_entry(struct qeth_card *card,
 	}
 
 	mutex_unlock(&card->ip_lock);
-	mutex_unlock(&card->conf_mutex);
 
 	return rc;
 }
@@ -570,7 +568,6 @@ int qeth_l3_del_ipato_entry(struct qeth_card *card,
 
 	QETH_CARD_TEXT(card, 2, "delipato");
 
-	mutex_lock(&card->conf_mutex);
 	mutex_lock(&card->ip_lock);
 
 	list_for_each_entry_safe(ipatoe, tmp, &card->ipato.entries, entry) {
@@ -587,7 +584,6 @@ int qeth_l3_del_ipato_entry(struct qeth_card *card,
 	}
 
 	mutex_unlock(&card->ip_lock);
-	mutex_unlock(&card->conf_mutex);
 
 	return rc;
 }
diff --git a/drivers/s390/net/qeth_l3_sys.c b/drivers/s390/net/qeth_l3_sys.c
index dd0b39082534..ca9c95b6bf35 100644
--- a/drivers/s390/net/qeth_l3_sys.c
+++ b/drivers/s390/net/qeth_l3_sys.c
@@ -301,19 +301,21 @@ static ssize_t qeth_l3_dev_ipato_enable_store(struct device *dev,
 		goto out;
 	}
 
+	mutex_lock(&card->ip_lock);
 	if (sysfs_streq(buf, "toggle")) {
 		enable = !card->ipato.enabled;
 	} else if (kstrtobool(buf, &enable)) {
 		rc = -EINVAL;
-		goto out;
+		goto unlock_ip;
 	}
 
 	if (card->ipato.enabled != enable) {
 		card->ipato.enabled = enable;
-		mutex_lock(&card->ip_lock);
 		qeth_l3_update_ipato(card);
-		mutex_unlock(&card->ip_lock);
 	}
+
+unlock_ip:
+	mutex_unlock(&card->ip_lock);
 out:
 	mutex_unlock(&card->conf_mutex);
 	return rc ? rc : count;
@@ -339,7 +341,7 @@ static ssize_t qeth_l3_dev_ipato_invert4_store(struct device *dev,
 	bool invert;
 	int rc = 0;
 
-	mutex_lock(&card->conf_mutex);
+	mutex_lock(&card->ip_lock);
 	if (sysfs_streq(buf, "toggle")) {
 		invert = !card->ipato.invert4;
 	} else if (kstrtobool(buf, &invert)) {
@@ -349,12 +351,11 @@ static ssize_t qeth_l3_dev_ipato_invert4_store(struct device *dev,
 
 	if (card->ipato.invert4 != invert) {
 		card->ipato.invert4 = invert;
-		mutex_lock(&card->ip_lock);
 		qeth_l3_update_ipato(card);
-		mutex_unlock(&card->ip_lock);
 	}
+
 out:
-	mutex_unlock(&card->conf_mutex);
+	mutex_unlock(&card->ip_lock);
 	return rc ? rc : count;
 }
 
@@ -510,7 +511,7 @@ static ssize_t qeth_l3_dev_ipato_invert6_store(struct device *dev,
 	bool invert;
 	int rc = 0;
 
-	mutex_lock(&card->conf_mutex);
+	mutex_lock(&card->ip_lock);
 	if (sysfs_streq(buf, "toggle")) {
 		invert = !card->ipato.invert6;
 	} else if (kstrtobool(buf, &invert)) {
@@ -520,12 +521,11 @@ static ssize_t qeth_l3_dev_ipato_invert6_store(struct device *dev,
 
 	if (card->ipato.invert6 != invert) {
 		card->ipato.invert6 = invert;
-		mutex_lock(&card->ip_lock);
 		qeth_l3_update_ipato(card);
-		mutex_unlock(&card->ip_lock);
 	}
+
 out:
-	mutex_unlock(&card->conf_mutex);
+	mutex_unlock(&card->ip_lock);
 	return rc ? rc : count;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH net-next 3/9] s390/qeth: clean up string ops in qeth_l3_parse_ipatoe()
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 1/9] s390/qeth: don't init refcount twice for mcast IPs Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 2/9] s390/qeth: relax locking for ipato config data Julian Wiedmann
@ 2020-09-23  8:36 ` Julian Wiedmann
  2020-09-23  9:55   ` David Laight
  2020-09-23  8:36 ` [PATCH net-next 4/9] s390/qeth: replace deprecated simple_stroul() Julian Wiedmann
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

Indicate the max number of to-be-parsed characters, and avoid copying
the address sub-string.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_l3_sys.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/s390/net/qeth_l3_sys.c b/drivers/s390/net/qeth_l3_sys.c
index ca9c95b6bf35..05fa986e30fc 100644
--- a/drivers/s390/net/qeth_l3_sys.c
+++ b/drivers/s390/net/qeth_l3_sys.c
@@ -409,21 +409,22 @@ static ssize_t qeth_l3_dev_ipato_add4_show(struct device *dev,
 static int qeth_l3_parse_ipatoe(const char *buf, enum qeth_prot_versions proto,
 		  u8 *addr, int *mask_bits)
 {
-	const char *start, *end;
-	char *tmp;
-	char buffer[40] = {0, };
+	const char *start;
+	char *sep, *tmp;
+	int rc;
 
-	start = buf;
-	/* get address string */
-	end = strchr(start, '/');
-	if (!end || (end - start >= 40)) {
+	/* Expected input pattern: %addr/%mask */
+	sep = strnchr(buf, 40, '/');
+	if (!sep)
 		return -EINVAL;
-	}
-	strncpy(buffer, start, end - start);
-	if (qeth_l3_string_to_ipaddr(buffer, proto, addr)) {
-		return -EINVAL;
-	}
-	start = end + 1;
+
+	/* Terminate the %addr sub-string, and parse it: */
+	*sep = '\0';
+	rc = qeth_l3_string_to_ipaddr(buf, proto, addr);
+	if (rc)
+		return rc;
+
+	start = sep + 1;
 	*mask_bits = simple_strtoul(start, &tmp, 10);
 	if (!strlen(start) ||
 	    (tmp == start) ||
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH net-next 4/9] s390/qeth: replace deprecated simple_stroul()
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
                   ` (2 preceding siblings ...)
  2020-09-23  8:36 ` [PATCH net-next 3/9] s390/qeth: clean up string ops in qeth_l3_parse_ipatoe() Julian Wiedmann
@ 2020-09-23  8:36 ` Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 5/9] s390/qeth: tighten ucast IP locking Julian Wiedmann
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

Convert the remaining occurences in sysfs code to kstrtouint().

While at it move some input parsing out of locked sections, replace an
open-coded clamp() and remove some unnecessary run-time checks for
ipatoe->mask_bits that are already enforced when creating the object.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h     |  4 +-
 drivers/s390/net/qeth_core_sys.c | 65 +++++++++++++++++---------------
 drivers/s390/net/qeth_l3.h       |  4 +-
 drivers/s390/net/qeth_l3_main.c  |  8 ++--
 drivers/s390/net/qeth_l3_sys.c   | 21 +++++------
 5 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 1b3fe384dcc9..f931b764d6a4 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -195,8 +195,8 @@ struct qeth_vnicc_info {
 #define QETH_IN_BUF_SIZE_DEFAULT 65536
 #define QETH_IN_BUF_COUNT_DEFAULT 64
 #define QETH_IN_BUF_COUNT_HSDEFAULT 128
-#define QETH_IN_BUF_COUNT_MIN 8
-#define QETH_IN_BUF_COUNT_MAX 128
+#define QETH_IN_BUF_COUNT_MIN	8U
+#define QETH_IN_BUF_COUNT_MAX	128U
 #define QETH_MAX_BUFFER_ELEMENTS(card) ((card)->qdio.in_buf_size >> 12)
 #define QETH_IN_BUF_REQUEUE_THRESHOLD(card) \
 		 ((card)->qdio.in_buf_pool.buf_count / 2)
diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
index 8def82336f53..74c70364edc1 100644
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -103,21 +103,21 @@ static ssize_t qeth_dev_portno_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct qeth_card *card = dev_get_drvdata(dev);
-	char *tmp;
 	unsigned int portno, limit;
 	int rc = 0;
 
+	rc = kstrtouint(buf, 16, &portno);
+	if (rc)
+		return rc;
+	if (portno > QETH_MAX_PORTNO)
+		return -EINVAL;
+
 	mutex_lock(&card->conf_mutex);
 	if (card->state != CARD_STATE_DOWN) {
 		rc = -EPERM;
 		goto out;
 	}
 
-	portno = simple_strtoul(buf, &tmp, 16);
-	if (portno > QETH_MAX_PORTNO) {
-		rc = -EINVAL;
-		goto out;
-	}
 	limit = (card->ssqd.pcnt ? card->ssqd.pcnt - 1 : card->ssqd.pcnt);
 	if (portno > limit) {
 		rc = -EINVAL;
@@ -248,19 +248,19 @@ static ssize_t qeth_dev_bufcnt_store(struct device *dev,
 {
 	struct qeth_card *card = dev_get_drvdata(dev);
 	unsigned int cnt;
-	char *tmp;
 	int rc = 0;
 
+	rc = kstrtouint(buf, 10, &cnt);
+	if (rc)
+		return rc;
+
 	mutex_lock(&card->conf_mutex);
 	if (card->state != CARD_STATE_DOWN) {
 		rc = -EPERM;
 		goto out;
 	}
 
-	cnt = simple_strtoul(buf, &tmp, 10);
-	cnt = (cnt < QETH_IN_BUF_COUNT_MIN) ? QETH_IN_BUF_COUNT_MIN :
-		((cnt > QETH_IN_BUF_COUNT_MAX) ? QETH_IN_BUF_COUNT_MAX : cnt);
-
+	cnt = clamp(cnt, QETH_IN_BUF_COUNT_MIN, QETH_IN_BUF_COUNT_MAX);
 	rc = qeth_resize_buffer_pool(card, cnt);
 
 out:
@@ -341,18 +341,15 @@ static ssize_t qeth_dev_layer2_store(struct device *dev,
 {
 	struct qeth_card *card = dev_get_drvdata(dev);
 	struct net_device *ndev;
-	char *tmp;
-	int i, rc = 0;
 	enum qeth_discipline_id newdis;
+	unsigned int input;
+	int rc;
 
-	mutex_lock(&card->discipline_mutex);
-	if (card->state != CARD_STATE_DOWN) {
-		rc = -EPERM;
-		goto out;
-	}
+	rc = kstrtouint(buf, 16, &input);
+	if (rc)
+		return rc;
 
-	i = simple_strtoul(buf, &tmp, 16);
-	switch (i) {
+	switch (input) {
 	case 0:
 		newdis = QETH_DISCIPLINE_LAYER3;
 		break;
@@ -360,7 +357,12 @@ static ssize_t qeth_dev_layer2_store(struct device *dev,
 		newdis = QETH_DISCIPLINE_LAYER2;
 		break;
 	default:
-		rc = -EINVAL;
+		return -EINVAL;
+	}
+
+	mutex_lock(&card->discipline_mutex);
+	if (card->state != CARD_STATE_DOWN) {
+		rc = -EPERM;
 		goto out;
 	}
 
@@ -551,20 +553,21 @@ static DEVICE_ATTR(hw_trap, 0644, qeth_hw_trap_show,
 static ssize_t qeth_dev_blkt_store(struct qeth_card *card,
 		const char *buf, size_t count, int *value, int max_value)
 {
-	char *tmp;
-	int i, rc = 0;
+	unsigned int input;
+	int rc;
+
+	rc = kstrtouint(buf, 10, &input);
+	if (rc)
+		return rc;
+
+	if (input > max_value)
+		return -EINVAL;
 
 	mutex_lock(&card->conf_mutex);
-	if (card->state != CARD_STATE_DOWN) {
+	if (card->state != CARD_STATE_DOWN)
 		rc = -EPERM;
-		goto out;
-	}
-	i = simple_strtoul(buf, &tmp, 10);
-	if (i <= max_value)
-		*value = i;
 	else
-		rc = -EINVAL;
-out:
+		*value = input;
 	mutex_unlock(&card->conf_mutex);
 	return rc ? rc : count;
 }
diff --git a/drivers/s390/net/qeth_l3.h b/drivers/s390/net/qeth_l3.h
index 6ccfe2121095..acd130cfbab3 100644
--- a/drivers/s390/net/qeth_l3.h
+++ b/drivers/s390/net/qeth_l3.h
@@ -96,7 +96,7 @@ struct qeth_ipato_entry {
 	struct list_head entry;
 	enum qeth_prot_versions proto;
 	char addr[16];
-	int mask_bits;
+	unsigned int mask_bits;
 };
 
 extern const struct attribute_group *qeth_l3_attr_groups[];
@@ -110,7 +110,7 @@ int qeth_l3_setrouting_v6(struct qeth_card *);
 int qeth_l3_add_ipato_entry(struct qeth_card *, struct qeth_ipato_entry *);
 int qeth_l3_del_ipato_entry(struct qeth_card *card,
 			    enum qeth_prot_versions proto, u8 *addr,
-			    int mask_bits);
+			    unsigned int mask_bits);
 void qeth_l3_update_ipato(struct qeth_card *card);
 int qeth_l3_modify_hsuid(struct qeth_card *card, bool add);
 int qeth_l3_modify_rxip_vipa(struct qeth_card *card, bool add, const u8 *ip,
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 810d65fd48b4..876a21d451f5 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -105,11 +105,9 @@ static bool qeth_l3_is_addr_covered_by_ipato(struct qeth_card *card,
 					  (ipatoe->proto == QETH_PROT_IPV4) ?
 					  4 : 16);
 		if (addr->proto == QETH_PROT_IPV4)
-			rc = !memcmp(addr_bits, ipatoe_bits,
-				     min(32, ipatoe->mask_bits));
+			rc = !memcmp(addr_bits, ipatoe_bits, ipatoe->mask_bits);
 		else
-			rc = !memcmp(addr_bits, ipatoe_bits,
-				     min(128, ipatoe->mask_bits));
+			rc = !memcmp(addr_bits, ipatoe_bits, ipatoe->mask_bits);
 		if (rc)
 			break;
 	}
@@ -561,7 +559,7 @@ int qeth_l3_add_ipato_entry(struct qeth_card *card,
 
 int qeth_l3_del_ipato_entry(struct qeth_card *card,
 			    enum qeth_prot_versions proto, u8 *addr,
-			    int mask_bits)
+			    unsigned int mask_bits)
 {
 	struct qeth_ipato_entry *ipatoe, *tmp;
 	int rc = -ENOENT;
diff --git a/drivers/s390/net/qeth_l3_sys.c b/drivers/s390/net/qeth_l3_sys.c
index 05fa986e30fc..351763ae9b9c 100644
--- a/drivers/s390/net/qeth_l3_sys.c
+++ b/drivers/s390/net/qeth_l3_sys.c
@@ -407,10 +407,9 @@ static ssize_t qeth_l3_dev_ipato_add4_show(struct device *dev,
 }
 
 static int qeth_l3_parse_ipatoe(const char *buf, enum qeth_prot_versions proto,
-		  u8 *addr, int *mask_bits)
+				u8 *addr, unsigned int *mask_bits)
 {
-	const char *start;
-	char *sep, *tmp;
+	char *sep;
 	int rc;
 
 	/* Expected input pattern: %addr/%mask */
@@ -424,13 +423,13 @@ static int qeth_l3_parse_ipatoe(const char *buf, enum qeth_prot_versions proto,
 	if (rc)
 		return rc;
 
-	start = sep + 1;
-	*mask_bits = simple_strtoul(start, &tmp, 10);
-	if (!strlen(start) ||
-	    (tmp == start) ||
-	    (*mask_bits > ((proto == QETH_PROT_IPV4) ? 32 : 128))) {
+	rc = kstrtouint(sep + 1, 10, mask_bits);
+	if (rc)
+		return rc;
+
+	if (*mask_bits > ((proto == QETH_PROT_IPV4) ? 32 : 128))
 		return -EINVAL;
-	}
+
 	return 0;
 }
 
@@ -438,8 +437,8 @@ static ssize_t qeth_l3_dev_ipato_add_store(const char *buf, size_t count,
 			 struct qeth_card *card, enum qeth_prot_versions proto)
 {
 	struct qeth_ipato_entry *ipatoe;
+	unsigned int mask_bits;
 	u8 addr[16];
-	int mask_bits;
 	int rc = 0;
 
 	rc = qeth_l3_parse_ipatoe(buf, proto, addr, &mask_bits);
@@ -476,8 +475,8 @@ static QETH_DEVICE_ATTR(ipato_add4, add4, 0644,
 static ssize_t qeth_l3_dev_ipato_del_store(const char *buf, size_t count,
 			 struct qeth_card *card, enum qeth_prot_versions proto)
 {
+	unsigned int mask_bits;
 	u8 addr[16];
-	int mask_bits;
 	int rc = 0;
 
 	rc = qeth_l3_parse_ipatoe(buf, proto, addr, &mask_bits);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH net-next 5/9] s390/qeth: tighten ucast IP locking
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
                   ` (3 preceding siblings ...)
  2020-09-23  8:36 ` [PATCH net-next 4/9] s390/qeth: replace deprecated simple_stroul() Julian Wiedmann
@ 2020-09-23  8:36 ` Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 6/9] s390/qeth: cancel cmds earlier during teardown Julian Wiedmann
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

The programming of ucast IPs via qeth_l3_modify_ip() is driven
independently from any of our typical locking mechanisms (eg. detaching
the netdevice, or holding the conf_mutex).
So when we inspect the card state to check whether the required cmd IO
should be deferred, there is no protection against concurrent state
changes.

But by slightly re-ordering the teardown sequence, we can rely on the
ip_lock to sufficiently serialize things:

1. when running concurrently to qeth_l3_set_online(), any instance of
   qeth_l3_modify_ip() that aquires the ip_lock _after_
   qeth_l3_recover_ip() will observe the state as CARD_STATE_SOFTSETUP
   and not defer the IO.
2. when running concurrently to qeth_l3_set_offline(), any instance of
   qeth_l3_modify_ip() that aquires the ip_lock _after_
   qeth_l3_clear_ip_htable() will observe the state as CARD_STATE_DOWN
   and defer the IO.

These guarantees in mind, we can now drop the conf_mutex from the
qeth_l3_modify_rxip_vipa() wrapper.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_l3_main.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 876a21d451f5..0815b64c9797 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -591,7 +591,6 @@ int qeth_l3_modify_rxip_vipa(struct qeth_card *card, bool add, const u8 *ip,
 			     enum qeth_prot_versions proto)
 {
 	struct qeth_ipaddr addr;
-	int rc;
 
 	qeth_l3_init_ipaddr(&addr, type, proto);
 	if (proto == QETH_PROT_IPV4)
@@ -599,11 +598,7 @@ int qeth_l3_modify_rxip_vipa(struct qeth_card *card, bool add, const u8 *ip,
 	else
 		memcpy(&addr.u.a6.addr, ip, 16);
 
-	mutex_lock(&card->conf_mutex);
-	rc = qeth_l3_modify_ip(card, &addr, add);
-	mutex_unlock(&card->conf_mutex);
-
-	return rc;
+	return qeth_l3_modify_ip(card, &addr, add);
 }
 
 int qeth_l3_modify_hsuid(struct qeth_card *card, bool add)
@@ -1161,9 +1156,9 @@ static void qeth_l3_stop_card(struct qeth_card *card)
 		qeth_diags_trace(card, QETH_DIAGS_CMD_TRACE_DISABLE);
 
 	if (card->state == CARD_STATE_SOFTSETUP) {
+		card->state = CARD_STATE_DOWN;
 		qeth_l3_clear_ip_htable(card, 1);
 		qeth_clear_ipacmd_list(card);
-		card->state = CARD_STATE_DOWN;
 	}
 
 	qeth_qdio_clear_card(card, 0);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH net-next 6/9] s390/qeth: cancel cmds earlier during teardown
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
                   ` (4 preceding siblings ...)
  2020-09-23  8:36 ` [PATCH net-next 5/9] s390/qeth: tighten ucast IP locking Julian Wiedmann
@ 2020-09-23  8:36 ` Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 7/9] s390/qeth: consolidate online code Julian Wiedmann
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

Originators of cmd IO typically hold the rtnl or conf_mutex to protect
against a concurrent teardown.
Since qeth_set_offline() already holds the conf_mutex, the main reason
why we still care about cancelling pending cmds is so that they release
the rtnl when we need it ourselves.

So move this step a little earlier into the teardown sequence.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      | 1 -
 drivers/s390/net/qeth_core_main.c | 6 ++++--
 drivers/s390/net/qeth_l2_main.c   | 4 +---
 drivers/s390/net/qeth_l3_main.c   | 1 -
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index f931b764d6a4..711ab5097bd1 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -1067,7 +1067,6 @@ void qeth_put_cmd(struct qeth_cmd_buffer *iob);
 int qeth_schedule_recovery(struct qeth_card *card);
 void qeth_flush_local_addrs(struct qeth_card *card);
 int qeth_poll(struct napi_struct *napi, int budget);
-void qeth_clear_ipacmd_list(struct qeth_card *);
 int qeth_qdio_clear_card(struct qeth_card *, int);
 void qeth_clear_working_pool_list(struct qeth_card *);
 void qeth_drain_output_queues(struct qeth_card *card);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 7cd0cbf8a4f0..286787845cae 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -965,7 +965,7 @@ static struct qeth_ipa_cmd *qeth_check_ipa_data(struct qeth_card *card,
 	}
 }
 
-void qeth_clear_ipacmd_list(struct qeth_card *card)
+static void qeth_clear_ipacmd_list(struct qeth_card *card)
 {
 	struct qeth_cmd_buffer *iob;
 	unsigned long flags;
@@ -977,7 +977,6 @@ void qeth_clear_ipacmd_list(struct qeth_card *card)
 		qeth_notify_cmd(iob, -ECANCELED);
 	spin_unlock_irqrestore(&card->lock, flags);
 }
-EXPORT_SYMBOL_GPL(qeth_clear_ipacmd_list);
 
 static int qeth_check_idx_response(struct qeth_card *card,
 	unsigned char *buffer)
@@ -5334,6 +5333,9 @@ int qeth_set_offline(struct qeth_card *card, bool resetting)
 		card->info.hwtrap = 1;
 	}
 
+	/* cancel any stalled cmd that might block the rtnl: */
+	qeth_clear_ipacmd_list(card);
+
 	rtnl_lock();
 	card->info.open_when_online = card->dev->flags & IFF_UP;
 	dev_close(card->dev);
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index e12ac32b8b47..cbd1ab71e785 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -315,10 +315,8 @@ static void qeth_l2_stop_card(struct qeth_card *card)
 	cancel_work_sync(&card->rx_mode_work);
 	qeth_l2_drain_rx_mode_cache(card);
 
-	if (card->state == CARD_STATE_SOFTSETUP) {
-		qeth_clear_ipacmd_list(card);
+	if (card->state == CARD_STATE_SOFTSETUP)
 		card->state = CARD_STATE_DOWN;
-	}
 
 	qeth_qdio_clear_card(card, 0);
 	qeth_drain_output_queues(card);
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 0815b64c9797..410c35ca8f4a 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1158,7 +1158,6 @@ static void qeth_l3_stop_card(struct qeth_card *card)
 	if (card->state == CARD_STATE_SOFTSETUP) {
 		card->state = CARD_STATE_DOWN;
 		qeth_l3_clear_ip_htable(card, 1);
-		qeth_clear_ipacmd_list(card);
 	}
 
 	qeth_qdio_clear_card(card, 0);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH net-next 7/9] s390/qeth: consolidate online code
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
                   ` (5 preceding siblings ...)
  2020-09-23  8:36 ` [PATCH net-next 6/9] s390/qeth: cancel cmds earlier during teardown Julian Wiedmann
@ 2020-09-23  8:36 ` Julian Wiedmann
  2020-09-23  8:36 ` [PATCH net-next 8/9] s390/qeth: consolidate teardown code Julian Wiedmann
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

Move duplicated code from the disciplines into the core path.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      |  6 +----
 drivers/s390/net/qeth_core_main.c | 41 ++++++++++++++++++++++++-------
 drivers/s390/net/qeth_l2_main.c   | 20 +--------------
 drivers/s390/net/qeth_l3_main.c   | 21 ++--------------
 4 files changed, 36 insertions(+), 52 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 711ab5097bd1..242558bdb5ac 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -753,7 +753,7 @@ struct qeth_discipline {
 	const struct device_type *devtype;
 	int (*setup) (struct ccwgroup_device *);
 	void (*remove) (struct ccwgroup_device *);
-	int (*set_online)(struct qeth_card *card);
+	int (*set_online)(struct qeth_card *card, bool carrier_ok);
 	void (*set_offline)(struct qeth_card *card);
 	int (*do_ioctl)(struct net_device *dev, struct ifreq *rq, int cmd);
 	int (*control_event_handler)(struct qeth_card *card,
@@ -1037,11 +1037,8 @@ struct net_device *qeth_clone_netdev(struct net_device *orig);
 struct qeth_card *qeth_get_card_by_busid(char *bus_id);
 void qeth_set_allowed_threads(struct qeth_card *, unsigned long , int);
 int qeth_threads_running(struct qeth_card *, unsigned long);
-int qeth_core_hardsetup_card(struct qeth_card *card, bool *carrier_ok);
-int qeth_stop_channel(struct qeth_channel *channel);
 int qeth_set_offline(struct qeth_card *card, bool resetting);
 
-void qeth_print_status_message(struct qeth_card *);
 int qeth_send_ipa_cmd(struct qeth_card *, struct qeth_cmd_buffer *,
 		  int (*reply_cb)
 		  (struct qeth_card *, struct qeth_reply *, unsigned long),
@@ -1093,7 +1090,6 @@ 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_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 *);
 int qeth_setassparms_cb(struct qeth_card *, struct qeth_reply *, unsigned long);
 int qeth_setup_netdev(struct qeth_card *card);
 int qeth_set_features(struct net_device *, netdev_features_t);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 286787845cae..5a132241b4dd 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1753,7 +1753,7 @@ static int qeth_halt_channel(struct qeth_card *card,
 	return 0;
 }
 
-int qeth_stop_channel(struct qeth_channel *channel)
+static int qeth_stop_channel(struct qeth_channel *channel)
 {
 	struct ccw_device *cdev = channel->ccwdev;
 	int rc;
@@ -1771,7 +1771,6 @@ int qeth_stop_channel(struct qeth_channel *channel)
 
 	return rc;
 }
-EXPORT_SYMBOL_GPL(qeth_stop_channel);
 
 static int qeth_start_channel(struct qeth_channel *channel)
 {
@@ -2866,7 +2865,7 @@ static int qeth_mpc_initialize(struct qeth_card *card)
 	return 0;
 }
 
-void qeth_print_status_message(struct qeth_card *card)
+static void qeth_print_status_message(struct qeth_card *card)
 {
 	switch (card->info.type) {
 	case QETH_CARD_TYPE_OSD:
@@ -2907,7 +2906,6 @@ void qeth_print_status_message(struct qeth_card *card)
 		 (card->info.mcl_level[0]) ? ")" : "",
 		 qeth_get_cardname_short(card));
 }
-EXPORT_SYMBOL_GPL(qeth_print_status_message);
 
 static void qeth_initialize_working_pool_list(struct qeth_card *card)
 {
@@ -5123,7 +5121,7 @@ static void qeth_core_free_card(struct qeth_card *card)
 	kfree(card);
 }
 
-void qeth_trace_features(struct qeth_card *card)
+static void qeth_trace_features(struct qeth_card *card)
 {
 	QETH_CARD_TEXT(card, 2, "features");
 	QETH_CARD_HEX(card, 2, &card->options.ipa4, sizeof(card->options.ipa4));
@@ -5132,7 +5130,6 @@ void qeth_trace_features(struct qeth_card *card)
 	QETH_CARD_HEX(card, 2, &card->info.diagass_support,
 		      sizeof(card->info.diagass_support));
 }
-EXPORT_SYMBOL_GPL(qeth_trace_features);
 
 static struct ccw_device_id qeth_ids[] = {
 	{CCW_DEVICE_DEVTYPE(0x1731, 0x01, 0x1732, 0x01),
@@ -5163,7 +5160,7 @@ static struct ccw_driver qeth_ccw_driver = {
 	.remove = ccwgroup_remove_ccwdev,
 };
 
-int qeth_core_hardsetup_card(struct qeth_card *card, bool *carrier_ok)
+static int qeth_hardsetup_card(struct qeth_card *card, bool *carrier_ok)
 {
 	int retries = 3;
 	int rc;
@@ -5277,6 +5274,8 @@ int qeth_core_hardsetup_card(struct qeth_card *card, bool *carrier_ok)
 			QETH_CARD_TEXT_(card, 2, "8err%d", rc);
 	}
 
+	qeth_trace_features(card);
+
 	if (!qeth_is_diagass_supported(card, QETH_DIAGS_CMD_TRAP) ||
 	    (card->info.hwtrap && qeth_hw_trap(card, QETH_DIAGS_TRAP_ARM)))
 		card->info.hwtrap = 0;
@@ -5302,21 +5301,45 @@ int qeth_core_hardsetup_card(struct qeth_card *card, bool *carrier_ok)
 			 CARD_DEVID(card), rc);
 	return rc;
 }
-EXPORT_SYMBOL_GPL(qeth_core_hardsetup_card);
 
 static int qeth_set_online(struct qeth_card *card)
 {
+	bool carrier_ok;
 	int rc;
 
 	mutex_lock(&card->discipline_mutex);
 	mutex_lock(&card->conf_mutex);
 	QETH_CARD_TEXT(card, 2, "setonlin");
 
-	rc = card->discipline->set_online(card);
+	rc = qeth_hardsetup_card(card, &carrier_ok);
+	if (rc) {
+		QETH_CARD_TEXT_(card, 2, "2err%04x", rc);
+		rc = -ENODEV;
+		goto err_hardsetup;
+	}
+
+	qeth_print_status_message(card);
+
+	rc = card->discipline->set_online(card, carrier_ok);
+	if (rc)
+		goto err_online;
+
+	/* let user_space know that device is online */
+	kobject_uevent(&card->gdev->dev.kobj, KOBJ_CHANGE);
 
 	mutex_unlock(&card->conf_mutex);
 	mutex_unlock(&card->discipline_mutex);
+	return 0;
+
+err_online:
+err_hardsetup:
+	qeth_stop_channel(&card->data);
+	qeth_stop_channel(&card->write);
+	qeth_stop_channel(&card->read);
+	qdio_free(CARD_DDEV(card));
 
+	mutex_unlock(&card->conf_mutex);
+	mutex_unlock(&card->discipline_mutex);
 	return rc;
 }
 
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index cbd1ab71e785..6e8d5113d435 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1138,19 +1138,10 @@ static void qeth_l2_enable_brport_features(struct qeth_card *card)
 	}
 }
 
-static int qeth_l2_set_online(struct qeth_card *card)
+static int qeth_l2_set_online(struct qeth_card *card, bool carrier_ok)
 {
-	struct ccwgroup_device *gdev = card->gdev;
 	struct net_device *dev = card->dev;
 	int rc = 0;
-	bool carrier_ok;
-
-	rc = qeth_core_hardsetup_card(card, &carrier_ok);
-	if (rc) {
-		QETH_CARD_TEXT_(card, 2, "2err%04x", rc);
-		rc = -ENODEV;
-		goto out_remove;
-	}
 
 	/* query before bridgeport_notification may be enabled */
 	qeth_l2_detect_dev2br_support(card);
@@ -1169,11 +1160,8 @@ static int qeth_l2_set_online(struct qeth_card *card)
 	/* for the rx_bcast characteristic, init VNICC after setmac */
 	qeth_l2_vnicc_init(card);
 
-	qeth_trace_features(card);
 	qeth_l2_trace_features(card);
 
-	qeth_print_status_message(card);
-
 	/* softsetup */
 	QETH_CARD_TEXT(card, 2, "softsetp");
 
@@ -1205,16 +1193,10 @@ static int qeth_l2_set_online(struct qeth_card *card)
 		}
 		rtnl_unlock();
 	}
-	/* let user_space know that device is online */
-	kobject_uevent(&gdev->dev.kobj, KOBJ_CHANGE);
 	return 0;
 
 out_remove:
 	qeth_l2_stop_card(card);
-	qeth_stop_channel(&card->data);
-	qeth_stop_channel(&card->write);
-	qeth_stop_channel(&card->read);
-	qdio_free(CARD_DDEV(card));
 	return rc;
 }
 
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 410c35ca8f4a..08285af6a5ff 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2012,21 +2012,10 @@ static void qeth_l3_remove_device(struct ccwgroup_device *cgdev)
 	qeth_l3_clear_ipato_list(card);
 }
 
-static int qeth_l3_set_online(struct qeth_card *card)
+static int qeth_l3_set_online(struct qeth_card *card, bool carrier_ok)
 {
-	struct ccwgroup_device *gdev = card->gdev;
 	struct net_device *dev = card->dev;
 	int rc = 0;
-	bool carrier_ok;
-
-	rc = qeth_core_hardsetup_card(card, &carrier_ok);
-	if (rc) {
-		QETH_CARD_TEXT_(card, 2, "2err%04x", rc);
-		rc = -ENODEV;
-		goto out_remove;
-	}
-
-	qeth_print_status_message(card);
 
 	/* softsetup */
 	QETH_CARD_TEXT(card, 2, "softsetp");
@@ -2073,16 +2062,10 @@ static int qeth_l3_set_online(struct qeth_card *card)
 		}
 		rtnl_unlock();
 	}
-	qeth_trace_features(card);
-	/* let user_space know that device is online */
-	kobject_uevent(&gdev->dev.kobj, KOBJ_CHANGE);
 	return 0;
+
 out_remove:
 	qeth_l3_stop_card(card);
-	qeth_stop_channel(&card->data);
-	qeth_stop_channel(&card->write);
-	qeth_stop_channel(&card->read);
-	qdio_free(CARD_DDEV(card));
 	return rc;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH net-next 8/9] s390/qeth: consolidate teardown code
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
                   ` (6 preceding siblings ...)
  2020-09-23  8:36 ` [PATCH net-next 7/9] s390/qeth: consolidate online code Julian Wiedmann
@ 2020-09-23  8:36 ` Julian Wiedmann
  2020-09-23  8:37 ` [PATCH net-next 9/9] s390/qeth: remove forward declarations in L2 code Julian Wiedmann
  2020-09-23 19:08 ` [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 David Miller
  9 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:36 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

Clarify which discipline-specific steps are needed to roll back after
error in qeth_l?_set_online(), and which are common to roll back
from qeth_hardsetup_card().

Some steps (cancelling the RX modeset, draining the TX queues) are only
necessary if the netdev was potentially UP before, so move them to the
common qeth_set_offline().

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      |  4 ---
 drivers/s390/net/qeth_core_main.c | 24 ++++++++++-----
 drivers/s390/net/qeth_l2_main.c   | 50 +++++++++++--------------------
 drivers/s390/net/qeth_l3_main.c   | 46 ++++++++++------------------
 4 files changed, 50 insertions(+), 74 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 242558bdb5ac..f321eabefbe4 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -1062,11 +1062,7 @@ void qeth_notify_cmd(struct qeth_cmd_buffer *iob, int reason);
 void qeth_put_cmd(struct qeth_cmd_buffer *iob);
 
 int qeth_schedule_recovery(struct qeth_card *card);
-void qeth_flush_local_addrs(struct qeth_card *card);
 int qeth_poll(struct napi_struct *napi, int budget);
-int qeth_qdio_clear_card(struct qeth_card *, int);
-void qeth_clear_working_pool_list(struct qeth_card *);
-void qeth_drain_output_queues(struct qeth_card *card);
 void qeth_setadp_promisc_mode(struct qeth_card *card, bool enable);
 int qeth_setadpparms_change_macaddr(struct qeth_card *);
 void qeth_tx_timeout(struct net_device *, unsigned int txqueue);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 5a132241b4dd..fc2c3db9259f 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -201,7 +201,7 @@ int qeth_threads_running(struct qeth_card *card, unsigned long threads)
 }
 EXPORT_SYMBOL_GPL(qeth_threads_running);
 
-void qeth_clear_working_pool_list(struct qeth_card *card)
+static void qeth_clear_working_pool_list(struct qeth_card *card)
 {
 	struct qeth_buffer_pool_entry *pool_entry, *tmp;
 	struct qeth_qdio_q *queue = card->qdio.in_q;
@@ -216,7 +216,6 @@ void qeth_clear_working_pool_list(struct qeth_card *card)
 	for (i = 0; i < ARRAY_SIZE(queue->bufs); i++)
 		queue->bufs[i].pool_entry = NULL;
 }
-EXPORT_SYMBOL_GPL(qeth_clear_working_pool_list);
 
 static void qeth_free_pool_entry(struct qeth_buffer_pool_entry *entry)
 {
@@ -658,12 +657,11 @@ static void qeth_flush_local_addrs6(struct qeth_card *card)
 	spin_unlock_irq(&card->local_addrs6_lock);
 }
 
-void qeth_flush_local_addrs(struct qeth_card *card)
+static void qeth_flush_local_addrs(struct qeth_card *card)
 {
 	qeth_flush_local_addrs4(card);
 	qeth_flush_local_addrs6(card);
 }
-EXPORT_SYMBOL_GPL(qeth_flush_local_addrs);
 
 static void qeth_add_local_addrs4(struct qeth_card *card,
 				  struct qeth_ipacmd_local_addrs4 *cmd)
@@ -1501,7 +1499,7 @@ static void qeth_drain_output_queue(struct qeth_qdio_out_q *q, bool free)
 	}
 }
 
-void qeth_drain_output_queues(struct qeth_card *card)
+static void qeth_drain_output_queues(struct qeth_card *card)
 {
 	int i;
 
@@ -1512,7 +1510,6 @@ void qeth_drain_output_queues(struct qeth_card *card)
 			qeth_drain_output_queue(card->qdio.out_qs[i], false);
 	}
 }
-EXPORT_SYMBOL_GPL(qeth_drain_output_queues);
 
 static int qeth_osa_set_output_queues(struct qeth_card *card, bool single)
 {
@@ -1840,7 +1837,7 @@ static int qeth_clear_halt_card(struct qeth_card *card, int halt)
 	return qeth_clear_channels(card);
 }
 
-int qeth_qdio_clear_card(struct qeth_card *card, int use_halt)
+static int qeth_qdio_clear_card(struct qeth_card *card, int use_halt)
 {
 	int rc = 0;
 
@@ -1868,7 +1865,6 @@ int qeth_qdio_clear_card(struct qeth_card *card, int use_halt)
 		QETH_CARD_TEXT_(card, 3, "2err%d", rc);
 	return rc;
 }
-EXPORT_SYMBOL_GPL(qeth_qdio_clear_card);
 
 static enum qeth_discipline_id qeth_vm_detect_layer(struct qeth_card *card)
 {
@@ -5333,6 +5329,10 @@ static int qeth_set_online(struct qeth_card *card)
 
 err_online:
 err_hardsetup:
+	qeth_qdio_clear_card(card, 0);
+	qeth_clear_working_pool_list(card);
+	qeth_flush_local_addrs(card);
+
 	qeth_stop_channel(&card->data);
 	qeth_stop_channel(&card->write);
 	qeth_stop_channel(&card->read);
@@ -5366,8 +5366,16 @@ int qeth_set_offline(struct qeth_card *card, bool resetting)
 	netif_carrier_off(card->dev);
 	rtnl_unlock();
 
+	cancel_work_sync(&card->rx_mode_work);
+
 	card->discipline->set_offline(card);
 
+	qeth_qdio_clear_card(card, 0);
+	qeth_drain_output_queues(card);
+	qeth_clear_working_pool_list(card);
+	qeth_flush_local_addrs(card);
+	card->info.promisc_mode = 0;
+
 	rc  = qeth_stop_channel(&card->data);
 	rc2 = qeth_stop_channel(&card->write);
 	rc3 = qeth_stop_channel(&card->read);
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 6e8d5113d435..fd6891494c69 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -304,34 +304,6 @@ static void qeth_l2_dev2br_fdb_flush(struct qeth_card *card)
 				 card->dev, &info.info, NULL);
 }
 
-static void qeth_l2_stop_card(struct qeth_card *card)
-{
-	struct qeth_priv *priv = netdev_priv(card->dev);
-
-	QETH_CARD_TEXT(card, 2, "stopcard");
-
-	qeth_set_allowed_threads(card, 0, 1);
-
-	cancel_work_sync(&card->rx_mode_work);
-	qeth_l2_drain_rx_mode_cache(card);
-
-	if (card->state == CARD_STATE_SOFTSETUP)
-		card->state = CARD_STATE_DOWN;
-
-	qeth_qdio_clear_card(card, 0);
-	qeth_drain_output_queues(card);
-	qeth_clear_working_pool_list(card);
-	qeth_l2_set_pnso_mode(card, QETH_PNSO_NONE);
-	qeth_flush_local_addrs(card);
-	card->info.promisc_mode = 0;
-
-	if (priv->brport_features & BR_LEARNING_SYNC) {
-		rtnl_lock();
-		qeth_l2_dev2br_fdb_flush(card);
-		rtnl_unlock();
-	}
-}
-
 static int qeth_l2_request_initial_mac(struct qeth_card *card)
 {
 	int rc = 0;
@@ -1172,7 +1144,7 @@ static int qeth_l2_set_online(struct qeth_card *card, bool carrier_ok)
 	if (dev->reg_state != NETREG_REGISTERED) {
 		rc = qeth_l2_setup_netdev(card);
 		if (rc)
-			goto out_remove;
+			goto err_setup;
 
 		if (carrier_ok)
 			netif_carrier_on(dev);
@@ -1195,14 +1167,28 @@ static int qeth_l2_set_online(struct qeth_card *card, bool carrier_ok)
 	}
 	return 0;
 
-out_remove:
-	qeth_l2_stop_card(card);
+err_setup:
+	qeth_set_allowed_threads(card, 0, 1);
+	card->state = CARD_STATE_DOWN;
 	return rc;
 }
 
 static void qeth_l2_set_offline(struct qeth_card *card)
 {
-	qeth_l2_stop_card(card);
+	struct qeth_priv *priv = netdev_priv(card->dev);
+
+	qeth_set_allowed_threads(card, 0, 1);
+	qeth_l2_drain_rx_mode_cache(card);
+
+	if (card->state == CARD_STATE_SOFTSETUP)
+		card->state = CARD_STATE_DOWN;
+
+	qeth_l2_set_pnso_mode(card, QETH_PNSO_NONE);
+	if (priv->brport_features & BR_LEARNING_SYNC) {
+		rtnl_lock();
+		qeth_l2_dev2br_fdb_flush(card);
+		rtnl_unlock();
+	}
 }
 
 static int __init qeth_l2_init(void)
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 08285af6a5ff..a6f8878b55c6 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1142,32 +1142,6 @@ static int qeth_l3_vlan_rx_kill_vid(struct net_device *dev,
 	return 0;
 }
 
-static void qeth_l3_stop_card(struct qeth_card *card)
-{
-	QETH_CARD_TEXT(card, 2, "stopcard");
-
-	qeth_set_allowed_threads(card, 0, 1);
-
-	cancel_work_sync(&card->rx_mode_work);
-	qeth_l3_drain_rx_mode_cache(card);
-
-	if (card->options.sniffer &&
-	    (card->info.promisc_mode == SET_PROMISC_MODE_ON))
-		qeth_diags_trace(card, QETH_DIAGS_CMD_TRACE_DISABLE);
-
-	if (card->state == CARD_STATE_SOFTSETUP) {
-		card->state = CARD_STATE_DOWN;
-		qeth_l3_clear_ip_htable(card, 1);
-	}
-
-	qeth_qdio_clear_card(card, 0);
-	qeth_drain_output_queues(card);
-	qeth_clear_working_pool_list(card);
-	flush_workqueue(card->event_wq);
-	qeth_flush_local_addrs(card);
-	card->info.promisc_mode = 0;
-}
-
 static void qeth_l3_set_promisc_mode(struct qeth_card *card)
 {
 	bool enable = card->dev->flags & IFF_PROMISC;
@@ -2042,7 +2016,7 @@ static int qeth_l3_set_online(struct qeth_card *card, bool carrier_ok)
 	if (dev->reg_state != NETREG_REGISTERED) {
 		rc = qeth_l3_setup_netdev(card);
 		if (rc)
-			goto out_remove;
+			goto err_setup;
 
 		if (carrier_ok)
 			netif_carrier_on(dev);
@@ -2064,14 +2038,26 @@ static int qeth_l3_set_online(struct qeth_card *card, bool carrier_ok)
 	}
 	return 0;
 
-out_remove:
-	qeth_l3_stop_card(card);
+err_setup:
+	qeth_set_allowed_threads(card, 0, 1);
+	card->state = CARD_STATE_DOWN;
+	qeth_l3_clear_ip_htable(card, 1);
 	return rc;
 }
 
 static void qeth_l3_set_offline(struct qeth_card *card)
 {
-	qeth_l3_stop_card(card);
+	qeth_set_allowed_threads(card, 0, 1);
+	qeth_l3_drain_rx_mode_cache(card);
+
+	if (card->options.sniffer &&
+	    (card->info.promisc_mode == SET_PROMISC_MODE_ON))
+		qeth_diags_trace(card, QETH_DIAGS_CMD_TRACE_DISABLE);
+
+	if (card->state == CARD_STATE_SOFTSETUP) {
+		card->state = CARD_STATE_DOWN;
+		qeth_l3_clear_ip_htable(card, 1);
+	}
 }
 
 /* Returns zero if the command is successfully "consumed" */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH net-next 9/9] s390/qeth: remove forward declarations in L2 code
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
                   ` (7 preceding siblings ...)
  2020-09-23  8:36 ` [PATCH net-next 8/9] s390/qeth: consolidate teardown code Julian Wiedmann
@ 2020-09-23  8:37 ` Julian Wiedmann
  2020-09-23 19:08 ` [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 David Miller
  9 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23  8:37 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul,
	Julian Wiedmann

Shuffle some code around (primarily all the discipline-related stuff) to
get rid of all the unnecessary forward declarations.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_l2.h      |   7 +
 drivers/s390/net/qeth_l2_main.c | 378 +++++++++++++++-----------------
 2 files changed, 188 insertions(+), 197 deletions(-)

diff --git a/drivers/s390/net/qeth_l2.h b/drivers/s390/net/qeth_l2.h
index cc95675c8bc4..296d73d84326 100644
--- a/drivers/s390/net/qeth_l2.h
+++ b/drivers/s390/net/qeth_l2.h
@@ -31,4 +31,11 @@ struct qeth_mac {
 	struct hlist_node hnode;
 };
 
+static inline bool qeth_bridgeport_is_in_use(struct qeth_card *card)
+{
+	return card->options.sbp.role ||
+	       card->options.sbp.reflect_promisc ||
+	       card->options.sbp.hostnotification;
+}
+
 #endif /* __QETH_L2_H__ */
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index fd6891494c69..1852d0a3c10a 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -28,17 +28,6 @@
 #include "qeth_core.h"
 #include "qeth_l2.h"
 
-static void qeth_bridgeport_query_support(struct qeth_card *card);
-static void qeth_bridge_state_change(struct qeth_card *card,
-					struct qeth_ipa_cmd *cmd);
-static void qeth_addr_change_event(struct qeth_card *card,
-				   struct qeth_ipa_cmd *cmd);
-static bool qeth_bridgeport_is_in_use(struct qeth_card *card);
-static void qeth_l2_vnicc_set_defaults(struct qeth_card *card);
-static void qeth_l2_vnicc_init(struct qeth_card *card);
-static bool qeth_l2_vnicc_recover_timeout(struct qeth_card *card, u32 vnicc,
-					  u32 *timeout);
-
 static int qeth_l2_setdelmac_makerc(struct qeth_card *card, u16 retcode)
 {
 	int rc;
@@ -587,49 +576,6 @@ static u16 qeth_l2_select_queue(struct net_device *dev, struct sk_buff *skb,
 				 qeth_get_priority_queue(card, skb);
 }
 
-static const struct device_type qeth_l2_devtype = {
-	.name = "qeth_layer2",
-	.groups = qeth_l2_attr_groups,
-};
-
-static int qeth_l2_probe_device(struct ccwgroup_device *gdev)
-{
-	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
-	int rc;
-
-	if (IS_OSN(card))
-		dev_notice(&gdev->dev, "OSN support will be dropped in 2021\n");
-
-	qeth_l2_vnicc_set_defaults(card);
-	mutex_init(&card->sbp_lock);
-
-	if (gdev->dev.type == &qeth_generic_devtype) {
-		rc = qeth_l2_create_device_attributes(&gdev->dev);
-		if (rc)
-			return rc;
-	}
-
-	INIT_WORK(&card->rx_mode_work, qeth_l2_rx_mode_work);
-	return 0;
-}
-
-static void qeth_l2_remove_device(struct ccwgroup_device *cgdev)
-{
-	struct qeth_card *card = dev_get_drvdata(&cgdev->dev);
-
-	if (cgdev->dev.type == &qeth_generic_devtype)
-		qeth_l2_remove_device_attributes(&cgdev->dev);
-	qeth_set_allowed_threads(card, 0, 1);
-	wait_event(card->wait_q, qeth_threads_running(card, 0xffffffff) == 0);
-
-	if (cgdev->state == CCWGROUP_ONLINE)
-		qeth_set_offline(card, false);
-
-	cancel_work_sync(&card->close_dev_work);
-	if (card->dev->reg_state == NETREG_REGISTERED)
-		unregister_netdev(card->dev);
-}
-
 static void qeth_l2_set_rx_mode(struct net_device *dev)
 {
 	struct qeth_card *card = dev->ml_priv;
@@ -1110,130 +1056,6 @@ static void qeth_l2_enable_brport_features(struct qeth_card *card)
 	}
 }
 
-static int qeth_l2_set_online(struct qeth_card *card, bool carrier_ok)
-{
-	struct net_device *dev = card->dev;
-	int rc = 0;
-
-	/* query before bridgeport_notification may be enabled */
-	qeth_l2_detect_dev2br_support(card);
-
-	mutex_lock(&card->sbp_lock);
-	qeth_bridgeport_query_support(card);
-	if (card->options.sbp.supported_funcs) {
-		qeth_l2_setup_bridgeport_attrs(card);
-		dev_info(&card->gdev->dev,
-			 "The device represents a Bridge Capable Port\n");
-	}
-	mutex_unlock(&card->sbp_lock);
-
-	qeth_l2_register_dev_addr(card);
-
-	/* for the rx_bcast characteristic, init VNICC after setmac */
-	qeth_l2_vnicc_init(card);
-
-	qeth_l2_trace_features(card);
-
-	/* softsetup */
-	QETH_CARD_TEXT(card, 2, "softsetp");
-
-	card->state = CARD_STATE_SOFTSETUP;
-
-	qeth_set_allowed_threads(card, 0xffffffff, 0);
-
-	if (dev->reg_state != NETREG_REGISTERED) {
-		rc = qeth_l2_setup_netdev(card);
-		if (rc)
-			goto err_setup;
-
-		if (carrier_ok)
-			netif_carrier_on(dev);
-	} else {
-		rtnl_lock();
-		if (carrier_ok)
-			netif_carrier_on(dev);
-		else
-			netif_carrier_off(dev);
-
-		netif_device_attach(dev);
-		qeth_enable_hw_features(dev);
-		qeth_l2_enable_brport_features(card);
-
-		if (card->info.open_when_online) {
-			card->info.open_when_online = 0;
-			dev_open(dev, NULL);
-		}
-		rtnl_unlock();
-	}
-	return 0;
-
-err_setup:
-	qeth_set_allowed_threads(card, 0, 1);
-	card->state = CARD_STATE_DOWN;
-	return rc;
-}
-
-static void qeth_l2_set_offline(struct qeth_card *card)
-{
-	struct qeth_priv *priv = netdev_priv(card->dev);
-
-	qeth_set_allowed_threads(card, 0, 1);
-	qeth_l2_drain_rx_mode_cache(card);
-
-	if (card->state == CARD_STATE_SOFTSETUP)
-		card->state = CARD_STATE_DOWN;
-
-	qeth_l2_set_pnso_mode(card, QETH_PNSO_NONE);
-	if (priv->brport_features & BR_LEARNING_SYNC) {
-		rtnl_lock();
-		qeth_l2_dev2br_fdb_flush(card);
-		rtnl_unlock();
-	}
-}
-
-static int __init qeth_l2_init(void)
-{
-	pr_info("register layer 2 discipline\n");
-	return 0;
-}
-
-static void __exit qeth_l2_exit(void)
-{
-	pr_info("unregister layer 2 discipline\n");
-}
-
-/* Returns zero if the command is successfully "consumed" */
-static int qeth_l2_control_event(struct qeth_card *card,
-					struct qeth_ipa_cmd *cmd)
-{
-	switch (cmd->hdr.command) {
-	case IPA_CMD_SETBRIDGEPORT_OSA:
-	case IPA_CMD_SETBRIDGEPORT_IQD:
-		if (cmd->data.sbp.hdr.command_code ==
-				IPA_SBP_BRIDGE_PORT_STATE_CHANGE) {
-			qeth_bridge_state_change(card, cmd);
-			return 0;
-		} else
-			return 1;
-	case IPA_CMD_ADDRESS_CHANGE_NOTIF:
-		qeth_addr_change_event(card, cmd);
-		return 0;
-	default:
-		return 1;
-	}
-}
-
-struct qeth_discipline qeth_l2_discipline = {
-	.devtype = &qeth_l2_devtype,
-	.setup = qeth_l2_probe_device,
-	.remove = qeth_l2_remove_device,
-	.set_online = qeth_l2_set_online,
-	.set_offline = qeth_l2_set_offline,
-	.do_ioctl = NULL,
-	.control_event_handler = qeth_l2_control_event,
-};
-EXPORT_SYMBOL_GPL(qeth_l2_discipline);
-
 #ifdef CONFIG_QETH_OSN
 static void qeth_osn_assist_cb(struct qeth_card *card,
 			       struct qeth_cmd_buffer *iob,
@@ -1953,12 +1775,6 @@ int qeth_bridgeport_an_set(struct qeth_card *card, int enable)
 	return rc;
 }
 
-static bool qeth_bridgeport_is_in_use(struct qeth_card *card)
-{
-	return (card->options.sbp.role || card->options.sbp.reflect_promisc ||
-		card->options.sbp.hostnotification);
-}
-
 /* VNIC Characteristics support */
 
 /* handle VNICC IPA command return codes; convert to error codes */
@@ -2104,6 +1920,19 @@ static int qeth_l2_vnicc_getset_timeout(struct qeth_card *card, u32 vnicc,
 	return qeth_send_ipa_cmd(card, iob, qeth_l2_vnicc_request_cb, timeout);
 }
 
+/* recover user timeout setting */
+static bool qeth_l2_vnicc_recover_timeout(struct qeth_card *card, u32 vnicc,
+					  u32 *timeout)
+{
+	if (card->options.vnicc.sup_chars & vnicc &&
+	    card->options.vnicc.getset_timeout_sup & vnicc &&
+	    !qeth_l2_vnicc_getset_timeout(card, vnicc, IPA_VNICC_SET_TIMEOUT,
+					  timeout))
+		return false;
+	*timeout = QETH_VNICC_DEFAULT_TIMEOUT;
+	return true;
+}
+
 /* set current VNICC flag state; called from sysfs store function */
 int qeth_l2_vnicc_set_state(struct qeth_card *card, u32 vnicc, bool state)
 {
@@ -2274,19 +2103,6 @@ bool qeth_bridgeport_allowed(struct qeth_card *card)
 		!(priv->brport_features & BR_LEARNING_SYNC));
 }
 
-/* recover user timeout setting */
-static bool qeth_l2_vnicc_recover_timeout(struct qeth_card *card, u32 vnicc,
-					  u32 *timeout)
-{
-	if (card->options.vnicc.sup_chars & vnicc &&
-	    card->options.vnicc.getset_timeout_sup & vnicc &&
-	    !qeth_l2_vnicc_getset_timeout(card, vnicc, IPA_VNICC_SET_TIMEOUT,
-					  timeout))
-		return false;
-	*timeout = QETH_VNICC_DEFAULT_TIMEOUT;
-	return true;
-}
-
 /* recover user characteristic setting */
 static bool qeth_l2_vnicc_recover_char(struct qeth_card *card, u32 vnicc,
 				       bool enable)
@@ -2375,6 +2191,174 @@ static void qeth_l2_vnicc_set_defaults(struct qeth_card *card)
 	card->options.vnicc.wanted_chars = QETH_VNICC_DEFAULT;
 }
 
+static const struct device_type qeth_l2_devtype = {
+	.name = "qeth_layer2",
+	.groups = qeth_l2_attr_groups,
+};
+
+static int qeth_l2_probe_device(struct ccwgroup_device *gdev)
+{
+	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
+	int rc;
+
+	if (IS_OSN(card))
+		dev_notice(&gdev->dev, "OSN support will be dropped in 2021\n");
+
+	qeth_l2_vnicc_set_defaults(card);
+	mutex_init(&card->sbp_lock);
+
+	if (gdev->dev.type == &qeth_generic_devtype) {
+		rc = qeth_l2_create_device_attributes(&gdev->dev);
+		if (rc)
+			return rc;
+	}
+
+	INIT_WORK(&card->rx_mode_work, qeth_l2_rx_mode_work);
+	return 0;
+}
+
+static void qeth_l2_remove_device(struct ccwgroup_device *gdev)
+{
+	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
+
+	if (gdev->dev.type == &qeth_generic_devtype)
+		qeth_l2_remove_device_attributes(&gdev->dev);
+	qeth_set_allowed_threads(card, 0, 1);
+	wait_event(card->wait_q, qeth_threads_running(card, 0xffffffff) == 0);
+
+	if (gdev->state == CCWGROUP_ONLINE)
+		qeth_set_offline(card, false);
+
+	cancel_work_sync(&card->close_dev_work);
+	if (card->dev->reg_state == NETREG_REGISTERED)
+		unregister_netdev(card->dev);
+}
+
+static int qeth_l2_set_online(struct qeth_card *card, bool carrier_ok)
+{
+	struct net_device *dev = card->dev;
+	int rc = 0;
+
+	/* query before bridgeport_notification may be enabled */
+	qeth_l2_detect_dev2br_support(card);
+
+	mutex_lock(&card->sbp_lock);
+	qeth_bridgeport_query_support(card);
+	if (card->options.sbp.supported_funcs) {
+		qeth_l2_setup_bridgeport_attrs(card);
+		dev_info(&card->gdev->dev,
+			 "The device represents a Bridge Capable Port\n");
+	}
+	mutex_unlock(&card->sbp_lock);
+
+	qeth_l2_register_dev_addr(card);
+
+	/* for the rx_bcast characteristic, init VNICC after setmac */
+	qeth_l2_vnicc_init(card);
+
+	qeth_l2_trace_features(card);
+
+	/* softsetup */
+	QETH_CARD_TEXT(card, 2, "softsetp");
+
+	card->state = CARD_STATE_SOFTSETUP;
+
+	qeth_set_allowed_threads(card, 0xffffffff, 0);
+
+	if (dev->reg_state != NETREG_REGISTERED) {
+		rc = qeth_l2_setup_netdev(card);
+		if (rc)
+			goto err_setup;
+
+		if (carrier_ok)
+			netif_carrier_on(dev);
+	} else {
+		rtnl_lock();
+		if (carrier_ok)
+			netif_carrier_on(dev);
+		else
+			netif_carrier_off(dev);
+
+		netif_device_attach(dev);
+		qeth_enable_hw_features(dev);
+		qeth_l2_enable_brport_features(card);
+
+		if (card->info.open_when_online) {
+			card->info.open_when_online = 0;
+			dev_open(dev, NULL);
+		}
+		rtnl_unlock();
+	}
+	return 0;
+
+err_setup:
+	qeth_set_allowed_threads(card, 0, 1);
+	card->state = CARD_STATE_DOWN;
+	return rc;
+}
+
+static void qeth_l2_set_offline(struct qeth_card *card)
+{
+	struct qeth_priv *priv = netdev_priv(card->dev);
+
+	qeth_set_allowed_threads(card, 0, 1);
+	qeth_l2_drain_rx_mode_cache(card);
+
+	if (card->state == CARD_STATE_SOFTSETUP)
+		card->state = CARD_STATE_DOWN;
+
+	qeth_l2_set_pnso_mode(card, QETH_PNSO_NONE);
+	if (priv->brport_features & BR_LEARNING_SYNC) {
+		rtnl_lock();
+		qeth_l2_dev2br_fdb_flush(card);
+		rtnl_unlock();
+	}
+}
+
+/* Returns zero if the command is successfully "consumed" */
+static int qeth_l2_control_event(struct qeth_card *card,
+				 struct qeth_ipa_cmd *cmd)
+{
+	switch (cmd->hdr.command) {
+	case IPA_CMD_SETBRIDGEPORT_OSA:
+	case IPA_CMD_SETBRIDGEPORT_IQD:
+		if (cmd->data.sbp.hdr.command_code ==
+		    IPA_SBP_BRIDGE_PORT_STATE_CHANGE) {
+			qeth_bridge_state_change(card, cmd);
+			return 0;
+		}
+
+		return 1;
+	case IPA_CMD_ADDRESS_CHANGE_NOTIF:
+		qeth_addr_change_event(card, cmd);
+		return 0;
+	default:
+		return 1;
+	}
+}
+
+struct qeth_discipline qeth_l2_discipline = {
+	.devtype = &qeth_l2_devtype,
+	.setup = qeth_l2_probe_device,
+	.remove = qeth_l2_remove_device,
+	.set_online = qeth_l2_set_online,
+	.set_offline = qeth_l2_set_offline,
+	.do_ioctl = NULL,
+	.control_event_handler = qeth_l2_control_event,
+};
+EXPORT_SYMBOL_GPL(qeth_l2_discipline);
+
+static int __init qeth_l2_init(void)
+{
+	pr_info("register layer 2 discipline\n");
+	return 0;
+}
+
+static void __exit qeth_l2_exit(void)
+{
+	pr_info("unregister layer 2 discipline\n");
+}
+
 module_init(qeth_l2_init);
 module_exit(qeth_l2_exit);
 MODULE_AUTHOR("Frank Blaschka <frank.blaschka@de.ibm.com>");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread
* RE: [PATCH net-next 3/9] s390/qeth: clean up string ops in qeth_l3_parse_ipatoe()
  2020-09-23  8:36 ` [PATCH net-next 3/9] s390/qeth: clean up string ops in qeth_l3_parse_ipatoe() Julian Wiedmann
@ 2020-09-23  9:55   ` David Laight
  2020-09-23 10:43     ` Julian Wiedmann
  0 siblings, 1 reply; 13+ messages in thread
From: David Laight @ 2020-09-23  9:55 UTC (permalink / raw)
  To: 'Julian Wiedmann', David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul

From: Julian Wiedmann
> Sent: 23 September 2020 09:37
> 
> Indicate the max number of to-be-parsed characters, and avoid copying
> the address sub-string.
> 
> Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
> ---
>  drivers/s390/net/qeth_l3_sys.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/s390/net/qeth_l3_sys.c b/drivers/s390/net/qeth_l3_sys.c
> index ca9c95b6bf35..05fa986e30fc 100644
> --- a/drivers/s390/net/qeth_l3_sys.c
> +++ b/drivers/s390/net/qeth_l3_sys.c
> @@ -409,21 +409,22 @@ static ssize_t qeth_l3_dev_ipato_add4_show(struct device *dev,
>  static int qeth_l3_parse_ipatoe(const char *buf, enum qeth_prot_versions proto,
>  		  u8 *addr, int *mask_bits)
>  {
> -	const char *start, *end;
> -	char *tmp;
> -	char buffer[40] = {0, };
> +	const char *start;
> +	char *sep, *tmp;
> +	int rc;
> 
> -	start = buf;
> -	/* get address string */
> -	end = strchr(start, '/');
> -	if (!end || (end - start >= 40)) {
> +	/* Expected input pattern: %addr/%mask */
> +	sep = strnchr(buf, 40, '/');
> +	if (!sep)
>  		return -EINVAL;
> -	}
> -	strncpy(buffer, start, end - start);
> -	if (qeth_l3_string_to_ipaddr(buffer, proto, addr)) {
> -		return -EINVAL;
> -	}
> -	start = end + 1;
> +
> +	/* Terminate the %addr sub-string, and parse it: */
> +	*sep = '\0';

Is it valid to write into the input buffer here?

> +	rc = qeth_l3_string_to_ipaddr(buf, proto, addr);
> +	if (rc)
> +		return rc;
> +
> +	start = sep + 1;
>  	*mask_bits = simple_strtoul(start, &tmp, 10);

The use of strnchr() rather implies that the input
buffer may not be '\0' terminated.
If that is true then you've just run off the end of the
input buffer.

>  	if (!strlen(start) ||
>  	    (tmp == start) ||

Hmmm... delete the strlen() clause.
It ought to test start[0], but the 'tmp == start' test
covers that case.

I don't understand why simple_strtoul() is deprecated.
I don't recall any of the replacements returning the
address of the terminating character.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH net-next 3/9] s390/qeth: clean up string ops in qeth_l3_parse_ipatoe()
  2020-09-23  9:55   ` David Laight
@ 2020-09-23 10:43     ` Julian Wiedmann
  0 siblings, 0 replies; 13+ messages in thread
From: Julian Wiedmann @ 2020-09-23 10:43 UTC (permalink / raw)
  To: David Laight, David Miller, Jakub Kicinski
  Cc: netdev, linux-s390, Heiko Carstens, Ursula Braun, Karsten Graul

On 23.09.20 12:55, David Laight wrote:
> From: Julian Wiedmann
>> Sent: 23 September 2020 09:37
>>
>> Indicate the max number of to-be-parsed characters, and avoid copying
>> the address sub-string.
>>
>> Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
>> ---
>>  drivers/s390/net/qeth_l3_sys.c | 27 ++++++++++++++-------------
>>  1 file changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/s390/net/qeth_l3_sys.c b/drivers/s390/net/qeth_l3_sys.c
>> index ca9c95b6bf35..05fa986e30fc 100644
>> --- a/drivers/s390/net/qeth_l3_sys.c
>> +++ b/drivers/s390/net/qeth_l3_sys.c
>> @@ -409,21 +409,22 @@ static ssize_t qeth_l3_dev_ipato_add4_show(struct device *dev,
>>  static int qeth_l3_parse_ipatoe(const char *buf, enum qeth_prot_versions proto,
>>  		  u8 *addr, int *mask_bits)
>>  {
>> -	const char *start, *end;
>> -	char *tmp;
>> -	char buffer[40] = {0, };
>> +	const char *start;
>> +	char *sep, *tmp;
>> +	int rc;
>>
>> -	start = buf;
>> -	/* get address string */
>> -	end = strchr(start, '/');
>> -	if (!end || (end - start >= 40)) {
>> +	/* Expected input pattern: %addr/%mask */
>> +	sep = strnchr(buf, 40, '/');
>> +	if (!sep)
>>  		return -EINVAL;
>> -	}
>> -	strncpy(buffer, start, end - start);
>> -	if (qeth_l3_string_to_ipaddr(buffer, proto, addr)) {
>> -		return -EINVAL;
>> -	}
>> -	start = end + 1;
>> +
>> +	/* Terminate the %addr sub-string, and parse it: */
>> +	*sep = '\0';
> 
> Is it valid to write into the input buffer here?
> 

It's a private buffer that was handed to us by the kernfs write code.

>> +	rc = qeth_l3_string_to_ipaddr(buf, proto, addr);
>> +	if (rc)
>> +		return rc;
>> +
>> +	start = sep + 1;
>>  	*mask_bits = simple_strtoul(start, &tmp, 10);
> 
> The use of strnchr() rather implies that the input
> buffer may not be '\0' terminated.

It's a kernfs write buffer, so guaranteed to be terminated.

> If that is true then you've just run off the end of the
> input buffer.
> 
>>  	if (!strlen(start) ||
>>  	    (tmp == start) ||
> 
> Hmmm... delete the strlen() clause.
> It ought to test start[0], but the 'tmp == start' test
> covers that case.
> 

See the next patch in this series, all this goes away.

> I don't understand why simple_strtoul() is deprecated.
> I don't recall any of the replacements returning the
> address of the terminating character.
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 

b

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/9] s390/qeth: updates 2020-09-23
  2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
                   ` (8 preceding siblings ...)
  2020-09-23  8:37 ` [PATCH net-next 9/9] s390/qeth: remove forward declarations in L2 code Julian Wiedmann
@ 2020-09-23 19:08 ` David Miller
  9 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2020-09-23 19:08 UTC (permalink / raw)
  To: jwi; +Cc: kuba, netdev, linux-s390, hca, ubraun, kgraul

From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Wed, 23 Sep 2020 10:36:51 +0200

> please apply the following patch series for qeth to netdev's net-next tree.
> 
> This brings all sorts of cleanups. Highlights are more code sharing in
> the init/teardown paths, and more fine-grained rollback on errors during
> initialization (instead of a full-blown teardown).

Series applied, thank you.

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-09-23 19:08 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23  8:36 [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 Julian Wiedmann
2020-09-23  8:36 ` [PATCH net-next 1/9] s390/qeth: don't init refcount twice for mcast IPs Julian Wiedmann
2020-09-23  8:36 ` [PATCH net-next 2/9] s390/qeth: relax locking for ipato config data Julian Wiedmann
2020-09-23  8:36 ` [PATCH net-next 3/9] s390/qeth: clean up string ops in qeth_l3_parse_ipatoe() Julian Wiedmann
2020-09-23  9:55   ` David Laight
2020-09-23 10:43     ` Julian Wiedmann
2020-09-23  8:36 ` [PATCH net-next 4/9] s390/qeth: replace deprecated simple_stroul() Julian Wiedmann
2020-09-23  8:36 ` [PATCH net-next 5/9] s390/qeth: tighten ucast IP locking Julian Wiedmann
2020-09-23  8:36 ` [PATCH net-next 6/9] s390/qeth: cancel cmds earlier during teardown Julian Wiedmann
2020-09-23  8:36 ` [PATCH net-next 7/9] s390/qeth: consolidate online code Julian Wiedmann
2020-09-23  8:36 ` [PATCH net-next 8/9] s390/qeth: consolidate teardown code Julian Wiedmann
2020-09-23  8:37 ` [PATCH net-next 9/9] s390/qeth: remove forward declarations in L2 code Julian Wiedmann
2020-09-23 19:08 ` [PATCH net-next 0/9] s390/qeth: updates 2020-09-23 David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.