Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 01/11] s390/qeth: fix race in used-buffer accounting
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>

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.

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

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index d80972b9bfc7..f7ddd6455638 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -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 */
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 08/11] s390/qeth: add statistics for consumed buffer elements
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>

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.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 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(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 994ac7f434d5..6d8005af67f5 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -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;
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 717511c167e7..84f1e1e33f3f 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -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;
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 668f80680575..a785c5ff73cd 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -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)
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 078b891a6d24..c12aeb7d8f26 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -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)
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 07/11] s390/qeth: use core MTU range checking
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>

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.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 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(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 6f02a6cbe59e..994ac7f434d5 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -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,
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index ab3d63f98779..717511c167e7 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -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;
diff --git a/drivers/s390/net/qeth_core_mpc.h b/drivers/s390/net/qeth_core_mpc.h
index 54c35224262a..cf5ad94e960a 100644
--- a/drivers/s390/net/qeth_core_mpc.h
+++ b/drivers/s390/net/qeth_core_mpc.h
@@ -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 */
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 00d8bb1d2a41..668f80680575 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -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,
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index e5fa8e5ac1b3..078b891a6d24 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -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,
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 05/11] s390/qeth: don't cache HW port number
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>

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

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 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(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 4d6827c8aba4..04b900c7060d 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -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;
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 6df2226417f1..39c429787c4d 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -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
diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
index 9bef19ed7e04..25d0be25bcb3 100644
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -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);
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 06/11] s390/qeth: simplify max MTU handling
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>

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.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 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(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 04b900c7060d..6f02a6cbe59e 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -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;
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 39c429787c4d..ab3d63f98779 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -2278,19 +2278,42 @@ static int qeth_cm_setup(struct qeth_card *card)
 
 }
 
-static int qeth_get_initial_mtu_for_card(struct qeth_card *card)
+static int qeth_update_max_mtu(struct qeth_card *card, unsigned int max_mtu)
 {
-	switch (card->info.type) {
-	case QETH_CARD_TYPE_IQD:
-		return card->info.max_mtu;
-	case QETH_CARD_TYPE_OSD:
-	case QETH_CARD_TYPE_OSX:
-		if (!card->options.layer2)
-			return ETH_DATA_LEN - 8; /* L3: allow for LLC + SNAP */
-		/* fall through */
-	default:
-		return ETH_DATA_LEN;
+	struct net_device *dev = card->dev;
+	unsigned int new_mtu;
+
+	if (!max_mtu) {
+		/* IQD needs accurate max MTU to set up its RX buffers: */
+		if (IS_IQD(card))
+			return -EINVAL;
+		/* tolerate quirky HW: */
+		max_mtu = ETH_MAX_MTU;
+	}
+
+	rtnl_lock();
+	if (IS_IQD(card)) {
+		/* move any device with default MTU to new max MTU: */
+		new_mtu = (dev->mtu == dev->max_mtu) ? max_mtu : dev->mtu;
+
+		/* adjust RX buffer size to new max MTU: */
+		card->qdio.in_buf_size = max_mtu + 2 * PAGE_SIZE;
+		if (dev->max_mtu && dev->max_mtu != max_mtu)
+			qeth_free_qdio_buffers(card);
+	} else {
+		if (dev->mtu)
+			new_mtu = dev->mtu;
+		/* default MTUs for first setup: */
+		else if (card->options.layer2)
+			new_mtu = ETH_DATA_LEN;
+		else
+			new_mtu = ETH_DATA_LEN - 8; /* allow for LLC + SNAP */
 	}
+
+	dev->max_mtu = max_mtu;
+	dev->mtu = min(new_mtu, max_mtu);
+	rtnl_unlock();
+	return 0;
 }
 
 static int qeth_get_mtu_outof_framesize(int framesize)
@@ -2316,8 +2339,7 @@ static int qeth_mtu_is_valid(struct qeth_card *card, int mtu)
 	case QETH_CARD_TYPE_OSM:
 	case QETH_CARD_TYPE_OSX:
 	case QETH_CARD_TYPE_IQD:
-		return ((mtu >= 576) &&
-			(mtu <= card->info.max_mtu));
+		return ((mtu >= 576) && (mtu <= card->dev->max_mtu));
 	case QETH_CARD_TYPE_OSN:
 	default:
 		return 1;
@@ -2342,28 +2364,10 @@ static int qeth_ulp_enable_cb(struct qeth_card *card, struct qeth_reply *reply,
 	if (card->info.type == QETH_CARD_TYPE_IQD) {
 		memcpy(&framesize, QETH_ULP_ENABLE_RESP_MAX_MTU(iob->data), 2);
 		mtu = qeth_get_mtu_outof_framesize(framesize);
-		if (!mtu) {
-			iob->rc = -EINVAL;
-			QETH_DBF_TEXT_(SETUP, 2, "  rc%d", iob->rc);
-			return 0;
-		}
-		if (card->info.initial_mtu && (card->info.initial_mtu != mtu)) {
-			/* frame size has changed */
-			if ((card->dev->mtu == card->info.initial_mtu) ||
-			    (card->dev->mtu > mtu))
-				card->dev->mtu = mtu;
-			qeth_free_qdio_buffers(card);
-		}
-		card->info.initial_mtu = mtu;
-		card->info.max_mtu = mtu;
-		card->qdio.in_buf_size = mtu + 2 * PAGE_SIZE;
 	} else {
-		card->info.max_mtu = *(__u16 *)QETH_ULP_ENABLE_RESP_MAX_MTU(
-			iob->data);
-		card->info.initial_mtu = min(card->info.max_mtu,
-					qeth_get_initial_mtu_for_card(card));
-		card->qdio.in_buf_size = QETH_IN_BUF_SIZE_DEFAULT;
+		mtu = *(__u16 *)QETH_ULP_ENABLE_RESP_MAX_MTU(iob->data);
 	}
+	*(u16 *)reply->param = mtu;
 
 	memcpy(&len, QETH_ULP_ENABLE_RESP_DIFINFO_LEN(iob->data), 2);
 	if (len >= QETH_MPC_DIFINFO_LEN_INDICATES_LINK_TYPE) {
@@ -2382,6 +2386,7 @@ static int qeth_ulp_enable(struct qeth_card *card)
 	int rc;
 	char prot_type;
 	struct qeth_cmd_buffer *iob;
+	u16 max_mtu;
 
 	/*FIXME: trace view callbacks*/
 	QETH_DBF_TEXT(SETUP, 2, "ulpenabl");
@@ -2404,9 +2409,10 @@ static int qeth_ulp_enable(struct qeth_card *card)
 	memcpy(QETH_ULP_ENABLE_FILTER_TOKEN(iob->data),
 	       &card->token.ulp_filter_w, QETH_MPC_TOKEN_LENGTH);
 	rc = qeth_send_control_data(card, ULP_ENABLE_SIZE, iob,
-				    qeth_ulp_enable_cb, NULL);
-	return rc;
-
+				    qeth_ulp_enable_cb, &max_mtu);
+	if (rc)
+		return rc;
+	return qeth_update_max_mtu(card, max_mtu);
 }
 
 static int qeth_ulp_setup_cb(struct qeth_card *card, struct qeth_reply *reply,
@@ -5691,7 +5697,9 @@ 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->max_mtu = ETH_MAX_MTU;
+	 /* initialized when device first goes online: */
+	dev->max_mtu = 0;
+	dev->mtu = 0;
 	SET_NETDEV_DEV(dev, &card->gdev->dev);
 	netif_carrier_off(dev);
 	return dev;
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 8392fc7e809c..00d8bb1d2a41 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -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;
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 72c8f323801b..e5fa8e5ac1b3 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -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;
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 04/11] s390/qeth: allocate netdevice early
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>

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.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 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(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index a932aac62d0e..4d6827c8aba4 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -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);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 7c3b643550f6..6df2226417f1 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -653,8 +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_off(card->dev);
+				netif_carrier_off(card->dev);
 				return NULL;
 			case IPA_CMD_STARTLAN:
 				dev_info(&card->gdev->dev,
@@ -2350,9 +2349,8 @@ static int qeth_ulp_enable_cb(struct qeth_card *card, struct qeth_reply *reply,
 		}
 		if (card->info.initial_mtu && (card->info.initial_mtu != mtu)) {
 			/* frame size has changed */
-			if (card->dev &&
-			    ((card->dev->mtu == card->info.initial_mtu) ||
-			     (card->dev->mtu > mtu)))
+			if ((card->dev->mtu == card->info.initial_mtu) ||
+			    (card->dev->mtu > mtu))
 				card->dev->mtu = mtu;
 			qeth_free_qdio_buffers(card);
 		}
@@ -3578,7 +3576,7 @@ static void qeth_qdio_start_poll(struct ccw_device *ccwdev, int queue,
 {
 	struct qeth_card *card = (struct qeth_card *)card_ptr;
 
-	if (card->dev && (card->dev->flags & IFF_UP))
+	if (card->dev->flags & IFF_UP)
 		napi_schedule(&card->napi);
 }
 
@@ -4794,9 +4792,6 @@ int qeth_vm_request_mac(struct qeth_card *card)
 
 	QETH_DBF_TEXT(SETUP, 2, "vmreqmac");
 
-	if (!card->dev)
-		return -ENODEV;
-
 	request = kzalloc(sizeof(*request), GFP_KERNEL | GFP_DMA);
 	response = kzalloc(sizeof(*response), GFP_KERNEL | GFP_DMA);
 	if (!request || !response) {
@@ -5676,6 +5671,44 @@ static void qeth_clear_dbf_list(void)
 	mutex_unlock(&qeth_dbf_list_mutex);
 }
 
+static struct net_device *qeth_alloc_netdev(struct qeth_card *card)
+{
+	struct net_device *dev;
+
+	switch (card->info.type) {
+	case QETH_CARD_TYPE_IQD:
+		dev = alloc_netdev(0, "hsi%d", NET_NAME_UNKNOWN, ether_setup);
+		break;
+	case QETH_CARD_TYPE_OSN:
+		dev = alloc_netdev(0, "osn%d", NET_NAME_UNKNOWN, ether_setup);
+		break;
+	default:
+		dev = alloc_etherdev(0);
+	}
+
+	if (!dev)
+		return NULL;
+
+	dev->ml_priv = card;
+	dev->watchdog_timeo = QETH_TX_TIMEOUT;
+	dev->min_mtu = 64;
+	dev->max_mtu = ETH_MAX_MTU;
+	SET_NETDEV_DEV(dev, &card->gdev->dev);
+	netif_carrier_off(dev);
+	return dev;
+}
+
+struct net_device *qeth_clone_netdev(struct net_device *orig)
+{
+	struct net_device *clone = qeth_alloc_netdev(orig->ml_priv);
+
+	if (!clone)
+		return NULL;
+
+	clone->dev_port = orig->dev_port;
+	return clone;
+}
+
 static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 {
 	struct qeth_card *card;
@@ -5725,6 +5758,10 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 		goto err_card;
 	}
 
+	card->dev = qeth_alloc_netdev(card);
+	if (!card->dev)
+		goto err_card;
+
 	qeth_determine_capabilities(card);
 	enforced_disc = qeth_enforce_discipline(card);
 	switch (enforced_disc) {
@@ -5735,7 +5772,7 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 		card->info.layer_enforced = true;
 		rc = qeth_core_load_discipline(card, enforced_disc);
 		if (rc)
-			goto err_card;
+			goto err_load;
 
 		gdev->dev.type = (card->info.type != QETH_CARD_TYPE_OSN)
 					? card->discipline->devtype
@@ -5753,6 +5790,8 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 
 err_disc:
 	qeth_core_free_discipline(card);
+err_load:
+	free_netdev(card->dev);
 err_card:
 	qeth_core_free_card(card);
 err_dev:
@@ -5775,10 +5814,10 @@ static void qeth_core_remove_device(struct ccwgroup_device *gdev)
 	write_lock_irqsave(&qeth_core_card_list.rwlock, flags);
 	list_del(&card->list);
 	write_unlock_irqrestore(&qeth_core_card_list.rwlock, flags);
+	free_netdev(card->dev);
 	qeth_core_free_card(card);
 	dev_set_drvdata(&gdev->dev, NULL);
 	put_device(&gdev->dev);
-	return;
 }
 
 static int qeth_core_set_online(struct ccwgroup_device *gdev)
diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
index cfb659747693..9bef19ed7e04 100644
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -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);
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 089bde458fd5..8392fc7e809c 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -899,13 +899,7 @@ static void qeth_l2_remove_device(struct ccwgroup_device *cgdev)
 
 	if (cgdev->state == CCWGROUP_ONLINE)
 		qeth_l2_set_offline(cgdev);
-
-	if (card->dev) {
-		unregister_netdev(card->dev);
-		free_netdev(card->dev);
-		card->dev = NULL;
-	}
-	return;
+	unregister_netdev(card->dev);
 }
 
 static const struct ethtool_ops qeth_l2_ethtool_ops = {
@@ -944,29 +938,13 @@ static const struct net_device_ops qeth_l2_netdev_ops = {
 
 static int qeth_l2_setup_netdev(struct qeth_card *card)
 {
-	switch (card->info.type) {
-	case QETH_CARD_TYPE_IQD:
-		card->dev = alloc_netdev(0, "hsi%d", NET_NAME_UNKNOWN,
-					 ether_setup);
-		break;
-	case QETH_CARD_TYPE_OSN:
-		card->dev = alloc_netdev(0, "osn%d", NET_NAME_UNKNOWN,
-					 ether_setup);
-		break;
-	default:
-		card->dev = alloc_etherdev(0);
-	}
+	int rc;
 
-	if (!card->dev)
-		return -ENODEV;
+	if (card->dev->netdev_ops)
+		return 0;
 
-	card->dev->ml_priv = card;
 	card->dev->priv_flags |= IFF_UNICAST_FLT;
-	card->dev->watchdog_timeo = QETH_TX_TIMEOUT;
 	card->dev->mtu = card->info.initial_mtu;
-	card->dev->min_mtu = 64;
-	card->dev->max_mtu = ETH_MAX_MTU;
-	card->dev->dev_port = card->info.portno;
 	card->dev->netdev_ops = &qeth_l2_netdev_ops;
 	if (card->info.type == QETH_CARD_TYPE_OSN) {
 		card->dev->ethtool_ops = &qeth_l2_osn_ops;
@@ -1006,12 +984,12 @@ static int qeth_l2_setup_netdev(struct qeth_card *card)
 		card->dev->vlan_features |= NETIF_F_RXCSUM;
 	}
 
-	card->info.broadcast_capable = 1;
 	qeth_l2_request_initial_mac(card);
-	SET_NETDEV_DEV(card->dev, &card->gdev->dev);
 	netif_napi_add(card->dev, &card->napi, qeth_poll, QETH_NAPI_WEIGHT);
-	netif_carrier_off(card->dev);
-	return register_netdev(card->dev);
+	rc = register_netdev(card->dev);
+	if (rc)
+		card->dev->netdev_ops = NULL;
+	return rc;
 }
 
 static int qeth_l2_start_ipassists(struct qeth_card *card)
@@ -1057,10 +1035,9 @@ static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
 		dev_info(&card->gdev->dev,
 		"The device represents a Bridge Capable Port\n");
 
-	if (!card->dev && qeth_l2_setup_netdev(card)) {
-		rc = -ENODEV;
+	rc = qeth_l2_setup_netdev(card);
+	if (rc)
 		goto out_remove;
-	}
 
 	if (card->info.type != QETH_CARD_TYPE_OSN &&
 	    !qeth_l2_send_setmac(card, card->dev->dev_addr))
@@ -1163,8 +1140,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_off(card->dev);
+	netif_carrier_off(card->dev);
 	recover_flag = card->state;
 	if ((!recovery_mode && card->info.hwtrap) || card->info.hwtrap == 2) {
 		qeth_hw_trap(card, QETH_DIAGS_TRAP_DISARM);
@@ -1237,8 +1213,7 @@ static int qeth_l2_pm_suspend(struct ccwgroup_device *gdev)
 {
 	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
 
-	if (card->dev)
-		netif_device_detach(card->dev);
+	netif_device_detach(card->dev);
 	qeth_set_allowed_threads(card, 0, 1);
 	wait_event(card->wait_q, qeth_threads_running(card, 0xffffffff) == 0);
 	if (gdev->state == CCWGROUP_OFFLINE)
@@ -1271,8 +1246,7 @@ static int qeth_l2_pm_resume(struct ccwgroup_device *gdev)
 		rc = __qeth_l2_set_online(card->gdev, 0);
 out:
 	qeth_set_allowed_threads(card, 0xffffffff, 0);
-	if (card->dev)
-		netif_device_attach(card->dev);
+	netif_device_attach(card->dev);
 	if (rc)
 		dev_warn(&card->gdev->dev, "The qeth device driver "
 			"failed to recover an error on the device\n");
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index ee99af08b2c4..72c8f323801b 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2536,6 +2536,9 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 {
 	int rc;
 
+	if (card->dev->netdev_ops)
+		return 0;
+
 	if (card->info.type == QETH_CARD_TYPE_OSD ||
 	    card->info.type == QETH_CARD_TYPE_OSX) {
 		if ((card->info.link_type == QETH_LINK_TYPE_LANE_TR) ||
@@ -2544,9 +2547,6 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 			return -ENODEV;
 		}
 
-		card->dev = alloc_etherdev(0);
-		if (!card->dev)
-			return -ENODEV;
 		card->dev->netdev_ops = &qeth_l3_osa_netdev_ops;
 
 		/*IPv6 address autoconfiguration stuff*/
@@ -2567,27 +2567,19 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 			card->dev->vlan_features |= NETIF_F_IPV6_CSUM;
 		}
 	} else if (card->info.type == QETH_CARD_TYPE_IQD) {
-		card->dev = alloc_netdev(0, "hsi%d", NET_NAME_UNKNOWN,
-					 ether_setup);
-		if (!card->dev)
-			return -ENODEV;
 		card->dev->flags |= IFF_NOARP;
 		card->dev->netdev_ops = &qeth_l3_netdev_ops;
 
 		rc = qeth_l3_iqd_read_initial_mac(card);
 		if (rc)
-			return rc;
+			goto out;
+
 		if (card->options.hsuid[0])
 			memcpy(card->dev->perm_addr, card->options.hsuid, 9);
 	} else
 		return -ENODEV;
 
-	card->dev->ml_priv = card;
-	card->dev->watchdog_timeo = QETH_TX_TIMEOUT;
 	card->dev->mtu = card->info.initial_mtu;
-	card->dev->min_mtu = 64;
-	card->dev->max_mtu = ETH_MAX_MTU;
-	card->dev->dev_port = card->info.portno;
 	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;
@@ -2602,10 +2594,12 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 		netif_set_gso_max_size(card->dev,
 				       PAGE_SIZE * (QETH_MAX_BUFFER_ELEMENTS(card) - 1));
 
-	SET_NETDEV_DEV(card->dev, &card->gdev->dev);
 	netif_napi_add(card->dev, &card->napi, qeth_poll, QETH_NAPI_WEIGHT);
-	netif_carrier_off(card->dev);
-	return register_netdev(card->dev);
+	rc = register_netdev(card->dev);
+out:
+	if (rc)
+		card->dev->netdev_ops = NULL;
+	return rc;
 }
 
 static const struct device_type qeth_l3_devtype = {
@@ -2643,15 +2637,9 @@ static void qeth_l3_remove_device(struct ccwgroup_device *cgdev)
 	if (cgdev->state == CCWGROUP_ONLINE)
 		qeth_l3_set_offline(cgdev);
 
-	if (card->dev) {
-		unregister_netdev(card->dev);
-		free_netdev(card->dev);
-		card->dev = NULL;
-	}
-
+	unregister_netdev(card->dev);
 	qeth_l3_clear_ip_htable(card, 0);
 	qeth_l3_clear_ipato_list(card);
-	return;
 }
 
 static int __qeth_l3_set_online(struct ccwgroup_device *gdev, int recovery_mode)
@@ -2673,10 +2661,9 @@ static int __qeth_l3_set_online(struct ccwgroup_device *gdev, int recovery_mode)
 		goto out_remove;
 	}
 
-	if (!card->dev && qeth_l3_setup_netdev(card)) {
-		rc = -ENODEV;
+	rc = qeth_l3_setup_netdev(card);
+	if (rc)
 		goto out_remove;
-	}
 
 	if (qeth_is_diagass_supported(card, QETH_DIAGS_CMD_TRAP)) {
 		if (card->info.hwtrap &&
@@ -2773,8 +2760,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_off(card->dev);
+	netif_carrier_off(card->dev);
 	recover_flag = card->state;
 	if ((!recovery_mode && card->info.hwtrap) || card->info.hwtrap == 2) {
 		qeth_hw_trap(card, QETH_DIAGS_TRAP_DISARM);
@@ -2842,8 +2828,7 @@ static int qeth_l3_pm_suspend(struct ccwgroup_device *gdev)
 {
 	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
 
-	if (card->dev)
-		netif_device_detach(card->dev);
+	netif_device_detach(card->dev);
 	qeth_set_allowed_threads(card, 0, 1);
 	wait_event(card->wait_q, qeth_threads_running(card, 0xffffffff) == 0);
 	if (gdev->state == CCWGROUP_OFFLINE)
@@ -2876,8 +2861,7 @@ static int qeth_l3_pm_resume(struct ccwgroup_device *gdev)
 		rc = __qeth_l3_set_online(card->gdev, 0);
 out:
 	qeth_set_allowed_threads(card, 0xffffffff, 0);
-	if (card->dev)
-		netif_device_attach(card->dev);
+	netif_device_attach(card->dev);
 	if (rc)
 		dev_warn(&card->gdev->dev, "The qeth device driver "
 			"failed to recover an error on the device\n");
diff --git a/drivers/s390/net/qeth_l3_sys.c b/drivers/s390/net/qeth_l3_sys.c
index f61192a048f4..45ac6d8705c6 100644
--- a/drivers/s390/net/qeth_l3_sys.c
+++ b/drivers/s390/net/qeth_l3_sys.c
@@ -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);
 
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 02/11] s390/qeth: reset layer2 attribute on layer switch
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>

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.

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

diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
index c3f18afb368b..cfb659747693 100644
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -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);
-- 
2.16.4

^ permalink raw reply related

* [net-next v4 3/3] net/tls: Remove redundant array allocation.
From: Vakul Garg @ 2018-07-19 15:59 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719155931.21497-1-vakul.garg@nxp.com>

In function decrypt_skb(), array allocation in case when sgout is NULL
is unnecessary. Instead, local variable sgin_arr[] can be used.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
 net/tls/tls_sw.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index e62f288fda31..c33ba3f1c408 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -703,7 +703,6 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
 	memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
 	if (!sgout) {
 		nsg = skb_cow_data(skb, 0, &unused) + 1;
-		sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
 		sgout = sgin;
 	}
 
@@ -724,9 +723,6 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
 				rxm->full_len - tls_ctx->rx.overhead_size,
 				skb, sk->sk_allocation);
 
-	if (sgin != &sgin_arr[0])
-		kfree(sgin);
-
 	return ret;
 }
 
-- 
2.13.6

^ permalink raw reply related

* [net-next v4 2/3] net/tls: Remove redundant variable assignments and wakeup
From: Vakul Garg @ 2018-07-19 15:59 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719155931.21497-1-vakul.garg@nxp.com>

In function decrypt_skb_update(), the assignment to tls receive context
variable 'decrypted' is redundant as the same is being done in function
tls_sw_recvmsg() after calling decrypt_skb_update(). Also calling callback
function to wakeup processes sleeping on socket data availability is
useless as decrypt_skb_update() is invoked from user processes only. This
patch cleans these up.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
 net/tls/tls_sw.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index a58661c624ec..e62f288fda31 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -659,7 +659,6 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
 			      struct scatterlist *sgout, bool *zc)
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
-	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
 	struct strp_msg *rxm = strp_msg(skb);
 	int err = 0;
 
@@ -679,8 +678,6 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
 	rxm->offset += tls_ctx->rx.prepend_size;
 	rxm->full_len -= tls_ctx->rx.overhead_size;
 	tls_advance_record_sn(sk, &tls_ctx->rx);
-	ctx->decrypted = true;
-	ctx->saved_data_ready(sk);
 
 	return err;
 }
-- 
2.13.6

^ permalink raw reply related

* [net-next v4 1/3] net/tls: Use socket data_ready callback on record availability
From: Vakul Garg @ 2018-07-19 15:59 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719155931.21497-1-vakul.garg@nxp.com>

On receipt of a complete tls record, use socket's saved data_ready
callback instead of state_change callback.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
 net/tls/tls_sw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 7d194c0cd6cf..a58661c624ec 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1023,7 +1023,7 @@ static void tls_queue(struct strparser *strp, struct sk_buff *skb)
 	ctx->recv_pkt = skb;
 	strp_pause(strp);
 
-	strp->sk->sk_state_change(strp->sk);
+	ctx->saved_data_ready(strp->sk);
 }
 
 static void tls_data_ready(struct sock *sk)
-- 
2.13.6

^ permalink raw reply related

* [net-next v4 0/3] net/tls: Minor code cleanup patches
From: Vakul Garg @ 2018-07-19 15:59 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg

This patch series improves tls_sw.c code by:

1) Using correct socket callback for flagging data availability.
2) Removing redundant variable assignments and wakeup callbacks.
3) Removing redundant dynamic array allocation.

The patches do not fix any functional bug. Hence "Fixes:" tag has not
been used. From patch series v3, this series v4 contains two patches
less. They will be submitted separately.

Vakul Garg (3):
  net/tls: Use socket data_ready callback on record availability
  net/tls: Remove redundant variable assignments and wakeup
  net/tls: Remove redundant array allocation.

 net/tls/tls_sw.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

-- 
2.13.6

^ permalink raw reply

* RE: [net-next v3 1/5] net/tls: Do not enable zero-copy prematurely
From: Vakul Garg @ 2018-07-19 10:32 UTC (permalink / raw)
  To: Boris Pismenny, netdev@vger.kernel.org
  Cc: aviadye@mellanox.com, davejwatson@fb.com, davem@davemloft.net
In-Reply-To: <1601b472-319e-f1c8-6a71-be883aa18bd2@mellanox.com>

Thanks for the comment.
I will take this patch out of the series.

> -----Original Message-----
> From: Boris Pismenny [mailto:borisp@mellanox.com]
> Sent: Thursday, July 19, 2018 3:58 PM
> To: Vakul Garg <vakul.garg@nxp.com>; netdev@vger.kernel.org
> Cc: aviadye@mellanox.com; davejwatson@fb.com; davem@davemloft.net
> Subject: Re: [net-next v3 1/5] net/tls: Do not enable zero-copy prematurely
> 
> Hi Vakul,
> 
> On 7/19/2018 7:16 AM, Vakul Garg wrote:
> > Zero-copy mode was left enabled even when zerocopy_from_iter() failed.
> > Set the zero-copy mode only when zerocopy_from_iter() succeeds. This
> > leads to removal of argument 'zc' of function decrypt_skb_update().
> > Function decrypt_skb_update() does not need to check whether
> > ctx->decrypted is set since it is never called if ctx->decrypted is
> > true.
> >
> 
> This patch breaks our tls_device code for the following 2 reasons:
> 1. We need to disable zerocopy if the device decrypted the record, because
> decrypted data has to be copied to user buffers.
> 2. ctx->decrypted must be checked in decrypt_skb_update, because it might
> change after calling tls_device_decrypted.

^ permalink raw reply

* Re: [net 4/8] net/mlx5e: Don't allow aRFS for encapsulated packets
From: Or Gerlitz @ 2018-07-19 10:29 UTC (permalink / raw)
  To: Eran Ben Elisha
  Cc: Saeed Mahameed, Eran Ben Elisha, David S. Miller,
	Linux Netdev List, Alexander Duyck, Tom Herbert
In-Reply-To: <CAKHjkjm8-hf5LEYZpYVGO_RhJrAXqwtiDu4Ya4ohzGez_UdxTg@mail.gmail.com>

On Thu, Jul 19, 2018 at 12:02 PM, Eran Ben Elisha
<eranlinuxmellanox@gmail.com> wrote:
> On Thu, Jul 19, 2018 at 10:50 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Thu, Jul 19, 2018 at 9:55 AM, Eran Ben Elisha
>> <eranlinuxmellanox@gmail.com> wrote:
>>> On Thu, Jul 19, 2018 at 9:23 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>> On Thu, Jul 19, 2018 at 4:26 AM, Saeed Mahameed <saeedm@mellanox.com> wrote:
>>>>> From: Eran Ben Elisha <eranbe@mellanox.com>
>>>>>
>>>>> Driver is yet to support aRFS for encapsulated packets, return early
>>>>> error in such case.
>>>>
>>>>
>>>> Eran,
>>>>
>>>> Isn't that something which is done wrong by the arfs stack code?
>>>>
>>>> If the kernel has an SKB which has encap set and an arfs steering
>>>> rule is programed into the driver, the API should include a driver neutral
>>>> description for the encap header for the HW to match, so maybe we can just do
>>>>
>>>
>>> Hi Or,
>>> This could break existing drivers support for tunneled aRFS, and hurts
>>> their RX performance dramatically..
>>
>>> IMHO, it is expected from the driver to figure out that the skb holds
>>> encap packet and act accordingly.
>>
>> I don't think this one bit indication on the skb is enough for
>> any HW driver (e.g mlx4, mlx5 and others) to properly set
>> the steering rules.
>
> why do you think it is not enough?
> mlx5e currently cannot offload tunneled packets, so this info is
> perfectly fit in order to reject.

Do we know that all the flows in stack that deals with reception of packets do
this marking for encapsulated packets? is this documented some/where? if
this is what we think, then the patch is ok.

^ permalink raw reply

* Re: [net-next v3 1/5] net/tls: Do not enable zero-copy prematurely
From: Boris Pismenny @ 2018-07-19 10:28 UTC (permalink / raw)
  To: Vakul Garg, netdev; +Cc: aviadye, davejwatson, davem
In-Reply-To: <20180719111643.12787-2-vakul.garg@nxp.com>

Hi Vakul,

On 7/19/2018 7:16 AM, Vakul Garg wrote:
> Zero-copy mode was left enabled even when zerocopy_from_iter() failed.
> Set the zero-copy mode only when zerocopy_from_iter() succeeds. This
> leads to removal of argument 'zc' of function decrypt_skb_update().
> Function decrypt_skb_update() does not need to check whether
> ctx->decrypted is set since it is never called if ctx->decrypted is
> true.
> 

This patch breaks our tls_device code for the following 2 reasons:
1. We need to disable zerocopy if the device decrypted the record, 
because decrypted data has to be copied to user buffers.
2. ctx->decrypted must be checked in decrypt_skb_update, because it 
might change after calling tls_device_decrypted.

^ permalink raw reply

* Re: [pci PATCH v8 4/4] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs
From: Sunil Kovvuri @ 2018-07-19 10:52 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Bjorn Helgaas, linux-pci, virtio-dev, kvm, Linux Netdev List,
	dan.daly, LKML, linux-nvme, keith.busch, netanel, ddutile, mheyne,
	liang-min.wang, mark.d.rustad, David Woodhouse, Christoph Hellwig,
	dwmw
In-Reply-To: <20180420163109.46077.60334.stgit@ahduyck-green-test.jf.intel.com>

Hi,

> +static struct pci_driver pf_stub_driver = {
> +       .name                   = "pci-pf-stub",
> +       .id_table               = pci_pf_stub_white_list,
> +       .probe                  = pci_pf_stub_probe,
> +       .sriov_configure        = pci_sriov_configure_simple,
> +};
> +

Is there any specific reason for not disabling SRIOV upon driver unload/remove ?

i.e if a user sets numVFs to a non-zero value and unloads this driver
then we have a
situation where there is no PF driver in kernel but VFs still exist.

Thanks,
Sunil.

^ permalink raw reply

* Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-07-19 10:06 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Bartosz Golaszewski, Sekhar Nori, Kevin Hilman, Russell King,
	Grygorii Strashko, David S . Miller, Srinivas Kandagatla,
	Lukas Wunner, Rob Herring, Florian Fainelli, Ivan Khoronzhuk,
	David Lechner, Greg Kroah-Hartman, Andrew Lunn, arm-soc, LKML,
	Linux-OMAP, netdev
In-Reply-To: <20180719090929.arbysqo2bk3esoar@mwanda>

2018-07-19 11:09 GMT+02:00 Dan Carpenter <dan.carpenter@oracle.com>:
>
> Maybe it would be simpler as three separate functions:
>
> int of_eth_get_mac_address() <- rename existing function to this
> int nvmem_eth_get_mac_address() <- patch 2
> int mtd_eth_nvmem_get_mac_address() patch 3
>
>         int ret;
>
>         ret = of_eth_get_mac_address(dev, mac_addr);
>         if (!ret)
>                 return 0;
>         ret = nvmem_eth_get_mac_address(dev, mac_addr);
>         if (ret == -EPROBEDEFER)
>                 return ret;
>         if (!ret)
>                 return 0;
>         ret = mtd_eth_nvmem_get_mac_address(dev, mac_addr);
>         if (!ret)
>                 return 0;
>
>         return -ENODEV;
>
> regards,
> dan carpenter

It looks simpler as long as you don't add all the new routines
resulting from this approach. I've just tried to quickly implement
this solution and it resulted in much bigger and duplicated code
(checking the validity of mac_addr, copying it etc.). I would prefer
the current approach and would like to read someone else's opinion on
that.

Thanks,
Bart
>

^ permalink raw reply

* Re: [PATCH net-next,v2] net: rename ndo_setup_tc to ndo_setup_offload
From: Martin Habets @ 2018-07-19  9:17 UTC (permalink / raw)
  To: Pablo Neira Ayuso, netdev
  Cc: jiri, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
	santosh, madalin.bucur, yisen.zhuang, salil.mehta,
	jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, ganeshgr,
	jakub.kicinski, linux-net-drivers, peppe.cavallaro,
	alexandre.torgue, joabreu, grygorii.strashko, andrew,
	vivien.didelot
In-Reply-To: <20180719001114.4883-1-pablo@netfilter.org>

On 19/07/18 01:11, Pablo Neira Ayuso wrote:
> One of the recurring complaints is that we do not have, as a driver
> writer, a central location from which we would be fed offloading rules
> into a NIC. This was brought up again during Netconf'18 in Boston.
> 
> This patch just renames ndo_setup_tc to ndo_setup_offload as a very
> early initial work to prepare for follow up patch that discuss unified
> flow representation for the existing offload programming APIs.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Martin Habets <mhabets@solarflare.com>

> ---
> v2: Missing function definition update in drivers/net/ethernet/sfc/falcon/tx.c
>     apparently I forgot to turn on that driver when doing compile-testing,
>     problem spotted by Martin Habets. Keeping Jakub and Jiri Acked-by tags,
>     as this is the only change in the v1 patch.
> 

^ permalink raw reply

* Re: [net-next, v6, 6/7] net-sysfs: Add interface for Rx queue(s) map per Tx queue
From: Peter Zijlstra @ 2018-07-19  9:16 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: Nambiar, Amritha, Ingo Molnar, netdev, davem, alexander.h.duyck,
	willemdebruijn.kernel, sridhar.samudrala, alexander.duyck,
	edumazet, hannes, tom, tom, jasowang, gaowanlong
In-Reply-To: <20180718182235.GA28548@outlook.office365.com>

On Wed, Jul 18, 2018 at 11:22:36AM -0700, Andrei Vagin wrote:
> > > [    1.085679]   lock(cpu_hotplug_lock.rw_sem);
> > > [    1.085753]   lock(cpu_hotplug_lock.rw_sem);
> > > [    1.085828] 
> > > [    1.085828]  *** DEADLOCK ***

> Peter and Ingo, maybe you could explain why it isn't safe to take one
> reader lock twice?

Very simple, because rwsems are fair and !recursive.

So if another CPU were to issue a write-lock in between these, then the
second would block because there is a writer pending. But because we
then already have a reader we're deadlocked.

^ permalink raw reply

* Re: [net 4/8] net/mlx5e: Don't allow aRFS for encapsulated packets
From: Eran Ben Elisha @ 2018-07-19  9:02 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Saeed Mahameed, Eran Ben Elisha, David S. Miller,
	Linux Netdev List, Alexander Duyck, Tom Herbert
In-Reply-To: <CAJ3xEMivDFLfrXpXWj_t6tHbk7apfHrgFknxs13QgUjrhv92Jw@mail.gmail.com>

On Thu, Jul 19, 2018 at 10:50 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Thu, Jul 19, 2018 at 9:55 AM, Eran Ben Elisha
> <eranlinuxmellanox@gmail.com> wrote:
>> On Thu, Jul 19, 2018 at 9:23 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>> On Thu, Jul 19, 2018 at 4:26 AM, Saeed Mahameed <saeedm@mellanox.com> wrote:
>>>> From: Eran Ben Elisha <eranbe@mellanox.com>
>>>>
>>>> Driver is yet to support aRFS for encapsulated packets, return early
>>>> error in such case.
>>>
>>>
>>> Eran,
>>>
>>> Isn't that something which is done wrong by the arfs stack code?
>>>
>>> If the kernel has an SKB which has encap set and an arfs steering
>>> rule is programed into the driver, the API should include a driver neutral
>>> description for the encap header for the HW to match, so maybe we can just do
>>>
>>
>> Hi Or,
>> This could break existing drivers support for tunneled aRFS, and hurts
>> their RX performance dramatically..
>
>> IMHO, it is expected from the driver to figure out that the skb holds
>> encap packet and act accordingly.
>
> I don't think this one bit indication on the skb is enough for
> any HW driver (e.g mlx4, mlx5 and others) to properly set
> the steering rules.

why do you think it is not enough?
mlx5e currently cannot offload tunneled packets, so this info is
perfectly fit in order to reject.

>
> The problem you indicate typically doesn't come into play in the presence
> of VMs, since the host TCP stack isn't active on such traffic.
>
> This is maybe why it wasn't pointed earlier.
>
> I believe that more drivers are broken (mlx4?)

We can do a fix for that latter.

>
> Looking now on bnxt, I see they dissect the skb and then check the
> FLOW_DIS_ENCAPSULATION flag but not always err.
>
> If we want to make sure we don't break anyone else, we can indeed have
> the check done in our drivers.
>
> It seems that the check done by bnxt is more general, thoughts?

I see bnxt driver checking protocol using this mechanism as well,
in mlx5e it was selected via skb metadata, let's keep going with this approach.

>
> Or.

^ permalink raw reply

* Re: [PATCH v2 02/17] y2038: Remove newstat family from default syscall set
From: Arnd Bergmann @ 2018-07-19  8:53 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Christoph Hellwig, Thomas Gleixner, y2038 Mailman List, Linux API,
	linux-arch, GNU C Library, Albert ARIBAUD, Networking, Al Viro,
	Peter Zijlstra, Darren Hart, Eric W . Biederman,
	Dominik Brodowski
In-Reply-To: <alpine.DEB.2.20.1807182010290.7221@digraph.polyomino.org.uk>

On Wed, Jul 18, 2018 at 10:15 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 17 Jul 2018, Arnd Bergmann wrote:
>
>> That would definitely help. See below for the stat implementation
>> I did in my musl libc prototype based on statx(). It passes the
>> LTP syscall tests, but that doesn't mean all the corner cases
>> are correct.
>
> Well, you definitely need explicit timestamp conversions on the result of
> statx to be usable in struct stat when userspace "long" is 64-bit, for BE
> because otherwise the integer nanoseconds will be in the wrong place for
> struct timespec, and for LE if the "__reserved is held in case we need a
> yet finer resolution." might start being returned as nonzero (for integer
> attoseconds, I suppose) in future.

Right. The musl implementation I sent did that by simply copying each
field we care about from the statx structure into the stat structure
individually. This is the more or less the same thing that the kernel
does to implement stat() as well.

     Arnd

^ permalink raw reply

* Re: [PATCH V4 net-next 2/3] tcp: Add tcp_retransmit_stamp() helper routine
From: Sergei Shtylyov @ 2018-07-19  9:31 UTC (permalink / raw)
  To: Jon Maxwell, davem
  Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
	linux-kernel, jmaxwell
In-Reply-To: <20180719011444.4694-3-jmaxwell37@gmail.com>

Hello!

On 7/19/2018 4:14 AM, Jon Maxwell wrote:

> Create a seperate helper routine as per Neal Cardwells suggestion. To

    Cardwell's. Perhaps copuld be fixed while applying...

> be used by the final commit in this series and retransmits_timed_out().
> 
> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
[...]

MBR, Sergei

^ permalink raw reply

* Re: [net-next 10/16] net/mlx5: Support PCIe buffer congestion handling via Devlink
From: Eran Ben Elisha @ 2018-07-19  8:49 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Saeed Mahameed, David S. Miller, Linux Netdev List,
	Eran Ben Elisha
In-Reply-To: <20180719082400.GG2184@nanopsycho.orion>

>
> This should not be num. It should be a string. Same for "mode".

will fix for v2, thanks.
>
>

^ permalink raw reply

* [PATCH net-next] tipc: make some functions static
From: YueHaibing @ 2018-07-19  9:16 UTC (permalink / raw)
  To: davem, jon.maloy, ying.xue
  Cc: linux-kernel, netdev, tipc-discussion, YueHaibing

Fixes the following sparse warnings:

net/tipc/link.c:376:5: warning: symbol 'link_bc_rcv_gap' was not declared. Should it be static?
net/tipc/link.c:823:6: warning: symbol 'link_prepare_wakeup' was not declared. Should it be static?
net/tipc/link.c:959:6: warning: symbol 'tipc_link_advance_backlog' was not declared. Should it be static?
net/tipc/link.c:1009:5: warning: symbol 'tipc_link_retrans' was not declared. Should it be static?
net/tipc/monitor.c:687:5: warning: symbol '__tipc_nl_add_monitor_peer' was not declared. Should it be static?
net/tipc/group.c:230:20: warning: symbol 'tipc_group_find_member' was not declared. Should it be static?

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 net/tipc/group.c   |  4 ++--
 net/tipc/link.c    | 11 ++++++-----
 net/tipc/monitor.c |  3 ++-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/tipc/group.c b/net/tipc/group.c
index 8f43e7d..e82f13c 100644
--- a/net/tipc/group.c
+++ b/net/tipc/group.c
@@ -227,8 +227,8 @@ void tipc_group_delete(struct net *net, struct tipc_group *grp)
 	kfree(grp);
 }
 
-struct tipc_member *tipc_group_find_member(struct tipc_group *grp,
-					   u32 node, u32 port)
+static struct tipc_member *tipc_group_find_member(struct tipc_group *grp,
+						  u32 node, u32 port)
 {
 	struct rb_node *n = grp->members.rb_node;
 	u64 nkey, key = (u64)node << 32 | port;
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 6987ffc..b1f0bee 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -373,7 +373,7 @@ int tipc_link_bc_peers(struct tipc_link *l)
 	return l->ackers;
 }
 
-u16 link_bc_rcv_gap(struct tipc_link *l)
+static u16 link_bc_rcv_gap(struct tipc_link *l)
 {
 	struct sk_buff *skb = skb_peek(&l->deferdq);
 	u16 gap = 0;
@@ -820,7 +820,7 @@ static int link_schedule_user(struct tipc_link *l, struct tipc_msg *hdr)
  * Wake up a number of waiting users, as permitted by available space
  * in the send queue
  */
-void link_prepare_wakeup(struct tipc_link *l)
+static void link_prepare_wakeup(struct tipc_link *l)
 {
 	struct sk_buff *skb, *tmp;
 	int imp, i = 0;
@@ -956,7 +956,8 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
 	return rc;
 }
 
-void tipc_link_advance_backlog(struct tipc_link *l, struct sk_buff_head *xmitq)
+static void tipc_link_advance_backlog(struct tipc_link *l,
+				      struct sk_buff_head *xmitq)
 {
 	struct sk_buff *skb, *_skb;
 	struct tipc_msg *hdr;
@@ -1006,8 +1007,8 @@ static void link_retransmit_failure(struct tipc_link *l, struct sk_buff *skb)
  * @to: retransmit to (inclusive) this sequence number
  * xmitq: queue for accumulating the retransmitted packets
  */
-int tipc_link_retrans(struct tipc_link *l, struct tipc_link *r,
-		      u16 from, u16 to, struct sk_buff_head *xmitq)
+static int tipc_link_retrans(struct tipc_link *l, struct tipc_link *r,
+			     u16 from, u16 to, struct sk_buff_head *xmitq)
 {
 	struct sk_buff *_skb, *skb = skb_peek(&l->transmq);
 	u16 bc_ack = l->bc_rcvlink->rcv_nxt - 1;
diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c
index 5453e56..67f6938 100644
--- a/net/tipc/monitor.c
+++ b/net/tipc/monitor.c
@@ -684,7 +684,8 @@ int tipc_nl_monitor_get_threshold(struct net *net)
 	return tn->mon_threshold;
 }
 
-int __tipc_nl_add_monitor_peer(struct tipc_peer *peer, struct tipc_nl_msg *msg)
+static int __tipc_nl_add_monitor_peer(struct tipc_peer *peer,
+				      struct tipc_nl_msg *msg)
 {
 	struct tipc_mon_domain *dom = peer->domain;
 	struct nlattr *attrs;
-- 
2.7.0

^ permalink raw reply related

* Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
From: Dan Carpenter @ 2018-07-19  9:09 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Bartosz Golaszewski, Sekhar Nori, Kevin Hilman, Russell King,
	Grygorii Strashko, David S . Miller, Srinivas Kandagatla,
	Lukas Wunner, Rob Herring, Florian Fainelli, Ivan Khoronzhuk,
	David Lechner, Greg Kroah-Hartman, Andrew Lunn, arm-soc, LKML,
	Linux-OMAP, netdev
In-Reply-To: <CAMpxmJUgyB1hYi-G3KVxMsm0P+aKGuHOB8LSuH+bJbYfjengJA@mail.gmail.com>


Maybe it would be simpler as three separate functions:

int of_eth_get_mac_address() <- rename existing function to this
int nvmem_eth_get_mac_address() <- patch 2
int mtd_eth_nvmem_get_mac_address() patch 3

	int ret;

	ret = of_eth_get_mac_address(dev, mac_addr);
	if (!ret)
		return 0;
	ret = nvmem_eth_get_mac_address(dev, mac_addr);
	if (ret == -EPROBEDEFER)
		return ret;
	if (!ret)
		return 0;
	ret = mtd_eth_nvmem_get_mac_address(dev, mac_addr);
	if (!ret)
		return 0;

	return -ENODEV;

regards,
dan carpenter

^ permalink raw reply

* Re: [net-next 10/16] net/mlx5: Support PCIe buffer congestion handling via Devlink
From: Jiri Pirko @ 2018-07-19  8:24 UTC (permalink / raw)
  To: Saeed Mahameed; +Cc: David S. Miller, netdev, Eran Ben Elisha
In-Reply-To: <20180719010107.22363-11-saeedm@mellanox.com>

Thu, Jul 19, 2018 at 03:01:01AM CEST, saeedm@mellanox.com wrote:
>From: Eran Ben Elisha <eranbe@mellanox.com>
>
>Add support for two driver parameters via devlink params interface:
>- Congestion action
>	HW mechanism in the PCIe buffer which monitors the amount of
>	consumed PCIe buffer per host.  This mechanism supports the
>	following actions in case of threshold overflow:
>	- Disabled - NOP (Default)
>	- Drop
>	- Mark - Mark CE bit in the CQE of received packet
>- Congestion mode
>	- Aggressive - Aggressive static trigger threshold (Default)
>	- Dynamic - Dynamically change the trigger threshold
>
>Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>---
> .../net/ethernet/mellanox/mlx5/core/devlink.c | 105 +++++++++++++++++-
> 1 file changed, 104 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>index 9800c98b01d3..1f04decef043 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>@@ -153,12 +153,115 @@ static int mlx5_devlink_query_tx_overflow_sense(struct mlx5_core_dev *mdev,
> 	return 0;
> }
> 
>+static int mlx5_devlink_set_congestion_action(struct devlink *devlink, u32 id,
>+					      struct devlink_param_gset_ctx *ctx)
>+{
>+	struct mlx5_core_dev *dev = devlink_priv(devlink);
>+	u8 max = MLX5_DEVLINK_CONGESTION_ACTION_MAX;
>+	u8 sense;
>+	int err;
>+
>+	if (!MLX5_CAP_MCAM_FEATURE(dev, mark_tx_action_cqe) &&
>+	    !MLX5_CAP_MCAM_FEATURE(dev, mark_tx_action_cnp))
>+		max = MLX5_DEVLINK_CONGESTION_ACTION_MARK - 1;
>+
>+	if (ctx->val.vu8 > max)

This should not be num. It should be a string. Same for "mode".


>+		return -ERANGE;
>+
>+	err = mlx5_devlink_query_tx_overflow_sense(dev, &sense);
>+	if (err)
>+		return err;
>+
>+	if (ctx->val.vu8 == MLX5_DEVLINK_CONGESTION_ACTION_DISABLED &&
>+	    sense != MLX5_DEVLINK_CONGESTION_MODE_AGGRESSIVE)
>+		return -EINVAL;
>+
>+	return mlx5_devlink_set_tx_lossy_overflow(dev, ctx->val.vu8);
>+}
>+
>+static int mlx5_devlink_get_congestion_action(struct devlink *devlink, u32 id,
>+					      struct devlink_param_gset_ctx *ctx)
>+{
>+	struct mlx5_core_dev *dev = devlink_priv(devlink);
>+
>+	return mlx5_devlink_query_tx_lossy_overflow(dev, &ctx->val.vu8);
>+}
>+
>+static int mlx5_devlink_set_congestion_mode(struct devlink *devlink, u32 id,
>+					    struct devlink_param_gset_ctx *ctx)
>+{
>+	struct mlx5_core_dev *dev = devlink_priv(devlink);
>+	u8 tx_lossy_overflow;
>+	int err;
>+
>+	if (ctx->val.vu8 > MLX5_DEVLINK_CONGESTION_MODE_MAX)
>+		return -ERANGE;
>+
>+	err = mlx5_devlink_query_tx_lossy_overflow(dev, &tx_lossy_overflow);
>+	if (err)
>+		return err;
>+
>+	if (ctx->val.vu8 != MLX5_DEVLINK_CONGESTION_MODE_AGGRESSIVE &&
>+	    tx_lossy_overflow == MLX5_DEVLINK_CONGESTION_ACTION_DISABLED)
>+		return -EINVAL;
>+
>+	return mlx5_devlink_set_tx_overflow_sense(dev, ctx->val.vu8);
>+}
>+
>+static int mlx5_devlink_get_congestion_mode(struct devlink *devlink, u32 id,
>+					    struct devlink_param_gset_ctx *ctx)
>+{
>+	struct mlx5_core_dev *dev = devlink_priv(devlink);
>+
>+	return mlx5_devlink_query_tx_overflow_sense(dev, &ctx->val.vu8);
>+}
>+
>+enum mlx5_devlink_param_id {
>+	MLX5_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
>+	MLX5_DEVLINK_PARAM_ID_CONGESTION_ACTION,
>+	MLX5_DEVLINK_PARAM_ID_CONGESTION_MODE,
>+};
>+
>+static const struct devlink_param mlx5_devlink_params[] = {
>+	DEVLINK_PARAM_DRIVER(MLX5_DEVLINK_PARAM_ID_CONGESTION_ACTION,
>+			     "congestion_action",
>+			     DEVLINK_PARAM_TYPE_U8,
>+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
>+			     mlx5_devlink_get_congestion_action,
>+			     mlx5_devlink_set_congestion_action, NULL),
>+	DEVLINK_PARAM_DRIVER(MLX5_DEVLINK_PARAM_ID_CONGESTION_MODE,
>+			     "congestion_mode",
>+			     DEVLINK_PARAM_TYPE_U8,
>+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
>+			     mlx5_devlink_get_congestion_mode,
>+			     mlx5_devlink_set_congestion_mode, NULL),
>+};
>+
> int mlx5_devlink_register(struct devlink *devlink, struct device *dev)
> {
>-	return devlink_register(devlink, dev);
>+	int err;
>+
>+	err = devlink_register(devlink, dev);
>+	if (err)
>+		return err;
>+
>+	err = devlink_params_register(devlink, mlx5_devlink_params,
>+				      ARRAY_SIZE(mlx5_devlink_params));
>+	if (err) {
>+		dev_err(dev, "devlink_params_register failed, err = %d\n", err);
>+		goto unregister;
>+	}
>+
>+	return 0;
>+
>+unregister:
>+	devlink_unregister(devlink);
>+	return err;
> }
> 
> void mlx5_devlink_unregister(struct devlink *devlink)
> {
>+	devlink_params_unregister(devlink, mlx5_devlink_params,
>+				  ARRAY_SIZE(mlx5_devlink_params));
> 	devlink_unregister(devlink);
> }
>-- 
>2.17.0
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox