Skip to contentSkip to conversation

s390/qeth: updates 2018-07-19

PATCH In review Julian Wiedmann 2 participants 13 messages Jul 19, 2018 – Jul 21, 2018

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover s390/qeth: updates 2018-07-19

Hi Dave,

please apply one more round of qeth patches to net-next. This brings additional performance improvements for the transmit code, and some refactoring to pave the way for using netdev_priv. Also, two minor fixes for rare corner cases.

Thanks,
Julian

Julian Wiedmann (11):
s390/qeth: fix race in used-buffer accounting s390/qeth: reset layer2 attribute on layer switch s390/qeth: remove redundant netif_carrier_ok() checks s390/qeth: allocate netdevice early
s390/qeth: don't cache HW port number
s390/qeth: simplify max MTU handling
s390/qeth: use core MTU range checking
s390/qeth: add statistics for consumed buffer elements s390/qeth: merge linearize-check into HW header construction s390/qeth: add support for constrained HW headers s390/qeth: speed up L2 IQD xmit

 drivers/s390/net/qeth_core.h      |  11 +-
 drivers/s390/net/qeth_core_main.c | 283 ++++++++++++++++++++++++--------------
 drivers/s390/net/qeth_core_mpc.h  |   1 +
 drivers/s390/net/qeth_core_sys.c  |  18 ++-
 drivers/s390/net/qeth_l2_main.c   | 176 +++++++-----------------
 drivers/s390/net/qeth_l3_main.c   | 112 +++++----------
 drivers/s390/net/qeth_l3_sys.c    |   6 +-
 7 files changed, 287 insertions(+), 320 deletions(-)
Signature · 1 line
2.16.4
#1 lore raw

PATCH 1/11 s390/qeth: fix race in used-buffer accounting

By updating q->used_buffers only _after_ do_QDIO() has completed, there is a potential race against the buffer's TX completion. In the unlikely case that the TX completion path wins, qeth_qdio_output_handler() would decrement the counter before qeth_flush_buffers() even incremented it.

---
 drivers/s390/net/qeth_core_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/s390/net/qeth_core_main.c +2 -1
index d80972b9bfc7..f7ddd6455638 100644
@@ -3506,13 +3506,14 @@ static void qeth_flush_buffers(struct qeth_qdio_out_q *queue, int index,
 	qdio_flags = QDIO_FLAG_SYNC_OUTPUT;
 	if (atomic_read(&queue->set_pci_flags_count))
 		qdio_flags |= QDIO_FLAG_PCI_OUT;
+	atomic_add(count, &queue->used_buffers);
+
 	rc = do_QDIO(CARD_DDEV(queue->card), qdio_flags,
 		     queue->queue_no, index, count);
 	if (queue->card->options.performance_stats)
 		queue->card->perf_stats.outbound_do_qdio_time +=
 			qeth_get_micros() -
 			queue->card->perf_stats.outbound_do_qdio_start_time;
-	atomic_add(count, &queue->used_buffers);
 	if (rc) {
 		queue->card->stats.tx_errors += count;
 		/* ignore temporary SIGA errors without busy condition */
Signature · 1 line
2.16.4
#2 lore raw

PATCH 2/11 s390/qeth: reset layer2 attribute on layer switch

In reply to Julian Wiedmann

After the subdriver's remove() routine has completed, the card's layer mode is undetermined again. Reflect this in the layer2 field.

If qeth_dev_layer2_store() hits an error after remove() was called, the card _always_ requires a setup(), even if the previous layer mode is requested again.
But qeth_dev_layer2_store() bails out early if the requested layer mode still matches the current one. So unless we reset the layer2 field, re-probing the card back to its previous mode is currently not possible.

---
 drivers/s390/net/qeth_core_sys.c | 1 +
 1 file changed, 1 insertion(+)
drivers/s390/net/qeth_core_sys.c +1 -0
index c3f18afb368b..cfb659747693 100644
@@ -426,6 +426,7 @@ static ssize_t qeth_dev_layer2_store(struct device *dev,
 	if (card->discipline) {
 		card->discipline->remove(card->gdev);
 		qeth_core_free_discipline(card);
+		card->options.layer2 = -1;
 	}
 
 	rc = qeth_core_load_discipline(card, newdis);
Signature · 1 line
2.16.4
#3 lore raw

PATCH 3/11 s390/qeth: remove redundant netif_carrier_ok() checks

In reply to Julian Wiedmann

netif_carrier_off() does its own checking.

---
 drivers/s390/net/qeth_core_main.c | 2 +-
 drivers/s390/net/qeth_l2_main.c   | 2 +-
 drivers/s390/net/qeth_l3_main.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/s390/net/qeth_core_main.c +1 -1
index f7ddd6455638..7c3b643550f6 100644
@@ -653,7 +653,7 @@ static struct qeth_ipa_cmd *qeth_check_ipa_data(struct qeth_card *card,
 						cmd->hdr.return_code, card);
 				}
 				card->lan_online = 0;
-				if (card->dev && netif_carrier_ok(card->dev))
+				if (card->dev)
 					netif_carrier_off(card->dev);
 				return NULL;
 			case IPA_CMD_STARTLAN:
drivers/s390/net/qeth_l2_main.c +1 -1
index 8ac243de7a9e..089bde458fd5 100644
@@ -1163,7 +1163,7 @@ static int __qeth_l2_set_offline(struct ccwgroup_device *cgdev,
 	QETH_DBF_TEXT(SETUP, 3, "setoffl");
 	QETH_DBF_HEX(SETUP, 3, &card, sizeof(void *));
 
-	if (card->dev && netif_carrier_ok(card->dev))
+	if (card->dev)
 		netif_carrier_off(card->dev);
 	recover_flag = card->state;
 	if ((!recovery_mode && card->info.hwtrap) || card->info.hwtrap == 2) {
drivers/s390/net/qeth_l3_main.c +1 -1
index 062f62b49294..ee99af08b2c4 100644
@@ -2773,7 +2773,7 @@ static int __qeth_l3_set_offline(struct ccwgroup_device *cgdev,
 	QETH_DBF_TEXT(SETUP, 3, "setoffl");
 	QETH_DBF_HEX(SETUP, 3, &card, sizeof(void *));
 
-	if (card->dev && netif_carrier_ok(card->dev))
+	if (card->dev)
 		netif_carrier_off(card->dev);
 	recover_flag = card->state;
 	if ((!recovery_mode && card->info.hwtrap) || card->info.hwtrap == 2) {
Signature · 1 line
2.16.4
#4 lore raw

PATCH 4/11 s390/qeth: allocate netdevice early

In reply to Julian Wiedmann

Allocation of the netdevice is currently delayed until a qeth card first goes online. This complicates matters in several places, where we need to cache values instead of applying them straight to the netdevice.

Improve on this by moving the allocation up to where the qeth card itself is created. This is also one step in direction of eventually placing the qeth card into netdev_priv().

In all subsequent code, remove the now redundant checks whether card->dev is valid.

---
 drivers/s390/net/qeth_core.h      |  1 +
 drivers/s390/net/qeth_core_main.c | 61 ++++++++++++++++++++++++++++++++-------
 drivers/s390/net/qeth_core_sys.c  | 14 +++++++--
 drivers/s390/net/qeth_l2_main.c   | 52 +++++++++------------------------
 drivers/s390/net/qeth_l3_main.c   | 48 ++++++++++--------------------
 drivers/s390/net/qeth_l3_sys.c    |  6 ++--
 6 files changed, 94 insertions(+), 88 deletions(-)
drivers/s390/net/qeth_core.h +1 -0
index a932aac62d0e..4d6827c8aba4 100644
@@ -966,6 +966,7 @@ extern struct qeth_card_list_struct qeth_core_card_list;
 extern struct kmem_cache *qeth_core_header_cache;
 extern struct qeth_dbf_info qeth_dbf[QETH_DBF_INFOS];
 
+struct net_device *qeth_clone_netdev(struct net_device *orig);
 void qeth_set_recovery_task(struct qeth_card *);
 void qeth_clear_recovery_task(struct qeth_card *);
 void qeth_set_allowed_threads(struct qeth_card *, unsigned long , int);
drivers/s390/net/qeth_core_main.c +50 -11

118 lines not on this page · raw message

drivers/s390/net/qeth_core_sys.c +12 -2
index cfb659747693..9bef19ed7e04 100644
@@ -144,8 +144,7 @@ static ssize_t qeth_dev_portno_store(struct device *dev,
 		goto out;
 	}
 	card->info.portno = portno;
-	if (card->dev)
-		card->dev->dev_port = portno;
+	card->dev->dev_port = portno;
 out:
 	mutex_unlock(&card->conf_mutex);
 	return rc ? rc : count;
@@ -388,6 +387,7 @@ static ssize_t qeth_dev_layer2_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct qeth_card *card = dev_get_drvdata(dev);
+	struct net_device *ndev;
 	char *tmp;
 	int i, rc = 0;
 	enum qeth_discipline_id newdis;
@@ -424,9 +424,19 @@ static ssize_t qeth_dev_layer2_store(struct device *dev,
 
 	card->info.mac_bits = 0;
 	if (card->discipline) {
+		/* start with a new, pristine netdevice: */
+		ndev = qeth_clone_netdev(card->dev);
+		if (!ndev) {
+			rc = -ENOMEM;
+			goto out;
+		}
+
 		card->discipline->remove(card->gdev);
 		qeth_core_free_discipline(card);
 		card->options.layer2 = -1;
+
+		free_netdev(card->dev);
+		card->dev = ndev;
 	}
 
 	rc = qeth_core_load_discipline(card, newdis);
drivers/s390/net/qeth_l2_main.c +13 -39

101 lines not on this page · raw message

drivers/s390/net/qeth_l3_main.c +16 -32

117 lines not on this page · raw message

drivers/s390/net/qeth_l3_sys.c +2 -4
index f61192a048f4..45ac6d8705c6 100644
@@ -299,8 +299,7 @@ static ssize_t qeth_l3_dev_hsuid_store(struct device *dev,
 	if (strlen(tmp) == 0) {
 		/* delete ip address only */
 		card->options.hsuid[0] = '\0';
-		if (card->dev)
-			memcpy(card->dev->perm_addr, card->options.hsuid, 9);
+		memcpy(card->dev->perm_addr, card->options.hsuid, 9);
 		qeth_configure_cq(card, QETH_CQ_DISABLED);
 		return count;
 	}
@@ -311,8 +310,7 @@ static ssize_t qeth_l3_dev_hsuid_store(struct device *dev,
 	snprintf(card->options.hsuid, sizeof(card->options.hsuid),
 		 "%-8s", tmp);
 	ASCEBC(card->options.hsuid, 8);
-	if (card->dev)
-		memcpy(card->dev->perm_addr, card->options.hsuid, 9);
+	memcpy(card->dev->perm_addr, card->options.hsuid, 9);
 
 	rc = qeth_l3_modify_hsuid(card, true);
 
Signature · 1 line
2.16.4
#5 lore raw

PATCH 5/11 s390/qeth: don't cache HW port number

In reply to Julian Wiedmann

The netdevice is always available now, so get the portno from there.

---
 drivers/s390/net/qeth_core.h      | 1 -
 drivers/s390/net/qeth_core_main.c | 7 +++----
 drivers/s390/net/qeth_core_sys.c  | 3 +--
 3 files changed, 4 insertions(+), 7 deletions(-)
drivers/s390/net/qeth_core.h +0 -1
index 4d6827c8aba4..04b900c7060d 100644
@@ -658,7 +658,6 @@ struct qeth_card_info {
 	char mcl_level[QETH_MCL_LENGTH + 1];
 	int guestlan;
 	int mac_bits;
-	int portno;
 	enum qeth_card_types type;
 	enum qeth_link_types link_type;
 	int initial_mtu;
drivers/s390/net/qeth_core_main.c +3 -4
index 6df2226417f1..39c429787c4d 100644
@@ -1920,7 +1920,7 @@ static int qeth_idx_activate_channel(struct qeth_channel *channel,
 		memcpy(QETH_TRANSPORT_HEADER_SEQ_NO(iob->data),
 		       &card->seqno.trans_hdr, QETH_SEQ_NO_LENGTH);
 	}
-	tmp = ((__u8)card->info.portno) | 0x80;
+	tmp = ((u8)card->dev->dev_port) | 0x80;
 	memcpy(QETH_IDX_ACT_PNO(iob->data), &tmp, 1);
 	memcpy(QETH_IDX_ACT_ISSUER_RM_TOKEN(iob->data),
 	       &card->token.issuer_rm_w, QETH_MPC_TOKEN_LENGTH);
@@ -2389,8 +2389,7 @@ static int qeth_ulp_enable(struct qeth_card *card)
 	iob = qeth_wait_for_buffer(&card->write);
 	memcpy(iob->data, ULP_ENABLE, ULP_ENABLE_SIZE);
 
-	*(QETH_ULP_ENABLE_LINKNUM(iob->data)) =
-		(__u8) card->info.portno;
+	*(QETH_ULP_ENABLE_LINKNUM(iob->data)) = (u8) card->dev->dev_port;
 	if (card->options.layer2)
 		if (card->info.type == QETH_CARD_TYPE_OSN)
 			prot_type = QETH_PROT_OSN2;
@@ -2918,7 +2917,7 @@ static void qeth_fill_ipacmd_header(struct qeth_card *card,
 	cmd->hdr.initiator = IPA_CMD_INITIATOR_HOST;
 	/* cmd->hdr.seqno is set by qeth_send_control_data() */
 	cmd->hdr.adapter_type = qeth_get_ipa_adp_type(card->info.link_type);
-	cmd->hdr.rel_adapter_no = (__u8) card->info.portno;
+	cmd->hdr.rel_adapter_no = (u8) card->dev->dev_port;
 	if (card->options.layer2)
 		cmd->hdr.prim_version_no = 2;
 	else
drivers/s390/net/qeth_core_sys.c +1 -2
index 9bef19ed7e04..25d0be25bcb3 100644
@@ -112,7 +112,7 @@ static ssize_t qeth_dev_portno_show(struct device *dev,
 	if (!card)
 		return -EINVAL;
 
-	return sprintf(buf, "%i\n", card->info.portno);
+	return sprintf(buf, "%i\n", card->dev->dev_port);
 }
 
 static ssize_t qeth_dev_portno_store(struct device *dev,
@@ -143,7 +143,6 @@ static ssize_t qeth_dev_portno_store(struct device *dev,
 		rc = -EINVAL;
 		goto out;
 	}
-	card->info.portno = portno;
 	card->dev->dev_port = portno;
 out:
 	mutex_unlock(&card->conf_mutex);
Signature · 1 line
2.16.4
#6 lore raw

PATCH 6/11 s390/qeth: simplify max MTU handling

In reply to Julian Wiedmann

When the MPC initialization code discovers the HW-specific max MTU, apply the resulting changes straight to the netdevice.

If this is the device's first initialization, also set its MTU (HiperSockets: the max MTU; else: a layer-specific default value). Then cap the current MTU by the new max MTU.

---
 drivers/s390/net/qeth_core.h      |  2 -
 drivers/s390/net/qeth_core_main.c | 82 +++++++++++++++++++++------------------
 drivers/s390/net/qeth_l2_main.c   |  1 -
 drivers/s390/net/qeth_l3_main.c   |  1 -
 4 files changed, 45 insertions(+), 41 deletions(-)
drivers/s390/net/qeth_core.h +0 -2
index 04b900c7060d..6f02a6cbe59e 100644
@@ -660,8 +660,6 @@ struct qeth_card_info {
 	int mac_bits;
 	enum qeth_card_types type;
 	enum qeth_link_types link_type;
-	int initial_mtu;
-	int max_mtu;
 	int broadcast_capable;
 	int unique_id;
 	bool layer_enforced;
drivers/s390/net/qeth_core_main.c +45 -37

122 lines not on this page · raw message

drivers/s390/net/qeth_l2_main.c +0 -1
index 8392fc7e809c..00d8bb1d2a41 100644
@@ -944,7 +944,6 @@ static int qeth_l2_setup_netdev(struct qeth_card *card)
 		return 0;
 
 	card->dev->priv_flags |= IFF_UNICAST_FLT;
-	card->dev->mtu = card->info.initial_mtu;
 	card->dev->netdev_ops = &qeth_l2_netdev_ops;
 	if (card->info.type == QETH_CARD_TYPE_OSN) {
 		card->dev->ethtool_ops = &qeth_l2_osn_ops;
drivers/s390/net/qeth_l3_main.c +0 -1
index 72c8f323801b..e5fa8e5ac1b3 100644
@@ -2579,7 +2579,6 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 	} else
 		return -ENODEV;
 
-	card->dev->mtu = card->info.initial_mtu;
 	card->dev->ethtool_ops = &qeth_l3_ethtool_ops;
 	card->dev->priv_flags &= ~IFF_TX_SKB_SHARING;
 	card->dev->needed_headroom = sizeof(struct qeth_hdr) - ETH_HLEN;
Signature · 1 line
2.16.4
#7 lore raw

PATCH 7/11 s390/qeth: use core MTU range checking

In reply to Julian Wiedmann

qeth's ndo_change_mtu() only applies some trivial bounds checking. Set up dev->min_mtu properly, so that dev_set_mtu() can do this for us.

---
 drivers/s390/net/qeth_core.h      |  1 -
 drivers/s390/net/qeth_core_main.c | 34 +---------------------------------
 drivers/s390/net/qeth_core_mpc.h  |  1 +
 drivers/s390/net/qeth_l2_main.c   |  1 -
 drivers/s390/net/qeth_l3_main.c   |  2 --
 5 files changed, 2 insertions(+), 37 deletions(-)
drivers/s390/net/qeth_core.h +0 -1
index 6f02a6cbe59e..994ac7f434d5 100644
@@ -993,7 +993,6 @@ void qeth_clear_cmd_buffers(struct qeth_channel *);
 void qeth_clear_qdio_buffers(struct qeth_card *);
 void qeth_setadp_promisc_mode(struct qeth_card *);
 struct net_device_stats *qeth_get_stats(struct net_device *);
-int qeth_change_mtu(struct net_device *, int);
 int qeth_setadpparms_change_macaddr(struct qeth_card *);
 void qeth_tx_timeout(struct net_device *);
 void qeth_prepare_control_data(struct qeth_card *, int,
drivers/s390/net/qeth_core_main.c +1 -33
index ab3d63f98779..717511c167e7 100644
@@ -2332,20 +2332,6 @@ static int qeth_get_mtu_outof_framesize(int framesize)
 	}
 }
 
-static int qeth_mtu_is_valid(struct qeth_card *card, int mtu)
-{
-	switch (card->info.type) {
-	case QETH_CARD_TYPE_OSD:
-	case QETH_CARD_TYPE_OSM:
-	case QETH_CARD_TYPE_OSX:
-	case QETH_CARD_TYPE_IQD:
-		return ((mtu >= 576) && (mtu <= card->dev->max_mtu));
-	case QETH_CARD_TYPE_OSN:
-	default:
-		return 1;
-	}
-}
-
 static int qeth_ulp_enable_cb(struct qeth_card *card, struct qeth_reply *reply,
 		unsigned long data)
 {
@@ -4204,24 +4190,6 @@ void qeth_setadp_promisc_mode(struct qeth_card *card)
 }
 EXPORT_SYMBOL_GPL(qeth_setadp_promisc_mode);
 
-int qeth_change_mtu(struct net_device *dev, int new_mtu)
-{
-	struct qeth_card *card;
-	char dbf_text[15];
-
-	card = dev->ml_priv;
-
-	QETH_CARD_TEXT(card, 4, "chgmtu");
-	sprintf(dbf_text, "%8x", new_mtu);
-	QETH_CARD_TEXT(card, 4, dbf_text);
-
-	if (!qeth_mtu_is_valid(card, new_mtu))
-		return -EINVAL;
-	dev->mtu = new_mtu;
-	return 0;
-}
-EXPORT_SYMBOL_GPL(qeth_change_mtu);
-
 struct net_device_stats *qeth_get_stats(struct net_device *dev)
 {
 	struct qeth_card *card;
@@ -5696,7 +5664,7 @@ static struct net_device *qeth_alloc_netdev(struct qeth_card *card)
 
 	dev->ml_priv = card;
 	dev->watchdog_timeo = QETH_TX_TIMEOUT;
-	dev->min_mtu = 64;
+	dev->min_mtu = IS_OSN(card) ? 64 : 576;
 	 /* initialized when device first goes online: */
 	dev->max_mtu = 0;
 	dev->mtu = 0;
drivers/s390/net/qeth_core_mpc.h +1 -0
index 54c35224262a..cf5ad94e960a 100644
@@ -65,6 +65,7 @@ enum qeth_card_types {
 };
 
 #define IS_IQD(card)	((card)->info.type == QETH_CARD_TYPE_IQD)
+#define IS_OSN(card)	((card)->info.type == QETH_CARD_TYPE_OSN)
 
 #define QETH_MPC_DIFINFO_LEN_INDICATES_LINK_TYPE 0x18
 /* only the first two bytes are looked at in qeth_get_cardname_short */
drivers/s390/net/qeth_l2_main.c +0 -1
index 00d8bb1d2a41..668f80680575 100644
@@ -928,7 +928,6 @@ static const struct net_device_ops qeth_l2_netdev_ops = {
 	.ndo_set_rx_mode	= qeth_l2_set_rx_mode,
 	.ndo_do_ioctl		= qeth_do_ioctl,
 	.ndo_set_mac_address    = qeth_l2_set_mac_address,
-	.ndo_change_mtu	   	= qeth_change_mtu,
 	.ndo_vlan_rx_add_vid	= qeth_l2_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid   = qeth_l2_vlan_rx_kill_vid,
 	.ndo_tx_timeout	   	= qeth_tx_timeout,
drivers/s390/net/qeth_l3_main.c +0 -2
index e5fa8e5ac1b3..078b891a6d24 100644
@@ -2506,7 +2506,6 @@ static const struct net_device_ops qeth_l3_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_rx_mode	= qeth_l3_set_rx_mode,
 	.ndo_do_ioctl		= qeth_do_ioctl,
-	.ndo_change_mtu		= qeth_change_mtu,
 	.ndo_fix_features	= qeth_fix_features,
 	.ndo_set_features	= qeth_set_features,
 	.ndo_vlan_rx_add_vid	= qeth_l3_vlan_rx_add_vid,
@@ -2523,7 +2522,6 @@ static const struct net_device_ops qeth_l3_osa_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_rx_mode	= qeth_l3_set_rx_mode,
 	.ndo_do_ioctl		= qeth_do_ioctl,
-	.ndo_change_mtu		= qeth_change_mtu,
 	.ndo_fix_features	= qeth_fix_features,
 	.ndo_set_features	= qeth_set_features,
 	.ndo_vlan_rx_add_vid	= qeth_l3_vlan_rx_add_vid,
Signature · 1 line
2.16.4
#8 lore raw

PATCH 8/11 s390/qeth: add statistics for consumed buffer elements

In reply to Julian Wiedmann

Nowadays an skb fragment typically spans over multiple pages. So replace the obsolete, SG-only 'fragments' counter with one that tracks the consumed buffer elements. This is what actually matters for performance.

---
 drivers/s390/net/qeth_core.h      |  2 +-
 drivers/s390/net/qeth_core_main.c |  4 ++--
 drivers/s390/net/qeth_l2_main.c   | 13 +++++++------
 drivers/s390/net/qeth_l3_main.c   | 28 ++++++++++++++--------------
 4 files changed, 24 insertions(+), 23 deletions(-)
drivers/s390/net/qeth_core.h +1 -1
index 994ac7f434d5..6d8005af67f5 100644
@@ -104,6 +104,7 @@ struct qeth_dbf_info {
 struct qeth_perf_stats {
 	unsigned int bufs_rec;
 	unsigned int bufs_sent;
+	unsigned int buf_elements_sent;
 
 	unsigned int skbs_sent_pack;
 	unsigned int bufs_sent_pack;
@@ -137,7 +138,6 @@ struct qeth_perf_stats {
 	unsigned int large_send_bytes;
 	unsigned int large_send_cnt;
 	unsigned int sg_skbs_sent;
-	unsigned int sg_frags_sent;
 	/* initial values when measuring starts */
 	unsigned long initial_rx_packets;
 	unsigned long initial_tx_packets;
drivers/s390/net/qeth_core_main.c +2 -2
index 717511c167e7..84f1e1e33f3f 100644
@@ -5970,7 +5970,7 @@ static struct {
 	{"tx skbs packing"},
 	{"tx buffers packing"},
 	{"tx sg skbs"},
-	{"tx sg frags"},
+	{"tx buffer elements"},
 /* 10 */{"rx sg skbs"},
 	{"rx sg frags"},
 	{"rx sg page allocs"},
@@ -6029,7 +6029,7 @@ void qeth_core_get_ethtool_stats(struct net_device *dev,
 	data[6] = card->perf_stats.skbs_sent_pack;
 	data[7] = card->perf_stats.bufs_sent_pack;
 	data[8] = card->perf_stats.sg_skbs_sent;
-	data[9] = card->perf_stats.sg_frags_sent;
+	data[9] = card->perf_stats.buf_elements_sent;
 	data[10] = card->perf_stats.sg_skbs_rx;
 	data[11] = card->perf_stats.sg_frags_rx;
 	data[12] = card->perf_stats.sg_alloc_page_rx;
drivers/s390/net/qeth_l2_main.c +7 -6
index 668f80680575..a785c5ff73cd 100644
@@ -672,10 +672,11 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
 			    int ipv)
 {
 	int push_len = sizeof(struct qeth_hdr);
-	unsigned int elements, nr_frags;
 	unsigned int hdr_elements = 0;
 	struct qeth_hdr *hdr = NULL;
 	unsigned int hd_len = 0;
+	unsigned int elements;
+	bool is_sg;
 	int rc;
 
 	/* fix hardware limitation: as long as we do not have sbal
@@ -693,7 +694,6 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
 		if (rc)
 			return rc;
 	}
-	nr_frags = skb_shinfo(skb)->nr_frags;
 
 	rc = skb_cow_head(skb, push_len);
 	if (rc)
@@ -720,15 +720,16 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
 	}
 	elements += hdr_elements;
 
+	is_sg = skb_is_nonlinear(skb);
 	/* TODO: remove the skb_orphan() once TX completion is fast enough */
 	skb_orphan(skb);
 	rc = qeth_do_send_packet(card, queue, skb, hdr, 0, hd_len, elements);
 out:
 	if (!rc) {
-		if (card->options.performance_stats && nr_frags) {
-			card->perf_stats.sg_skbs_sent++;
-			/* nr_frags + skb->data */
-			card->perf_stats.sg_frags_sent += nr_frags + 1;
+		if (card->options.performance_stats) {
+			card->perf_stats.buf_elements_sent += elements;
+			if (is_sg)
+				card->perf_stats.sg_skbs_sent++;
 		}
 	} else {
 		if (hd_len)
drivers/s390/net/qeth_l3_main.c +14 -14
index 078b891a6d24..c12aeb7d8f26 100644
@@ -2166,12 +2166,13 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 				int cast_type)
 {
 	const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
-	unsigned int frame_len, nr_frags;
 	unsigned char eth_hdr[ETH_HLEN];
 	unsigned int hdr_elements = 0;
 	struct qeth_hdr *hdr = NULL;
 	int elements, push_len, rc;
 	unsigned int hd_len = 0;
+	unsigned int frame_len;
+	bool is_sg;
 
 	/* compress skb to fit into one IO buffer: */
 	if (!qeth_get_elements_no(card, skb, 0, 0)) {
@@ -2194,7 +2195,6 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 	skb_copy_from_linear_data(skb, eth_hdr, ETH_HLEN);
 	skb_pull(skb, ETH_HLEN);
 	frame_len = skb->len;
-	nr_frags = skb_shinfo(skb)->nr_frags;
 
 	push_len = qeth_push_hdr(skb, &hdr, hw_hdr_len);
 	if (push_len < 0)
@@ -2217,6 +2217,7 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 	else
 		qeth_l3_fill_header(card, hdr, skb, ipv, cast_type, frame_len);
 
+	is_sg = skb_is_nonlinear(skb);
 	if (IS_IQD(card)) {
 		rc = qeth_do_send_packet_fast(queue, skb, hdr, 0, hd_len);
 	} else {
@@ -2227,10 +2228,10 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 	}
 out:
 	if (!rc) {
-		if (card->options.performance_stats && nr_frags) {
-			card->perf_stats.sg_skbs_sent++;
-			/* nr_frags + skb->data */
-			card->perf_stats.sg_frags_sent += nr_frags + 1;
+		if (card->options.performance_stats) {
+			card->perf_stats.buf_elements_sent += elements;
+			if (is_sg)
+				card->perf_stats.sg_skbs_sent++;
 		}
 	} else {
 		if (!push_len)
@@ -2248,14 +2249,14 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 static int qeth_l3_xmit(struct qeth_card *card, struct sk_buff *skb,
 			struct qeth_qdio_out_q *queue, int ipv, int cast_type)
 {
-	unsigned int hd_len, nr_frags;
 	int elements, len, rc;
 	__be16 *tag;
 	struct qeth_hdr *hdr = NULL;
 	int hdr_elements = 0;
 	struct sk_buff *new_skb = NULL;
 	int tx_bytes = skb->len;
-	bool use_tso;
+	unsigned int hd_len;
+	bool use_tso, is_sg;
 
 	/* Ignore segment size from skb_is_gso(), 1 page is always used. */
 	use_tso = skb_is_gso(skb) &&
@@ -2297,7 +2298,6 @@ static int qeth_l3_xmit(struct qeth_card *card, struct sk_buff *skb,
 		if (rc)
 			goto out;
 	}
-	nr_frags = skb_shinfo(new_skb)->nr_frags;
 
 	if (use_tso) {
 		hdr = skb_push(new_skb, sizeof(struct qeth_hdr_tso));
@@ -2334,6 +2334,8 @@ static int qeth_l3_xmit(struct qeth_card *card, struct sk_buff *skb,
 		rc = -EINVAL;
 		goto out;
 	}
+
+	is_sg = skb_is_nonlinear(new_skb);
 	rc = qeth_do_send_packet(card, queue, new_skb, hdr, hd_len, hd_len,
 				 elements);
 out:
@@ -2341,15 +2343,13 @@ static int qeth_l3_xmit(struct qeth_card *card, struct sk_buff *skb,
 		if (new_skb != skb)
 			dev_kfree_skb_any(skb);
 		if (card->options.performance_stats) {
+			card->perf_stats.buf_elements_sent += elements;
+			if (is_sg)
+				card->perf_stats.sg_skbs_sent++;
 			if (use_tso) {
 				card->perf_stats.large_send_bytes += tx_bytes;
 				card->perf_stats.large_send_cnt++;
 			}
-			if (nr_frags) {
-				card->perf_stats.sg_skbs_sent++;
-				/* nr_frags + skb->data */
-				card->perf_stats.sg_frags_sent += nr_frags + 1;
-			}
 		}
 	} else {
 		if (new_skb != skb)
Signature · 1 line
2.16.4
#9 lore raw

PATCH 9/11 s390/qeth: merge linearize-check into HW header construction

In reply to Julian Wiedmann

When checking whether an skb needs to be linearized to fit into an IO buffer, it's desirable to consider the skb's final size and layout (ie. after the HW header was added). But a subsequent linearization can then cause the re-positioned HW header to violate its alignment restrictions.

Dealing with this situation in two different code paths is quite tricky. This patch integrates a) linearize-check and b) HW header construction into one 3 step-sequence:
1. evaluate how the HW header needs to be added (to identify if it takes up an additional buffer element), then
2. check if the required buffer elements exceed the device's limit. Linearize when necessary and re-evaluate the HW header placement.
3. Add the HW header in the best-possible way:
a) push, without taking up an additional buffer element
b) push, but consume another buffer element
c) allocate a header object from the cache.

---
 drivers/s390/net/qeth_core.h      |  4 +-
 drivers/s390/net/qeth_core_main.c | 86 ++++++++++++++++++++++++++++++++-------
 drivers/s390/net/qeth_l2_main.c   | 29 +------------
 drivers/s390/net/qeth_l3_main.c   | 31 ++------------
 4 files changed, 80 insertions(+), 70 deletions(-)
drivers/s390/net/qeth_core.h +3 -1
index 6d8005af67f5..2a5ec99643df 100644
@@ -1047,7 +1047,9 @@ netdev_features_t qeth_features_check(struct sk_buff *skb,
 				      struct net_device *dev,
 				      netdev_features_t features);
 int qeth_vm_request_mac(struct qeth_card *card);
-int qeth_push_hdr(struct sk_buff *skb, struct qeth_hdr **hdr, unsigned int len);
+int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
+		       struct qeth_hdr **hdr, unsigned int len,
+		       unsigned int *elements);
 
 /* exports for OSN */
 int qeth_osn_assist(struct net_device *, void *, int);
drivers/s390/net/qeth_core_main.c +71 -15

118 lines not on this page · raw message

drivers/s390/net/qeth_l2_main.c +2 -27
index a785c5ff73cd..905f3bb3a87c 100644
@@ -672,39 +672,21 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
 			    int ipv)
 {
 	int push_len = sizeof(struct qeth_hdr);
-	unsigned int hdr_elements = 0;
 	struct qeth_hdr *hdr = NULL;
 	unsigned int hd_len = 0;
 	unsigned int elements;
 	bool is_sg;
 	int rc;
 
-	/* fix hardware limitation: as long as we do not have sbal
-	 * chaining we can not send long frag lists
-	 */
-	if (!qeth_get_elements_no(card, skb, 0, 0)) {
-		rc = skb_linearize(skb);
-
-		if (card->options.performance_stats) {
-			if (rc)
-				card->perf_stats.tx_linfail++;
-			else
-				card->perf_stats.tx_lin++;
-		}
-		if (rc)
-			return rc;
-	}
-
 	rc = skb_cow_head(skb, push_len);
 	if (rc)
 		return rc;
-	push_len = qeth_push_hdr(skb, &hdr, push_len);
+	push_len = qeth_add_hw_header(card, skb, &hdr, push_len, &elements);
 	if (push_len < 0)
 		return push_len;
 	if (!push_len) {
 		/* hdr was allocated from cache */
 		hd_len = sizeof(*hdr);
-		hdr_elements = 1;
 	}
 	qeth_l2_fill_header(hdr, skb, cast_type, skb->len - push_len);
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
@@ -713,18 +695,11 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
 			card->perf_stats.tx_csum++;
 	}
 
-	elements = qeth_get_elements_no(card, skb, hdr_elements, 0);
-	if (!elements) {
-		rc = -E2BIG;
-		goto out;
-	}
-	elements += hdr_elements;
-
 	is_sg = skb_is_nonlinear(skb);
 	/* TODO: remove the skb_orphan() once TX completion is fast enough */
 	skb_orphan(skb);
 	rc = qeth_do_send_packet(card, queue, skb, hdr, 0, hd_len, elements);
-out:
+
 	if (!rc) {
 		if (card->options.performance_stats) {
 			card->perf_stats.buf_elements_sent += elements;
drivers/s390/net/qeth_l3_main.c +4 -27
index c12aeb7d8f26..f7bcc4853c45 100644
@@ -2166,28 +2166,13 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 				int cast_type)
 {
 	const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
+	unsigned int frame_len, elements;
 	unsigned char eth_hdr[ETH_HLEN];
-	unsigned int hdr_elements = 0;
 	struct qeth_hdr *hdr = NULL;
-	int elements, push_len, rc;
 	unsigned int hd_len = 0;
-	unsigned int frame_len;
+	int push_len, rc;
 	bool is_sg;
 
-	/* compress skb to fit into one IO buffer: */
-	if (!qeth_get_elements_no(card, skb, 0, 0)) {
-		rc = skb_linearize(skb);
-
-		if (card->options.performance_stats) {
-			if (rc)
-				card->perf_stats.tx_linfail++;
-			else
-				card->perf_stats.tx_lin++;
-		}
-		if (rc)
-			return rc;
-	}
-
 	/* re-use the L2 header area for the HW header: */
 	rc = skb_cow_head(skb, hw_hdr_len - ETH_HLEN);
 	if (rc)
@@ -2196,22 +2181,14 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 	skb_pull(skb, ETH_HLEN);
 	frame_len = skb->len;
 
-	push_len = qeth_push_hdr(skb, &hdr, hw_hdr_len);
+	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, &elements);
 	if (push_len < 0)
 		return push_len;
 	if (!push_len) {
 		/* hdr was added discontiguous from skb->data */
 		hd_len = hw_hdr_len;
-		hdr_elements = 1;
 	}
 
-	elements = qeth_get_elements_no(card, skb, hdr_elements, 0);
-	if (!elements) {
-		rc = -E2BIG;
-		goto out;
-	}
-	elements += hdr_elements;
-
 	if (skb->protocol == htons(ETH_P_AF_IUCV))
 		qeth_l3_fill_af_iucv_hdr(hdr, skb, frame_len);
 	else
@@ -2226,7 +2203,7 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 		rc = qeth_do_send_packet(card, queue, skb, hdr, 0, hd_len,
 					 elements);
 	}
-out:
+
 	if (!rc) {
 		if (card->options.performance_stats) {
 			card->perf_stats.buf_elements_sent += elements;
Signature · 1 line
2.16.4
#10 lore raw

PATCH 10/11 s390/qeth: add support for constrained HW headers

In reply to Julian Wiedmann

Some transmit modes require that the HW header is located in the same page as the initial protocol headers in skb->data. Let callers specify the size of this contiguous header range, and enforce it when building the HW header.

While at it, apply some gentle renaming to the relevant L2 code so that it matches the L3 code.

---
 drivers/s390/net/qeth_core.h      |  4 ++--
 drivers/s390/net/qeth_core_main.c | 33 +++++++++++++++++++--------------
 drivers/s390/net/qeth_l2_main.c   | 12 +++++++-----
 drivers/s390/net/qeth_l3_main.c   |  3 ++-
 4 files changed, 30 insertions(+), 22 deletions(-)
drivers/s390/net/qeth_core.h +2 -2
index 2a5ec99643df..605ec4706773 100644
@@ -1048,8 +1048,8 @@ netdev_features_t qeth_features_check(struct sk_buff *skb,
 				      netdev_features_t features);
 int qeth_vm_request_mac(struct qeth_card *card);
 int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
-		       struct qeth_hdr **hdr, unsigned int len,
-		       unsigned int *elements);
+		       struct qeth_hdr **hdr, unsigned int hdr_len,
+		       unsigned int proto_len, unsigned int *elements);
 
 /* exports for OSN */
 int qeth_osn_assist(struct net_device *, void *, int);
drivers/s390/net/qeth_core_main.c +19 -14
index e7b34624df1e..732b517369c7 100644
@@ -3895,7 +3895,9 @@ EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
  * @skb: skb that the HW header should be added to.
  * @hdr: double pointer to a qeth_hdr. When returning with >= 0,
  *	 it contains a valid pointer to a qeth_hdr.
- * @len: length of the HW header.
+ * @hdr_len: length of the HW header.
+ * @proto_len: length of protocol headers that need to be in same page as the
+ *	       HW header.
  *
  * Returns the pushed length. If the header can't be pushed on
  * (eg. because it would cross a page boundary), it is allocated from
@@ -3904,31 +3906,32 @@ EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
  * Error to create the hdr is indicated by returning with < 0.
  */
 int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
-		       struct qeth_hdr **hdr, unsigned int len,
-		       unsigned int *elements)
+		       struct qeth_hdr **hdr, unsigned int hdr_len,
+		       unsigned int proto_len, unsigned int *elements)
 {
 	const unsigned int max_elements = QETH_MAX_BUFFER_ELEMENTS(card);
+	const unsigned int contiguous = proto_len ? proto_len : 1;
 	unsigned int __elements;
 	addr_t start, end;
 	bool push_ok;
 	int rc;
 
 check_layout:
-	start = (addr_t)skb->data - len;
+	start = (addr_t)skb->data - hdr_len;
 	end = (addr_t)skb->data;
 
-	if (qeth_get_elements_for_range(start, end + 1) == 1) {
+	if (qeth_get_elements_for_range(start, end + contiguous) == 1) {
 		/* Push HW header into same page as first protocol header. */
 		push_ok = true;
 		__elements = qeth_count_elements(skb, 0);
-	} else {
+	} else if (!proto_len && qeth_get_elements_for_range(start, end) == 1) {
+		/* Push HW header into a new page. */
+		push_ok = true;
 		__elements = 1 + qeth_count_elements(skb, 0);
-		if (qeth_get_elements_for_range(start, end) == 1)
-			/* Push HW header into a new page. */
-			push_ok = true;
-		else
-			/* Use header cache. */
-			push_ok = false;
+	} else {
+		/* Use header cache, copy protocol headers up. */
+		push_ok = false;
+		__elements = 1 + qeth_count_elements(skb, proto_len);
 	}
 
 	/* Compress skb to fit into one IO buffer: */
@@ -3957,13 +3960,15 @@ int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
 	*elements = __elements;
 	/* Add the header: */
 	if (push_ok) {
-		*hdr = skb_push(skb, len);
-		return len;
+		*hdr = skb_push(skb, hdr_len);
+		return hdr_len;
 	}
 	/* fall back */
 	*hdr = kmem_cache_alloc(qeth_core_header_cache, GFP_ATOMIC);
 	if (!*hdr)
 		return -ENOMEM;
+	/* Copy protocol headers behind HW header: */
+	skb_copy_from_linear_data(skb, ((char *)*hdr) + hdr_len, proto_len);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qeth_add_hw_header);
drivers/s390/net/qeth_l2_main.c +7 -5
index 905f3bb3a87c..c302002e422f 100644
@@ -671,17 +671,19 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
 			    struct qeth_qdio_out_q *queue, int cast_type,
 			    int ipv)
 {
-	int push_len = sizeof(struct qeth_hdr);
+	const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
 	struct qeth_hdr *hdr = NULL;
 	unsigned int hd_len = 0;
 	unsigned int elements;
+	int push_len, rc;
 	bool is_sg;
-	int rc;
 
-	rc = skb_cow_head(skb, push_len);
+	rc = skb_cow_head(skb, hw_hdr_len);
 	if (rc)
 		return rc;
-	push_len = qeth_add_hw_header(card, skb, &hdr, push_len, &elements);
+
+	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, 0,
+				      &elements);
 	if (push_len < 0)
 		return push_len;
 	if (!push_len) {
@@ -707,7 +709,7 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
 				card->perf_stats.sg_skbs_sent++;
 		}
 	} else {
-		if (hd_len)
+		if (!push_len)
 			kmem_cache_free(qeth_core_header_cache, hdr);
 		if (rc == -EBUSY)
 			/* roll back to ETH header */
drivers/s390/net/qeth_l3_main.c +2 -1
index f7bcc4853c45..b8e828556cf6 100644
@@ -2181,7 +2181,8 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 	skb_pull(skb, ETH_HLEN);
 	frame_len = skb->len;
 
-	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, &elements);
+	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, 0,
+				      &elements);
 	if (push_len < 0)
 		return push_len;
 	if (!push_len) {
Signature · 1 line
2.16.4
#11 lore raw

PATCH 11/11 s390/qeth: speed up L2 IQD xmit

In reply to Julian Wiedmann

Modify the L2 OSA xmit path so that it also supports L2 IQD devices (in particular, their HW header requirements). This allows IQD devices to advertise NETIF_F_SG support, and eliminates the allocation overhead for the HW header.

---
 drivers/s390/net/qeth_core_main.c |  7 ++++
 drivers/s390/net/qeth_l2_main.c   | 76 ++++++++++++---------------------------
 drivers/s390/net/qeth_l3_main.c   |  3 --
 3 files changed, 30 insertions(+), 56 deletions(-)
drivers/s390/net/qeth_core_main.c +7 -0
index 732b517369c7..d09a7110b381 100644
@@ -5731,6 +5731,13 @@ static struct net_device *qeth_alloc_netdev(struct qeth_card *card)
 	dev->mtu = 0;
 	SET_NETDEV_DEV(dev, &card->gdev->dev);
 	netif_carrier_off(dev);
+
+	if (!IS_OSN(card)) {
+		dev->priv_flags &= ~IFF_TX_SKB_SHARING;
+		dev->hw_features |= NETIF_F_SG;
+		dev->vlan_features |= NETIF_F_SG;
+	}
+
 	return dev;
 }
 
drivers/s390/net/qeth_l2_main.c +23 -53

120 lines not on this page · raw message

drivers/s390/net/qeth_l3_main.c +0 -3
index b8e828556cf6..1833e7505aca 100644
@@ -2556,13 +2556,10 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 		return -ENODEV;
 
 	card->dev->ethtool_ops = &qeth_l3_ethtool_ops;
-	card->dev->priv_flags &= ~IFF_TX_SKB_SHARING;
 	card->dev->needed_headroom = sizeof(struct qeth_hdr) - ETH_HLEN;
 	card->dev->features |=	NETIF_F_HW_VLAN_CTAG_TX |
 				NETIF_F_HW_VLAN_CTAG_RX |
 				NETIF_F_HW_VLAN_CTAG_FILTER;
-	card->dev->hw_features |= NETIF_F_SG;
-	card->dev->vlan_features |= NETIF_F_SG;
 
 	netif_keep_dst(card->dev);
 	if (card->dev->hw_features & NETIF_F_TSO)
Signature · 1 line
2.16.4
maintainer
#12 lore raw
In reply to Julian Wiedmann

From: Julian Wiedmann <[email protected]>
Date: Thu, 19 Jul 2018 12:43:47 +0200

Quoted 4 lines

please apply one more round of qeth patches to net-next. This brings additional performance improvements for the transmit code, and some refactoring to pave the way for using netdev_priv. Also, two minor fixes for rare corner cases.

Series applied, thanks Julian.