Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 05/15] s390/qeth: limit csum offload erratum to L3 devices
From: Julian Wiedmann @ 2018-09-17 15:35 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

Combined L3+L4 csum offload is only required for some L3 HW. So for
L2 devices, don't offload the IP header csum calculation.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
Reference-ID: JUP 394553
---
 drivers/s390/net/qeth_core.h    | 5 -----
 drivers/s390/net/qeth_l3_main.c | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 0857b1286660..b47fb95a49e9 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -892,11 +892,6 @@ static inline void qeth_tx_csum(struct sk_buff *skb, u8 *flags, int ipv)
 	if ((ipv == 4 && ip_hdr(skb)->protocol == IPPROTO_UDP) ||
 	    (ipv == 6 && ipv6_hdr(skb)->nexthdr == IPPROTO_UDP))
 		*flags |= QETH_HDR_EXT_UDP;
-	if (ipv == 4) {
-		/* some HW requires combined L3+L4 csum offload: */
-		*flags |= QETH_HDR_EXT_CSUM_HDR_REQ;
-		ip_hdr(skb)->check = 0;
-	}
 }
 
 static inline void qeth_put_buffer_pool_entry(struct qeth_card *card,
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 1d92584e01b3..d4a967077279 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2055,6 +2055,11 @@ static void qeth_l3_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
 
 	if (!skb_is_gso(skb) && skb->ip_summed == CHECKSUM_PARTIAL) {
 		qeth_tx_csum(skb, &hdr->hdr.l3.ext_flags, ipv);
+		/* some HW requires combined L3+L4 csum offload: */
+		if (ipv == 4) {
+			hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_HDR_REQ;
+			ip_hdr(skb)->check = 0;
+		}
 		if (card->options.performance_stats)
 			card->perf_stats.tx_csum++;
 	}
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 06/15] s390/qeth: fix up protocol headers early
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

When qeth_add_hw_header() falls back to the HW header cache, it also
copies over the necessary protocol headers. Thus any manipulation to
the protocol headers needs to happen before adding the HW header.

For current usage this doesn't matter, but it becomes relevant when
moving TSO transmission over to the faster code path.

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

diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index d4a967077279..0c085f57f7e2 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2056,10 +2056,8 @@ static void qeth_l3_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
 	if (!skb_is_gso(skb) && skb->ip_summed == CHECKSUM_PARTIAL) {
 		qeth_tx_csum(skb, &hdr->hdr.l3.ext_flags, ipv);
 		/* some HW requires combined L3+L4 csum offload: */
-		if (ipv == 4) {
+		if (ipv == 4)
 			hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_HDR_REQ;
-			ip_hdr(skb)->check = 0;
-		}
 		if (card->options.performance_stats)
 			card->perf_stats.tx_csum++;
 	}
@@ -2168,6 +2166,15 @@ static int qeth_l3_get_elements_no_tso(struct qeth_card *card,
 	return elements;
 }
 
+static void qeth_l3_fixup_headers(struct sk_buff *skb)
+{
+	struct iphdr *iph = ip_hdr(skb);
+
+	/* this is safe, IPv6 traffic takes a different path */
+	if (skb->ip_summed == CHECKSUM_PARTIAL)
+		iph->check = 0;
+}
+
 static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 				struct qeth_qdio_out_q *queue, int ipv,
 				int cast_type)
@@ -2188,6 +2195,7 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 	skb_pull(skb, ETH_HLEN);
 	frame_len = skb->len;
 
+	qeth_l3_fixup_headers(skb);
 	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, 0,
 				      &elements);
 	if (push_len < 0)
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 09/15] s390/qeth: speed up TSO transmission
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

Switch TSO over to the faster transmit path, and remove all the unused
old TSO code.

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

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index d86eea9db2a7..1c9fce609eb9 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -1004,7 +1004,6 @@ int qeth_send_control_data(struct qeth_card *, int, struct qeth_cmd_buffer *,
 	int (*reply_cb)(struct qeth_card *, struct qeth_reply*, unsigned long),
 	void *reply_param);
 unsigned int qeth_count_elements(struct sk_buff *skb, unsigned int data_offset);
-int qeth_get_elements_for_frags(struct sk_buff *);
 int qeth_do_send_packet_fast(struct qeth_qdio_out_q *queue, struct sk_buff *skb,
 			     struct qeth_hdr *hdr, unsigned int offset,
 			     unsigned int hd_len);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 460ffdf1b200..7426167eace2 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -3788,7 +3788,7 @@ EXPORT_SYMBOL_GPL(qeth_get_priority_queue);
  * Returns the number of pages, and thus QDIO buffer elements, needed to cover
  * fragmented part of the SKB. Returns zero for linear SKB.
  */
-int qeth_get_elements_for_frags(struct sk_buff *skb)
+static int qeth_get_elements_for_frags(struct sk_buff *skb)
 {
 	int cnt, elements = 0;
 
@@ -3801,7 +3801,6 @@ int qeth_get_elements_for_frags(struct sk_buff *skb)
 	}
 	return elements;
 }
-EXPORT_SYMBOL_GPL(qeth_get_elements_for_frags);
 
 /**
  * qeth_count_elements() -	Counts the number of QDIO buffer elements needed
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index fe55218802d7..5d7e2921ab36 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -33,7 +33,6 @@
 #include <net/ipv6.h>
 #include <net/ip6_route.h>
 #include <net/ip6_fib.h>
-#include <net/ip6_checksum.h>
 #include <net/iucv/af_iucv.h>
 #include <linux/hashtable.h>
 
@@ -2118,70 +2117,6 @@ static void qeth_l3_fill_tso_ext(struct qeth_hdr_tso *hdr,
 	ext->dg_hdr_len = proto_len;
 }
 
-static void qeth_tso_fill_header(struct qeth_card *card,
-		struct qeth_hdr *qhdr, struct sk_buff *skb)
-{
-	struct qeth_hdr_tso *hdr = (struct qeth_hdr_tso *)qhdr;
-	struct tcphdr *tcph = tcp_hdr(skb);
-	struct iphdr *iph = ip_hdr(skb);
-	struct ipv6hdr *ip6h = ipv6_hdr(skb);
-
-	/*set values which are fix for the first approach ...*/
-	hdr->ext.hdr_tot_len = (__u16) sizeof(struct qeth_hdr_ext_tso);
-	hdr->ext.imb_hdr_no  = 1;
-	hdr->ext.hdr_type    = 1;
-	hdr->ext.hdr_version = 1;
-	hdr->ext.hdr_len     = 28;
-	/*insert non-fix values */
-	hdr->ext.mss = skb_shinfo(skb)->gso_size;
-	hdr->ext.dg_hdr_len = (__u16)(ip_hdrlen(skb) + tcp_hdrlen(skb));
-	hdr->ext.payload_len = (__u16)(skb->len - hdr->ext.dg_hdr_len -
-				       sizeof(struct qeth_hdr_tso));
-	tcph->check = 0;
-	if (be16_to_cpu(skb->protocol) == ETH_P_IPV6) {
-		ip6h->payload_len = 0;
-		tcph->check = ~csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
-					       0, IPPROTO_TCP, 0);
-	} else {
-		/*OSA want us to set these values ...*/
-		tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-					 0, IPPROTO_TCP, 0);
-		iph->tot_len = 0;
-		iph->check = 0;
-	}
-}
-
-/**
- * qeth_l3_get_elements_no_tso() - find number of SBALEs for skb data for tso
- * @card:			   qeth card structure, to check max. elems.
- * @skb:			   SKB address
- * @extra_elems:		   extra elems needed, to check against max.
- *
- * Returns the number of pages, and thus QDIO buffer elements, needed to cover
- * skb data, including linear part and fragments, but excluding TCP header.
- * Checks if the result plus extra_elems fits under the limit for the card.
- * Returns 0 if it does not.
- * Note: extra_elems is not included in the returned result.
- */
-static int qeth_l3_get_elements_no_tso(struct qeth_card *card,
-			struct sk_buff *skb, int extra_elems)
-{
-	addr_t start = (addr_t)tcp_hdr(skb) + tcp_hdrlen(skb);
-	addr_t end = (addr_t)skb->data + skb_headlen(skb);
-	int elements = qeth_get_elements_for_frags(skb);
-
-	if (start != end)
-		elements += qeth_get_elements_for_range(start, end);
-
-	if ((elements + extra_elems) > QETH_MAX_BUFFER_ELEMENTS(card)) {
-		QETH_DBF_MESSAGE(2,
-	"Invalid size of TSO IP packet (Number=%d / Length=%d). Discarded.\n",
-				elements + extra_elems, skb->len);
-		return 0;
-	}
-	return elements;
-}
-
 static void qeth_l3_fixup_headers(struct sk_buff *skb)
 {
 	struct iphdr *iph = ip_hdr(skb);
@@ -2196,9 +2131,8 @@ static void qeth_l3_fixup_headers(struct sk_buff *skb)
 	}
 }
 
-static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
-				struct qeth_qdio_out_q *queue, int ipv,
-				int cast_type)
+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 hw_hdr_len, proto_len, frame_len, elements;
 	unsigned char eth_hdr[ETH_HLEN];
@@ -2282,81 +2216,6 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 	return rc;
 }
 
-static int qeth_l3_xmit(struct qeth_card *card, struct sk_buff *skb,
-			struct qeth_qdio_out_q *queue, int ipv, int cast_type)
-{
-	struct qeth_hdr *hdr = NULL;
-	struct sk_buff *new_skb = NULL;
-	int tx_bytes = skb->len;
-	unsigned int hd_len;
-	int elements, rc;
-	bool is_sg;
-
-	/* create a clone with writeable headroom */
-	new_skb = skb_realloc_headroom(skb, sizeof(struct qeth_hdr_tso));
-	if (!new_skb)
-		return -ENOMEM;
-
-	skb_pull(new_skb, ETH_HLEN);
-
-	/* fix hardware limitation: as long as we do not have sbal
-	 * chaining we can not send long frag lists
-	 */
-	if (!qeth_l3_get_elements_no_tso(card, new_skb, 1)) {
-		rc = skb_linearize(new_skb);
-
-		if (card->options.performance_stats) {
-			if (rc)
-				card->perf_stats.tx_linfail++;
-			else
-				card->perf_stats.tx_lin++;
-		}
-		if (rc)
-			goto out;
-	}
-
-	hdr = skb_push(new_skb, sizeof(struct qeth_hdr_tso));
-	memset(hdr, 0, sizeof(struct qeth_hdr_tso));
-	qeth_l3_fill_header(card, hdr, new_skb, ipv, cast_type,
-			    new_skb->len - sizeof(struct qeth_hdr_tso));
-	qeth_tso_fill_header(card, hdr, new_skb);
-
-	elements = qeth_l3_get_elements_no_tso(card, new_skb, 1);
-	if (!elements) {
-		rc = -E2BIG;
-		goto out;
-	}
-	elements++;
-
-	hd_len = sizeof(struct qeth_hdr_tso) + ip_hdrlen(new_skb) +
-		 tcp_hdrlen(new_skb);
-
-	if (qeth_hdr_chk_and_bounce(new_skb, &hdr, hd_len)) {
-		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:
-	if (!rc) {
-		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++;
-			card->perf_stats.large_send_bytes += tx_bytes;
-			card->perf_stats.large_send_cnt++;
-		}
-	} else {
-		if (new_skb != skb)
-			dev_kfree_skb_any(new_skb);
-	}
-	return rc;
-}
-
 static netdev_tx_t qeth_l3_hard_start_xmit(struct sk_buff *skb,
 					   struct net_device *dev)
 {
@@ -2392,9 +2251,7 @@ static netdev_tx_t qeth_l3_hard_start_xmit(struct sk_buff *skb,
 	}
 	netif_stop_queue(dev);
 
-	if (IS_IQD(card) || (!skb_is_gso(skb) && ipv == 4))
-		rc = qeth_l3_xmit_offload(card, skb, queue, ipv, cast_type);
-	else if (skb_is_gso(skb))
+	if (ipv == 4 || IS_IQD(card))
 		rc = qeth_l3_xmit(card, skb, queue, ipv, cast_type);
 	else
 		rc = qeth_xmit(card, skb, queue, ipv, cast_type,
@@ -2560,6 +2417,8 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 		card->dev->needed_headroom = sizeof(struct qeth_hdr);
 		/* allow for de-acceleration of NETIF_F_HW_VLAN_CTAG_TX: */
 		card->dev->needed_headroom += VLAN_HLEN;
+		if (qeth_is_supported(card, IPA_OUTBOUND_TSO))
+			card->dev->needed_headroom = sizeof(struct qeth_hdr_tso);
 
 		/*IPv6 address autoconfiguration stuff*/
 		qeth_l3_get_unique_id(card);
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 10/15] s390/qeth: remove qeth_hdr_chk_and_bounce()
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

Restructure the OSN xmit path to handle misaligned HW headers properly,
without shifting the packet data around.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      |  1 -
 drivers/s390/net/qeth_core_main.c | 21 ---------------------
 drivers/s390/net/qeth_l2_main.c   | 37 ++++++++++++++++++++++++++++---------
 3 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 1c9fce609eb9..be213b5c2552 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -1021,7 +1021,6 @@ void qeth_dbf_longtext(debug_info_t *id, int level, char *text, ...);
 int qeth_core_ethtool_get_link_ksettings(struct net_device *netdev,
 					 struct ethtool_link_ksettings *cmd);
 int qeth_set_access_ctrl_online(struct qeth_card *card, int fallback);
-int qeth_hdr_chk_and_bounce(struct sk_buff *, struct qeth_hdr **, int);
 int qeth_configure_cq(struct qeth_card *, enum qeth_cq);
 int qeth_hw_trap(struct qeth_card *, enum qeth_diags_trap_action);
 void qeth_trace_features(struct qeth_card *);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 7426167eace2..c7f7061a7205 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -3823,27 +3823,6 @@ unsigned int qeth_count_elements(struct sk_buff *skb, unsigned int data_offset)
 }
 EXPORT_SYMBOL_GPL(qeth_count_elements);
 
-int qeth_hdr_chk_and_bounce(struct sk_buff *skb, struct qeth_hdr **hdr, int len)
-{
-	int hroom, inpage, rest;
-
-	if (((unsigned long)skb->data & PAGE_MASK) !=
-	    (((unsigned long)skb->data + len - 1) & PAGE_MASK)) {
-		hroom = skb_headroom(skb);
-		inpage = PAGE_SIZE - ((unsigned long) skb->data % PAGE_SIZE);
-		rest = len - inpage;
-		if (rest > hroom)
-			return 1;
-		memmove(skb->data - rest, skb->data, skb_headlen(skb));
-		skb->data -= rest;
-		skb->tail -= rest;
-		*hdr = (struct qeth_hdr *)skb->data;
-		QETH_DBF_MESSAGE(2, "skb bounce len: %d rest: %d\n", len, rest);
-	}
-	return 0;
-}
-EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
-
 #define QETH_HDR_CACHE_OBJ_SIZE		(sizeof(struct qeth_hdr_tso) + \
 					 MAX_TCP_HEADER)
 
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 24b531ca2827..33b65471a68a 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -650,19 +650,38 @@ static void qeth_l2_set_rx_mode(struct net_device *dev)
 static int qeth_l2_xmit_osn(struct qeth_card *card, struct sk_buff *skb,
 			    struct qeth_qdio_out_q *queue)
 {
-	unsigned int elements;
-	struct qeth_hdr *hdr;
+	struct qeth_hdr *hdr = (struct qeth_hdr *)skb->data;
+	addr_t end = (addr_t)(skb->data + sizeof(*hdr));
+	addr_t start = (addr_t)skb->data;
+	unsigned int elements = 0;
+	unsigned int hd_len = 0;
+	int rc;
 
 	if (skb->protocol == htons(ETH_P_IPV6))
 		return -EPROTONOSUPPORT;
 
-	hdr = (struct qeth_hdr *)skb->data;
-	elements = qeth_count_elements(skb, 0);
-	if (elements > QETH_MAX_BUFFER_ELEMENTS(card))
-		return -E2BIG;
-	if (qeth_hdr_chk_and_bounce(skb, &hdr, sizeof(*hdr)))
-		return -EINVAL;
-	return qeth_do_send_packet(card, queue, skb, hdr, 0, 0, elements);
+	if (qeth_get_elements_for_range(start, end) > 1) {
+		/* Misaligned HW header, move it to its own buffer element. */
+		hdr = kmem_cache_alloc(qeth_core_header_cache, GFP_ATOMIC);
+		if (!hdr)
+			return -ENOMEM;
+		hd_len = sizeof(*hdr);
+		skb_copy_from_linear_data(skb, (char *)hdr, hd_len);
+		elements++;
+	}
+
+	elements += qeth_count_elements(skb, hd_len);
+	if (elements > QETH_MAX_BUFFER_ELEMENTS(card)) {
+		rc = -E2BIG;
+		goto out;
+	}
+
+	rc = qeth_do_send_packet(card, queue, skb, hdr, hd_len, hd_len,
+				 elements);
+out:
+	if (rc && hd_len)
+		kmem_cache_free(qeth_core_header_cache, hdr);
+	return rc;
 }
 
 static netdev_tx_t qeth_l2_hard_start_xmit(struct sk_buff *skb,
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 11/15] s390/qeth: uninstall IRQ handler on device removal
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

When setting up, qeth installs its IRQ handler on the ccw devices. But
the IRQ handler is not cleared on removal - so even after qeth yields
control of the ccw devices, spurious interrupts would still be presented
to us.

Make (de-)installation of the IRQ handler part of the ccw channel
setup/removal helpers, and while at it also add the appropriate locking.
Shift around qeth_setup_channel() to avoid a forward declaration for
qeth_irq().

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

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index c7f7061a7205..b76e844811e1 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -901,44 +901,6 @@ static void qeth_send_control_data_cb(struct qeth_channel *channel,
 	qeth_release_buffer(channel, iob);
 }
 
-static int qeth_setup_channel(struct qeth_channel *channel, bool alloc_buffers)
-{
-	int cnt;
-
-	QETH_DBF_TEXT(SETUP, 2, "setupch");
-
-	channel->ccw = kmalloc(sizeof(struct ccw1), GFP_KERNEL | GFP_DMA);
-	if (!channel->ccw)
-		return -ENOMEM;
-	channel->state = CH_STATE_DOWN;
-	atomic_set(&channel->irq_pending, 0);
-	init_waitqueue_head(&channel->wait_q);
-
-	if (!alloc_buffers)
-		return 0;
-
-	for (cnt = 0; cnt < QETH_CMD_BUFFER_NO; cnt++) {
-		channel->iob[cnt].data =
-			kzalloc(QETH_BUFSIZE, GFP_DMA|GFP_KERNEL);
-		if (channel->iob[cnt].data == NULL)
-			break;
-		channel->iob[cnt].state = BUF_STATE_FREE;
-		channel->iob[cnt].channel = channel;
-		channel->iob[cnt].callback = qeth_send_control_data_cb;
-		channel->iob[cnt].rc = 0;
-	}
-	if (cnt < QETH_CMD_BUFFER_NO) {
-		kfree(channel->ccw);
-		while (cnt-- > 0)
-			kfree(channel->iob[cnt].data);
-		return -ENOMEM;
-	}
-	channel->io_buf_no = 0;
-	spin_lock_init(&channel->iob_lock);
-
-	return 0;
-}
-
 static int qeth_set_thread_start_bit(struct qeth_card *card,
 		unsigned long thread)
 {
@@ -1337,14 +1299,61 @@ static void qeth_free_buffer_pool(struct qeth_card *card)
 
 static void qeth_clean_channel(struct qeth_channel *channel)
 {
+	struct ccw_device *cdev = channel->ccwdev;
 	int cnt;
 
 	QETH_DBF_TEXT(SETUP, 2, "freech");
+
+	spin_lock_irq(get_ccwdev_lock(cdev));
+	cdev->handler = NULL;
+	spin_unlock_irq(get_ccwdev_lock(cdev));
+
 	for (cnt = 0; cnt < QETH_CMD_BUFFER_NO; cnt++)
 		kfree(channel->iob[cnt].data);
 	kfree(channel->ccw);
 }
 
+static int qeth_setup_channel(struct qeth_channel *channel, bool alloc_buffers)
+{
+	struct ccw_device *cdev = channel->ccwdev;
+	int cnt;
+
+	QETH_DBF_TEXT(SETUP, 2, "setupch");
+
+	channel->ccw = kmalloc(sizeof(struct ccw1), GFP_KERNEL | GFP_DMA);
+	if (!channel->ccw)
+		return -ENOMEM;
+	channel->state = CH_STATE_DOWN;
+	atomic_set(&channel->irq_pending, 0);
+	init_waitqueue_head(&channel->wait_q);
+
+	spin_lock_irq(get_ccwdev_lock(cdev));
+	cdev->handler = qeth_irq;
+	spin_unlock_irq(get_ccwdev_lock(cdev));
+
+	if (!alloc_buffers)
+		return 0;
+
+	for (cnt = 0; cnt < QETH_CMD_BUFFER_NO; cnt++) {
+		channel->iob[cnt].data =
+			kzalloc(QETH_BUFSIZE, GFP_DMA|GFP_KERNEL);
+		if (channel->iob[cnt].data == NULL)
+			break;
+		channel->iob[cnt].state = BUF_STATE_FREE;
+		channel->iob[cnt].channel = channel;
+		channel->iob[cnt].callback = qeth_send_control_data_cb;
+		channel->iob[cnt].rc = 0;
+	}
+	if (cnt < QETH_CMD_BUFFER_NO) {
+		qeth_clean_channel(channel);
+		return -ENOMEM;
+	}
+	channel->io_buf_no = 0;
+	spin_lock_init(&channel->iob_lock);
+
+	return 0;
+}
+
 static void qeth_set_single_write_queues(struct qeth_card *card)
 {
 	if ((atomic_read(&card->qdio.state) != QETH_QDIO_UNINITIALIZED) &&
@@ -1495,7 +1504,7 @@ static void qeth_core_sl_print(struct seq_file *m, struct service_level *slr)
 			CARD_BUS_ID(card), card->info.mcl_level);
 }
 
-static struct qeth_card *qeth_alloc_card(void)
+static struct qeth_card *qeth_alloc_card(struct ccwgroup_device *gdev)
 {
 	struct qeth_card *card;
 
@@ -1504,6 +1513,11 @@ static struct qeth_card *qeth_alloc_card(void)
 	if (!card)
 		goto out;
 	QETH_DBF_HEX(SETUP, 2, &card, sizeof(void *));
+
+	card->gdev = gdev;
+	CARD_RDEV(card) = gdev->cdev[0];
+	CARD_WDEV(card) = gdev->cdev[1];
+	CARD_DDEV(card) = gdev->cdev[2];
 	if (qeth_setup_channel(&card->read, true))
 		goto out_ip;
 	if (qeth_setup_channel(&card->write, true))
@@ -5773,7 +5787,7 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 
 	QETH_DBF_TEXT_(SETUP, 2, "%s", dev_name(&gdev->dev));
 
-	card = qeth_alloc_card();
+	card = qeth_alloc_card(gdev);
 	if (!card) {
 		QETH_DBF_TEXT_(SETUP, 2, "1err%d", -ENOMEM);
 		rc = -ENOMEM;
@@ -5789,15 +5803,7 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 			goto err_card;
 	}
 
-	card->read.ccwdev  = gdev->cdev[0];
-	card->write.ccwdev = gdev->cdev[1];
-	card->data.ccwdev  = gdev->cdev[2];
 	dev_set_drvdata(&gdev->dev, card);
-	card->gdev = gdev;
-	gdev->cdev[0]->handler = qeth_irq;
-	gdev->cdev[1]->handler = qeth_irq;
-	gdev->cdev[2]->handler = qeth_irq;
-
 	qeth_setup_card(card);
 	qeth_update_from_chp_desc(card);
 
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 13/15] s390/qeth: fix typo in return value
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

Assuming this was just a typo, as returning an actual negative value
from a cmd callback would make no sense either.

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

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index b76e844811e1..f09bef4a49ca 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -3058,7 +3058,7 @@ static int qeth_query_ipassists_cb(struct qeth_card *card,
 		QETH_DBF_TEXT(SETUP, 2, "ipaunsup");
 		card->options.ipa4.supported_funcs |= IPA_SETADAPTERPARMS;
 		card->options.ipa6.supported_funcs |= IPA_SETADAPTERPARMS;
-		return -0;
+		return 0;
 	default:
 		if (cmd->hdr.return_code) {
 			QETH_DBF_MESSAGE(1, "%s IPA_CMD_QIPASSIST: Unhandled "
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 14/15] s390/qeth: fine-tune spinlocks
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

For quite a lot of code paths it's obvious that they will never run in
IRQ context. So replace their spin_lock_irqsave() calls with
spin_lock_irq().

While at it, get rid of the redundant card pointer in struct qeth_reply
that was used by qeth_send_control_data() to access the card's lock.

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

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index be213b5c2552..0dbe81f958f0 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -639,7 +639,6 @@ struct qeth_reply {
 	atomic_t received;
 	int rc;
 	void *param;
-	struct qeth_card *card;
 	refcount_t refcnt;
 };
 
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index f09bef4a49ca..100b6f4a3fb8 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -592,7 +592,6 @@ static struct qeth_reply *qeth_alloc_reply(struct qeth_card *card)
 	if (reply) {
 		refcount_set(&reply->refcnt, 1);
 		atomic_set(&reply->received, 0);
-		reply->card = card;
 	}
 	return reply;
 }
@@ -1541,15 +1540,14 @@ static struct qeth_card *qeth_alloc_card(struct ccwgroup_device *gdev)
 
 static int qeth_clear_channel(struct qeth_channel *channel)
 {
-	unsigned long flags;
 	struct qeth_card *card;
 	int rc;
 
 	card = CARD_FROM_CDEV(channel->ccwdev);
 	QETH_CARD_TEXT(card, 3, "clearch");
-	spin_lock_irqsave(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
 	rc = ccw_device_clear(channel->ccwdev, QETH_CLEAR_CHANNEL_PARM);
-	spin_unlock_irqrestore(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
 
 	if (rc)
 		return rc;
@@ -1565,15 +1563,14 @@ static int qeth_clear_channel(struct qeth_channel *channel)
 
 static int qeth_halt_channel(struct qeth_channel *channel)
 {
-	unsigned long flags;
 	struct qeth_card *card;
 	int rc;
 
 	card = CARD_FROM_CDEV(channel->ccwdev);
 	QETH_CARD_TEXT(card, 3, "haltch");
-	spin_lock_irqsave(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
 	rc = ccw_device_halt(channel->ccwdev, QETH_HALT_CHANNEL_PARM);
-	spin_unlock_irqrestore(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
 
 	if (rc)
 		return rc;
@@ -1667,7 +1664,6 @@ static int qeth_read_conf_data(struct qeth_card *card, void **buffer,
 	char *rcd_buf;
 	int ret;
 	struct qeth_channel *channel = &card->data;
-	unsigned long flags;
 
 	/*
 	 * scan for RCD command in extended SenseID data
@@ -1681,11 +1677,11 @@ static int qeth_read_conf_data(struct qeth_card *card, void **buffer,
 
 	qeth_setup_ccw(channel->ccw, ciw->cmd, ciw->count, rcd_buf);
 	channel->state = CH_STATE_RCD;
-	spin_lock_irqsave(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
 	ret = ccw_device_start_timeout(channel->ccwdev, channel->ccw,
 				       QETH_RCD_PARM, LPM_ANYPATH, 0,
 				       QETH_RCD_TIMEOUT);
-	spin_unlock_irqrestore(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
 	if (!ret)
 		wait_event(card->wait_q,
 			   (channel->state == CH_STATE_RCD_DONE ||
@@ -1843,7 +1839,6 @@ static int qeth_idx_activate_get_answer(struct qeth_channel *channel,
 			struct qeth_cmd_buffer *))
 {
 	struct qeth_cmd_buffer *iob;
-	unsigned long flags;
 	int rc;
 	struct qeth_card *card;
 
@@ -1858,10 +1853,10 @@ static int qeth_idx_activate_get_answer(struct qeth_channel *channel,
 	wait_event(card->wait_q,
 		   atomic_cmpxchg(&channel->irq_pending, 0, 1) == 0);
 	QETH_DBF_TEXT(SETUP, 6, "noirqpnd");
-	spin_lock_irqsave(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
 	rc = ccw_device_start_timeout(channel->ccwdev, channel->ccw,
 				      (addr_t) iob, 0, 0, QETH_TIMEOUT);
-	spin_unlock_irqrestore(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
 
 	if (rc) {
 		QETH_DBF_MESSAGE(2, "Error2 in activating channel rc=%d\n", rc);
@@ -1888,7 +1883,6 @@ static int qeth_idx_activate_channel(struct qeth_channel *channel,
 {
 	struct qeth_card *card;
 	struct qeth_cmd_buffer *iob;
-	unsigned long flags;
 	__u16 temp;
 	__u8 tmp;
 	int rc;
@@ -1928,10 +1922,10 @@ static int qeth_idx_activate_channel(struct qeth_channel *channel,
 	wait_event(card->wait_q,
 		   atomic_cmpxchg(&channel->irq_pending, 0, 1) == 0);
 	QETH_DBF_TEXT(SETUP, 6, "noirqpnd");
-	spin_lock_irqsave(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
 	rc = ccw_device_start_timeout(channel->ccwdev, channel->ccw,
 				      (addr_t) iob, 0, 0, QETH_TIMEOUT);
-	spin_unlock_irqrestore(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
 
 	if (rc) {
 		QETH_DBF_MESSAGE(2, "Error1 in activating channel. rc=%d\n",
@@ -2112,7 +2106,6 @@ int qeth_send_control_data(struct qeth_card *card, int len,
 {
 	struct qeth_channel *channel = iob->channel;
 	int rc;
-	unsigned long flags;
 	struct qeth_reply *reply = NULL;
 	unsigned long timeout, event_timeout;
 	struct qeth_ipa_cmd *cmd = NULL;
@@ -2145,26 +2138,26 @@ int qeth_send_control_data(struct qeth_card *card, int len,
 	}
 	qeth_prepare_control_data(card, len, iob);
 
-	spin_lock_irqsave(&card->lock, flags);
+	spin_lock_irq(&card->lock);
 	list_add_tail(&reply->list, &card->cmd_waiter_list);
-	spin_unlock_irqrestore(&card->lock, flags);
+	spin_unlock_irq(&card->lock);
 
 	timeout = jiffies + event_timeout;
 
 	QETH_CARD_TEXT(card, 6, "noirqpnd");
-	spin_lock_irqsave(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
 	rc = ccw_device_start_timeout(channel->ccwdev, channel->ccw,
 				      (addr_t) iob, 0, 0, event_timeout);
-	spin_unlock_irqrestore(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
 	if (rc) {
 		QETH_DBF_MESSAGE(2, "%s qeth_send_control_data: "
 			"ccw_device_start rc = %i\n",
 			dev_name(&channel->ccwdev->dev), rc);
 		QETH_CARD_TEXT_(card, 2, " err%d", rc);
-		spin_lock_irqsave(&card->lock, flags);
+		spin_lock_irq(&card->lock);
 		list_del_init(&reply->list);
 		qeth_put_reply(reply);
-		spin_unlock_irqrestore(&card->lock, flags);
+		spin_unlock_irq(&card->lock);
 		qeth_release_buffer(channel, iob);
 		atomic_set(&channel->irq_pending, 0);
 		wake_up(&card->wait_q);
@@ -2192,9 +2185,9 @@ int qeth_send_control_data(struct qeth_card *card, int len,
 
 time_err:
 	reply->rc = -ETIME;
-	spin_lock_irqsave(&reply->card->lock, flags);
+	spin_lock_irq(&card->lock);
 	list_del_init(&reply->list);
-	spin_unlock_irqrestore(&reply->card->lock, flags);
+	spin_unlock_irq(&card->lock);
 	atomic_inc(&reply->received);
 	rc = reply->rc;
 	qeth_put_reply(reply);
@@ -5776,7 +5769,6 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 	struct device *dev;
 	int rc;
 	enum qeth_discipline_id enforced_disc;
-	unsigned long flags;
 	char dbf_name[DBF_NAME_LEN];
 
 	QETH_DBF_TEXT(SETUP, 2, "probedev");
@@ -5834,9 +5826,9 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 		break;
 	}
 
-	write_lock_irqsave(&qeth_core_card_list.rwlock, flags);
+	write_lock_irq(&qeth_core_card_list.rwlock);
 	list_add_tail(&card->list, &qeth_core_card_list.list);
-	write_unlock_irqrestore(&qeth_core_card_list.rwlock, flags);
+	write_unlock_irq(&qeth_core_card_list.rwlock);
 	return 0;
 
 err_disc:
@@ -5852,7 +5844,6 @@ static int qeth_core_probe_device(struct ccwgroup_device *gdev)
 
 static void qeth_core_remove_device(struct ccwgroup_device *gdev)
 {
-	unsigned long flags;
 	struct qeth_card *card = dev_get_drvdata(&gdev->dev);
 
 	QETH_DBF_TEXT(SETUP, 2, "removedv");
@@ -5862,9 +5853,9 @@ static void qeth_core_remove_device(struct ccwgroup_device *gdev)
 		qeth_core_free_discipline(card);
 	}
 
-	write_lock_irqsave(&qeth_core_card_list.rwlock, flags);
+	write_lock_irq(&qeth_core_card_list.rwlock);
 	list_del(&card->list);
-	write_unlock_irqrestore(&qeth_core_card_list.rwlock, flags);
+	write_unlock_irq(&qeth_core_card_list.rwlock);
 	free_netdev(card->dev);
 	qeth_core_free_card(card);
 	dev_set_drvdata(&gdev->dev, NULL);
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 6285af373bdf..12858ffa28cf 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1209,7 +1209,6 @@ static int qeth_osn_send_control_data(struct qeth_card *card, int len,
 			   struct qeth_cmd_buffer *iob)
 {
 	struct qeth_channel *channel = iob->channel;
-	unsigned long flags;
 	int rc = 0;
 
 	QETH_CARD_TEXT(card, 5, "osndctrd");
@@ -1218,10 +1217,10 @@ static int qeth_osn_send_control_data(struct qeth_card *card, int len,
 		   atomic_cmpxchg(&channel->irq_pending, 0, 1) == 0);
 	qeth_prepare_control_data(card, len, iob);
 	QETH_CARD_TEXT(card, 6, "osnoirqp");
-	spin_lock_irqsave(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
 	rc = ccw_device_start_timeout(channel->ccwdev, channel->ccw,
 				      (addr_t) iob, 0, 0, QETH_IPA_TIMEOUT);
-	spin_unlock_irqrestore(get_ccwdev_lock(channel->ccwdev), flags);
+	spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
 	if (rc) {
 		QETH_DBF_MESSAGE(2, "qeth_osn_send_control_data: "
 			   "ccw_device_start rc = %i\n", rc);
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 15/15] s390/qeth: reduce 0-initializing when building IPA cmds
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

qeth_get_ipacmd_buffer() obtains its buffers for building IPA cmds from
__qeth_get_buffer(), where they are fully cleared. So get rid of all the
additional zero-ing in various other places.

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

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 100b6f4a3fb8..89e09e7b8fff 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -780,7 +780,6 @@ void qeth_release_buffer(struct qeth_channel *channel,
 
 	QETH_CARD_TEXT(CARD_FROM_CDEV(channel->ccwdev), 6, "relbuff");
 	spin_lock_irqsave(&channel->iob_lock, flags);
-	memset(iob->data, 0, QETH_BUFSIZE);
 	iob->state = BUF_STATE_FREE;
 	iob->callback = qeth_send_control_data_cb;
 	iob->rc = 0;
@@ -1334,8 +1333,8 @@ static int qeth_setup_channel(struct qeth_channel *channel, bool alloc_buffers)
 		return 0;
 
 	for (cnt = 0; cnt < QETH_CMD_BUFFER_NO; cnt++) {
-		channel->iob[cnt].data =
-			kzalloc(QETH_BUFSIZE, GFP_DMA|GFP_KERNEL);
+		channel->iob[cnt].data = kmalloc(QETH_BUFSIZE,
+						 GFP_KERNEL | GFP_DMA);
 		if (channel->iob[cnt].data == NULL)
 			break;
 		channel->iob[cnt].state = BUF_STATE_FREE;
@@ -2888,10 +2887,10 @@ static __u8 qeth_get_ipa_adp_type(enum qeth_link_types link_type)
 }
 
 static void qeth_fill_ipacmd_header(struct qeth_card *card,
-		struct qeth_ipa_cmd *cmd, __u8 command,
-		enum qeth_prot_versions prot)
+				    struct qeth_ipa_cmd *cmd,
+				    enum qeth_ipa_cmds command,
+				    enum qeth_prot_versions prot)
 {
-	memset(cmd, 0, sizeof(struct qeth_ipa_cmd));
 	cmd->hdr.command = command;
 	cmd->hdr.initiator = IPA_CMD_INITIATOR_HOST;
 	/* cmd->hdr.seqno is set by qeth_send_control_data() */
@@ -2903,8 +2902,6 @@ static void qeth_fill_ipacmd_header(struct qeth_card *card,
 		cmd->hdr.prim_version_no = 1;
 	cmd->hdr.param_count = 1;
 	cmd->hdr.prot_version = prot;
-	cmd->hdr.ipa_supported = 0;
-	cmd->hdr.ipa_enabled = 0;
 }
 
 struct qeth_cmd_buffer *qeth_get_ipacmd_buffer(struct qeth_card *card,
@@ -5494,8 +5491,6 @@ struct qeth_cmd_buffer *qeth_get_setassparms_cmd(struct qeth_card *card,
 		cmd->data.setassparms.hdr.assist_no = ipa_func;
 		cmd->data.setassparms.hdr.length = 8 + len;
 		cmd->data.setassparms.hdr.command_code = cmd_code;
-		cmd->data.setassparms.hdr.return_code = 0;
-		cmd->data.setassparms.hdr.seq_no = 0;
 	}
 
 	return iob;
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 12/15] s390/qeth: invoke softirqs after napi_schedule()
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

Calling napi_schedule() from process context does not ensure that the
NET_RX softirq is run in a timely fashion. So trigger it manually.

This is no big issue with current code. A call to ndo_open() is usually
followed by a ndo_set_rx_mode() call, and for qeth this contains a
spin_unlock_bh(). Except for OSN, where qeth_l2_set_rx_mode() bails out
early.
Nevertheless it's best to not depend on this behaviour, and just fix
the issue at its source like all other drivers do. For instance see
commit 83a0c6e58901 ("i40e: Invoke softirqs after napi_reschedule").

Fixes: a1c3ed4c9ca0 ("qeth: NAPI support for l2 and l3 discipline")
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_l2_main.c | 3 +++
 drivers/s390/net/qeth_l3_main.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 33b65471a68a..6285af373bdf 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -755,7 +755,10 @@ static int __qeth_l2_open(struct net_device *dev)
 
 	if (qdio_stop_irq(card->data.ccwdev, 0) >= 0) {
 		napi_enable(&card->napi);
+		local_bh_disable();
 		napi_schedule(&card->napi);
+		/* kick-start the NAPI softirq: */
+		local_bh_enable();
 	} else
 		rc = -EIO;
 	return rc;
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 5d7e2921ab36..8930d2a9fcad 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2293,7 +2293,10 @@ static int __qeth_l3_open(struct net_device *dev)
 
 	if (qdio_stop_irq(card->data.ccwdev, 0) >= 0) {
 		napi_enable(&card->napi);
+		local_bh_disable();
 		napi_schedule(&card->napi);
+		/* kick-start the NAPI softirq: */
+		local_bh_enable();
 	} else
 		rc = -EIO;
 	return rc;
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 08/15] s390/qeth: prepare for copy-free TSO transmission
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

Add all the necessary TSO plumbing to the copy-less transmit path.
This includes calculating the right length of required protocol headers,
and always building a separate buffer element for the TSO headers.

A follow-up patch will then switch TSO traffic over to this path.

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

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index b47fb95a49e9..d86eea9db2a7 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -26,6 +26,7 @@
 #include <net/ipv6.h>
 #include <net/if_inet6.h>
 #include <net/addrconf.h>
+#include <net/tcp.h>
 
 #include <asm/debug.h>
 #include <asm/qdio.h>
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 79ebe8a5687b..460ffdf1b200 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -3845,7 +3845,8 @@ int qeth_hdr_chk_and_bounce(struct sk_buff *skb, struct qeth_hdr **hdr, int len)
 }
 EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
 
-#define QETH_HDR_CACHE_OBJ_SIZE		(sizeof(struct qeth_hdr) + ETH_HLEN)
+#define QETH_HDR_CACHE_OBJ_SIZE		(sizeof(struct qeth_hdr_tso) + \
+					 MAX_TCP_HEADER)
 
 /**
  * qeth_add_hw_header() - add a HW header to an skb.
@@ -3880,7 +3881,11 @@ int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
 	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);
+		/* ... but TSO always needs a separate element for headers: */
+		if (skb_is_gso(skb))
+			__elements = 1 + qeth_count_elements(skb, proto_len);
+		else
+			__elements = qeth_count_elements(skb, 0);
 	} else if (!proto_len && qeth_get_elements_for_range(start, end) == 1) {
 		/* Push HW header into a new page. */
 		push_ok = true;
@@ -4193,6 +4198,7 @@ int qeth_xmit(struct qeth_card *card, struct sk_buff *skb,
 		hd_len = hw_hdr_len + proto_len;
 		data_offset = proto_len;
 	}
+	memset(hdr, 0, hw_hdr_len);
 	fill_header(card, hdr, skb, ipv, cast_type, frame_len);
 
 	is_sg = skb_is_nonlinear(skb);
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 87cb71d5dae8..24b531ca2827 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -199,7 +199,6 @@ static void qeth_l2_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
 {
 	struct vlan_ethhdr *veth = (struct vlan_ethhdr *)skb_mac_header(skb);
 
-	memset(hdr, 0, sizeof(struct qeth_hdr));
 	hdr->hdr.l2.id = QETH_HEADER_TYPE_LAYER2;
 	hdr->hdr.l2.pkt_length = data_len;
 
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 0c085f57f7e2..fe55218802d7 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2008,7 +2008,6 @@ static void qeth_l3_fill_af_iucv_hdr(struct qeth_hdr *hdr, struct sk_buff *skb,
 	char daddr[16];
 	struct af_iucv_trans_hdr *iucv_hdr;
 
-	memset(hdr, 0, sizeof(struct qeth_hdr));
 	hdr->hdr.l3.id = QETH_HEADER_TYPE_LAYER3;
 	hdr->hdr.l3.length = data_len;
 	hdr->hdr.l3.flags = QETH_HDR_IPV6 | QETH_CAST_UNICAST;
@@ -2038,10 +2037,22 @@ static void qeth_l3_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
 {
 	struct vlan_ethhdr *veth = vlan_eth_hdr(skb);
 
-	memset(hdr, 0, sizeof(struct qeth_hdr));
-	hdr->hdr.l3.id = QETH_HEADER_TYPE_LAYER3;
 	hdr->hdr.l3.length = data_len;
 
+	if (skb_is_gso(skb)) {
+		hdr->hdr.l3.id = QETH_HEADER_TYPE_TSO;
+	} else {
+		hdr->hdr.l3.id = QETH_HEADER_TYPE_LAYER3;
+		if (skb->ip_summed == CHECKSUM_PARTIAL) {
+			qeth_tx_csum(skb, &hdr->hdr.l3.ext_flags, ipv);
+			/* some HW requires combined L3+L4 csum offload: */
+			if (ipv == 4)
+				hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_HDR_REQ;
+			if (card->options.performance_stats)
+				card->perf_stats.tx_csum++;
+		}
+	}
+
 	if (ipv == 4 || IS_IQD(card)) {
 		/* NETIF_F_HW_VLAN_CTAG_TX */
 		if (skb_vlan_tag_present(skb)) {
@@ -2053,15 +2064,6 @@ static void qeth_l3_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
 		hdr->hdr.l3.vlan_id = ntohs(veth->h_vlan_TCI);
 	}
 
-	if (!skb_is_gso(skb) && skb->ip_summed == CHECKSUM_PARTIAL) {
-		qeth_tx_csum(skb, &hdr->hdr.l3.ext_flags, ipv);
-		/* some HW requires combined L3+L4 csum offload: */
-		if (ipv == 4)
-			hdr->hdr.l3.ext_flags |= QETH_HDR_EXT_CSUM_HDR_REQ;
-		if (card->options.performance_stats)
-			card->perf_stats.tx_csum++;
-	}
-
 	/* OSA only: */
 	if (!ipv) {
 		hdr->hdr.l3.flags = QETH_HDR_PASSTHRU;
@@ -2100,6 +2102,22 @@ static void qeth_l3_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
 	rcu_read_unlock();
 }
 
+static void qeth_l3_fill_tso_ext(struct qeth_hdr_tso *hdr,
+				 unsigned int payload_len, struct sk_buff *skb,
+				 unsigned int proto_len)
+{
+	struct qeth_hdr_ext_tso *ext = &hdr->ext;
+
+	ext->hdr_tot_len = sizeof(*ext);
+	ext->imb_hdr_no = 1;
+	ext->hdr_type = 1;
+	ext->hdr_version = 1;
+	ext->hdr_len = 28;
+	ext->payload_len = payload_len;
+	ext->mss = skb_shinfo(skb)->gso_size;
+	ext->dg_hdr_len = proto_len;
+}
+
 static void qeth_tso_fill_header(struct qeth_card *card,
 		struct qeth_hdr *qhdr, struct sk_buff *skb)
 {
@@ -2108,8 +2126,6 @@ static void qeth_tso_fill_header(struct qeth_card *card,
 	struct iphdr *iph = ip_hdr(skb);
 	struct ipv6hdr *ip6h = ipv6_hdr(skb);
 
-	/*fix header to TSO values ...*/
-	hdr->hdr.hdr.l3.id = QETH_HEADER_TYPE_TSO;
 	/*set values which are fix for the first approach ...*/
 	hdr->ext.hdr_tot_len = (__u16) sizeof(struct qeth_hdr_ext_tso);
 	hdr->ext.imb_hdr_no  = 1;
@@ -2173,20 +2189,35 @@ static void qeth_l3_fixup_headers(struct sk_buff *skb)
 	/* this is safe, IPv6 traffic takes a different path */
 	if (skb->ip_summed == CHECKSUM_PARTIAL)
 		iph->check = 0;
+	if (skb_is_gso(skb)) {
+		iph->tot_len = 0;
+		tcp_hdr(skb)->check = ~tcp_v4_check(0, iph->saddr,
+						    iph->daddr, 0);
+	}
 }
 
 static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 				struct qeth_qdio_out_q *queue, int ipv,
 				int cast_type)
 {
-	const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
-	unsigned int frame_len, elements;
+	unsigned int hw_hdr_len, proto_len, frame_len, elements;
 	unsigned char eth_hdr[ETH_HLEN];
+	bool is_tso = skb_is_gso(skb);
+	unsigned int data_offset = 0;
 	struct qeth_hdr *hdr = NULL;
 	unsigned int hd_len = 0;
 	int push_len, rc;
 	bool is_sg;
 
+	if (is_tso) {
+		hw_hdr_len = sizeof(struct qeth_hdr_tso);
+		proto_len = skb_transport_offset(skb) + tcp_hdrlen(skb) -
+			    ETH_HLEN;
+	} else {
+		hw_hdr_len = sizeof(struct qeth_hdr);
+		proto_len = 0;
+	}
+
 	/* re-use the L2 header area for the HW header: */
 	rc = skb_cow_head(skb, hw_hdr_len - ETH_HLEN);
 	if (rc)
@@ -2196,28 +2227,36 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 	frame_len = skb->len;
 
 	qeth_l3_fixup_headers(skb);
-	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, 0,
+	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, proto_len,
 				      &elements);
 	if (push_len < 0)
 		return push_len;
-	if (!push_len) {
-		/* hdr was added discontiguous from skb->data */
-		hd_len = hw_hdr_len;
+	if (is_tso || !push_len) {
+		/* HW header needs its own buffer element. */
+		hd_len = hw_hdr_len + proto_len;
+		data_offset = push_len + proto_len;
 	}
+	memset(hdr, 0, hw_hdr_len);
 
-	if (skb->protocol == htons(ETH_P_AF_IUCV))
+	if (skb->protocol == htons(ETH_P_AF_IUCV)) {
 		qeth_l3_fill_af_iucv_hdr(hdr, skb, frame_len);
-	else
+	} else {
 		qeth_l3_fill_header(card, hdr, skb, ipv, cast_type, frame_len);
+		if (is_tso)
+			qeth_l3_fill_tso_ext((struct qeth_hdr_tso *) hdr,
+					     frame_len - proto_len, skb,
+					     proto_len);
+	}
 
 	is_sg = skb_is_nonlinear(skb);
 	if (IS_IQD(card)) {
-		rc = qeth_do_send_packet_fast(queue, skb, hdr, 0, hd_len);
+		rc = qeth_do_send_packet_fast(queue, skb, hdr, data_offset,
+					      hd_len);
 	} else {
 		/* TODO: drop skb_orphan() once TX completion is fast enough */
 		skb_orphan(skb);
-		rc = qeth_do_send_packet(card, queue, skb, hdr, 0, hd_len,
-					 elements);
+		rc = qeth_do_send_packet(card, queue, skb, hdr, data_offset,
+					 hd_len, elements);
 	}
 
 	if (!rc) {
@@ -2225,6 +2264,10 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
 			card->perf_stats.buf_elements_sent += elements;
 			if (is_sg)
 				card->perf_stats.sg_skbs_sent++;
+			if (is_tso) {
+				card->perf_stats.large_send_bytes += frame_len;
+				card->perf_stats.large_send_cnt++;
+			}
 		}
 	} else {
 		if (!push_len)
-- 
2.16.4

^ permalink raw reply related

* Re: [PATCH iproute2] libnetlink: fix leak and using unused memory on error
From: Stephen Hemminger @ 2018-09-17 15:37 UTC (permalink / raw)
  To: Mahesh Bandewar; +Cc: netdev
In-Reply-To: <20180913193338.20233-1-stephen@networkplumber.org>

On Thu, 13 Sep 2018 12:33:38 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> If an error happens in multi-segment message (tc only)
> then report the error and stop processing further responses.
> This also fixes refering to the buffer after free.
> 
> The sequence check is not necessary here because the
> response message has already been validated to be in
> the window of the sequence number of the iov.
> 
> Reported-by: Mahesh Bandewar <mahesh@bandewar.net>
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Bad habit of working on two projects as once.

^ permalink raw reply

* [PATCH net-next 07/15] s390/qeth: check size of required HW header cache object
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

When qeth_add_hw_header() falls back to the header cache, ensure that
the requested length doesn't exceed the object size.

For current usage this is a no-brainer, but TSO transmission will
introduce protocol headers of varying length.

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

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index eaf01dc62e91..79ebe8a5687b 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -16,6 +16,7 @@
 #include <linux/string.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
+#include <linux/log2.h>
 #include <linux/ip.h>
 #include <linux/tcp.h>
 #include <linux/mii.h>
@@ -3844,6 +3845,8 @@ int qeth_hdr_chk_and_bounce(struct sk_buff *skb, struct qeth_hdr **hdr, int len)
 }
 EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
 
+#define QETH_HDR_CACHE_OBJ_SIZE		(sizeof(struct qeth_hdr) + ETH_HLEN)
+
 /**
  * qeth_add_hw_header() - add a HW header to an skb.
  * @skb: skb that the HW header should be added to.
@@ -3918,6 +3921,8 @@ int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
 		return hdr_len;
 	}
 	/* fall back */
+	if (hdr_len + proto_len > QETH_HDR_CACHE_OBJ_SIZE)
+		return -E2BIG;
 	*hdr = kmem_cache_alloc(qeth_core_header_cache, GFP_ATOMIC);
 	if (!*hdr)
 		return -ENOMEM;
@@ -6661,8 +6666,10 @@ static int __init qeth_core_init(void)
 	rc = PTR_ERR_OR_ZERO(qeth_core_root_dev);
 	if (rc)
 		goto register_err;
-	qeth_core_header_cache = kmem_cache_create("qeth_hdr",
-			sizeof(struct qeth_hdr) + ETH_HLEN, 64, 0, NULL);
+	qeth_core_header_cache =
+		kmem_cache_create("qeth_hdr", QETH_HDR_CACHE_OBJ_SIZE,
+				  roundup_pow_of_two(QETH_HDR_CACHE_OBJ_SIZE),
+				  0, NULL);
 	if (!qeth_core_header_cache) {
 		rc = -ENOMEM;
 		goto slab_err;
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 01/15] s390/qeth: move L2 xmit code to core module
From: Julian Wiedmann @ 2018-09-17 15:35 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

We need the exact same transmit path for non-offload-eligible traffic on
L3 OSAs. So make it accessible from both sub-drivers.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      |  5 +++
 drivers/s390/net/qeth_core_main.c | 59 +++++++++++++++++++++++++++++++
 drivers/s390/net/qeth_l2_main.c   | 74 ++++++---------------------------------
 3 files changed, 75 insertions(+), 63 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 34e0d476c5c6..2110fabdcc7a 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -1052,6 +1052,11 @@ 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 hdr_len,
 		       unsigned int proto_len, unsigned int *elements);
+int qeth_xmit(struct qeth_card *card, struct sk_buff *skb,
+	      struct qeth_qdio_out_q *queue, int ipv, int cast_type,
+	      void (*fill_header)(struct qeth_card *card, struct qeth_hdr *hdr,
+				  struct sk_buff *skb, int ipv, int cast_type,
+				  unsigned int data_len));
 
 /* exports for OSN */
 int qeth_osn_assist(struct net_device *, void *, int);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index de8282420f96..d2ca33a9330a 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4176,6 +4176,65 @@ int qeth_do_send_packet(struct qeth_card *card, struct qeth_qdio_out_q *queue,
 }
 EXPORT_SYMBOL_GPL(qeth_do_send_packet);
 
+int qeth_xmit(struct qeth_card *card, struct sk_buff *skb,
+	      struct qeth_qdio_out_q *queue, int ipv, int cast_type,
+	      void (*fill_header)(struct qeth_card *card, struct qeth_hdr *hdr,
+				  struct sk_buff *skb, int ipv, int cast_type,
+				  unsigned int data_len))
+{
+	const unsigned int proto_len = IS_IQD(card) ? ETH_HLEN : 0;
+	const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
+	unsigned int frame_len = skb->len;
+	unsigned int data_offset = 0;
+	struct qeth_hdr *hdr = NULL;
+	unsigned int hd_len = 0;
+	unsigned int elements;
+	int push_len, rc;
+	bool is_sg;
+
+	rc = skb_cow_head(skb, hw_hdr_len);
+	if (rc)
+		return rc;
+
+	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, proto_len,
+				      &elements);
+	if (push_len < 0)
+		return push_len;
+	if (!push_len) {
+		/* HW header needs its own buffer element. */
+		hd_len = hw_hdr_len + proto_len;
+		data_offset = proto_len;
+	}
+	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, data_offset,
+					      hd_len);
+	} else {
+		/* TODO: drop skb_orphan() once TX completion is fast enough */
+		skb_orphan(skb);
+		rc = qeth_do_send_packet(card, queue, skb, hdr, data_offset,
+					 hd_len, elements);
+	}
+
+	if (!rc) {
+		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)
+			kmem_cache_free(qeth_core_header_cache, hdr);
+		if (rc == -EBUSY)
+			/* roll back to ETH header */
+			skb_pull(skb, push_len);
+	}
+	return rc;
+}
+EXPORT_SYMBOL_GPL(qeth_xmit);
+
 static int qeth_setadp_promisc_mode_cb(struct qeth_card *card,
 		struct qeth_reply *reply, unsigned long data)
 {
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index b5e38531733f..715d58af5fc4 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -193,8 +193,9 @@ static int qeth_l2_get_cast_type(struct qeth_card *card, struct sk_buff *skb)
 	return RTN_UNICAST;
 }
 
-static void qeth_l2_fill_header(struct qeth_hdr *hdr, struct sk_buff *skb,
-				int cast_type, unsigned int data_len)
+static void qeth_l2_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
+				struct sk_buff *skb, int ipv, int cast_type,
+				unsigned int data_len)
 {
 	struct vlan_ethhdr *veth = (struct vlan_ethhdr *)skb_mac_header(skb);
 
@@ -202,6 +203,12 @@ static void qeth_l2_fill_header(struct qeth_hdr *hdr, struct sk_buff *skb,
 	hdr->hdr.l2.id = QETH_HEADER_TYPE_LAYER2;
 	hdr->hdr.l2.pkt_length = data_len;
 
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		qeth_tx_csum(skb, &hdr->hdr.l2.flags[1], ipv);
+		if (card->options.performance_stats)
+			card->perf_stats.tx_csum++;
+	}
+
 	/* set byte byte 3 to casting flags */
 	if (cast_type == RTN_MULTICAST)
 		hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_MULTICAST;
@@ -641,66 +648,6 @@ static void qeth_l2_set_rx_mode(struct net_device *dev)
 		qeth_promisc_to_bridge(card);
 }
 
-static int qeth_l2_xmit(struct qeth_card *card, struct sk_buff *skb,
-			struct qeth_qdio_out_q *queue, int cast_type, int ipv)
-{
-	const unsigned int proto_len = IS_IQD(card) ? ETH_HLEN : 0;
-	const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
-	unsigned int frame_len = skb->len;
-	unsigned int data_offset = 0;
-	struct qeth_hdr *hdr = NULL;
-	unsigned int hd_len = 0;
-	unsigned int elements;
-	int push_len, rc;
-	bool is_sg;
-
-	rc = skb_cow_head(skb, hw_hdr_len);
-	if (rc)
-		return rc;
-
-	push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, proto_len,
-				      &elements);
-	if (push_len < 0)
-		return push_len;
-	if (!push_len) {
-		/* HW header needs its own buffer element. */
-		hd_len = hw_hdr_len + proto_len;
-		data_offset = proto_len;
-	}
-	qeth_l2_fill_header(hdr, skb, cast_type, frame_len);
-	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		qeth_tx_csum(skb, &hdr->hdr.l2.flags[1], ipv);
-		if (card->options.performance_stats)
-			card->perf_stats.tx_csum++;
-	}
-
-	is_sg = skb_is_nonlinear(skb);
-	if (IS_IQD(card)) {
-		rc = qeth_do_send_packet_fast(queue, skb, hdr, data_offset,
-					      hd_len);
-	} else {
-		/* TODO: drop skb_orphan() once TX completion is fast enough */
-		skb_orphan(skb);
-		rc = qeth_do_send_packet(card, queue, skb, hdr, data_offset,
-					 hd_len, elements);
-	}
-
-	if (!rc) {
-		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)
-			kmem_cache_free(qeth_core_header_cache, hdr);
-		if (rc == -EBUSY)
-			/* roll back to ETH header */
-			skb_pull(skb, push_len);
-	}
-	return rc;
-}
-
 static int qeth_l2_xmit_osn(struct qeth_card *card, struct sk_buff *skb,
 			    struct qeth_qdio_out_q *queue)
 {
@@ -745,7 +692,8 @@ static netdev_tx_t qeth_l2_hard_start_xmit(struct sk_buff *skb,
 	if (IS_OSN(card))
 		rc = qeth_l2_xmit_osn(card, skb, queue);
 	else
-		rc = qeth_l2_xmit(card, skb, queue, cast_type, ipv);
+		rc = qeth_xmit(card, skb, queue, ipv, cast_type,
+			       qeth_l2_fill_header);
 
 	if (!rc) {
 		card->stats.tx_packets++;
-- 
2.16.4

^ permalink raw reply related

* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
From: Andrew Lunn @ 2018-09-17 15:38 UTC (permalink / raw)
  To: Simon Horman; +Cc: David Miller, netdev, Florian Fainelli
In-Reply-To: <20180917151302.l6mzzy5xmqlbgejj@verge.net.au>

On Mon, Sep 17, 2018 at 05:13:07PM +0200, Simon Horman wrote:
> On Wed, Sep 12, 2018 at 01:53:14AM +0200, Andrew Lunn wrote:
> > Some MAC hardware cannot support a subset of link modes. e.g. often
> > 1Gbps Full duplex is supported, but Half duplex is not. Add a helper
> > to remove such a link mode.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> >  drivers/net/ethernet/apm/xgene/xgene_enet_hw.c |  6 +++---
> >  drivers/net/ethernet/cadence/macb_main.c       |  5 ++---
> >  drivers/net/ethernet/freescale/fec_main.c      |  3 ++-
> >  drivers/net/ethernet/microchip/lan743x_main.c  |  2 +-
> >  drivers/net/ethernet/renesas/ravb_main.c       |  3 ++-
> >  .../net/ethernet/stmicro/stmmac/stmmac_main.c  | 12 ++++++++----
> >  drivers/net/phy/phy_device.c                   | 18 ++++++++++++++++++
> >  drivers/net/usb/lan78xx.c                      |  2 +-
> >  include/linux/phy.h                            |  1 +
> >  9 files changed, 38 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
> > index 078a04dc1182..4831f9de5945 100644
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> > index aff5516b781e..fb2a1125780d 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
> >  	}
> >  
> >  	/* 10BASE is not supported */
> > -	phydev->supported &= ~PHY_10BT_FEATURES;
> > +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> > +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
> >  
> >  	phy_attached_info(phydev);
> >  
> 
> ...
> 
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index db1172db1e7c..e9ca83a438b0 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
> >  }
> >  EXPORT_SYMBOL(phy_set_max_speed);
> >  
> > +/**
> > + * phy_remove_link_mode - Remove a supported link mode
> > + * @phydev: phy_device structure to remove link mode from
> > + * @link_mode: Link mode to be removed
> > + *
> > + * Description: Some MACs don't support all link modes which the PHY
> > + * does.  e.g. a 1G MAC often does not support 1000Half. Add a helper
> > + * to remove a link mode.
> > + */
> > +void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
> > +{
> > +	WARN_ON(link_mode > 31);
> > +
> > +	phydev->supported &= ~BIT(link_mode);
> > +	phydev->advertising = phydev->supported;
> > +}
> > +EXPORT_SYMBOL(phy_remove_link_mode);
> > +
> >  static void of_set_phy_supported(struct phy_device *phydev)
> >  {
> >  	struct device_node *node = phydev->mdio.dev.of_node;
> 
> Hi Andrew,
> 
> I believe that for the RAVB the overall effect of this change is that
> 10-BaseT modes are no longer advertised (although both with and without
> this patch they are not supported).
> 
> Unfortunately on R-Car Gen3 M3-W (r8a7796) based Salvator-X board
> I have observed that this results in the link no longer being negotiated
> on one switch (the one I usually use) while it seemed fine on another.

Hi Simon

Thanks for testing this.

Could you dump the PHY registers with and without this patch:

$ mii-tool -vv eth0

Once difference is that phy_remove_link_mode() does
phydev->advertising = phydev->supported where as the old code does
not. I though phylib would do this anyway, it does at some point in
time, but i didn't check when. It could be you are actually
advertising 10, even if you don't support it.

Thanks
	Andrew

^ permalink raw reply

* Re: [PATCH] net: phy: phylink: fix SFP interface autodetection
From: Baruch Siach @ 2018-09-17 15:39 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Andrew Lunn, Florian Fainelli, netdev, Antoine Tenart,
	Gregory CLEMENT
In-Reply-To: <20180917151230.GF30658@n2100.armlinux.org.uk>

Hi Russell,

Russell King - ARM Linux writes:

> On Mon, Sep 17, 2018 at 05:19:57PM +0300, Baruch Siach wrote:
>> When the switching to the SFP detected link mode update the main
>> link_interface field as well. Otherwise, the link fails to come up when
>> the configured 'phy-mode' defers from the SFP detected mode.
>> 
>> This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
>> is configured in the DT to "2500base-x" phy-mode.
>
> link_interface isn't supposed to track the SFP link mode.  In any case,
> this is only used when a PHY is attached.  For a PHY on a SFP,
> phylink_connect_phy() should be using link_config.interface and not
> link_interface there.

You mean something like this?

@@ -750,7 +750,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 		pl->link_config.interface = pl->link_interface;
 	}
 
-	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_interface);
+	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_config.interface);
 	if (ret)
 		return ret;
 
This fixes a similar issue on the Armada 8040 based Clearfog GT-8K where
the SFP interface phy-mode is set to "10gbase-kr". But the Macchiatobin
PHY-less interface has phy-mode set to "2500base-x", so I hit a separate
problem:

[   49.599455] WARNING: CPU: 1 PID: 1223 at drivers/net/phy/phylink.c:741 phylink_connect_phy+0x34/0xb4
[   49.608629] CPU: 1 PID: 1223 Comm: ifconfig Not tainted 4.19.0-rc4-00006-g18cf7eb4b625-dirty #952
[   49.617537] Hardware name: Marvell 8040 MACCHIATOBin (DT)
[   49.622957] pstate: 60000005 (nZCv daif -PAN -UAO)
[   49.627767] pc : phylink_connect_phy+0x34/0xb4
[   49.632229] lr : phylink_sfp_connect_phy+0xc/0x14
[   49.636950] sp : ffff00000b273830
[   49.640276] x29: ffff00000b273830 x28: ffff80007a342df0 
[   49.645610] x27: 00000000000004b0 x26: 0000000000000004 
[   49.650945] x25: ffff80007a342940 x24: ffff80007b90cc18 
[   49.656278] x23: ffff8000799b2538 x22: ffff000008bda000 
[   49.661613] x21: ffff80007a342000 x20: ffff80007bca4000 
[   49.666947] x19: ffff80007b814a80 x18: 000000000000000a 
[   49.672282] x17: 0000000000000000 x16: 0000000000000004 
[   49.677616] x15: 0000000000000416 x14: 0000000000000400 
[   49.682951] x13: 0000000000000400 x12: 0000000000000000 
[   49.688284] x11: 0000000000000000 x10: 0000000000000000 
[   49.693618] x9 : 00000000000002c6 x8 : 0000000000000000 
[   49.698952] x7 : ffff80007ff88b40 x6 : 0000000000000001 
[   49.704286] x5 : 0000000000000003 x4 : ffff80007bca4048 
[   49.709621] x3 : 0000000000000004 x2 : 0000000000000001 
[   49.714955] x1 : ffff80007bca4000 x0 : ffff80007a337400 
[   49.720289] Call trace:
[   49.722745]  phylink_connect_phy+0x34/0xb4
[   49.726857]  phylink_sfp_connect_phy+0xc/0x14
[   49.731231]  sfp_add_phy+0x48/0x50
[   49.734647]  sfp_sm_event+0x6e0/0x86c
[   49.738323]  sfp_start+0x10/0x18
[   49.741563]  sfp_upstream_start+0x28/0x3c
[   49.745588]  phylink_start+0x120/0x164
[   49.749352]  mvpp2_start_dev+0x184/0x260
[   49.753290]  mvpp2_open+0x394/0x3e8
[   49.756793]  __dev_open+0x110/0x124
[   49.760294]  __dev_change_flags+0xe8/0x190
[   49.764406]  dev_change_flags+0x20/0x5c
[   49.768258]  devinet_ioctl+0x270/0x528
[   49.772020]  inet_ioctl+0x13c/0x2e4
[   49.775524]  sock_do_ioctl+0x4c/0x1f4
[   49.779200]  sock_ioctl+0xf4/0x360
[   49.782616]  vfs_ioctl+0x24/0x40
[   49.785855]  do_vfs_ioctl+0xa4/0x7d4
[   49.789443]  ksys_ioctl+0x44/0x74
[   49.792770]  __arm64_sys_ioctl+0x18/0x24
[   49.796708]  el0_svc_common+0x8c/0xd0
[   49.800384]  el0_svc_handler+0x3c/0x6c
[   49.804148]  el0_svc+0x8/0xc
[   49.807039] ---[ end trace 0d1e0898561fce6e ]---
[   49.811729] sfp sfp-eth3: sfp_add_phy failed: -22

This hunk fixes that one:

@@ -738,7 +738,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 
 	if (WARN_ON(pl->link_an_mode == MLO_AN_FIXED ||
 		    (pl->link_an_mode == MLO_AN_INBAND &&
-		     phy_interface_mode_is_8023z(pl->link_interface))))
+		     phy_interface_mode_is_8023z(pl->link_config.interface))))
 		return -EINVAL;
 
 	if (pl->phydev)

Is that the correct fix?

Thanks,
baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply

* KASAN: slab-out-of-bounds Read in ip6_tnl_parse_tlv_enc_lim
From: syzbot @ 2018-09-17 21:23 UTC (permalink / raw)
  To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji

Hello,

syzbot found the following crash on:

HEAD commit:    3a5af36b6d0e Merge tag '4.19-rc3-smb3-cifs' of git://git.s..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15c727be400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=9c4a80625153107e
dashboard link: https://syzkaller.appspot.com/bug?extid=68dce7caebd8543121de
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1068a44e400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=146386c6400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+68dce7caebd8543121de@syzkaller.appspotmail.com

IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
==================================================================
BUG: KASAN: slab-out-of-bounds in ip6_tnl_parse_tlv_enc_lim+0x5df/0x660  
net/ipv6/ip6_tunnel.c:417
Read of size 1 at addr ffff8801d21187c7 by task syz-executor281/5345

CPU: 1 PID: 5345 Comm: syz-executor281 Not tainted 4.19.0-rc3+ #238
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
  print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
  __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
  ip6_tnl_parse_tlv_enc_lim+0x5df/0x660 net/ipv6/ip6_tunnel.c:417
  ip6ip6_tnl_xmit net/ipv6/ip6_tunnel.c:1339 [inline]
  ip6_tnl_start_xmit+0x3e2/0x2370 net/ipv6/ip6_tunnel.c:1403
  __netdev_start_xmit include/linux/netdevice.h:4287 [inline]
  netdev_start_xmit include/linux/netdevice.h:4296 [inline]
  xmit_one net/core/dev.c:3216 [inline]
  dev_hard_start_xmit+0x27f/0xc70 net/core/dev.c:3232
  __dev_queue_xmit+0x2f3b/0x3980 net/core/dev.c:3802
  dev_queue_xmit+0x17/0x20 net/core/dev.c:3835
  __bpf_tx_skb net/core/filter.c:2012 [inline]
  __bpf_redirect_common net/core/filter.c:2050 [inline]
  __bpf_redirect+0x5cf/0xb20 net/core/filter.c:2057
  ____bpf_clone_redirect net/core/filter.c:2090 [inline]
  bpf_clone_redirect+0x2f6/0x490 net/core/filter.c:2062
  bpf_prog_759a992c578a3894+0x741/0x1000

Allocated by task 5345:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
  __do_kmalloc_node mm/slab.c:3682 [inline]
  __kmalloc_node_track_caller+0x47/0x70 mm/slab.c:3696
  __kmalloc_reserve.isra.39+0x41/0xe0 net/core/skbuff.c:137
  pskb_expand_head+0x230/0x10f0 net/core/skbuff.c:1460
  skb_ensure_writable+0x3dd/0x640 net/core/skbuff.c:5126
  __bpf_try_make_writable net/core/filter.c:1633 [inline]
  bpf_try_make_writable net/core/filter.c:1639 [inline]
  bpf_try_make_head_writable net/core/filter.c:1647 [inline]
  ____bpf_clone_redirect net/core/filter.c:2084 [inline]
  bpf_clone_redirect+0x14a/0x490 net/core/filter.c:2062
  bpf_prog_759a992c578a3894+0x741/0x1000

Freed by task 3895:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
  __cache_free mm/slab.c:3498 [inline]
  kfree+0xcf/0x230 mm/slab.c:3813
  load_elf_binary+0x25b4/0x5620 fs/binfmt_elf.c:1118
  search_binary_handler+0x17d/0x570 fs/exec.c:1653
  exec_binprm fs/exec.c:1695 [inline]
  __do_execve_file.isra.33+0x162f/0x2540 fs/exec.c:1819
  do_execveat_common fs/exec.c:1866 [inline]
  do_execve fs/exec.c:1883 [inline]
  __do_sys_execve fs/exec.c:1964 [inline]
  __se_sys_execve fs/exec.c:1959 [inline]
  __x64_sys_execve+0x8f/0xc0 fs/exec.c:1959
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff8801d21185c0
  which belongs to the cache kmalloc-512 of size 512
The buggy address is located 7 bytes to the right of
  512-byte region [ffff8801d21185c0, ffff8801d21187c0)
The buggy address belongs to the page:
page:ffffea0007484600 count:1 mapcount:0 mapping:ffff8801da800940 index:0x0
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffffea0007484008 ffffea0007487e88 ffff8801da800940
raw: 0000000000000000 ffff8801d21180c0 0000000100000006 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8801d2118680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff8801d2118700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8801d2118780: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
                                            ^
  ffff8801d2118800: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
  ffff8801d2118880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: What is the best forum (mailing list, irc etc) to ask questions about the usage of AF_XDP sockets.
From: Konrad Djimeli @ 2018-09-17 16:04 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, bjorn.topel
In-Reply-To: <20180913095227.26b7305d@cakuba.netronome.com>

On 2018-09-13 18:52, Jakub Kicinski wrote:
> On Thu, 13 Sep 2018 18:31:55 +0200, Konrad Djimeli wrote:
>> Hello,
>>
>> I have been working on trying to make use of AF_XDP sockets as part of a
>> project I working on, and I have been facing some issues but I am not
>> sure where to ask questions related to the usage of AF_XDP, since this
>> is a development mailing list.
> 
> IMHO AF_XDP is quite fresh so it should be okay to ask questions on
> netdev.  There is also xdp-newbies mailing list which seems very
> appropriate for less advanced questions!

Thanks a lot for the suggestions. Though I am still very new to the
technology, I really enjoy working with AF_XDP and I hope to share my
experience the and queries with the community. Below is link to what I
have been working on, so far.
-
https://github.com/djkonro/snabb/commit/74310142882a5f09487c676f61f75b3bd686783c

Thanks
Konrad
www.djimeli.me

^ permalink raw reply

* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Andy Lutomirski @ 2018-09-17 16:07 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Ard Biesheuvel, Andrew Lutomirski, David S. Miller, andrew,
	Eric Biggers, Greg KH, LKML, Network Development, Samuel Neves,
	Jean-Philippe Aumasson, Linux Crypto Mailing List
In-Reply-To: <CAHmME9o2sB-rQo0DRh56AFe7ZLJJh+CU6Z1m5XZ4tA8vATCy1g@mail.gmail.com>

On Mon, Sep 17, 2018 at 8:32 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Mon, Sep 17, 2018 at 4:52 PM Andy Lutomirski <luto@amacapital.net> wrote:
> > I think the module organization needs to change. It needs to be possible to have chacha20 built in but AES or whatever as a module.
>
> Okay, I'll do that for v5.
>
> > I might have agreed before Spectre :(. Unfortunately, unless we do some magic, I think the code would look something like:
> >
> > if (static_branch_likely(have_simd)) arch_chacha20();
> >
> > ...where arch_chacha20 is a *pointer*. And that will generate a retpoline and run very, very slowly.  (I just rewrote some of the x86 entry code to eliminate one retpoline. I got a 5% speedup on some tests according to the kbuild bot.)
>
> Actually, the way it works now benefits from the compilers inliner and
> the branch predictor. I benchmarked this without any retpoline
> slowdowns, and the branch predictor becomes correct pretty much all
> the time. We can tinker with this after the initial merge, if you
> really want, but avoiding function pointers and instead using ordinary
> branches really winds up being quite fast.

Indeed.  What I'm saying is that you shouldn't refactor it this way
because it will be slow.  I agree it would be conceptually nice to be
able to blacklist a chacha20_x86_64 module to disable the asm, but I
think it would be very hard to get good performance.

--Andy

^ permalink raw reply

* Re: [PATCH net-next 00/15] s390/qeth: updates 2018-09-17
From: David Miller @ 2018-09-17 16:10 UTC (permalink / raw)
  To: jwi; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>

From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Mon, 17 Sep 2018 17:35:54 +0200

> please apply the following patchset to net-next. This brings more restructuring
> of qeth's transmit code (eliminating its last usage of skb_realloc_headroom()),
> and the usual mix of minor improvements & cleanups.

Series applied, thank you.

^ permalink raw reply

* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: Peter Oskolkov @ 2018-09-17 16:11 UTC (permalink / raw)
  To: dsahern; +Cc: davem, netdev
In-Reply-To: <351e6fbc-5356-b5c1-0638-0554843ad833@gmail.com>

On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
>
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 3eed045c65a5..a3902f805305 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
> >
> >  static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
> >  {
> > -     rt->dst.flags |= fib6_info_dst_flags(ort);
> > +     rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
>
> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
> argument is passed to ip6_dst_alloc which is always invoked before
> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.

ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
If the flag assignment is completely removed from ip6_rt_init_dst(),
then DST_NOPOLICY flag will be lost.

Which may be OK, but is more than what this patch tries to solve (do not
copy DST_NOCOUNT flag).

>
> >
> >       if (ort->fib6_flags & RTF_REJECT) {
> >               ip6_rt_init_dst_reject(rt, ort);
> >
>

^ permalink raw reply

* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: David Ahern @ 2018-09-17 16:13 UTC (permalink / raw)
  To: Peter Oskolkov; +Cc: davem, netdev
In-Reply-To: <CAPNVh5f3_GPpvV6nequZ8JNBhMVVOpEsJQ_XRfTN-+eaMPOWpw@mail.gmail.com>

On 9/17/18 9:11 AM, Peter Oskolkov wrote:
> On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 3eed045c65a5..a3902f805305 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
>>>
>>>  static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
>>>  {
>>> -     rt->dst.flags |= fib6_info_dst_flags(ort);
>>> +     rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
>>
>> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
>> argument is passed to ip6_dst_alloc which is always invoked before
>> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
> 
> ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
> one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
> If the flag assignment is completely removed from ip6_rt_init_dst(),
> then DST_NOPOLICY flag will be lost.
> 
> Which may be OK, but is more than what this patch tries to solve (do not
> copy DST_NOCOUNT flag).

After 5+ days mostly offline I just started looking at this problem.
Give me some time to chase down a thought I had from my last response.

^ permalink raw reply

* Re: [PATCH net] net/ipv4: defensive cipso option parsing
From: Paul Moore @ 2018-09-17 16:35 UTC (permalink / raw)
  To: snu; +Cc: netdev, aams, yujuan.qi, stable
In-Reply-To: <20180917151149.22231-1-snu@amazon.com>

On Mon, Sep 17, 2018 at 11:12 AM Stefan Nuernberger <snu@amazon.com> wrote:
> commit 40413955ee26 ("Cipso: cipso_v4_optptr enter infinite loop") fixed
> a possible infinite loop in the IP option parsing of CIPSO. The fix
> assumes that ip_options_compile filtered out all zero length options and
> that no other one-byte options beside IPOPT_END and IPOPT_NOOP exist.
> While this assumption currently holds true, add explicit checks for zero
> length and invalid length options to be safe for the future. Even though
> ip_options_compile should have validated the options, the introduction of
> new one-byte options can still confuse this code without the additional
> checks.
>
> Signed-off-by: Stefan Nuernberger <snu@amazon.com>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> Reviewed-by: Simon Veith <sveith@amazon.de>
> Cc: stable@vger.kernel.org
> ---
>  net/ipv4/cipso_ipv4.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 82178cc69c96..f291b57b8474 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -1512,7 +1512,7 @@ static int cipso_v4_parsetag_loc(const struct cipso_v4_doi *doi_def,
>   *
>   * Description:
>   * Parse the packet's IP header looking for a CIPSO option.  Returns a pointer
> - * to the start of the CIPSO option on success, NULL if one if not found.
> + * to the start of the CIPSO option on success, NULL if one is not found.
>   *
>   */
>  unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> @@ -1522,9 +1522,11 @@ unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
>         int optlen;
>         int taglen;
>
> -       for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 0; ) {
> +       for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 1; ) {
>                 switch (optptr[0]) {
>                 case IPOPT_CIPSO:
> +                       if (!optptr[1] || optptr[1] > optlen)
> +                               return NULL;
>                         return optptr;
>                 case IPOPT_END:
>                         return NULL;
> @@ -1534,6 +1536,10 @@ unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
>                 default:
>                         taglen = optptr[1];
>                 }
> +
> +               if (!taglen || taglen > optlen)
> +                       break;

I tend to think that you reach a point where you simply need to trust
that the stack is doing the right thing and that by the time you hit a
certain point you can safely assume that the packet is well formed,
but I'm not going to fight about that here.

Regardless of the above, I don't like how you're doing the option
length check twice in this code, that looks ugly to me, I think we can
do better.  How about something like this:

  for (...) {
    switch(optptr[0]) {
    case IPOPT_END:
      return NULL;
    case IPOPT_NOOP:
      taglen = 1;
    default:
      taglen = optptr[1];
    }
    if (taglen == 0 || taglen > optlen)
      return NULL;
    if (optptr[0] == IPOPT_CIPSO)
      return optptr;
    ....
  }

>                 optlen -= taglen;
>                 optptr += taglen;
>         }

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Wang, Dongsheng @ 2018-09-17 16:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: timur@kernel.org, davem@davemloft.net, Zheng, Joey,
	netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20180917145028.GC5458@lunn.ch>

On 9/17/2018 10:50 PM, Andrew Lunn wrote:
> On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:
>> This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
>> Since emac include MDIO, so If the motherboard has more than one PHY
>> connected to an MDIO bus, this property will point to the MAC device
>> that has the MDIO bus.
>>
>> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
>> ---
>> V2: s/Since QDF2400 emac/Since emac/
>> ---
>>  Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
>> index 346e6c7f47b7..50db71771358 100644
>> --- a/Documentation/devicetree/bindings/net/qcom-emac.txt
>> +++ b/Documentation/devicetree/bindings/net/qcom-emac.txt
>> @@ -24,6 +24,9 @@ Internal PHY node:
>>  The external phy child node:
>>  - reg : The phy address
>>  
>> +Optional properties:
>> +- mdio-device : Shared MIDO bus.
> Hi Dongsheng
>
> I don't see why you need this property. The ethernet interface has a
> phy-handle which points to a PHY. That is all you need to find the PHY.
phy-handle is description PHY address. This property is describing an
MDIO controller.
Each QCOM emac include an MDIO controller, normally each emac only
connect one
PHY device, but when all of the PHY devices mdio lines connect one MDIO
controller
that is included in EMAC, we need to share this MDIO controller for
others EMAC.

Normally:

                                (MDIO)
MAC0 ---------------------------------------PHY0
     |                                                               |
     |                          (DATA)                         |
     ---------------------------------------------


                                (MDIO)
MAC1 ---------------------------------------PHY1
     |                                                               |
     |                          (DATA)                         |
     ---------------------------------------------



Shared MDIO bus: "mdio-device" = &MAC0, MAC1 will get MAC0's MDIO bus
and also get the corresponding PHY device.

                                (DATA)
MAC0 ---------------------------------------PHY0
     |                                                               |
     |                          (MDIO)                         |
     --------------------------------------------
                                                                     |
MAC1                                                        PHY0
     |                                                               |
     |                          (DATA)                         |
     --------------------------------------------

Cheers,
Dongsheng


>         emac0: ethernet@feb20000 {
>                 compatible = "qcom,fsm9900-emac";
>                 reg = <0xfeb20000 0x10000>,
>                       <0xfeb36000 0x1000>;
>                 interrupts = <76>;
>
>                 clocks = <&gcc 0>, <&gcc 1>, <&gcc 3>, <&gcc 4>, <&gcc 5>,
>                         <&gcc 6>, <&gcc 7>;
>                 clock-names = "axi_clk", "cfg_ahb_clk", "high_speed_clk",
>                         "mdio_clk", "tx_clk", "rx_clk", "sys_clk";
>
>                 internal-phy = <&emac_sgmii>;
>
>                 phy-handle = <&phy0>;
>
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 phy0: ethernet-phy@0 {
>                         reg = <0>;
>                 };
>
>                 phy1: ethernet-phy@1 {
>                         reg = <1>;
>                 };
>
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&mdio_pins_a>;
>         };
>
>         emac1: ethernet@38900000 {
>                 compatible = "qcom,fsm9900-emac";
> 		...
> 		...
>
>                 phy-handle = <&phy1>;
> 	};
>
> 	Andrew
>

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Florian Fainelli @ 2018-09-17 16:54 UTC (permalink / raw)
  To: Wang, Dongsheng, Andrew Lunn
  Cc: timur@kernel.org, davem@davemloft.net, Zheng, Joey,
	netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <b01bb33b30164088a9cae59a825acdc9@HXTBJIDCEMVIW02.hxtcorp.net>

On 09/17/2018 09:47 AM, Wang, Dongsheng wrote:
> On 9/17/2018 10:50 PM, Andrew Lunn wrote:
>> On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:
>>> This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
>>> Since emac include MDIO, so If the motherboard has more than one PHY
>>> connected to an MDIO bus, this property will point to the MAC device
>>> that has the MDIO bus.
>>>
>>> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
>>> ---
>>> V2: s/Since QDF2400 emac/Since emac/
>>> ---
>>>  Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> index 346e6c7f47b7..50db71771358 100644
>>> --- a/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> +++ b/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> @@ -24,6 +24,9 @@ Internal PHY node:
>>>  The external phy child node:
>>>  - reg : The phy address
>>>  
>>> +Optional properties:
>>> +- mdio-device : Shared MIDO bus.
>> Hi Dongsheng
>>
>> I don't see why you need this property. The ethernet interface has a
>> phy-handle which points to a PHY. That is all you need to find the PHY.
> phy-handle is description PHY address. This property is describing an
> MDIO controller.
> Each QCOM emac include an MDIO controller, normally each emac only
> connect one
> PHY device, but when all of the PHY devices mdio lines connect one MDIO
> controller
> that is included in EMAC, we need to share this MDIO controller for
> others EMAC.

If you want to describe the MDIO controller, then you embed a mdio
subnode into your Ethernet MAC node:

 emac0: ethernet@feb20000 {
	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		phy0: ethernet-phy@0 {
			reg = <0>;
		};
	};
};

And then each Ethernet MAC controller refers to their appropriate PHY
device tree node using a phy-handle property to point to either their
own MDIO controller, or another MAC's MDIO controller.

The IBM Emac is a old (not to say bad) example and it does not use the
PHY library and the standard Device Tree node property, please don't use
it as a reference.
-- 
Florian

^ permalink raw reply

* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: David Ahern @ 2018-09-17 16:59 UTC (permalink / raw)
  To: Peter Oskolkov; +Cc: davem, netdev
In-Reply-To: <CAPNVh5f3_GPpvV6nequZ8JNBhMVVOpEsJQ_XRfTN-+eaMPOWpw@mail.gmail.com>

On 9/17/18 9:11 AM, Peter Oskolkov wrote:
> On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 3eed045c65a5..a3902f805305 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
>>>
>>>  static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
>>>  {
>>> -     rt->dst.flags |= fib6_info_dst_flags(ort);
>>> +     rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
>>
>> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
>> argument is passed to ip6_dst_alloc which is always invoked before
>> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
> 
> ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
> one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
> If the flag assignment is completely removed from ip6_rt_init_dst(),
> then DST_NOPOLICY flag will be lost.
> 
> Which may be OK, but is more than what this patch tries to solve (do not
> copy DST_NOCOUNT flag).

In the 4.17 kernel (prior to the fib6_info change), ip6_rt_cache_alloc
calls __ip6_dst_alloc with 0 for flags so this is correct. The mistake
is ip6_rt_copy_init -> ip6_rt_init_dst -> fib6_info_dst_flags.

I believe the right fix is to drop the 'rt->dst.flags |=
fib6_info_dst_flags(ort);' from ip6_rt_init_dst.

^ 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