Netdev List
 help / color / mirror / Atom feed
* [RESENT PATCH v2] ixgbe: Use memzero_explicit directly in crypto cases
From: zhong jiang @ 2019-09-17 14:44 UTC (permalink / raw)
  To: jakub.kicinski, davem
  Cc: anna.schumaker, trond.myklebust, netdev, linux-kernel, zhongjiang

It's better to use memzero_explicit() to replace memset() in crypto cases.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 31629fc..7e4f32f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -960,10 +960,10 @@ int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
 	return 0;
 
 err_aead:
-	memset(xs->aead, 0, sizeof(*xs->aead));
+	memzero_explicit(xs->aead, sizeof(*xs->aead));
 	kfree(xs->aead);
 err_xs:
-	memset(xs, 0, sizeof(*xs));
+	memzero_explicit(xs, sizeof(*xs));
 	kfree(xs);
 err_out:
 	msgbuf[1] = err;
@@ -1049,7 +1049,7 @@ int ixgbe_ipsec_vf_del_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
 	ixgbe_ipsec_del_sa(xs);
 
 	/* remove the xs that was made-up in the add request */
-	memset(xs, 0, sizeof(*xs));
+	memzero_explicit(xs, sizeof(*xs));
 	kfree(xs);
 
 	return 0;
-- 
1.7.12.4


^ permalink raw reply related

* Re: [PATCH] netfilter: bridge: drop a broken include
From: Jeremy Sowden @ 2019-09-17 14:59 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Adam Borowski, Jozsef Kadlecsik, Florian Westphal, Roopa Prabhu,
	Nikolay Aleksandrov, netfilter-devel, coreteam, netdev
In-Reply-To: <20190917050946.kmzajvqh3kjr4ch5@salvia>

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

On 2019-09-17, at 07:09:46 +0200, Pablo Neira Ayuso wrote:
> On Mon, Sep 16, 2019 at 02:08:12PM +0100, Jeremy Sowden wrote:
> > On 2019-09-16, at 02:05:16 +0200, Adam Borowski wrote:
> > > This caused a build failure if CONFIG_NF_CONNTRACK_BRIDGE is set
> > > but CONFIG_NF_TABLES=n -- and appears to be unused anyway.
> [...]
> > There are already changes in the net-next tree that will fix it.
>
> If the fix needs to go to -stable 5.3 kernel release, then you have to
> point to the particular commit ID of this patch to fix this one.
> net-next contains 5.4-rc material. I'd appreciate also if you can help
> identify the patch with a Fixes: tag.

The commit in the mainline that needs fixing is:

  3c171f496ef5 ("netfilter: bridge: add connection tracking system")

The commit in net-next that fixes it is:

  47e640af2e49 ("netfilter: add missing IS_ENABLED(CONFIG_NF_TABLES) check to header-file.")

I applied it to the mainline and compile-tested it to verify that it
does indeed fix the build failure.

From my reading of stable-kernel-rules.rst and netdev-FAQ.rst, it
appears that the fix should come from the mainline, so I will wait for
it to get there.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v2 0/2] net/ibmvnic: serialization fixes
From: Juliet Kim @ 2019-09-17 14:52 UTC (permalink / raw)
  To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev

This series includes two fixes. The first improves reset code to allow 
linkwatch_event to proceed during reset. The second ensures that no more
than one thread runs in reset at a time. 

v2: 
- Separate change param reset from do_reset()
- Return IBMVNIC_OPEN_FAILED if __ibmvnic_open fails 
- Remove setting wait_for_reset to false from __ibmvnic_reset(), this 
  is done in wait_for_reset()
- Move the check for force_reset_recovery from patch 1 to patch 2

Juliet Kim (2):
  net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run
  net/ibmvnic: prevent more than one thread from running in reset

 drivers/net/ethernet/ibm/ibmvnic.c | 244 ++++++++++++++++++++++++++-----------
 drivers/net/ethernet/ibm/ibmvnic.h |   4 +
 2 files changed, 180 insertions(+), 68 deletions(-)

-- 
2.16.4


^ permalink raw reply

* [PATCH v2 1/2] net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run
From: Juliet Kim @ 2019-09-17 14:52 UTC (permalink / raw)
  To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev
In-Reply-To: <20190917145249.15334-1-julietk@linux.vnet.ibm.com>

Commit a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset") 
made the change to hold the RTNL lock during a reset to avoid deadlock 
but linkwatch_event is fired during the reset and needs the RTNL lock. 
That keeps linkwatch_event process from proceeding until the reset 
is complete. The reset process cannot tolerate the linkwatch_event 
processing after reset completes, so release the RTNL lock during the 
process to allow a chance for linkwatch_event to run during reset. 
This does not guarantee that the linkwatch_event will be processed as 
soon as link state changes, but is an improvement over the current code 
where linkwatch_event processing is always delayed, which prevents 
transmissions on the device from being deactivated leading transmit 
watchdog timer to time-out. 

Release the RTNL lock before link state change and re-acquire after 
the link state change to allow linkwatch_event to grab the RTNL lock 
and run during the reset.

Fixes: a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset")
Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 223 ++++++++++++++++++++++++++-----------
 drivers/net/ethernet/ibm/ibmvnic.h |   1 +
 2 files changed, 156 insertions(+), 68 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5cb55ea671e3..fc760c1eb0b0 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1723,6 +1723,86 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p)
 	return rc;
 }
 
+/**
+ * do_change_param_reset returns zero if we are able to keep processing reset
+ * events, or non-zero if we hit a fatal error and must halt.
+ */
+static int do_change_param_reset(struct ibmvnic_adapter *adapter,
+				 struct ibmvnic_rwi *rwi,
+				 u32 reset_state)
+{
+	struct net_device *netdev = adapter->netdev;
+	int i, rc;
+
+	netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n",
+		   rwi->reset_reason);
+
+	netif_carrier_off(netdev);
+	adapter->reset_reason = rwi->reset_reason;
+
+	ibmvnic_cleanup(netdev);
+
+	if (reset_state == VNIC_OPEN) {
+		rc = __ibmvnic_close(netdev);
+		if (rc)
+			return rc;
+	}
+
+	release_resources(adapter);
+	release_sub_crqs(adapter, 1);
+	release_crq_queue(adapter);
+
+	adapter->state = VNIC_PROBED;
+
+	rc = init_crq_queue(adapter);
+
+	if (rc) {
+		netdev_err(adapter->netdev,
+			   "Couldn't initialize crq. rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = ibmvnic_reset_init(adapter);
+	if (rc)
+		return IBMVNIC_INIT_FAILED;
+
+	/* If the adapter was in PROBE state prior to the reset,
+	 * exit here.
+	 */
+	if (reset_state == VNIC_PROBED)
+		return 0;
+
+	rc = ibmvnic_login(netdev);
+	if (rc) {
+		adapter->state = reset_state;
+		return rc;
+	}
+
+	rc = init_resources(adapter);
+	if (rc)
+		return rc;
+
+	ibmvnic_disable_irqs(adapter);
+
+	adapter->state = VNIC_CLOSED;
+
+	if (reset_state == VNIC_CLOSED)
+		return 0;
+
+	rc = __ibmvnic_open(netdev);
+	if (rc)
+		return IBMVNIC_OPEN_FAILED;
+
+	/* refresh device's multicast list */
+	ibmvnic_set_multi(netdev);
+
+	/* kick napi */
+	for (i = 0; i < adapter->req_rx_queues; i++)
+		napi_schedule(&adapter->napi[i]);
+
+	return 0;
+}
+
 /**
  * do_reset returns zero if we are able to keep processing reset events, or
  * non-zero if we hit a fatal error and must halt.
@@ -1738,6 +1818,8 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
 		   rwi->reset_reason);
 
+	rtnl_lock();
+
 	netif_carrier_off(netdev);
 	adapter->reset_reason = rwi->reset_reason;
 
@@ -1751,16 +1833,25 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	if (reset_state == VNIC_OPEN &&
 	    adapter->reset_reason != VNIC_RESET_MOBILITY &&
 	    adapter->reset_reason != VNIC_RESET_FAILOVER) {
-		rc = __ibmvnic_close(netdev);
+		adapter->state = VNIC_CLOSING;
+
+		/* Release the RTNL lock before link state change and
+		 * re-acquire after the link state change to allow
+		 * linkwatch_event to grab the RTNL lock and run during
+		 * a reset.
+		 */
+		rtnl_unlock();
+		rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+		rtnl_lock();
 		if (rc)
-			return rc;
-	}
+			goto out;
 
-	if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
-	    adapter->wait_for_reset) {
-		release_resources(adapter);
-		release_sub_crqs(adapter, 1);
-		release_crq_queue(adapter);
+		if (adapter->state != VNIC_CLOSING) {
+			rc = -1;
+			goto out;
+		}
+
+		adapter->state = VNIC_CLOSED;
 	}
 
 	if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
@@ -1769,9 +1860,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 		 */
 		adapter->state = VNIC_PROBED;
 
-		if (adapter->wait_for_reset) {
-			rc = init_crq_queue(adapter);
-		} else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
+		if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
 			rc = ibmvnic_reenable_crq_queue(adapter);
 			release_sub_crqs(adapter, 1);
 		} else {
@@ -1783,36 +1872,35 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 		if (rc) {
 			netdev_err(adapter->netdev,
 				   "Couldn't initialize crq. rc=%d\n", rc);
-			return rc;
+			goto out;
 		}
 
 		rc = ibmvnic_reset_init(adapter);
-		if (rc)
-			return IBMVNIC_INIT_FAILED;
+		if (rc) {
+			rc = IBMVNIC_INIT_FAILED;
+			goto out;
+		}
 
 		/* If the adapter was in PROBE state prior to the reset,
 		 * exit here.
 		 */
-		if (reset_state == VNIC_PROBED)
-			return 0;
+		if (reset_state == VNIC_PROBED) {
+			rc = 0;
+			goto out;
+		}
 
 		rc = ibmvnic_login(netdev);
 		if (rc) {
 			adapter->state = reset_state;
-			return rc;
+			goto out;
 		}
 
-		if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
-		    adapter->wait_for_reset) {
-			rc = init_resources(adapter);
-			if (rc)
-				return rc;
-		} else if (adapter->req_rx_queues != old_num_rx_queues ||
-			   adapter->req_tx_queues != old_num_tx_queues ||
-			   adapter->req_rx_add_entries_per_subcrq !=
-							old_num_rx_slots ||
-			   adapter->req_tx_entries_per_subcrq !=
-							old_num_tx_slots) {
+		if (adapter->req_rx_queues != old_num_rx_queues ||
+		    adapter->req_tx_queues != old_num_tx_queues ||
+		    adapter->req_rx_add_entries_per_subcrq !=
+		    old_num_rx_slots ||
+		    adapter->req_tx_entries_per_subcrq !=
+		    old_num_tx_slots) {
 			release_rx_pools(adapter);
 			release_tx_pools(adapter);
 			release_napi(adapter);
@@ -1820,32 +1908,30 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 
 			rc = init_resources(adapter);
 			if (rc)
-				return rc;
+				goto out;
 
 		} else {
 			rc = reset_tx_pools(adapter);
 			if (rc)
-				return rc;
+				goto out;
 
 			rc = reset_rx_pools(adapter);
 			if (rc)
-				return rc;
+				goto out;
 		}
 		ibmvnic_disable_irqs(adapter);
 	}
 	adapter->state = VNIC_CLOSED;
 
-	if (reset_state == VNIC_CLOSED)
-		return 0;
+	if (reset_state == VNIC_CLOSED) {
+		rc = 0;
+		goto out;
+	}
 
 	rc = __ibmvnic_open(netdev);
 	if (rc) {
-		if (list_empty(&adapter->rwi_list))
-			adapter->state = VNIC_CLOSED;
-		else
-			adapter->state = reset_state;
-
-		return 0;
+		rc = IBMVNIC_OPEN_FAILED;
+		goto out;
 	}
 
 	/* refresh device's multicast list */
@@ -1855,11 +1941,15 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	for (i = 0; i < adapter->req_rx_queues; i++)
 		napi_schedule(&adapter->napi[i]);
 
-	if (adapter->reset_reason != VNIC_RESET_FAILOVER &&
-	    adapter->reset_reason != VNIC_RESET_CHANGE_PARAM)
+	if (adapter->reset_reason != VNIC_RESET_FAILOVER)
 		call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, netdev);
 
-	return 0;
+	rc = 0;
+
+out:
+	rtnl_unlock();
+
+	return rc;
 }
 
 static int do_hard_reset(struct ibmvnic_adapter *adapter,
@@ -1919,14 +2009,8 @@ static int do_hard_reset(struct ibmvnic_adapter *adapter,
 		return 0;
 
 	rc = __ibmvnic_open(netdev);
-	if (rc) {
-		if (list_empty(&adapter->rwi_list))
-			adapter->state = VNIC_CLOSED;
-		else
-			adapter->state = reset_state;
-
-		return 0;
-	}
+	if (rc)
+		return IBMVNIC_OPEN_FAILED;
 
 	return 0;
 }
@@ -1965,20 +2049,11 @@ static void __ibmvnic_reset(struct work_struct *work)
 {
 	struct ibmvnic_rwi *rwi;
 	struct ibmvnic_adapter *adapter;
-	bool we_lock_rtnl = false;
 	u32 reset_state;
 	int rc = 0;
 
 	adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
 
-	/* netif_set_real_num_xx_queues needs to take rtnl lock here
-	 * unless wait_for_reset is set, in which case the rtnl lock
-	 * has already been taken before initializing the reset
-	 */
-	if (!adapter->wait_for_reset) {
-		rtnl_lock();
-		we_lock_rtnl = true;
-	}
 	reset_state = adapter->state;
 
 	rwi = get_next_rwi(adapter);
@@ -1990,14 +2065,31 @@ static void __ibmvnic_reset(struct work_struct *work)
 			break;
 		}
 
-		if (adapter->force_reset_recovery) {
-			adapter->force_reset_recovery = false;
-			rc = do_hard_reset(adapter, rwi, reset_state);
+		if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+			/* CHANGE_PARAM requestor holds rtnl_lock */
+			rc = do_change_param_reset(adapter, rwi, reset_state);
+		} else if (adapter->force_reset_recovery) {
+			/* Transport event occurred during previous reset */
+			if (adapter->wait_for_reset) {
+				/* Previous was CHANGE_PARAM; caller locked */
+				adapter->force_reset_recovery = false;
+				rc = do_hard_reset(adapter, rwi, reset_state);
+			} else {
+				rtnl_lock();
+				adapter->force_reset_recovery = false;
+				rc = do_hard_reset(adapter, rwi, reset_state);
+				rtnl_unlock();
+			}
 		} else {
 			rc = do_reset(adapter, rwi, reset_state);
 		}
 		kfree(rwi);
-		if (rc && rc != IBMVNIC_INIT_FAILED &&
+		if (rc == IBMVNIC_OPEN_FAILED) {
+			if (list_empty(&adapter->rwi_list))
+				adapter->state = VNIC_CLOSED;
+			else
+				adapter->state = reset_state;
+		} else if (rc && rc != IBMVNIC_INIT_FAILED &&
 		    !adapter->force_reset_recovery)
 			break;
 
@@ -2005,7 +2097,6 @@ static void __ibmvnic_reset(struct work_struct *work)
 	}
 
 	if (adapter->wait_for_reset) {
-		adapter->wait_for_reset = false;
 		adapter->reset_done_rc = rc;
 		complete(&adapter->reset_done);
 	}
@@ -2016,8 +2107,6 @@ static void __ibmvnic_reset(struct work_struct *work)
 	}
 
 	adapter->resetting = false;
-	if (we_lock_rtnl)
-		rtnl_unlock();
 }
 
 static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
@@ -2078,8 +2167,6 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 
 	return 0;
 err:
-	if (adapter->wait_for_reset)
-		adapter->wait_for_reset = false;
 	return -ret;
 }
 
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 70bd286f8932..9d3d35cc91d6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -20,6 +20,7 @@
 #define IBMVNIC_INVALID_MAP	-1
 #define IBMVNIC_STATS_TIMEOUT	1
 #define IBMVNIC_INIT_FAILED	2
+#define IBMVNIC_OPEN_FAILED	3
 
 /* basic structures plus 100 2k buffers */
 #define IBMVNIC_IO_ENTITLEMENT_DEFAULT	610305
-- 
2.16.4


^ permalink raw reply related

* [PATCH v2 2/2] net/ibmvnic: prevent more than one thread from running in reset
From: Juliet Kim @ 2019-09-17 14:52 UTC (permalink / raw)
  To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev
In-Reply-To: <20190917145249.15334-1-julietk@linux.vnet.ibm.com>

The current code allows more than one thread to run in reset. This can 
corrupt struct adapter data. Check adapter->resetting before performing 
a reset, if there is another reset running delay (100 msec) before trying 
again.

Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 23 ++++++++++++++++++++++-
 drivers/net/ethernet/ibm/ibmvnic.h |  3 +++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index fc760c1eb0b0..d3ebf4caa09c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2054,6 +2054,13 @@ static void __ibmvnic_reset(struct work_struct *work)
 
 	adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
 
+	if (adapter->resetting) {
+		schedule_delayed_work(&adapter->ibmvnic_delayed_reset,
+				      IBMVNIC_RESET_DELAY);
+		return;
+	}
+
+	adapter->resetting = true;
 	reset_state = adapter->state;
 
 	rwi = get_next_rwi(adapter);
@@ -2094,6 +2101,10 @@ static void __ibmvnic_reset(struct work_struct *work)
 			break;
 
 		rwi = get_next_rwi(adapter);
+
+		if (rwi && (rwi->reset_reason == VNIC_RESET_FAILOVER ||
+			    rwi->reset_reason == VNIC_RESET_MOBILITY))
+			adapter->force_reset_recovery = true;
 	}
 
 	if (adapter->wait_for_reset) {
@@ -2109,6 +2120,15 @@ static void __ibmvnic_reset(struct work_struct *work)
 	adapter->resetting = false;
 }
 
+static void __ibmvnic_delayed_reset(struct work_struct *work)
+{
+	struct ibmvnic_adapter *adapter;
+
+	adapter = container_of(work, struct ibmvnic_adapter,
+			       ibmvnic_delayed_reset.work);
+	__ibmvnic_reset(&adapter->ibmvnic_reset);
+}
+
 static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 			 enum ibmvnic_reset_reason reason)
 {
@@ -2161,7 +2181,6 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 	rwi->reset_reason = reason;
 	list_add_tail(&rwi->list, &adapter->rwi_list);
 	spin_unlock_irqrestore(&adapter->rwi_lock, flags);
-	adapter->resetting = true;
 	netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason);
 	schedule_work(&adapter->ibmvnic_reset);
 
@@ -4932,6 +4951,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	spin_lock_init(&adapter->stats_lock);
 
 	INIT_WORK(&adapter->ibmvnic_reset, __ibmvnic_reset);
+	INIT_DELAYED_WORK(&adapter->ibmvnic_delayed_reset,
+			  __ibmvnic_delayed_reset);
 	INIT_LIST_HEAD(&adapter->rwi_list);
 	spin_lock_init(&adapter->rwi_lock);
 	init_completion(&adapter->init_done);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 9d3d35cc91d6..4f4651d92cc1 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -39,6 +39,8 @@
 #define IBMVNIC_MAX_LTB_SIZE ((1 << (MAX_ORDER - 1)) * PAGE_SIZE)
 #define IBMVNIC_BUFFER_HLEN 500
 
+#define IBMVNIC_RESET_DELAY 100
+
 static const char ibmvnic_priv_flags[][ETH_GSTRING_LEN] = {
 #define IBMVNIC_USE_SERVER_MAXES 0x1
 	"use-server-maxes"
@@ -1077,6 +1079,7 @@ struct ibmvnic_adapter {
 	spinlock_t rwi_lock;
 	struct list_head rwi_list;
 	struct work_struct ibmvnic_reset;
+	struct delayed_work ibmvnic_delayed_reset;
 	bool resetting;
 	bool napi_enabled, from_passive_init;
 
-- 
2.16.4


^ permalink raw reply related

* Re: [PATCH bpf-next 1/5] perf/core: Add PERF_FORMAT_LOST read_format
From: Daniel Xu @ 2019-09-17 15:14 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, bpf, songliubraving, yhs, andriin, peterz, mingo,
	acme, Daniel Xu, ast, alexander.shishkin, jolsa, namhyung,
	linux-kernel, netdev, kernel-team

On Tue Sep 17, 2019 at 10:32 PM kbuild test robot wrote:
> All errors (new ones prefixed by >>):
> 
>    kernel/events/core.c: In function 'perf_event_lost':
> >> kernel/events/core.c:4753:11: error: implicit declaration of function 'perf_kprobe_missed'; did you mean 'perf_release'? [-Werror=implicit-function-declaration]
>       lost += perf_kprobe_missed(event);
>               ^~~~~~~~~~~~~~~~~~
>               perf_release
>    cc1: some warnings being treated as errors
> 

Ah forgot the #ifdef for CONFIG_KPROBE_EVENTS. I've applied the fix and
will send it in the next version.

^ permalink raw reply

* Re: [PATCH bpf-next 1/5] perf/core: Add PERF_FORMAT_LOST read_format
From: kbuild test robot @ 2019-09-17 15:22 UTC (permalink / raw)
  To: Daniel Xu
  Cc: kbuild-all, bpf, songliubraving, yhs, andriin, peterz, mingo,
	acme, Daniel Xu, ast, alexander.shishkin, jolsa, namhyung,
	linux-kernel, netdev, kernel-team
In-Reply-To: <20190917133056.5545-2-dxu@dxuuu.xyz>

[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]

Hi Daniel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Daniel-Xu/perf-core-Add-PERF_FORMAT_LOST-read_format/20190917-213515
base:   https://kernel.googlesource.com/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: i386-randconfig-e003-201937 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   kernel/events/core.c: In function 'perf_event_lost':
>> kernel/events/core.c:4753:11: error: implicit declaration of function 'perf_kprobe_missed'; did you mean 'perf_uprobe_init'? [-Werror=implicit-function-declaration]
      lost += perf_kprobe_missed(event);
              ^~~~~~~~~~~~~~~~~~
              perf_uprobe_init
   cc1: some warnings being treated as errors

vim +4753 kernel/events/core.c

  4739	
  4740	static struct pmu perf_kprobe;
  4741	static u64 perf_event_lost(struct perf_event *event)
  4742	{
  4743		struct ring_buffer *rb;
  4744		u64 lost = 0;
  4745	
  4746		rcu_read_lock();
  4747		rb = rcu_dereference(event->rb);
  4748		if (likely(!!rb))
  4749			lost += local_read(&rb->lost);
  4750		rcu_read_unlock();
  4751	
  4752		if (event->attr.type == perf_kprobe.type)
> 4753			lost += perf_kprobe_missed(event);
  4754	
  4755		return lost;
  4756	}
  4757	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36877 bytes --]

^ permalink raw reply

* Re: [PATCH iproute2 4/4] devlink: Fix devlink health set command
From: Stephen Hemminger @ 2019-09-17 15:23 UTC (permalink / raw)
  To: Tariq Toukan; +Cc: David Ahern, netdev, Moshe Shemesh, Aya Levin, Jiri Pirko
In-Reply-To: <1567687387-12993-5-git-send-email-tariqt@mellanox.com>

On Thu,  5 Sep 2019 15:43:07 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:

> From: Aya Levin <ayal@mellanox.com>
> 
> Prior to this patch both the reporter's name and the grace period
> attributes shared the same bit. This caused zeroing grace period when
> setting auto recovery. Let each parameter has its own bit.
> 
> Fixes: b18d89195b16 ("devlink: Add devlink health set command")
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>

Does not apply to current iproute2.

^ permalink raw reply

* [PATCH net] net/rds: Check laddr_check before calling it
From: Ka-Cheong Poon @ 2019-09-17 15:29 UTC (permalink / raw)
  To: netdev; +Cc: santosh.shilimkar, davem, rds-devel

In rds_bind(), laddr_check is called without checking if it is NULL or
not.  And rs_transport should be reset if rds_add_bound() fails.

Reported-by: syzbot+fae39afd2101a17ec624@syzkaller.appspotmail.com
Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
---
 net/rds/bind.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/rds/bind.c b/net/rds/bind.c
index 20c156a..5b5fb4c 100644
--- a/net/rds/bind.c
+++ b/net/rds/bind.c
@@ -244,7 +244,8 @@ int rds_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	 */
 	if (rs->rs_transport) {
 		trans = rs->rs_transport;
-		if (trans->laddr_check(sock_net(sock->sk),
+		if (!trans->laddr_check ||
+		    trans->laddr_check(sock_net(sock->sk),
 				       binding_addr, scope_id) != 0) {
 			ret = -ENOPROTOOPT;
 			goto out;
@@ -263,6 +264,8 @@ int rds_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 
 	sock_set_flag(sk, SOCK_RCU_FREE);
 	ret = rds_add_bound(rs, binding_addr, &port, scope_id);
+	if (ret)
+		rs->rs_transport = NULL;
 
 out:
 	release_sock(sk);
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH net] net/rds: Check laddr_check before calling it
From: santosh.shilimkar @ 2019-09-17 15:31 UTC (permalink / raw)
  To: Ka-Cheong Poon; +Cc: netdev, davem, rds-devel
In-Reply-To: <1568734158-18021-1-git-send-email-ka-cheong.poon@oracle.com>

On 9/17/19 8:29 AM, Ka-Cheong Poon wrote:
> In rds_bind(), laddr_check is called without checking if it is NULL or
> not.  And rs_transport should be reset if rds_add_bound() fails.
> 
> Reported-by: syzbot+fae39afd2101a17ec624@syzkaller.appspotmail.com
> Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
> ---
Thanks Ka-Cheong for getting this out quickly on list.

Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>


^ permalink raw reply

* Re: divide error in cdc_ncm_update_rxtx_max
From: Bjørn Mork @ 2019-09-17 15:46 UTC (permalink / raw)
  To: syzbot
  Cc: andreyknvl, davem, linux-kernel, linux-usb, netdev, oliver,
	syzkaller-bugs
In-Reply-To: <00000000000018e4250592c043c3@google.com>

syzbot <syzbot+ce366e2b8296e25d84f5@syzkaller.appspotmail.com> writes:

> syzbot has tested the proposed patch but the reproducer still
> triggered crash:
> divide error in usbnet_update_max_qlen
>
> cdc_ncm 5-1:1.0: setting tx_max = 16384
> divide error: 0000 [#1] SMP KASAN
> CPU: 1 PID: 1737 Comm: kworker/1:2 Not tainted 5.3.0-rc7+ #0
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 01/01/2011
> Workqueue: usb_hub_wq hub_event
> RIP: 0010:usbnet_update_max_qlen drivers/net/usb/usbnet.c:344 [inline]
> RIP: 0010:usbnet_update_max_qlen+0x231/0x370 drivers/net/usb/usbnet.c:338

Sure, but that's another error already fixed by Oliver..

I guess this fix worked.  But I believe we should see if this is a more
generic issue than just this single driver/bug.  I fear it is...



Bjørn

^ permalink raw reply

* Re: [PATCH] net: sysctl: cleanup net_sysctl_init error exit paths
From: George G. Davis @ 2019-09-17 15:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20190708224732.GA8009@mam-gdavis-lt>

Hello David,

On Mon, Jul 08, 2019 at 06:47:32PM -0400, George G. Davis wrote:
> Hello David,
> 
> On Fri, May 17, 2019 at 10:43:45AM -0400, George G. Davis wrote:
> > Hello David,
> > 
> > On Thu, May 16, 2019 at 02:27:44PM -0700, David Miller wrote:
> > > From: "George G. Davis" <george_davis@mentor.com>
> > > Date: Thu, 16 May 2019 11:23:08 -0400
> > > 
> > > > Unwind net_sysctl_init error exit goto spaghetti code
> > > > 
> > > > Suggested-by: Joshua Frkuska <joshua_frkuska@mentor.com>
> > > > Signed-off-by: George G. Davis <george_davis@mentor.com>
> > > 
> > > Cleanups are not appropriate until the net-next tree opens back up.
> > > 
> > > So please resubmit at that time.
> > 
> > I fear that I may be distracted by other shiny objects by then but
> > I'll make a reminder and try to resubmit during the next merge window.
> 
> Since the "Linux 5.2" kernel has been released [1], I'm guessing that the
> net-next merge window is open now? If yes, the patch remains unchanged
> since my initial post. Please consider applying or let me know when to
> resubmit when the net-next merge window is again open.

Ping, "Linux 5.3" kernel has been released [1] and it appears that the
5.4 merge window is open. The patch [2] remains unchanged since my initial
post. Please consider applying it.

Thanks!

-- 
Regards,
George
[1] https://lwn.net/Articles/799250/
[2] https://lore.kernel.org/patchwork/patch/1074969/

^ permalink raw reply

* Re: -Wsizeof-array-div warnings in ethernet drivers
From: Nick Desaulniers @ 2019-09-17 16:00 UTC (permalink / raw)
  To: Jose Abreu
  Cc: David Laight, Nathan Chancellor, Tom Lendacky, Giuseppe Cavallaro,
	Alexandre Torgue, David S. Miller, Ilie Halip, David Bolvansky,
	netdev@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <BN8PR12MB32662378E844E6ECBA3FE8D7D38F0@BN8PR12MB3266.namprd12.prod.outlook.com>

On Tue, Sep 17, 2019 at 6:27 AM Jose Abreu <Jose.Abreu@synopsys.com> wrote:
>
> From: David Laight <David.Laight@ACULAB.COM>
> Date: Sep/17/2019, 11:36:21 (UTC+00:00)
>
> > From: Jose Abreu
> > > Sent: 17 September 2019 08:59
> > > From: Nathan Chancellor <natechancellor@gmail.com>
> > > Date: Sep/17/2019, 08:32:32 (UTC+00:00)
> > >
> > > > Hi all,
> > > >
> > > > Clang recently added a new diagnostic in r371605, -Wsizeof-array-div,
> > > > that tries to warn when sizeof(X) / sizeof(Y) does not compute the
> > > > number of elements in an array X (i.e., sizeof(Y) is wrong). See that
> > > > commit for more details:
> > ...
> > > > ../drivers/net/ethernet/amd/xgbe/xgbe-dev.c:361:49: warning: expression
> > > > does not compute the number of elements in this array; element type is
> > > > 'u8' (aka 'unsigned char'), not 'u32' (aka 'unsigned int')
> > > > [-Wsizeof-array-div]
> > > >         unsigned int key_regs = sizeof(pdata->rss_key) / sizeof(u32);
> > > >                                        ~~~~~~~~~~~~~~  ^
> > ...
> > > > What is the reasoning behind having the key being an array of u8s but
> > > > seemlingly converting it into an array of u32s? It's not immediately
> > > > apparent from reading over the code but I am not familiar with it so I
> > > > might be making a mistake. I assume this is intentional? If so, the
> > > > warning can be silenced and we'll send patches to do so but we want to
> > > > make sure we aren't actually papering over a mistake.
> > >
> > > This is because we write 32 bits at a time to the reg but internally the
> > > driver uses 8 bits to store the array. If you look at
> > > dwxgmac2_rss_configure() you'll see that cfg->key is casted to u32 which
> > > is the value we use in HW writes. Then the for loop just does the math
> > > to check how many u32's has to write.
> >
> > That stinks of a possible misaligned data access.....
>
> It's possible to happen only if structure field is not aligned. I guess
> I can either change all to u32 or just __align the field of the struct

Would __aligning the struct still produce the warning?  It's good to
know that this case is intentional, but I would like to consider other
instances of it before we seriously consider turning it off.  If the
driver can be rewritten to just make use of u32, I would find that
preferrable.
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* Re: [patch iproute2-next v2] devlink: add reload failed indication
From: David Ahern @ 2019-09-17 16:46 UTC (permalink / raw)
  To: Jiri Pirko, netdev; +Cc: stephen, idosch, jakub.kicinski, tariqt, mlxsw
In-Reply-To: <20190916094448.26072-1-jiri@resnulli.us>

On 9/16/19 3:44 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Add indication about previous failed devlink reload.
> 
> Example outputs:
> 
> $ devlink dev
> netdevsim/netdevsim10: reload_failed true

odd output to user. Why not just "reload failed"?

^ permalink raw reply

* Re: BUG: sleeping function called from invalid context in tcf_chain0_head_change_cb_del
From: Cong Wang @ 2019-09-17 17:03 UTC (permalink / raw)
  To: Vlad Buslov
  Cc: syzbot, Alexei Starovoitov, Daniel Borkmann, David Miller,
	David Ahern, Florian Fainelli, hawk@kernel.org, Ido Schimmel,
	Jakub Kicinski, Jamal Hadi Salim, Jiri Pirko, Jiri Pirko,
	John Fastabend, Martin KaFai Lau, LKML,
	Linux Kernel Network Developers, Nikolay Aleksandrov,
	Petr Machata, Roopa Prabhu, Song Liu, syzkaller-bugs,
	xdp-newbies@vger.kernel.org, yhs@fb.com
In-Reply-To: <vbfk1a7cooq.fsf@mellanox.com>

On Tue, Sep 17, 2019 at 1:27 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> Hi Cong,
>
> Don't see why we would need qdisc tree lock while releasing the
> reference to (or destroying) previous Qdisc. I've skimmed through other
> scheds and it looks like sch_multiq, sch_htb and sch_tbf are also
> affected. Do you want me to send patches?

Yes, please do.

^ permalink raw reply

* Re: [PATCH] selftests: Add test cases for `ip nexthop flush proto XX`
From: David Ahern @ 2019-09-17 17:04 UTC (permalink / raw)
  To: Donald Sharp, netdev, dsahern
In-Reply-To: <20190916122650.24124-1-sharpd@cumulusnetworks.com>

On 9/16/19 6:26 AM, Donald Sharp wrote:
> Add some test cases to allow the fib_nexthops.sh test code
> to test the flushing of nexthops based upon the proto passed
> in upon creation of the nexthop group.
> 
> Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
> ---
>  tools/testing/selftests/net/fib_nexthops.sh | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 

Reviewed-by: David Ahern <dsahern@gmail.com>



^ permalink raw reply

* Re: [PATCH 3/6] Timer: expose monotonic clock and counter value
From: Marc Zyngier @ 2019-09-17 17:10 UTC (permalink / raw)
  To: Jianyong Wu, netdev, pbonzini, sean.j.christopherson,
	richardcochran, Mark.Rutland, Will.Deacon, suzuki.poulose
  Cc: linux-kernel, Steve.Capper, Kaly.Xin, justin.he, nd,
	linux-arm-kernel
In-Reply-To: <20190917112430.45680-4-jianyong.wu@arm.com>

On 17/09/2019 12:24, Jianyong Wu wrote:
> A number of PTP drivers (such as ptp-kvm) are assuming what the
> current clock source is, which could lead to interesting effects on
> systems where the clocksource can change depending on external events.
> 
> For this purpose, add a new API that retrives both the current
> monotonic clock as well as its counter value.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

There must be something wrong with the way you've taken this patch in
your tree. My authorship is gone (not that I deeply care about it, but
it is good practice to keep attributions), and the subject line has been
rewritten.

I'd appreciate it if you could fix this in a future revision of this
series. For reference, the original patch is here[1].

> ---
>  include/linux/timekeeping.h |  3 +++
>  kernel/time/timekeeping.c   | 13 +++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
> index a8ab0f143ac4..a5389adaa8bc 100644
> --- a/include/linux/timekeeping.h
> +++ b/include/linux/timekeeping.h
> @@ -247,6 +247,9 @@ extern int get_device_system_crosststamp(
>  			struct system_time_snapshot *history,
>  			struct system_device_crosststamp *xtstamp);
>  
> +/* Obtain current monotonic clock and its counter value  */
> +extern void get_current_counterval(struct system_counterval_t *sc);
> +
>  /*
>   * Simultaneously snapshot realtime and monotonic raw clocks
>   */
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 44b726bab4bd..07a0969625b1 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1098,6 +1098,19 @@ static bool cycle_between(u64 before, u64 test, u64 after)
>  	return false;
>  }
>  
> +/**
> + * get_current_counterval - Snapshot the current clocksource and counter value
> + * @sc:	Pointer to a struct containing the current clocksource and its value
> + */
> +void get_current_counterval(struct system_counterval_t *sc)
> +{
> +	struct timekeeper *tk = &tk_core.timekeeper;
> +
> +	sc->cs = READ_ONCE(tk->tkr_mono.clock);
> +	sc->cycles = sc->cs->read(sc->cs);
> +}
> +EXPORT_SYMBOL_GPL(get_current_counterval);

This export wasn't in my original patch. I guess you need it because
your ptp driver builds as a module? It'd be good to mention it in the
commit log.

> +
>  /**
>   * get_device_system_crosststamp - Synchronously capture system/device timestamp
>   * @get_time_fn:	Callback to get simultaneous device time and
> 

Thanks,

	M.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=timer/counterval&id=a6e8abce025691b6a55e1c195878d7f76bfeb9d1
-- 
Jazz is not dead, it just smells funny...

^ permalink raw reply

* Re: [PATCH v2] tcp: Add TCP_INFO counter for packets received out-of-order
From: Jason Baron @ 2019-09-17 17:12 UTC (permalink / raw)
  To: Eric Dumazet, Thomas Higdon; +Cc: netdev, Jonathan Lemon, Dave Jones
In-Reply-To: <CANn89iKCSae880bS3MTwrm=MeTyPsntyXfkhJS7CfgtpiEpOsQ@mail.gmail.com>



On 9/10/19 4:38 PM, Eric Dumazet wrote:
> On Tue, Sep 10, 2019 at 10:11 PM Thomas Higdon <tph@fb.com> wrote:
>>
>>
> ...
>> Because an additional 32-bit member in struct tcp_info would cause
>> a hole on 64-bit systems, we reserve a struct member '_reserved'.
> ...
>> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
>> index b3564f85a762..990a5bae3ac1 100644
>> --- a/include/uapi/linux/tcp.h
>> +++ b/include/uapi/linux/tcp.h
>> @@ -270,6 +270,9 @@ struct tcp_info {
>>         __u64   tcpi_bytes_retrans;  /* RFC4898 tcpEStatsPerfOctetsRetrans */
>>         __u32   tcpi_dsack_dups;     /* RFC4898 tcpEStatsStackDSACKDups */
>>         __u32   tcpi_reord_seen;     /* reordering events seen */
>> +
>> +       __u32   _reserved;           /* Reserved for future 32-bit member. */
>> +       __u32   tcpi_rcv_ooopack;    /* Out-of-order packets received */
>>  };
>>
> 
> Unfortunately we won't be able to use this hole, because the way the
> TCP_INFO works,
> 
> The kernel will report the same size after the reserved field is
> renamed to something else.
> 
> User space code is able to detect which fields are there or not based
> on what the kernel
> returns for the size of the structure.
> 

Hi,

I was interested in adding a field to tcp_info around the TFO state of a
socket. So for the server side it would indicate if TFO was used to
create the socket and on the client side it would report whether TFO
worked and if not that it failed with maybe some additional states
around why it failed. I'm thinking it would be maybe 3 bits.

My question is whether its reasonable to use the unused bits of
__u8    tcpi_delivery_rate_app_limited:1;. Or is this not good because
the size hasn't changed? What if I avoided using 0 for the new field to
avoid the possibility of not knowing if 0 because its the old kernel or
0 because that's now its a TFO state? IE the new field could always be >
0 for the new kernel.

Thanks,

-Jason

^ permalink raw reply

* Re: [PATCH v2] tcp: Add TCP_INFO counter for packets received out-of-order
From: Eric Dumazet @ 2019-09-17 17:21 UTC (permalink / raw)
  To: Jason Baron; +Cc: Thomas Higdon, netdev, Jonathan Lemon, Dave Jones
In-Reply-To: <2c6a44fd-3b7e-9fd9-4773-34796b64928f@akamai.com>

 Tue, Sep 17, 2019 at 10:13 AM Jason Baron <jbaron@akamai.com> wrote:
>
>
> Hi,
>
> I was interested in adding a field to tcp_info around the TFO state of a
> socket. So for the server side it would indicate if TFO was used to
> create the socket and on the client side it would report whether TFO
> worked and if not that it failed with maybe some additional states
> around why it failed. I'm thinking it would be maybe 3 bits.
>
> My question is whether its reasonable to use the unused bits of
> __u8    tcpi_delivery_rate_app_limited:1;. Or is this not good because
> the size hasn't changed? What if I avoided using 0 for the new field to
> avoid the possibility of not knowing if 0 because its the old kernel or
> 0 because that's now its a TFO state? IE the new field could always be >
> 0 for the new kernel.
>

I guess that storing the 'why it has failed' would need more bits.

I suggest maybe using an event for this, instead of TCP_INFO ?

As of using the bits, maybe the monitoring application does not really care
if running on an old kernel where the bits would be zero.

Commit eb8329e0a04db0061f714f033b4454326ba147f4 reserved a single
bit and did not bother about making sure the monitoring would detect if this
runs on an old kernel.

^ permalink raw reply

* Re: -Wsizeof-array-div warnings in ethernet drivers
From: Lendacky, Thomas @ 2019-09-17 17:23 UTC (permalink / raw)
  To: Nathan Chancellor, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, David S. Miller
  Cc: Nick Desaulniers, Ilie Halip, David Bolvansky,
	netdev@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190917073232.GA14291@archlinux-threadripper>

On 9/17/19 2:32 AM, Nathan Chancellor wrote:
> Hi all,
> 
> Clang recently added a new diagnostic in r371605, -Wsizeof-array-div,
> that tries to warn when sizeof(X) / sizeof(Y) does not compute the
> number of elements in an array X (i.e., sizeof(Y) is wrong). See that
> commit for more details:
> 
> https://github.com/llvm/llvm-project/commit/3240ad4ced0d3223149b72a4fc2a4d9b67589427
> 
> Some ethernet drivers have an instance of this warning due to receive
> side scaling support:
> 
> 
> ../drivers/net/ethernet/amd/xgbe/xgbe-dev.c:361:49: warning: expression
> does not compute the number of elements in this array; element type is
> 'u8' (aka 'unsigned char'), not 'u32' (aka 'unsigned int')
> [-Wsizeof-array-div]
>         unsigned int key_regs = sizeof(pdata->rss_key) / sizeof(u32);
>                                        ~~~~~~~~~~~~~~  ^
> ../drivers/net/ethernet/amd/xgbe/xgbe-dev.c:361:49: note: place
> parentheses around the 'sizeof(u32)' expression to silence this warning
> 
> 
> ../drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:537:36: warning:
> expression does not compute the number of elements in this array;
> element type is 'u8' (aka 'unsigned char'), not 'u32' (aka 'unsigned
> int') [-Wsizeof-array-div]
>         for (i = 0; i < (sizeof(cfg->key) / sizeof(u32)); i++) {
>                                 ~~~~~~~~  ^
> ../drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:537:36: note:
> place parentheses around the 'sizeof(u32)' expression to silence this
> warning
> 
> 
> ../drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c:2329:49: warning:
> expression does not compute the number of elements in this array;
> element type is 'u8' (aka 'unsigned char'), not 'u32' (aka 'unsigned
> int') [-Wsizeof-array-div]
>         unsigned int key_regs = sizeof(pdata->rss_key) / sizeof(u32);
>                                        ~~~~~~~~~~~~~~  ^
> ../drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c:2329:49: note: place
> parentheses around the 'sizeof(u32)' expression to silence this warning
> 
> 
> What is the reasoning behind having the key being an array of u8s but
> seemlingly converting it into an array of u32s? It's not immediately

Probably because the ethtool functions that get and set the RSS key passes
the key buffer in as a u8 pointer.  Having said that, there's no reason
that any casting can't be done in the ethtool callback functions, if
needed (which I don't think it is, since the key buffer is used in
memcpy() calls), instead.

Thanks,
Tom

> apparent from reading over the code but I am not familiar with it so I
> might be making a mistake. I assume this is intentional? If so, the
> warning can be silenced and we'll send patches to do so but we want to
> make sure we aren't actually papering over a mistake.>
> Cheers!
> Nathan
> 

^ permalink raw reply

* [PATCH v3 0/2] net/ibmvnic: serialization fixes
From: Juliet Kim @ 2019-09-17 17:15 UTC (permalink / raw)
  To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev

This series includes two fixes. The first improves reset code to allow
linkwatch_event to proceed during reset. The second ensures that no more
than one thread runs in reset at a time.

v2:
- Separate change param reset from do_reset()
- Return IBMVNIC_OPEN_FAILED in open failiure case
- Remove setting wait_for_reset to false from __ibmvnic_reset(), this
  is done in wait_for_reset()
- Move the check for force_reset_recovery from patch 1 to patch 2

v3:
- Restore reset’s successful return in open failure case

Juliet Kim (2):
  net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run
  net/ibmvnic: prevent more than one thread from running in reset

 drivers/net/ethernet/ibm/ibmvnic.c | 245 +++++++++++++++++++++++++++----------
 drivers/net/ethernet/ibm/ibmvnic.h |   4 +
 2 files changed, 181 insertions(+), 68 deletions(-)

-- 
2.16.4


^ permalink raw reply

* [PATCH v3 1/2] net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run
From: Juliet Kim @ 2019-09-17 17:15 UTC (permalink / raw)
  To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev
In-Reply-To: <20190917171552.32498-1-julietk@linux.vnet.ibm.com>

Commit a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset")
made the change to hold the RTNL lock during a reset to avoid deadlock
but linkwatch_event is fired during the reset and needs the RTNL lock.
That keeps linkwatch_event process from proceeding until the reset
is complete. The reset process cannot tolerate the linkwatch_event
processing after reset completes, so release the RTNL lock during the
process to allow a chance for linkwatch_event to run during reset.
This does not guarantee that the linkwatch_event will be processed as
soon as link state changes, but is an improvement over the current code
where linkwatch_event processing is always delayed, which prevents
transmissions on the device from being deactivated leading transmit
watchdog timer to time-out.

Release the RTNL lock before link state change and re-acquire after
the link state change to allow linkwatch_event to grab the RTNL lock
and run during the reset.

Fixes: a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset")
Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 224 ++++++++++++++++++++++++++-----------
 drivers/net/ethernet/ibm/ibmvnic.h |   1 +
 2 files changed, 157 insertions(+), 68 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5cb55ea671e3..ba340aaff1b3 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1723,6 +1723,86 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p)
 	return rc;
 }
 
+/**
+ * do_change_param_reset returns zero if we are able to keep processing reset
+ * events, or non-zero if we hit a fatal error and must halt.
+ */
+static int do_change_param_reset(struct ibmvnic_adapter *adapter,
+				 struct ibmvnic_rwi *rwi,
+				 u32 reset_state)
+{
+	struct net_device *netdev = adapter->netdev;
+	int i, rc;
+
+	netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n",
+		   rwi->reset_reason);
+
+	netif_carrier_off(netdev);
+	adapter->reset_reason = rwi->reset_reason;
+
+	ibmvnic_cleanup(netdev);
+
+	if (reset_state == VNIC_OPEN) {
+		rc = __ibmvnic_close(netdev);
+		if (rc)
+			return rc;
+	}
+
+	release_resources(adapter);
+	release_sub_crqs(adapter, 1);
+	release_crq_queue(adapter);
+
+	adapter->state = VNIC_PROBED;
+
+	rc = init_crq_queue(adapter);
+
+	if (rc) {
+		netdev_err(adapter->netdev,
+			   "Couldn't initialize crq. rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = ibmvnic_reset_init(adapter);
+	if (rc)
+		return IBMVNIC_INIT_FAILED;
+
+	/* If the adapter was in PROBE state prior to the reset,
+	 * exit here.
+	 */
+	if (reset_state == VNIC_PROBED)
+		return 0;
+
+	rc = ibmvnic_login(netdev);
+	if (rc) {
+		adapter->state = reset_state;
+		return rc;
+	}
+
+	rc = init_resources(adapter);
+	if (rc)
+		return rc;
+
+	ibmvnic_disable_irqs(adapter);
+
+	adapter->state = VNIC_CLOSED;
+
+	if (reset_state == VNIC_CLOSED)
+		return 0;
+
+	rc = __ibmvnic_open(netdev);
+	if (rc)
+		return IBMVNIC_OPEN_FAILED;
+
+	/* refresh device's multicast list */
+	ibmvnic_set_multi(netdev);
+
+	/* kick napi */
+	for (i = 0; i < adapter->req_rx_queues; i++)
+		napi_schedule(&adapter->napi[i]);
+
+	return 0;
+}
+
 /**
  * do_reset returns zero if we are able to keep processing reset events, or
  * non-zero if we hit a fatal error and must halt.
@@ -1738,6 +1818,8 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
 		   rwi->reset_reason);
 
+	rtnl_lock();
+
 	netif_carrier_off(netdev);
 	adapter->reset_reason = rwi->reset_reason;
 
@@ -1751,16 +1833,25 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	if (reset_state == VNIC_OPEN &&
 	    adapter->reset_reason != VNIC_RESET_MOBILITY &&
 	    adapter->reset_reason != VNIC_RESET_FAILOVER) {
-		rc = __ibmvnic_close(netdev);
+		adapter->state = VNIC_CLOSING;
+
+		/* Release the RTNL lock before link state change and
+		 * re-acquire after the link state change to allow
+		 * linkwatch_event to grab the RTNL lock and run during
+		 * a reset.
+		 */
+		rtnl_unlock();
+		rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+		rtnl_lock();
 		if (rc)
-			return rc;
-	}
+			goto out;
 
-	if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
-	    adapter->wait_for_reset) {
-		release_resources(adapter);
-		release_sub_crqs(adapter, 1);
-		release_crq_queue(adapter);
+		if (adapter->state != VNIC_CLOSING) {
+			rc = -1;
+			goto out;
+		}
+
+		adapter->state = VNIC_CLOSED;
 	}
 
 	if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
@@ -1769,9 +1860,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 		 */
 		adapter->state = VNIC_PROBED;
 
-		if (adapter->wait_for_reset) {
-			rc = init_crq_queue(adapter);
-		} else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
+		if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
 			rc = ibmvnic_reenable_crq_queue(adapter);
 			release_sub_crqs(adapter, 1);
 		} else {
@@ -1783,36 +1872,35 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 		if (rc) {
 			netdev_err(adapter->netdev,
 				   "Couldn't initialize crq. rc=%d\n", rc);
-			return rc;
+			goto out;
 		}
 
 		rc = ibmvnic_reset_init(adapter);
-		if (rc)
-			return IBMVNIC_INIT_FAILED;
+		if (rc) {
+			rc = IBMVNIC_INIT_FAILED;
+			goto out;
+		}
 
 		/* If the adapter was in PROBE state prior to the reset,
 		 * exit here.
 		 */
-		if (reset_state == VNIC_PROBED)
-			return 0;
+		if (reset_state == VNIC_PROBED) {
+			rc = 0;
+			goto out;
+		}
 
 		rc = ibmvnic_login(netdev);
 		if (rc) {
 			adapter->state = reset_state;
-			return rc;
+			goto out;
 		}
 
-		if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
-		    adapter->wait_for_reset) {
-			rc = init_resources(adapter);
-			if (rc)
-				return rc;
-		} else if (adapter->req_rx_queues != old_num_rx_queues ||
-			   adapter->req_tx_queues != old_num_tx_queues ||
-			   adapter->req_rx_add_entries_per_subcrq !=
-							old_num_rx_slots ||
-			   adapter->req_tx_entries_per_subcrq !=
-							old_num_tx_slots) {
+		if (adapter->req_rx_queues != old_num_rx_queues ||
+		    adapter->req_tx_queues != old_num_tx_queues ||
+		    adapter->req_rx_add_entries_per_subcrq !=
+		    old_num_rx_slots ||
+		    adapter->req_tx_entries_per_subcrq !=
+		    old_num_tx_slots) {
 			release_rx_pools(adapter);
 			release_tx_pools(adapter);
 			release_napi(adapter);
@@ -1820,32 +1908,30 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 
 			rc = init_resources(adapter);
 			if (rc)
-				return rc;
+				goto out;
 
 		} else {
 			rc = reset_tx_pools(adapter);
 			if (rc)
-				return rc;
+				goto out;
 
 			rc = reset_rx_pools(adapter);
 			if (rc)
-				return rc;
+				goto out;
 		}
 		ibmvnic_disable_irqs(adapter);
 	}
 	adapter->state = VNIC_CLOSED;
 
-	if (reset_state == VNIC_CLOSED)
-		return 0;
+	if (reset_state == VNIC_CLOSED) {
+		rc = 0;
+		goto out;
+	}
 
 	rc = __ibmvnic_open(netdev);
 	if (rc) {
-		if (list_empty(&adapter->rwi_list))
-			adapter->state = VNIC_CLOSED;
-		else
-			adapter->state = reset_state;
-
-		return 0;
+		rc = IBMVNIC_OPEN_FAILED;
+		goto out;
 	}
 
 	/* refresh device's multicast list */
@@ -1855,11 +1941,15 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	for (i = 0; i < adapter->req_rx_queues; i++)
 		napi_schedule(&adapter->napi[i]);
 
-	if (adapter->reset_reason != VNIC_RESET_FAILOVER &&
-	    adapter->reset_reason != VNIC_RESET_CHANGE_PARAM)
+	if (adapter->reset_reason != VNIC_RESET_FAILOVER)
 		call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, netdev);
 
-	return 0;
+	rc = 0;
+
+out:
+	rtnl_unlock();
+
+	return rc;
 }
 
 static int do_hard_reset(struct ibmvnic_adapter *adapter,
@@ -1919,14 +2009,8 @@ static int do_hard_reset(struct ibmvnic_adapter *adapter,
 		return 0;
 
 	rc = __ibmvnic_open(netdev);
-	if (rc) {
-		if (list_empty(&adapter->rwi_list))
-			adapter->state = VNIC_CLOSED;
-		else
-			adapter->state = reset_state;
-
-		return 0;
-	}
+	if (rc)
+		return IBMVNIC_OPEN_FAILED;
 
 	return 0;
 }
@@ -1965,20 +2049,11 @@ static void __ibmvnic_reset(struct work_struct *work)
 {
 	struct ibmvnic_rwi *rwi;
 	struct ibmvnic_adapter *adapter;
-	bool we_lock_rtnl = false;
 	u32 reset_state;
 	int rc = 0;
 
 	adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
 
-	/* netif_set_real_num_xx_queues needs to take rtnl lock here
-	 * unless wait_for_reset is set, in which case the rtnl lock
-	 * has already been taken before initializing the reset
-	 */
-	if (!adapter->wait_for_reset) {
-		rtnl_lock();
-		we_lock_rtnl = true;
-	}
 	reset_state = adapter->state;
 
 	rwi = get_next_rwi(adapter);
@@ -1990,14 +2065,32 @@ static void __ibmvnic_reset(struct work_struct *work)
 			break;
 		}
 
-		if (adapter->force_reset_recovery) {
-			adapter->force_reset_recovery = false;
-			rc = do_hard_reset(adapter, rwi, reset_state);
+		if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+			/* CHANGE_PARAM requestor holds rtnl_lock */
+			rc = do_change_param_reset(adapter, rwi, reset_state);
+		} else if (adapter->force_reset_recovery) {
+			/* Transport event occurred during previous reset */
+			if (adapter->wait_for_reset) {
+				/* Previous was CHANGE_PARAM; caller locked */
+				adapter->force_reset_recovery = false;
+				rc = do_hard_reset(adapter, rwi, reset_state);
+			} else {
+				rtnl_lock();
+				adapter->force_reset_recovery = false;
+				rc = do_hard_reset(adapter, rwi, reset_state);
+				rtnl_unlock();
+			}
 		} else {
 			rc = do_reset(adapter, rwi, reset_state);
 		}
 		kfree(rwi);
-		if (rc && rc != IBMVNIC_INIT_FAILED &&
+		if (rc == IBMVNIC_OPEN_FAILED) {
+			if (list_empty(&adapter->rwi_list))
+				adapter->state = VNIC_CLOSED;
+			else
+				adapter->state = reset_state;
+			rc = 0;
+		} else if (rc && rc != IBMVNIC_INIT_FAILED &&
 		    !adapter->force_reset_recovery)
 			break;
 
@@ -2005,7 +2098,6 @@ static void __ibmvnic_reset(struct work_struct *work)
 	}
 
 	if (adapter->wait_for_reset) {
-		adapter->wait_for_reset = false;
 		adapter->reset_done_rc = rc;
 		complete(&adapter->reset_done);
 	}
@@ -2016,8 +2108,6 @@ static void __ibmvnic_reset(struct work_struct *work)
 	}
 
 	adapter->resetting = false;
-	if (we_lock_rtnl)
-		rtnl_unlock();
 }
 
 static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
@@ -2078,8 +2168,6 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 
 	return 0;
 err:
-	if (adapter->wait_for_reset)
-		adapter->wait_for_reset = false;
 	return -ret;
 }
 
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 70bd286f8932..9d3d35cc91d6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -20,6 +20,7 @@
 #define IBMVNIC_INVALID_MAP	-1
 #define IBMVNIC_STATS_TIMEOUT	1
 #define IBMVNIC_INIT_FAILED	2
+#define IBMVNIC_OPEN_FAILED	3
 
 /* basic structures plus 100 2k buffers */
 #define IBMVNIC_IO_ENTITLEMENT_DEFAULT	610305
-- 
2.16.4


^ permalink raw reply related

* [PATCH v3 2/2] net/ibmvnic: prevent more than one thread from running in reset
From: Juliet Kim @ 2019-09-17 17:15 UTC (permalink / raw)
  To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev
In-Reply-To: <20190917171552.32498-1-julietk@linux.vnet.ibm.com>

Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 23 ++++++++++++++++++++++-
 drivers/net/ethernet/ibm/ibmvnic.h |  3 +++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index ba340aaff1b3..f344ccd68ad9 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2054,6 +2054,13 @@ static void __ibmvnic_reset(struct work_struct *work)
 
 	adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
 
+	if (adapter->resetting) {
+		schedule_delayed_work(&adapter->ibmvnic_delayed_reset,
+				      IBMVNIC_RESET_DELAY);
+		return;
+	}
+
+	adapter->resetting = true;
 	reset_state = adapter->state;
 
 	rwi = get_next_rwi(adapter);
@@ -2095,6 +2102,10 @@ static void __ibmvnic_reset(struct work_struct *work)
 			break;
 
 		rwi = get_next_rwi(adapter);
+
+		if (rwi && (rwi->reset_reason == VNIC_RESET_FAILOVER ||
+			    rwi->reset_reason == VNIC_RESET_MOBILITY))
+			adapter->force_reset_recovery = true;
 	}
 
 	if (adapter->wait_for_reset) {
@@ -2110,6 +2121,15 @@ static void __ibmvnic_reset(struct work_struct *work)
 	adapter->resetting = false;
 }
 
+static void __ibmvnic_delayed_reset(struct work_struct *work)
+{
+	struct ibmvnic_adapter *adapter;
+
+	adapter = container_of(work, struct ibmvnic_adapter,
+			       ibmvnic_delayed_reset.work);
+	__ibmvnic_reset(&adapter->ibmvnic_reset);
+}
+
 static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 			 enum ibmvnic_reset_reason reason)
 {
@@ -2162,7 +2182,6 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 	rwi->reset_reason = reason;
 	list_add_tail(&rwi->list, &adapter->rwi_list);
 	spin_unlock_irqrestore(&adapter->rwi_lock, flags);
-	adapter->resetting = true;
 	netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason);
 	schedule_work(&adapter->ibmvnic_reset);
 
@@ -4933,6 +4952,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	spin_lock_init(&adapter->stats_lock);
 
 	INIT_WORK(&adapter->ibmvnic_reset, __ibmvnic_reset);
+	INIT_DELAYED_WORK(&adapter->ibmvnic_delayed_reset,
+			  __ibmvnic_delayed_reset);
 	INIT_LIST_HEAD(&adapter->rwi_list);
 	spin_lock_init(&adapter->rwi_lock);
 	init_completion(&adapter->init_done);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 9d3d35cc91d6..4f4651d92cc1 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -39,6 +39,8 @@
 #define IBMVNIC_MAX_LTB_SIZE ((1 << (MAX_ORDER - 1)) * PAGE_SIZE)
 #define IBMVNIC_BUFFER_HLEN 500
 
+#define IBMVNIC_RESET_DELAY 100
+
 static const char ibmvnic_priv_flags[][ETH_GSTRING_LEN] = {
 #define IBMVNIC_USE_SERVER_MAXES 0x1
 	"use-server-maxes"
@@ -1077,6 +1079,7 @@ struct ibmvnic_adapter {
 	spinlock_t rwi_lock;
 	struct list_head rwi_list;
 	struct work_struct ibmvnic_reset;
+	struct delayed_work ibmvnic_delayed_reset;
 	bool resetting;
 	bool napi_enabled, from_passive_init;
 
-- 
2.16.4


^ permalink raw reply related

* [PATCH net] selftests: Update fib_tests to handle missing ping6
From: David Ahern @ 2019-09-17 17:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, David Ahern

From: David Ahern <dsahern@gmail.com>

Some distributions (e.g., debian buster) do not install ping6. Re-use
the hook in pmtu.sh to detect this and fallback to ping.

Fixes: a0e11da78f48 ("fib_tests: Add tests for metrics on routes")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 tools/testing/selftests/net/fib_tests.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 4465fc2dae14..cba83a12da82 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -17,6 +17,8 @@ PAUSE=no
 IP="ip -netns ns1"
 NS_EXEC="ip netns exec ns1"
 
+which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
+
 log_test()
 {
 	local rc=$1
@@ -1086,7 +1088,7 @@ ipv6_route_metrics_test()
 	log_test $rc 0 "Multipath route with mtu metric"
 
 	$IP -6 ro add 2001:db8:104::/64 via 2001:db8:101::2 mtu 1300
-	run_cmd "ip netns exec ns1 ping6 -w1 -c1 -s 1500 2001:db8:104::1"
+	run_cmd "ip netns exec ns1 ${ping6} -w1 -c1 -s 1500 2001:db8:104::1"
 	log_test $? 0 "Using route with mtu metric"
 
 	run_cmd "$IP -6 ro add 2001:db8:114::/64 via  2001:db8:101::2  congctl lock foo"
-- 
2.11.0


^ permalink raw reply related

* [PATCH] selftests: Update fib_nexthop_multiprefix to handle missing ping6
From: David Ahern @ 2019-09-17 17:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, David Ahern

From: David Ahern <dsahern@gmail.com>

Some distributions (e.g., debian buster) do not install ping6. Re-use
the hook in pmtu.sh to detect this and fallback to ping.

Fixes: 735ab2f65dce ("selftests: Add test with multiple prefixes using single nexthop")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 tools/testing/selftests/net/fib_nexthop_multiprefix.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/fib_nexthop_multiprefix.sh b/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
index e6828732843e..9dc35a16e415 100755
--- a/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
+++ b/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
@@ -15,6 +15,8 @@
 PAUSE_ON_FAIL=no
 VERBOSE=0
 
+which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
+
 ################################################################################
 # helpers
 
@@ -200,7 +202,7 @@ validate_v6_exception()
 	local rc
 
 	if [ ${ping_sz} != "0" ]; then
-		run_cmd ip netns exec h0 ping6 -s ${ping_sz} -c5 -w5 ${dst}
+		run_cmd ip netns exec h0 ${ping6} -s ${ping_sz} -c5 -w5 ${dst}
 	fi
 
 	if [ "$VERBOSE" = "1" ]; then
@@ -243,7 +245,7 @@ do
 		run_cmd taskset -c ${c} ip netns exec h0 ping -c1 -w1 172.16.10${i}.1
 		[ $? -ne 0 ] && printf "\nERROR: ping to h${i} failed\n" && ret=1
 
-		run_cmd taskset -c ${c} ip netns exec h0 ping6 -c1 -w1 2001:db8:10${i}::1
+		run_cmd taskset -c ${c} ip netns exec h0 ${ping6} -c1 -w1 2001:db8:10${i}::1
 		[ $? -ne 0 ] && printf "\nERROR: ping6 to h${i} failed\n" && ret=1
 
 		[ $ret -ne 0 ] && break
-- 
2.11.0


^ permalink raw reply related


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