All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 00/11] netpoll: second round of fixes.
@ 2018-09-27 16:31 Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi() Eric Dumazet
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC).

This capture, showing one ksoftirqd eating all cycles
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

It seems that all networking drivers that do use NAPI
for their TX completions, should not provide a ndo_poll_controller() :

Most NAPI drivers have netpoll support already handled
in core networking stack, since netpoll_poll_dev()
uses poll_napi(dev) to iterate through registered
NAPI contexts for a device.

First patch is a fix in poll_one_napi().

Then following patches take care of ten drivers.

Eric Dumazet (11):
  netpoll: do not test NAPI_STATE_SCHED in poll_one_napi()
  hinic: remove ndo_poll_controller
  ehea: remove ndo_poll_controller
  net: hns: remove ndo_poll_controller
  virtio_net: remove ndo_poll_controller
  qlcnic: remove ndo_poll_controller
  qlogic: netxen: remove ndo_poll_controller
  net: ena: remove ndo_poll_controller
  sfc: remove ndo_poll_controller
  sfc-falcon: remove ndo_poll_controller
  ibmvnic: remove ndo_poll_controller

 drivers/net/ethernet/amazon/ena/ena_netdev.c  | 22 ---------
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 18 --------
 .../net/ethernet/huawei/hinic/hinic_main.c    | 20 ---------
 drivers/net/ethernet/ibm/ehea/ehea_main.c     | 14 ------
 drivers/net/ethernet/ibm/ibmvnic.c            | 16 -------
 .../ethernet/qlogic/netxen/netxen_nic_main.c  | 23 ----------
 .../net/ethernet/qlogic/qlcnic/qlcnic_main.c  | 45 -------------------
 drivers/net/ethernet/sfc/efx.c                | 26 -----------
 drivers/net/ethernet/sfc/falcon/efx.c         | 26 -----------
 drivers/net/virtio_net.c                      | 14 ------
 net/core/netpoll.c                            | 20 +--------
 11 files changed, 1 insertion(+), 243 deletions(-)

-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi()
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-27 18:25   ` Michael Chan
  2018-09-27 16:31 ` [PATCH net 02/11] hinic: remove ndo_poll_controller Eric Dumazet
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon

Since we do no longer require NAPI drivers to provide
an ndo_poll_controller(), napi_schedule() has not been done
before poll_one_napi() invocation.

So testing NAPI_STATE_SCHED is likely to cause early returns.

While we are at it, remove outdated comment.

Note to future bisections : This change might surface prior
bugs in drivers. See commit 73f21c653f93 ("bnxt_en: Fix TX
timeout during netpoll.") for one occurrence.

Fixes: ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Tested-by: Song Liu <songliubraving@fb.com>
Cc: Michael Chan <michael.chan@broadcom.com>
---
 net/core/netpoll.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 3219a2932463096566ce8ff336ecdf699422dd65..3ae899805f8b674b4ffb7d791330f836d38eff7d 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -135,27 +135,9 @@ static void queue_process(struct work_struct *work)
 	}
 }
 
-/*
- * Check whether delayed processing was scheduled for our NIC. If so,
- * we attempt to grab the poll lock and use ->poll() to pump the card.
- * If this fails, either we've recursed in ->poll() or it's already
- * running on another CPU.
- *
- * Note: we don't mask interrupts with this lock because we're using
- * trylock here and interrupts are already disabled in the softirq
- * case. Further, we test the poll_owner to avoid recursion on UP
- * systems where the lock doesn't exist.
- */
 static void poll_one_napi(struct napi_struct *napi)
 {
-	int work = 0;
-
-	/* net_rx_action's ->poll() invocations and our's are
-	 * synchronized by this test which is only made while
-	 * holding the napi->poll_lock.
-	 */
-	if (!test_bit(NAPI_STATE_SCHED, &napi->state))
-		return;
+	int work;
 
 	/* If we set this bit but see that it has already been set,
 	 * that indicates that napi has been disabled and we need
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 02/11] hinic: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi() Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 03/11] ehea: " Eric Dumazet
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

hinic uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Note that hinic_netpoll() was incorrectly scheduling NAPI
on both RX and TX queues.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Aviad Krawczyk <aviad.krawczyk@huawei.com>
---
 .../net/ethernet/huawei/hinic/hinic_main.c    | 20 -------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index 09e9da10b786549b6232d8069c4e45857b95fd8c..4a8f82938ed5b87c8da6b09e88e08d387c652f0c 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -789,23 +789,6 @@ static void hinic_get_stats64(struct net_device *netdev,
 	stats->tx_errors  = nic_tx_stats->tx_dropped;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void hinic_netpoll(struct net_device *netdev)
-{
-	struct hinic_dev *nic_dev = netdev_priv(netdev);
-	int i, num_qps;
-
-	num_qps = hinic_hwdev_num_qps(nic_dev->hwdev);
-	for (i = 0; i < num_qps; i++) {
-		struct hinic_txq *txq = &nic_dev->txqs[i];
-		struct hinic_rxq *rxq = &nic_dev->rxqs[i];
-
-		napi_schedule(&txq->napi);
-		napi_schedule(&rxq->napi);
-	}
-}
-#endif
-
 static const struct net_device_ops hinic_netdev_ops = {
 	.ndo_open = hinic_open,
 	.ndo_stop = hinic_close,
@@ -818,9 +801,6 @@ static const struct net_device_ops hinic_netdev_ops = {
 	.ndo_start_xmit = hinic_xmit_frame,
 	.ndo_tx_timeout = hinic_tx_timeout,
 	.ndo_get_stats64 = hinic_get_stats64,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = hinic_netpoll,
-#endif
 };
 
 static void netdev_features_init(struct net_device *netdev)
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 03/11] ehea: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi() Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 02/11] hinic: remove ndo_poll_controller Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 04/11] net: hns: " Eric Dumazet
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

ehea uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Douglas Miller <dougmill@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ehea/ehea_main.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index ba580bfae512326d346e96e67718bec10ed716cc..03f64f40b2a3e0a3d9f3432cb6fbdd7bcd264a4c 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -921,17 +921,6 @@ static int ehea_poll(struct napi_struct *napi, int budget)
 	return rx;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void ehea_netpoll(struct net_device *dev)
-{
-	struct ehea_port *port = netdev_priv(dev);
-	int i;
-
-	for (i = 0; i < port->num_def_qps; i++)
-		napi_schedule(&port->port_res[i].napi);
-}
-#endif
-
 static irqreturn_t ehea_recv_irq_handler(int irq, void *param)
 {
 	struct ehea_port_res *pr = param;
@@ -2953,9 +2942,6 @@ static const struct net_device_ops ehea_netdev_ops = {
 	.ndo_open		= ehea_open,
 	.ndo_stop		= ehea_stop,
 	.ndo_start_xmit		= ehea_start_xmit,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller	= ehea_netpoll,
-#endif
 	.ndo_get_stats64	= ehea_get_stats64,
 	.ndo_set_mac_address	= ehea_set_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 04/11] net: hns: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (2 preceding siblings ...)
  2018-09-27 16:31 ` [PATCH net 03/11] ehea: " Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 05/11] virtio_net: " Eric Dumazet
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Salil Mehta

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

hns uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yisen Zhuang <yisen.zhuang@huawei.com>
Cc: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 5ce23d4b717ec2004d9fea5a222c7f0ba48eee06..28e907831b0eddbf760e0edb579ae7ae708520e0 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1503,21 +1503,6 @@ static int hns_nic_do_ioctl(struct net_device *netdev, struct ifreq *ifr,
 	return phy_mii_ioctl(phy_dev, ifr, cmd);
 }
 
-/* use only for netconsole to poll with the device without interrupt */
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void hns_nic_poll_controller(struct net_device *ndev)
-{
-	struct hns_nic_priv *priv = netdev_priv(ndev);
-	unsigned long flags;
-	int i;
-
-	local_irq_save(flags);
-	for (i = 0; i < priv->ae_handle->q_num * 2; i++)
-		napi_schedule(&priv->ring_data[i].napi);
-	local_irq_restore(flags);
-}
-#endif
-
 static netdev_tx_t hns_nic_net_xmit(struct sk_buff *skb,
 				    struct net_device *ndev)
 {
@@ -1970,9 +1955,6 @@ static const struct net_device_ops hns_nic_netdev_ops = {
 	.ndo_set_features = hns_nic_set_features,
 	.ndo_fix_features = hns_nic_fix_features,
 	.ndo_get_stats64 = hns_nic_get_stats64,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = hns_nic_poll_controller,
-#endif
 	.ndo_set_rx_mode = hns_nic_set_rx_mode,
 	.ndo_select_queue = hns_nic_select_queue,
 };
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 05/11] virtio_net: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (3 preceding siblings ...)
  2018-09-27 16:31 ` [PATCH net 04/11] net: hns: " Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 06/11] qlcnic: " Eric Dumazet
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

virto_net uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 76592090522607a4bdf5422b1d49ec99c6fd68ac..dab504ec5e502be401cbfe9a8e3f0f572c0220ba 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1699,17 +1699,6 @@ static void virtnet_stats(struct net_device *dev,
 	tot->rx_frame_errors = dev->stats.rx_frame_errors;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void virtnet_netpoll(struct net_device *dev)
-{
-	struct virtnet_info *vi = netdev_priv(dev);
-	int i;
-
-	for (i = 0; i < vi->curr_queue_pairs; i++)
-		napi_schedule(&vi->rq[i].napi);
-}
-#endif
-
 static void virtnet_ack_link_announce(struct virtnet_info *vi)
 {
 	rtnl_lock();
@@ -2447,9 +2436,6 @@ static const struct net_device_ops virtnet_netdev = {
 	.ndo_get_stats64     = virtnet_stats,
 	.ndo_vlan_rx_add_vid = virtnet_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = virtnet_vlan_rx_kill_vid,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = virtnet_netpoll,
-#endif
 	.ndo_bpf		= virtnet_xdp,
 	.ndo_xdp_xmit		= virtnet_xdp_xmit,
 	.ndo_features_check	= passthru_features_check,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 06/11] qlcnic: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (4 preceding siblings ...)
  2018-09-27 16:31 ` [PATCH net 05/11] virtio_net: " Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 07/11] qlogic: netxen: " Eric Dumazet
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

qlcnic uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Harish Patil <harish.patil@cavium.com>
Cc: Manish Chopra <manish.chopra@cavium.com>
---
 .../net/ethernet/qlogic/qlcnic/qlcnic_main.c  | 45 -------------------
 1 file changed, 45 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 2d38d1ac2aae58fd210030c7b143011f76b921cc..dbd48012224f2467d27134eedc692a68b92b1a04 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -59,9 +59,6 @@ static int qlcnic_close(struct net_device *netdev);
 static void qlcnic_tx_timeout(struct net_device *netdev);
 static void qlcnic_attach_work(struct work_struct *work);
 static void qlcnic_fwinit_work(struct work_struct *work);
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void qlcnic_poll_controller(struct net_device *netdev);
-#endif
 
 static void qlcnic_idc_debug_info(struct qlcnic_adapter *adapter, u8 encoding);
 static int qlcnic_can_start_firmware(struct qlcnic_adapter *adapter);
@@ -545,9 +542,6 @@ static const struct net_device_ops qlcnic_netdev_ops = {
 	.ndo_udp_tunnel_add	= qlcnic_add_vxlan_port,
 	.ndo_udp_tunnel_del	= qlcnic_del_vxlan_port,
 	.ndo_features_check	= qlcnic_features_check,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = qlcnic_poll_controller,
-#endif
 #ifdef CONFIG_QLCNIC_SRIOV
 	.ndo_set_vf_mac		= qlcnic_sriov_set_vf_mac,
 	.ndo_set_vf_rate	= qlcnic_sriov_set_vf_tx_rate,
@@ -3200,45 +3194,6 @@ static irqreturn_t qlcnic_msix_tx_intr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void qlcnic_poll_controller(struct net_device *netdev)
-{
-	struct qlcnic_adapter *adapter = netdev_priv(netdev);
-	struct qlcnic_host_sds_ring *sds_ring;
-	struct qlcnic_recv_context *recv_ctx;
-	struct qlcnic_host_tx_ring *tx_ring;
-	int ring;
-
-	if (!test_bit(__QLCNIC_DEV_UP, &adapter->state))
-		return;
-
-	recv_ctx = adapter->recv_ctx;
-
-	for (ring = 0; ring < adapter->drv_sds_rings; ring++) {
-		sds_ring = &recv_ctx->sds_rings[ring];
-		qlcnic_disable_sds_intr(adapter, sds_ring);
-		napi_schedule(&sds_ring->napi);
-	}
-
-	if (adapter->flags & QLCNIC_MSIX_ENABLED) {
-		/* Only Multi-Tx queue capable devices need to
-		 * schedule NAPI for TX rings
-		 */
-		if ((qlcnic_83xx_check(adapter) &&
-		     (adapter->flags & QLCNIC_TX_INTR_SHARED)) ||
-		    (qlcnic_82xx_check(adapter) &&
-		     !qlcnic_check_multi_tx(adapter)))
-			return;
-
-		for (ring = 0; ring < adapter->drv_tx_rings; ring++) {
-			tx_ring = &adapter->tx_ring[ring];
-			qlcnic_disable_tx_intr(adapter, tx_ring);
-			napi_schedule(&tx_ring->napi);
-		}
-	}
-}
-#endif
-
 static void
 qlcnic_idc_debug_info(struct qlcnic_adapter *adapter, u8 encoding)
 {
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 07/11] qlogic: netxen: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (5 preceding siblings ...)
  2018-09-27 16:31 ` [PATCH net 06/11] qlcnic: " Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 08/11] net: ena: " Eric Dumazet
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Rahul Verma

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

netxen uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Manish Chopra <manish.chopra@cavium.com>
Cc: Rahul Verma <rahul.verma@cavium.com>
---
 .../ethernet/qlogic/netxen/netxen_nic_main.c  | 23 -------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 69aa7fc392c5e4ad1cbcd9025f56bffdf3aa92c7..59c70be22a84c11262388529cf0ddf09887cea96 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -72,9 +72,6 @@ static void netxen_schedule_work(struct netxen_adapter *adapter,
 		work_func_t func, int delay);
 static void netxen_cancel_fw_work(struct netxen_adapter *adapter);
 static int netxen_nic_poll(struct napi_struct *napi, int budget);
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void netxen_nic_poll_controller(struct net_device *netdev);
-#endif
 
 static void netxen_create_sysfs_entries(struct netxen_adapter *adapter);
 static void netxen_remove_sysfs_entries(struct netxen_adapter *adapter);
@@ -581,9 +578,6 @@ static const struct net_device_ops netxen_netdev_ops = {
 	.ndo_tx_timeout	   = netxen_tx_timeout,
 	.ndo_fix_features = netxen_fix_features,
 	.ndo_set_features = netxen_set_features,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = netxen_nic_poll_controller,
-#endif
 };
 
 static inline bool netxen_function_zero(struct pci_dev *pdev)
@@ -2402,23 +2396,6 @@ static int netxen_nic_poll(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void netxen_nic_poll_controller(struct net_device *netdev)
-{
-	int ring;
-	struct nx_host_sds_ring *sds_ring;
-	struct netxen_adapter *adapter = netdev_priv(netdev);
-	struct netxen_recv_context *recv_ctx = &adapter->recv_ctx;
-
-	disable_irq(adapter->irq);
-	for (ring = 0; ring < adapter->max_sds_rings; ring++) {
-		sds_ring = &recv_ctx->sds_rings[ring];
-		netxen_intr(adapter->irq, sds_ring);
-	}
-	enable_irq(adapter->irq);
-}
-#endif
-
 static int
 nx_incr_dev_ref_cnt(struct netxen_adapter *adapter)
 {
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 08/11] net: ena: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (6 preceding siblings ...)
  2018-09-27 16:31 ` [PATCH net 07/11] qlogic: netxen: " Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-27 16:31 ` [PATCH net 09/11] sfc: " Eric Dumazet
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Saeed Bishara,
	Zorik Machulsky

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

ena uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Netanel Belgazal <netanel@amazon.com>
Cc: Saeed Bishara <saeedb@amazon.com>
Cc: Zorik Machulsky <zorik@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 22 --------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 29b5774dd32d47e5ad2c43e78b599493f011d129..25621a218f20754c29963c7ded3075c0c89a232c 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -2185,25 +2185,6 @@ static netdev_tx_t ena_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void ena_netpoll(struct net_device *netdev)
-{
-	struct ena_adapter *adapter = netdev_priv(netdev);
-	int i;
-
-	/* Dont schedule NAPI if the driver is in the middle of reset
-	 * or netdev is down.
-	 */
-
-	if (!test_bit(ENA_FLAG_DEV_UP, &adapter->flags) ||
-	    test_bit(ENA_FLAG_TRIGGER_RESET, &adapter->flags))
-		return;
-
-	for (i = 0; i < adapter->num_queues; i++)
-		napi_schedule(&adapter->ena_napi[i].napi);
-}
-#endif /* CONFIG_NET_POLL_CONTROLLER */
-
 static u16 ena_select_queue(struct net_device *dev, struct sk_buff *skb,
 			    struct net_device *sb_dev,
 			    select_queue_fallback_t fallback)
@@ -2369,9 +2350,6 @@ static const struct net_device_ops ena_netdev_ops = {
 	.ndo_change_mtu		= ena_change_mtu,
 	.ndo_set_mac_address	= NULL,
 	.ndo_validate_addr	= eth_validate_addr,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller	= ena_netpoll,
-#endif /* CONFIG_NET_POLL_CONTROLLER */
 };
 
 static int ena_device_validate_params(struct ena_adapter *adapter,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 09/11] sfc: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (7 preceding siblings ...)
  2018-09-27 16:31 ` [PATCH net 08/11] net: ena: " Eric Dumazet
@ 2018-09-27 16:31 ` Eric Dumazet
  2018-09-28  7:30   ` Bert Kenward
  2018-09-27 16:32 ` [PATCH net 10/11] sfc-falcon: " Eric Dumazet
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Edward Cree,
	Bert Kenward

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

sfc uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Edward Cree <ecree@solarflare.com>
Cc: Bert Kenward <bkenward@solarflare.com>
Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
---
 drivers/net/ethernet/sfc/efx.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 330233286e785254f5f29c87f9557a305974f606..3d0dd39c289e05b8a7a6778363461ef5698dc62b 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2206,29 +2206,6 @@ static void efx_fini_napi(struct efx_nic *efx)
 		efx_fini_napi_channel(channel);
 }
 
-/**************************************************************************
- *
- * Kernel netpoll interface
- *
- *************************************************************************/
-
-#ifdef CONFIG_NET_POLL_CONTROLLER
-
-/* Although in the common case interrupts will be disabled, this is not
- * guaranteed. However, all our work happens inside the NAPI callback,
- * so no locking is required.
- */
-static void efx_netpoll(struct net_device *net_dev)
-{
-	struct efx_nic *efx = netdev_priv(net_dev);
-	struct efx_channel *channel;
-
-	efx_for_each_channel(channel, efx)
-		efx_schedule_channel(channel);
-}
-
-#endif
-
 /**************************************************************************
  *
  * Kernel net device interface
@@ -2509,9 +2486,6 @@ static const struct net_device_ops efx_netdev_ops = {
 #endif
 	.ndo_get_phys_port_id   = efx_get_phys_port_id,
 	.ndo_get_phys_port_name	= efx_get_phys_port_name,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = efx_netpoll,
-#endif
 	.ndo_setup_tc		= efx_setup_tc,
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer	= efx_filter_rfs,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 10/11] sfc-falcon: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (8 preceding siblings ...)
  2018-09-27 16:31 ` [PATCH net 09/11] sfc: " Eric Dumazet
@ 2018-09-27 16:32 ` Eric Dumazet
  2018-09-28  7:30   ` Bert Kenward
  2018-09-27 16:32 ` [PATCH net 11/11] ibmvnic: " Eric Dumazet
  2018-09-28 18:14 ` [PATCH net 00/11] netpoll: second round of fixes David Miller
  11 siblings, 1 reply; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:32 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Edward Cree,
	Bert Kenward

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

sfc-falcon uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
Cc: Edward Cree <ecree@solarflare.com>
Cc: Bert Kenward <bkenward@solarflare.com>
---
 drivers/net/ethernet/sfc/falcon/efx.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/net/ethernet/sfc/falcon/efx.c b/drivers/net/ethernet/sfc/falcon/efx.c
index dd5530a4f8c8936868aed7171bd9481f93730d76..03e2455c502eacd9a4fd5c7fd320a9edcf265f77 100644
--- a/drivers/net/ethernet/sfc/falcon/efx.c
+++ b/drivers/net/ethernet/sfc/falcon/efx.c
@@ -2052,29 +2052,6 @@ static void ef4_fini_napi(struct ef4_nic *efx)
 		ef4_fini_napi_channel(channel);
 }
 
-/**************************************************************************
- *
- * Kernel netpoll interface
- *
- *************************************************************************/
-
-#ifdef CONFIG_NET_POLL_CONTROLLER
-
-/* Although in the common case interrupts will be disabled, this is not
- * guaranteed. However, all our work happens inside the NAPI callback,
- * so no locking is required.
- */
-static void ef4_netpoll(struct net_device *net_dev)
-{
-	struct ef4_nic *efx = netdev_priv(net_dev);
-	struct ef4_channel *channel;
-
-	ef4_for_each_channel(channel, efx)
-		ef4_schedule_channel(channel);
-}
-
-#endif
-
 /**************************************************************************
  *
  * Kernel net device interface
@@ -2250,9 +2227,6 @@ static const struct net_device_ops ef4_netdev_ops = {
 	.ndo_set_mac_address	= ef4_set_mac_address,
 	.ndo_set_rx_mode	= ef4_set_rx_mode,
 	.ndo_set_features	= ef4_set_features,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = ef4_netpoll,
-#endif
 	.ndo_setup_tc		= ef4_setup_tc,
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer	= ef4_filter_rfs,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net 11/11] ibmvnic: remove ndo_poll_controller
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (9 preceding siblings ...)
  2018-09-27 16:32 ` [PATCH net 10/11] sfc-falcon: " Eric Dumazet
@ 2018-09-27 16:32 ` Eric Dumazet
  2018-09-28 18:14 ` [PATCH net 00/11] netpoll: second round of fixes David Miller
  11 siblings, 0 replies; 17+ messages in thread
From: Eric Dumazet @ 2018-09-27 16:32 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, John Allen

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

ibmvnic uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

ibmvnic_netpoll_controller() was completely wrong anyway,
as it was scheduling NAPI to service RX queues (instead of TX),
so I doubt netpoll ever worked on this driver.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Cc: John Allen <jallen@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 4f0daf67b18df2dcf11d7a406ebc1982e0fee466..699ef942b615c3a22053ba1419399317a6642cfa 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2207,19 +2207,6 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
 	return frames_processed;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void ibmvnic_netpoll_controller(struct net_device *dev)
-{
-	struct ibmvnic_adapter *adapter = netdev_priv(dev);
-	int i;
-
-	replenish_pools(netdev_priv(dev));
-	for (i = 0; i < adapter->req_rx_queues; i++)
-		ibmvnic_interrupt_rx(adapter->rx_scrq[i]->irq,
-				     adapter->rx_scrq[i]);
-}
-#endif
-
 static int wait_for_reset(struct ibmvnic_adapter *adapter)
 {
 	int rc, ret;
@@ -2292,9 +2279,6 @@ static const struct net_device_ops ibmvnic_netdev_ops = {
 	.ndo_set_mac_address	= ibmvnic_set_mac,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_tx_timeout		= ibmvnic_tx_timeout,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller	= ibmvnic_netpoll_controller,
-#endif
 	.ndo_change_mtu		= ibmvnic_change_mtu,
 	.ndo_features_check     = ibmvnic_features_check,
 };
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi()
  2018-09-27 16:31 ` [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi() Eric Dumazet
@ 2018-09-27 18:25   ` Michael Chan
  2018-09-27 18:33     ` Song Liu
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Chan @ 2018-09-27 18:25 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Netdev, aviad.krawczyk, Song Liu, dougmill,
	yisen.zhuang, mst, jasowang, harish.patil, Chopra, Manish,
	netanel, linux-net-drivers, tlfalcon

On Thu, Sep 27, 2018 at 9:32 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Since we do no longer require NAPI drivers to provide
> an ndo_poll_controller(), napi_schedule() has not been done
> before poll_one_napi() invocation.
>
> So testing NAPI_STATE_SCHED is likely to cause early returns.
>
> While we are at it, remove outdated comment.
>
> Note to future bisections : This change might surface prior
> bugs in drivers. See commit 73f21c653f93 ("bnxt_en: Fix TX
> timeout during netpoll.") for one occurrence.
>
> Fixes: ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Song Liu <songliubraving@fb.com>
> Cc: Michael Chan <michael.chan@broadcom.com>

Reviewed-and-tested-by: Michael Chan <michael.chan@broadcom.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi()
  2018-09-27 18:25   ` Michael Chan
@ 2018-09-27 18:33     ` Song Liu
  0 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2018-09-27 18:33 UTC (permalink / raw)
  To: Michael Chan
  Cc: Eric Dumazet, David Miller, Netdev, aviad.krawczyk, dougmill,
	yisen.zhuang, mst, jasowang, harish.patil, Chopra, Manish,
	netanel, linux-net-drivers, tlfalcon



> On Sep 27, 2018, at 11:25 AM, Michael Chan <michael.chan@broadcom.com> wrote:
> 
> On Thu, Sep 27, 2018 at 9:32 AM Eric Dumazet <edumazet@google.com> wrote:
>> 
>> Since we do no longer require NAPI drivers to provide
>> an ndo_poll_controller(), napi_schedule() has not been done
>> before poll_one_napi() invocation.
>> 
>> So testing NAPI_STATE_SCHED is likely to cause early returns.
>> 
>> While we are at it, remove outdated comment.
>> 
>> Note to future bisections : This change might surface prior
>> bugs in drivers. See commit 73f21c653f93 ("bnxt_en: Fix TX
>> timeout during netpoll.") for one occurrence.
>> 
>> Fixes: ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Tested-by: Song Liu <songliubraving@fb.com>
>> Cc: Michael Chan <michael.chan@broadcom.com>
> 
> Reviewed-and-tested-by: Michael Chan <michael.chan@broadcom.com>

Reviewed-and-tested-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net 09/11] sfc: remove ndo_poll_controller
  2018-09-27 16:31 ` [PATCH net 09/11] sfc: " Eric Dumazet
@ 2018-09-28  7:30   ` Bert Kenward
  0 siblings, 0 replies; 17+ messages in thread
From: Bert Kenward @ 2018-09-28  7:30 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller
  Cc: netdev, Michael Chan, Aviad Krawczyk, Song Liu, Douglas Miller,
	Yisen Zhuang, Michael S . Tsirkin, Jason Wang, Harish Patil,
	Manish Chopra, Netanel Belgazal, Solarflare linux maintainers,
	Thomas Falcon, Edward Cree

On 27/09/18 17:31, Eric Dumazet wrote:
> As diagnosed by Song Liu, ndo_poll_controller() can
> be very dangerous on loaded hosts, since the cpu
> calling ndo_poll_controller() might steal all NAPI
> contexts (for all RX/TX queues of the NIC). This capture
> can last for unlimited amount of time, since one
> cpu is generally not able to drain all the queues under load.
> 
> sfc uses NAPI for TX completions, so we better let core
> networking stack call the napi->poll() to avoid the capture.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Edward Cree <ecree@solarflare.com>
> Cc: Bert Kenward <bkenward@solarflare.com>
> Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>

Acked-By: Bert Kenward <bkenward@solarflare.com>

> ---
>  drivers/net/ethernet/sfc/efx.c | 26 --------------------------
>  1 file changed, 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
> index 330233286e785254f5f29c87f9557a305974f606..3d0dd39c289e05b8a7a6778363461ef5698dc62b 100644
> --- a/drivers/net/ethernet/sfc/efx.c
> +++ b/drivers/net/ethernet/sfc/efx.c
> @@ -2206,29 +2206,6 @@ static void efx_fini_napi(struct efx_nic *efx)
>  		efx_fini_napi_channel(channel);
>  }
>  
> -/**************************************************************************
> - *
> - * Kernel netpoll interface
> - *
> - *************************************************************************/
> -
> -#ifdef CONFIG_NET_POLL_CONTROLLER
> -
> -/* Although in the common case interrupts will be disabled, this is not
> - * guaranteed. However, all our work happens inside the NAPI callback,
> - * so no locking is required.
> - */
> -static void efx_netpoll(struct net_device *net_dev)
> -{
> -	struct efx_nic *efx = netdev_priv(net_dev);
> -	struct efx_channel *channel;
> -
> -	efx_for_each_channel(channel, efx)
> -		efx_schedule_channel(channel);
> -}
> -
> -#endif
> -
>  /**************************************************************************
>   *
>   * Kernel net device interface
> @@ -2509,9 +2486,6 @@ static const struct net_device_ops efx_netdev_ops = {
>  #endif
>  	.ndo_get_phys_port_id   = efx_get_phys_port_id,
>  	.ndo_get_phys_port_name	= efx_get_phys_port_name,
> -#ifdef CONFIG_NET_POLL_CONTROLLER
> -	.ndo_poll_controller = efx_netpoll,
> -#endif
>  	.ndo_setup_tc		= efx_setup_tc,
>  #ifdef CONFIG_RFS_ACCEL
>  	.ndo_rx_flow_steer	= efx_filter_rfs,
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net 10/11] sfc-falcon: remove ndo_poll_controller
  2018-09-27 16:32 ` [PATCH net 10/11] sfc-falcon: " Eric Dumazet
@ 2018-09-28  7:30   ` Bert Kenward
  0 siblings, 0 replies; 17+ messages in thread
From: Bert Kenward @ 2018-09-28  7:30 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller
  Cc: netdev, Michael Chan, Aviad Krawczyk, Song Liu, Douglas Miller,
	Yisen Zhuang, Michael S . Tsirkin, Jason Wang, Harish Patil,
	Manish Chopra, Netanel Belgazal, Solarflare linux maintainers,
	Thomas Falcon, Edward Cree

On 27/09/18 17:32, Eric Dumazet wrote:
> As diagnosed by Song Liu, ndo_poll_controller() can
> be very dangerous on loaded hosts, since the cpu
> calling ndo_poll_controller() might steal all NAPI
> contexts (for all RX/TX queues of the NIC). This capture
> can last for unlimited amount of time, since one
> cpu is generally not able to drain all the queues under load.
> 
> sfc-falcon uses NAPI for TX completions, so we better let core
> networking stack call the napi->poll() to avoid the capture.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
> Cc: Edward Cree <ecree@solarflare.com>
> Cc: Bert Kenward <bkenward@solarflare.com>

Acked-By: Bert Kenward <bkenward@solarflare.com>

> ---
>  drivers/net/ethernet/sfc/falcon/efx.c | 26 --------------------------
>  1 file changed, 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/falcon/efx.c b/drivers/net/ethernet/sfc/falcon/efx.c
> index dd5530a4f8c8936868aed7171bd9481f93730d76..03e2455c502eacd9a4fd5c7fd320a9edcf265f77 100644
> --- a/drivers/net/ethernet/sfc/falcon/efx.c
> +++ b/drivers/net/ethernet/sfc/falcon/efx.c
> @@ -2052,29 +2052,6 @@ static void ef4_fini_napi(struct ef4_nic *efx)
>  		ef4_fini_napi_channel(channel);
>  }
>  
> -/**************************************************************************
> - *
> - * Kernel netpoll interface
> - *
> - *************************************************************************/
> -
> -#ifdef CONFIG_NET_POLL_CONTROLLER
> -
> -/* Although in the common case interrupts will be disabled, this is not
> - * guaranteed. However, all our work happens inside the NAPI callback,
> - * so no locking is required.
> - */
> -static void ef4_netpoll(struct net_device *net_dev)
> -{
> -	struct ef4_nic *efx = netdev_priv(net_dev);
> -	struct ef4_channel *channel;
> -
> -	ef4_for_each_channel(channel, efx)
> -		ef4_schedule_channel(channel);
> -}
> -
> -#endif
> -
>  /**************************************************************************
>   *
>   * Kernel net device interface
> @@ -2250,9 +2227,6 @@ static const struct net_device_ops ef4_netdev_ops = {
>  	.ndo_set_mac_address	= ef4_set_mac_address,
>  	.ndo_set_rx_mode	= ef4_set_rx_mode,
>  	.ndo_set_features	= ef4_set_features,
> -#ifdef CONFIG_NET_POLL_CONTROLLER
> -	.ndo_poll_controller = ef4_netpoll,
> -#endif
>  	.ndo_setup_tc		= ef4_setup_tc,
>  #ifdef CONFIG_RFS_ACCEL
>  	.ndo_rx_flow_steer	= ef4_filter_rfs,
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net 00/11] netpoll: second round of fixes.
  2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
                   ` (10 preceding siblings ...)
  2018-09-27 16:32 ` [PATCH net 11/11] ibmvnic: " Eric Dumazet
@ 2018-09-28 18:14 ` David Miller
  11 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2018-09-28 18:14 UTC (permalink / raw)
  To: edumazet
  Cc: netdev, michael.chan, aviad.krawczyk, songliubraving, dougmill,
	yisen.zhuang, mst, jasowang, harish.patil, manish.chopra,
	netanel, linux-net-drivers, tlfalcon

From: Eric Dumazet <edumazet@google.com>
Date: Thu, 27 Sep 2018 09:31:50 -0700

> As diagnosed by Song Liu, ndo_poll_controller() can
> be very dangerous on loaded hosts, since the cpu
> calling ndo_poll_controller() might steal all NAPI
> contexts (for all RX/TX queues of the NIC).
> 
> This capture, showing one ksoftirqd eating all cycles
> can last for unlimited amount of time, since one
> cpu is generally not able to drain all the queues under load.
> 
> It seems that all networking drivers that do use NAPI
> for their TX completions, should not provide a ndo_poll_controller() :
> 
> Most NAPI drivers have netpoll support already handled
> in core networking stack, since netpoll_poll_dev()
> uses poll_napi(dev) to iterate through registered
> NAPI contexts for a device.
> 
> First patch is a fix in poll_one_napi().
> 
> Then following patches take care of ten drivers.

Series applied, thanks Eric.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-09-29  0:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-27 16:31 [PATCH net 00/11] netpoll: second round of fixes Eric Dumazet
2018-09-27 16:31 ` [PATCH net 01/11] netpoll: do not test NAPI_STATE_SCHED in poll_one_napi() Eric Dumazet
2018-09-27 18:25   ` Michael Chan
2018-09-27 18:33     ` Song Liu
2018-09-27 16:31 ` [PATCH net 02/11] hinic: remove ndo_poll_controller Eric Dumazet
2018-09-27 16:31 ` [PATCH net 03/11] ehea: " Eric Dumazet
2018-09-27 16:31 ` [PATCH net 04/11] net: hns: " Eric Dumazet
2018-09-27 16:31 ` [PATCH net 05/11] virtio_net: " Eric Dumazet
2018-09-27 16:31 ` [PATCH net 06/11] qlcnic: " Eric Dumazet
2018-09-27 16:31 ` [PATCH net 07/11] qlogic: netxen: " Eric Dumazet
2018-09-27 16:31 ` [PATCH net 08/11] net: ena: " Eric Dumazet
2018-09-27 16:31 ` [PATCH net 09/11] sfc: " Eric Dumazet
2018-09-28  7:30   ` Bert Kenward
2018-09-27 16:32 ` [PATCH net 10/11] sfc-falcon: " Eric Dumazet
2018-09-28  7:30   ` Bert Kenward
2018-09-27 16:32 ` [PATCH net 11/11] ibmvnic: " Eric Dumazet
2018-09-28 18:14 ` [PATCH net 00/11] netpoll: second round of fixes David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.