[PATCH net 0/4] s390/qeth: fixes 2019-06-03

[PATCH net 0/4] s390/qeth: fixes 2019-06-03

[Julian Wiedmann]

Hi Dave,

please apply the following set of qeth fixes to -net.

- The first two patches fix issues in the L3 driver's cast type
  selection for transmitted skbs.
- Alexandra adds a sanity check when retrieving VLAN information from
  neighbour address events.
- The last patch adds some missing error handling for qeth's new
  multiqueue code.

Thanks,
Julian


Alexandra Winter (1):
  s390/qeth: fix VLAN attribute in bridge_hostnotify udev event

Julian Wiedmann (3):
  s390/qeth: handle limited IPv4 broadcast in L3 TX path
  s390/qeth: don't use obsolete dst entry
  s390/qeth: handle error when updating TX queue count

 drivers/s390/net/qeth_core.h      |  2 +-
 drivers/s390/net/qeth_core_main.c | 22 ++++++++----
 drivers/s390/net/qeth_l2_main.c   |  2 +-
 drivers/s390/net/qeth_l3_main.c   | 59 +++++++++++++++++++------------
 4 files changed, 55 insertions(+), 30 deletions(-)

-- 
2.17.1

[PATCH net 1/4] s390/qeth: handle limited IPv4 broadcast in L3 TX path

[Julian Wiedmann]

When selecting the cast type of a neighbourless IPv4 skb (eg. on a raw
socket), qeth_l3 falls back to the packet's destination IP address.
For this case we should classify traffic sent to 255.255.255.255 as
broadcast.
This fixes DHCP requests, which were misclassified as unicast
(and for IQD interfaces thus ended up on the wrong HW queue).

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

diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 0271833da6a2..2df67abdfde7 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1911,6 +1911,8 @@ static int qeth_l3_get_cast_type(struct sk_buff *skb)
 	/* no neighbour (eg AF_PACKET), fall back to target's IP address ... */
 	switch (qeth_get_ip_version(skb)) {
 	case 4:
+		if (ipv4_is_lbcast(ip_hdr(skb)->daddr))
+			return RTN_BROADCAST;
 		return ipv4_is_multicast(ip_hdr(skb)->daddr) ?
 				RTN_MULTICAST : RTN_UNICAST;
 	case 6:
-- 
2.17.1

[PATCH net 2/4] s390/qeth: don't use obsolete dst entry

[Julian Wiedmann]

While qeth_l3 uses netif_keep_dst() to hold onto the dst, a skb's dst
may still have been obsoleted (via dst_dev_put()) by the time that we
end up using it. The dst then points to the loopback interface, which
means the neighbour lookup in qeth_l3_get_cast_type() determines a bogus
cast type of RTN_BROADCAST.
For IQD interfaces this causes us to place such skbs on the wrong
HW queue, resulting in TX errors.

Fix-up the various call sites to check whether the dst is obsolete, and
fall back accordingly.

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

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 784a2e76a1b0..f89f5b0a9805 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -275,7 +275,7 @@ struct qeth_hdr_layer3 {
 		struct in6_addr ipv6_addr;
 		struct ipv4 {
 			u8 res[12];
-			u32 addr;
+			__be32 addr;
 		} ipv4;
 		/* RX: */
 		struct rx {
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 2df67abdfde7..93bcfc272f20 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1893,8 +1893,9 @@ static int qeth_l3_get_cast_type(struct sk_buff *skb)
 
 	rcu_read_lock();
 	dst = skb_dst(skb);
-	if (dst)
+	if (dst && dst->obsolete <= 0)
 		n = dst_neigh_lookup_skb(dst, skb);
+
 	if (n) {
 		int cast_type = n->type;
 
@@ -1924,6 +1925,33 @@ static int qeth_l3_get_cast_type(struct sk_buff *skb)
 	}
 }
 
+static void qeth_l3_get_next_hop_v4(struct sk_buff *skb, __be32 *next_hop)
+{
+	struct rtable *rt;
+
+	rcu_read_lock();
+	rt = skb_rtable(skb);
+	if (rt && rt->dst.obsolete <= 0)
+		*next_hop = rt_nexthop(rt, ip_hdr(skb)->daddr);
+	else
+		*next_hop = ip_hdr(skb)->daddr;
+	rcu_read_unlock();
+}
+
+static void qeth_l3_get_next_hop_v6(struct sk_buff *skb,
+				    struct in6_addr *next_hop)
+{
+	const struct rt6_info *rt;
+
+	rcu_read_lock();
+	rt = skb_rt6_info(skb);
+	if (rt && rt->dst.obsolete <= 0 && !ipv6_addr_any(&rt->rt6i_gateway))
+		*next_hop = rt->rt6i_gateway;
+	else
+		*next_hop = ipv6_hdr(skb)->daddr;
+	rcu_read_unlock();
+}
+
 static u8 qeth_l3_cast_type_to_flag(int cast_type)
 {
 	if (cast_type == RTN_MULTICAST)
@@ -1980,33 +2008,18 @@ static void qeth_l3_fill_header(struct qeth_qdio_out_q *queue,
 
 	l3_hdr->flags = qeth_l3_cast_type_to_flag(cast_type);
 
-	/* OSA only: */
-	if (!ipv) {
-		l3_hdr->flags |= QETH_HDR_PASSTHRU;
-		return;
-	}
-
-	rcu_read_lock();
 	if (ipv == 4) {
-		struct rtable *rt = skb_rtable(skb);
-
-		*((__be32 *) &hdr->hdr.l3.next_hop.ipv4.addr) = (rt) ?
-				rt_nexthop(rt, ip_hdr(skb)->daddr) :
-				ip_hdr(skb)->daddr;
-	} else {
-		/* IPv6 */
-		const struct rt6_info *rt = skb_rt6_info(skb);
-
-		if (rt && !ipv6_addr_any(&rt->rt6i_gateway))
-			l3_hdr->next_hop.ipv6_addr = rt->rt6i_gateway;
-		else
-			l3_hdr->next_hop.ipv6_addr = ipv6_hdr(skb)->daddr;
+		qeth_l3_get_next_hop_v4(skb, &l3_hdr->next_hop.ipv4.addr);
+	} else if (ipv == 6) {
+		qeth_l3_get_next_hop_v6(skb, &l3_hdr->next_hop.ipv6_addr);
 
 		hdr->hdr.l3.flags |= QETH_HDR_IPV6;
 		if (!IS_IQD(card))
 			hdr->hdr.l3.flags |= QETH_HDR_PASSTHRU;
+	} else {
+		/* OSA only */
+		l3_hdr->flags |= QETH_HDR_PASSTHRU;
 	}
-	rcu_read_unlock();
 }
 
 static void qeth_l3_fixup_headers(struct sk_buff *skb)
-- 
2.17.1

[PATCH net 3/4] s390/qeth: fix VLAN attribute in bridge_hostnotify udev event

[Julian Wiedmann]

From: Alexandra Winter <wintera@linux.ibm.com>

Enabling sysfs attribute bridge_hostnotify triggers a series of udev events
for the MAC addresses of all currently connected peers. In case no VLAN is
set for a peer, the device reports the corresponding MAC addresses with
VLAN ID 4096. This currently results in attribute VLAN=4096 for all
non-VLAN interfaces in the initial series of events after host-notify is
enabled.

Instead, no VLAN attribute should be reported in the udev event for
non-VLAN interfaces.

Only the initial events face this issue. For dynamic changes that are
reported later, the device uses a validity flag.

This also changes the code so that it now sets the VLAN attribute for
MAC addresses with VID 0. On Linux, no qeth interface will ever be
registered with VID 0: Linux kernel registers VID 0 on all network
interfaces initially, but qeth will drop .ndo_vlan_rx_add_vid for VID 0.
Peers with other OSs could register MACs with VID 0.

Fixes: 9f48b9db9a22 ("qeth: bridgeport support - address notifications")
Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_l2_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 218801232ca2..ff8a6cd790b1 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1680,7 +1680,7 @@ static void qeth_bridgeport_an_set_cb(void *priv,
 
 	l2entry = (struct qdio_brinfo_entry_l2 *)entry;
 	code = IPA_ADDR_CHANGE_CODE_MACADDR;
-	if (l2entry->addr_lnid.lnid)
+	if (l2entry->addr_lnid.lnid < VLAN_N_VID)
 		code |= IPA_ADDR_CHANGE_CODE_VLANID;
 	qeth_bridge_emit_host_event(card, anev_reg_unreg, code,
 		(struct net_if_token *)&l2entry->nit,
-- 
2.17.1

[PATCH net 4/4] s390/qeth: handle error when updating TX queue count

[Julian Wiedmann]

netif_set_real_num_tx_queues() can return an error, deal with it.

Fixes: 73dc2daf110f ("s390/qeth: add TX multiqueue support for OSA devices")
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core_main.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 009f2c0ec504..b1823d75dd35 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1274,16 +1274,20 @@ static int qeth_setup_channel(struct qeth_channel *channel, bool alloc_buffers)
 	return 0;
 }
 
-static void qeth_osa_set_output_queues(struct qeth_card *card, bool single)
+static int qeth_osa_set_output_queues(struct qeth_card *card, bool single)
 {
 	unsigned int count = single ? 1 : card->dev->num_tx_queues;
+	int rc;
 
 	rtnl_lock();
-	netif_set_real_num_tx_queues(card->dev, count);
+	rc = netif_set_real_num_tx_queues(card->dev, count);
 	rtnl_unlock();
 
+	if (rc)
+		return rc;
+
 	if (card->qdio.no_out_queues == count)
-		return;
+		return 0;
 
 	if (atomic_read(&card->qdio.state) != QETH_QDIO_UNINITIALIZED)
 		qeth_free_qdio_queues(card);
@@ -1293,12 +1297,14 @@ static void qeth_osa_set_output_queues(struct qeth_card *card, bool single)
 
 	card->qdio.default_out_queue = single ? 0 : QETH_DEFAULT_QUEUE;
 	card->qdio.no_out_queues = count;
+	return 0;
 }
 
 static int qeth_update_from_chp_desc(struct qeth_card *card)
 {
 	struct ccw_device *ccwdev;
 	struct channel_path_desc_fmt0 *chp_dsc;
+	int rc = 0;
 
 	QETH_DBF_TEXT(SETUP, 2, "chp_desc");
 
@@ -1311,12 +1317,12 @@ static int qeth_update_from_chp_desc(struct qeth_card *card)
 
 	if (IS_OSD(card) || IS_OSX(card))
 		/* CHPP field bit 6 == 1 -> single queue */
-		qeth_osa_set_output_queues(card, chp_dsc->chpp & 0x02);
+		rc = qeth_osa_set_output_queues(card, chp_dsc->chpp & 0x02);
 
 	kfree(chp_dsc);
 	QETH_DBF_TEXT_(SETUP, 2, "nr:%x", card->qdio.no_out_queues);
 	QETH_DBF_TEXT_(SETUP, 2, "lvl:%02x", card->info.func_level);
-	return 0;
+	return rc;
 }
 
 static void qeth_init_qdio_info(struct qeth_card *card)
@@ -5597,8 +5603,12 @@ static struct net_device *qeth_alloc_netdev(struct qeth_card *card)
 		dev->hw_features |= NETIF_F_SG;
 		dev->vlan_features |= NETIF_F_SG;
 		if (IS_IQD(card)) {
-			netif_set_real_num_tx_queues(dev, QETH_IQD_MIN_TXQ);
 			dev->features |= NETIF_F_SG;
+			if (netif_set_real_num_tx_queues(dev,
+							 QETH_IQD_MIN_TXQ)) {
+				free_netdev(dev);
+				return NULL;
+			}
 		}
 	}
 
-- 
2.17.1

Re: [PATCH net 2/4] s390/qeth: don't use obsolete dst entry

[David Miller]

From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Mon,  3 Jun 2019 17:04:44 +0200

> While qeth_l3 uses netif_keep_dst() to hold onto the dst, a skb's dst
> may still have been obsoleted (via dst_dev_put()) by the time that we
> end up using it. The dst then points to the loopback interface, which
> means the neighbour lookup in qeth_l3_get_cast_type() determines a bogus
> cast type of RTN_BROADCAST.
> For IQD interfaces this causes us to place such skbs on the wrong
> HW queue, resulting in TX errors.
> 
> Fix-up the various call sites to check whether the dst is obsolete, and
> fall back accordingly.
> 
> Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>

Please use "dst_check()".

Some routes have DST_OBSOLETE_FORCE_CHK set on them from the very beginning
so that uses of the route are forced through the dst->ops->check() method.

Simply use dst_check() and then you can just retain the 'rt == NULL' logic
as-is.

Thanks.

Re: [PATCH net 2/4] s390/qeth: don't use obsolete dst entry

[Julian Wiedmann]

On 03.06.19 21:43, David Miller wrote:
> From: Julian Wiedmann <jwi@linux.ibm.com>
> Date: Mon,  3 Jun 2019 17:04:44 +0200
> 
>> While qeth_l3 uses netif_keep_dst() to hold onto the dst, a skb's dst
>> may still have been obsoleted (via dst_dev_put()) by the time that we
>> end up using it. The dst then points to the loopback interface, which
>> means the neighbour lookup in qeth_l3_get_cast_type() determines a bogus
>> cast type of RTN_BROADCAST.
>> For IQD interfaces this causes us to place such skbs on the wrong
>> HW queue, resulting in TX errors.
>>
>> Fix-up the various call sites to check whether the dst is obsolete, and
>> fall back accordingly.
>>
>> Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
> 
> Please use "dst_check()".
> 
> Some routes have DST_OBSOLETE_FORCE_CHK set on them from the very beginning
> so that uses of the route are forced through the dst->ops->check() method.
> 
> Simply use dst_check() and then you can just retain the 'rt == NULL' logic
> as-is.
> 
> Thanks.
> 
Alright - I was hesitant to go down that path in the context of a driver,
but looks like rt6_get_cookie() should do the trick. v2 coming up... thanks.