netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v3 00/12]  net: ethernet: aquantia: improvements and fixes
@ 2017-02-20 11:41 Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev Pavel Belous
                   ` (12 more replies)
  0 siblings, 13 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

The following patchset contains improvements and fixes for aQuantia
AQtion ethernet driver from net-next tree.

Most fixes are based on the comments from Lino Sanfilippo.

Sanity testing was performed on real HW. No regression found.

v1->v2 :1)Removed buffers copying.
	2)Fixed dma error handling.

v2->v3 :1)Fixes for aq_ndev_change_mtu:
	-Use core MTU checking for min_mtu.
	-Removed extra new_mtu assigment.
	2)Reverse XMAS tree in aq_ring_rx_fill.

Please review.
Thanks.
Pavel Belous (12):
  net: ethernet: aquantia: Removed extra assignment for skb->dev.
  net: ethernet: aquantia: Removed busy_count field.
  net: ethernet: aquantia: Fixes for aq_ndev_change_mtu
  net: ethernet: aquantia: Using module_pci_driver.
  net: ethernet: aquantia: Superfluous initialization of "err".
  net: ethernet: aquantia: Fixed missing rtnl_unlock.
  net: ethernet: aquantia: Using NETDEV_TX_OK instead 0.
  net: ethernet: aquantia: Null pointer check for aq_nic_ndev_alloc.
  net: ethernet: aquantia: Call netdev_register after all initialized.
  net: ethernet: aquantia: Fixed incorrect buff->len calculation.
  net: ethernet: aquantia: Fixed memory allocation if
    AQ_CFG_RX_FRAME_MAX > 1 page.
  net: ethernet: aquantia: Copying tx buffers is not needed.

 drivers/net/ethernet/aquantia/atlantic/aq_main.c   |  40 +---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c    | 206 +++++++++++----------
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c   |  24 +--
 drivers/net/ethernet/aquantia/atlantic/aq_ring.h   |   3 -
 drivers/net/ethernet/aquantia/atlantic/aq_utils.h  |   1 -
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c  |   4 +-
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c  |   4 +-
 7 files changed, 122 insertions(+), 160 deletions(-)

-- 
2.7.4

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

* [PATCH net-next v3 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 02/12] net: ethernet: aquantia: Removed busy_count field Pavel Belous
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

This assignment is not needed.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index dea9e9b..4c40644 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -214,7 +214,6 @@ int aq_ring_rx_clean(struct aq_ring_s *self, int *work_done, int budget)
 				goto err_exit;
 			}
 
-			skb->dev = ndev;
 			skb_put(skb, buff->len);
 		} else {
 			skb = netdev_alloc_skb(ndev, ETH_HLEN);
-- 
2.7.4

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

* [PATCH net-next v3 02/12] net: ethernet: aquantia: Removed busy_count field.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu Pavel Belous
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

 busy_count field and is_busy flag is not needed at all.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c   | 11 -----------
 drivers/net/ethernet/aquantia/atlantic/aq_utils.h |  1 -
 2 files changed, 12 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index aa22a7c..a153750 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -122,14 +122,11 @@ static void aq_nic_service_timer_cb(unsigned long param)
 	struct aq_nic_s *self = (struct aq_nic_s *)param;
 	struct net_device *ndev = aq_nic_get_ndev(self);
 	int err = 0;
-	bool is_busy = false;
 	unsigned int i = 0U;
 	struct aq_hw_link_status_s link_status;
 	struct aq_ring_stats_rx_s stats_rx;
 	struct aq_ring_stats_tx_s stats_tx;
 
-	atomic_inc(&self->header.busy_count);
-	is_busy = true;
 	if (aq_utils_obj_test(&self->header.flags, AQ_NIC_FLAGS_IS_NOT_READY))
 		goto err_exit;
 
@@ -170,8 +167,6 @@ static void aq_nic_service_timer_cb(unsigned long param)
 	ndev->stats.tx_errors = stats_tx.errors;
 
 err_exit:
-	if (is_busy)
-		atomic_dec(&self->header.busy_count);
 	mod_timer(&self->service_timer,
 		  jiffies + AQ_CFG_SERVICE_TIMER_INTERVAL);
 }
@@ -574,16 +569,12 @@ __acquires(&ring->lock)
 	unsigned int trys = AQ_CFG_LOCK_TRYS;
 	int err = 0;
 	bool is_nic_in_bad_state;
-	bool is_busy = false;
 	struct aq_ring_buff_s buffers[AQ_CFG_SKB_FRAGS_MAX];
 
 	frags = skb_shinfo(skb)->nr_frags + 1;
 
 	ring = self->aq_ring_tx[AQ_NIC_TCVEC2RING(self, tc, vec)];
 
-	atomic_inc(&self->header.busy_count);
-	is_busy = true;
-
 	if (frags > AQ_CFG_SKB_FRAGS_MAX) {
 		dev_kfree_skb_any(skb);
 		goto err_exit;
@@ -629,8 +620,6 @@ __acquires(&ring->lock)
 	}
 
 err_exit:
-	if (is_busy)
-		atomic_dec(&self->header.busy_count);
 	return err;
 }
 
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_utils.h b/drivers/net/ethernet/aquantia/atlantic/aq_utils.h
index 4446bd9..f6012b3 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_utils.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_utils.h
@@ -19,7 +19,6 @@
 struct aq_obj_s {
 	spinlock_t lock; /* spinlock for nic/rings processing */
 	atomic_t flags;
-	atomic_t busy_count;
 };
 
 static inline void aq_utils_obj_set(atomic_t *flags, u32 mask)
-- 
2.7.4

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

* [PATCH net-next v3 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 02/12] net: ethernet: aquantia: Removed busy_count field Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 04/12] net: ethernet: aquantia: Using module_pci_driver Pavel Belous
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

1)Removed unnecessary comparsion "old_mtu == new_mtu".
This check is not needed. Function aq_ndev_change_mtu wont be called
if mtu has not changed.

2)Removed extra assignment ndev->mtu = new_mtu;
This assignment already done inside __dev_set_mtu().

3)Use core MTU checking for min_mtu.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_main.c | 12 +-----------
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c  |  1 +
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index c17c70a..45c769e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -100,20 +100,10 @@ static int aq_ndev_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 static int aq_ndev_change_mtu(struct net_device *ndev, int new_mtu)
 {
 	struct aq_nic_s *aq_nic = netdev_priv(ndev);
-	int err = 0;
+	int err = aq_nic_set_mtu(aq_nic, new_mtu + ETH_HLEN);
 
-	if (new_mtu == ndev->mtu) {
-		err = 0;
-		goto err_exit;
-	}
-	if (new_mtu < 68) {
-		err = -EINVAL;
-		goto err_exit;
-	}
-	err = aq_nic_set_mtu(aq_nic, new_mtu + ETH_HLEN);
 	if (err < 0)
 		goto err_exit;
-	ndev->mtu = new_mtu;
 
 	if (netif_running(ndev)) {
 		aq_ndev_close(ndev);
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index a153750..4b8d074 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -214,6 +214,7 @@ struct aq_nic_s *aq_nic_alloc_cold(const struct net_device_ops *ndev_ops,
 	SET_NETDEV_DEV(ndev, dev);
 
 	ndev->if_port = port;
+	ndev->min_mtu = ETH_MIN_MTU;
 	self->ndev = ndev;
 
 	self->aq_pci_func = aq_pci_func;
-- 
2.7.4

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

* [PATCH net-next v3 04/12] net: ethernet: aquantia: Using module_pci_driver.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (2 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 05/12] net: ethernet: aquantia: Superfluous initialization of "err" Pavel Belous
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

Remove boilerplate code by using macro module_pci_driver.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/aquantia/atlantic/aq_main.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index 45c769e..91c66f5 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -242,22 +242,4 @@ static struct pci_driver aq_pci_ops = {
 	.resume = aq_pci_resume,
 };
 
-static int __init aq_module_init(void)
-{
-	int err = 0;
-
-	err = pci_register_driver(&aq_pci_ops);
-	if (err < 0)
-		goto err_exit;
-
-err_exit:
-	return err;
-}
-
-static void __exit aq_module_exit(void)
-{
-	pci_unregister_driver(&aq_pci_ops);
-}
-
-module_init(aq_module_init);
-module_exit(aq_module_exit);
+module_pci_driver(aq_pci_ops);
-- 
2.7.4

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

* [PATCH net-next v3 05/12] net: ethernet: aquantia: Superfluous initialization of "err".
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (3 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 04/12] net: ethernet: aquantia: Using module_pci_driver Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 06/12] net: ethernet: aquantia: Fixed missing rtnl_unlock Pavel Belous
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

Fixed superfluous initialization of err.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/aquantia/atlantic/aq_main.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index 91c66f5..dad6362 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -87,14 +87,8 @@ static int aq_ndev_close(struct net_device *ndev)
 static int aq_ndev_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct aq_nic_s *aq_nic = netdev_priv(ndev);
-	int err = 0;
-
-	err = aq_nic_xmit(aq_nic, skb);
-	if (err < 0)
-		goto err_exit;
 
-err_exit:
-	return err;
+	return aq_nic_xmit(aq_nic, skb);
 }
 
 static int aq_ndev_change_mtu(struct net_device *ndev, int new_mtu)
-- 
2.7.4

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

* [PATCH net-next v3 06/12] net: ethernet: aquantia: Fixed missing rtnl_unlock.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (4 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 05/12] net: ethernet: aquantia: Superfluous initialization of "err" Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 07/12] net: ethernet: aquantia: Using NETDEV_TX_OK instead 0 Pavel Belous
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

rtnl_unlock should be called if error occurred.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 4b8d074..646314c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -932,7 +932,7 @@ int aq_nic_change_pm_state(struct aq_nic_s *self, pm_message_t *pm_msg)
 
 	if (!netif_running(self->ndev)) {
 		err = 0;
-		goto err_exit;
+		goto out;
 	}
 	rtnl_lock();
 	if (pm_msg->event & PM_EVENT_SLEEP || pm_msg->event & PM_EVENT_FREEZE) {
@@ -957,8 +957,9 @@ int aq_nic_change_pm_state(struct aq_nic_s *self, pm_message_t *pm_msg)
 		netif_device_attach(self->ndev);
 		netif_tx_start_all_queues(self->ndev);
 	}
-	rtnl_unlock();
 
 err_exit:
+	rtnl_unlock();
+out:
 	return err;
 }
-- 
2.7.4

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

* [PATCH net-next v3 07/12] net: ethernet: aquantia: Using NETDEV_TX_OK instead 0.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (5 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 06/12] net: ethernet: aquantia: Fixed missing rtnl_unlock Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 08/12] net: ethernet: aquantia: Null pointer check for aq_nic_ndev_alloc Pavel Belous
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

Use NETDEV_TX_OK as the return value for successful transmission.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 646314c..019bcc7 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -568,7 +568,7 @@ __acquires(&ring->lock)
 	unsigned int vec = skb->queue_mapping % self->aq_nic_cfg.vecs;
 	unsigned int tc = 0U;
 	unsigned int trys = AQ_CFG_LOCK_TRYS;
-	int err = 0;
+	int err = NETDEV_TX_OK;
 	bool is_nic_in_bad_state;
 	struct aq_ring_buff_s buffers[AQ_CFG_SKB_FRAGS_MAX];
 
-- 
2.7.4

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

* [PATCH net-next v3 08/12] net: ethernet: aquantia: Null pointer check for aq_nic_ndev_alloc.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (6 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 07/12] net: ethernet: aquantia: Using NETDEV_TX_OK instead 0 Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 09/12] net: ethernet: aquantia: Call netdev_register after all initialized Pavel Belous
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

We should check for a null pointer for aq_nic_ndev_alloc
instead netdev_priv.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 019bcc7..a8a27c5 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -202,12 +202,13 @@ struct aq_nic_s *aq_nic_alloc_cold(const struct net_device_ops *ndev_ops,
 	int err = 0;
 
 	ndev = aq_nic_ndev_alloc();
-	self = netdev_priv(ndev);
-	if (!self) {
-		err = -EINVAL;
+	if (!ndev) {
+		err = -ENOMEM;
 		goto err_exit;
 	}
 
+	self = netdev_priv(ndev);
+
 	ndev->netdev_ops = ndev_ops;
 	ndev->ethtool_ops = et_ops;
 
-- 
2.7.4

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

* [PATCH net-next v3 09/12] net: ethernet: aquantia: Call netdev_register after all initialized.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (7 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 08/12] net: ethernet: aquantia: Null pointer check for aq_nic_ndev_alloc Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 16:28   ` David Miller
  2017-02-20 11:41 ` [PATCH net-next v3 10/12] net: ethernet: aquantia: Fixed incorrect buff->len calculation Pavel Belous
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

netdev_register should be called when everything is initialized.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index a8a27c5..fdd7d72 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -261,16 +261,18 @@ int aq_nic_ndev_register(struct aq_nic_s *self)
 		ether_addr_copy(self->ndev->dev_addr, mac_addr_permanent);
 	}
 #endif
-	err = register_netdev(self->ndev);
-	if (err < 0)
-		goto err_exit;
 
-	self->is_ndev_registered = true;
 	netif_carrier_off(self->ndev);
 
 	for (i = AQ_CFG_VECS_MAX; i--;)
 		aq_nic_ndev_queue_stop(self, i);
 
+	err = register_netdev(self->ndev);
+	if (err < 0)
+		goto err_exit;
+
+	self->is_ndev_registered = true;
+
 err_exit:
 	return err;
 }
-- 
2.7.4

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

* [PATCH net-next v3 10/12] net: ethernet: aquantia: Fixed incorrect buff->len calculation.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (8 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 09/12] net: ethernet: aquantia: Call netdev_register after all initialized Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 11/12] net: ethernet: aquantia: Fixed memory allocation if AQ_CFG_RX_FRAME_MAX > 1 page Pavel Belous
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

rxd_wb->pkt_len is the total length of the packet.
If we received a large packet (with length > AQ_CFG_RX_FRAME_MAX) then we
will get multiple buffers. We need to fix the length of the last buffer.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c | 4 ++--
 drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
index 1f38805..a2b746a 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
@@ -659,8 +659,8 @@ static int hw_atl_a0_hw_ring_rx_receive(struct aq_hw_s *self,
 			}
 
 			if (HW_ATL_A0_RXD_WB_STAT2_EOP & rxd_wb->status) {
-				buff->len = (rxd_wb->pkt_len &
-						(AQ_CFG_RX_FRAME_MAX - 1U));
+				buff->len = rxd_wb->pkt_len %
+					AQ_CFG_RX_FRAME_MAX;
 				buff->len = buff->len ?
 					buff->len : AQ_CFG_RX_FRAME_MAX;
 				buff->next = 0U;
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index e7e694f..cab2931 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -673,8 +673,8 @@ static int hw_atl_b0_hw_ring_rx_receive(struct aq_hw_s *self,
 			}
 
 			if (HW_ATL_B0_RXD_WB_STAT2_EOP & rxd_wb->status) {
-				buff->len = (rxd_wb->pkt_len &
-						(AQ_CFG_RX_FRAME_MAX - 1U));
+				buff->len = rxd_wb->pkt_len %
+					AQ_CFG_RX_FRAME_MAX;
 				buff->len = buff->len ?
 					buff->len : AQ_CFG_RX_FRAME_MAX;
 				buff->next = 0U;
-- 
2.7.4

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

* [PATCH net-next v3 11/12] net: ethernet: aquantia: Fixed memory allocation if AQ_CFG_RX_FRAME_MAX > 1 page.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (9 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 10/12] net: ethernet: aquantia: Fixed incorrect buff->len calculation Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 11:41 ` [PATCH net-next v3 12/12] net: ethernet: aquantia: Copying tx buffers is not needed Pavel Belous
  2017-02-20 16:26 ` [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes David Miller
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

We should allocate the number of pages based on the config parameter
AQ_CFG_RX_FRAME_MAX.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c  | 4 ++--
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index fdd7d72..0b208c4 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -597,9 +597,9 @@ __acquires(&ring->lock)
 
 	do {
 		if (spin_trylock(&ring->header.lock)) {
-			frags = aq_nic_map_skb(self, skb, &buffers[0]);
+			frags = aq_nic_map_skb(self, skb, buffers);
 
-			aq_ring_tx_append_buffs(ring, &buffers[0], frags);
+			aq_ring_tx_append_buffs(ring, buffers, frags);
 
 			err = self->aq_hw_ops.hw_ring_tx_xmit(self->aq_hw,
 							      ring, frags);
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 4c40644..042976d 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -275,6 +275,8 @@ int aq_ring_rx_clean(struct aq_ring_s *self, int *work_done, int budget)
 
 int aq_ring_rx_fill(struct aq_ring_s *self)
 {
+	unsigned int pages_order = fls(AQ_CFG_RX_FRAME_MAX / PAGE_SIZE +
+		(AQ_CFG_RX_FRAME_MAX % PAGE_SIZE ? 1 : 0)) - 1;
 	struct aq_ring_buff_s *buff = NULL;
 	int err = 0;
 	int i = 0;
@@ -287,7 +289,7 @@ int aq_ring_rx_fill(struct aq_ring_s *self)
 		buff->len = AQ_CFG_RX_FRAME_MAX;
 
 		buff->page = alloc_pages(GFP_ATOMIC | __GFP_COLD |
-					 __GFP_COMP, 0);
+					 __GFP_COMP, pages_order);
 		if (!buff->page) {
 			err = -ENOMEM;
 			goto err_exit;
-- 
2.7.4

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

* [PATCH net-next v3 12/12] net: ethernet: aquantia: Copying tx buffers is not needed.
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (10 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 11/12] net: ethernet: aquantia: Fixed memory allocation if AQ_CFG_RX_FRAME_MAX > 1 page Pavel Belous
@ 2017-02-20 11:41 ` Pavel Belous
  2017-02-20 16:26 ` [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes David Miller
  12 siblings, 0 replies; 15+ messages in thread
From: Pavel Belous @ 2017-02-20 11:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Simon Edelhaus, Alexey Andriyanov, Lino Sanfilippo,
	Pavel Belous

From: Pavel Belous <pavel.belous@aquantia.com>

This fix removes copying of tx biffers.
Now we use ring->buff_fing directly.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c  | 170 +++++++++++++----------
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c |  19 ---
 drivers/net/ethernet/aquantia/atlantic/aq_ring.h |   3 -
 3 files changed, 97 insertions(+), 95 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 0b208c4..5c80162 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -470,95 +470,116 @@ int aq_nic_start(struct aq_nic_s *self)
 	return err;
 }
 
-static unsigned int aq_nic_map_skb_frag(struct aq_nic_s *self,
-					struct sk_buff *skb,
-					struct aq_ring_buff_s *dx)
+static unsigned int aq_nic_map_skb(struct aq_nic_s *self,
+				   struct sk_buff *skb,
+				   struct aq_ring_s *ring)
 {
 	unsigned int ret = 0U;
 	unsigned int nr_frags = skb_shinfo(skb)->nr_frags;
 	unsigned int frag_count = 0U;
+	unsigned int dx = ring->sw_tail;
+	struct aq_ring_buff_s *dx_buff = &ring->buff_ring[dx];
 
-	dx->flags = 0U;
-	dx->len = skb_headlen(skb);
-	dx->pa = dma_map_single(aq_nic_get_dev(self), skb->data, dx->len,
-				DMA_TO_DEVICE);
-	dx->len_pkt = skb->len;
-	dx->is_sop = 1U;
-	dx->is_mapped = 1U;
+	if (unlikely(skb_is_gso(skb))) {
+		dx_buff->flags = 0U;
+		dx_buff->len_pkt = skb->len;
+		dx_buff->len_l2 = ETH_HLEN;
+		dx_buff->len_l3 = ip_hdrlen(skb);
+		dx_buff->len_l4 = tcp_hdrlen(skb);
+		dx_buff->mss = skb_shinfo(skb)->gso_size;
+		dx_buff->is_txc = 1U;
+
+		dx = aq_ring_next_dx(ring, dx);
+		dx_buff = &ring->buff_ring[dx];
+		++ret;
+	}
+
+	dx_buff->flags = 0U;
+	dx_buff->len = skb_headlen(skb);
+	dx_buff->pa = dma_map_single(aq_nic_get_dev(self),
+				     skb->data,
+				     dx_buff->len,
+				     DMA_TO_DEVICE);
 
+	if (unlikely(dma_mapping_error(aq_nic_get_dev(self), dx_buff->pa)))
+		goto exit;
+
+	dx_buff->len_pkt = skb->len;
+	dx_buff->is_sop = 1U;
+	dx_buff->is_mapped = 1U;
 	++ret;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		dx->is_ip_cso = (htons(ETH_P_IP) == skb->protocol) ? 1U : 0U;
-		dx->is_tcp_cso =
+		dx_buff->is_ip_cso = (htons(ETH_P_IP) == skb->protocol) ?
+			1U : 0U;
+		dx_buff->is_tcp_cso =
 			(ip_hdr(skb)->protocol == IPPROTO_TCP) ? 1U : 0U;
-		dx->is_udp_cso =
+		dx_buff->is_udp_cso =
 			(ip_hdr(skb)->protocol == IPPROTO_UDP) ? 1U : 0U;
 	}
 
 	for (; nr_frags--; ++frag_count) {
-		unsigned int frag_len;
+		unsigned int frag_len = 0U;
 		dma_addr_t frag_pa;
 		skb_frag_t *frag = &skb_shinfo(skb)->frags[frag_count];
 
 		frag_len = skb_frag_size(frag);
-
 		frag_pa = skb_frag_dma_map(aq_nic_get_dev(self), frag, 0,
 					   frag_len, DMA_TO_DEVICE);
 
+		if (unlikely(dma_mapping_error(aq_nic_get_dev(self), frag_pa)))
+			goto mapping_error;
+
 		while (frag_len > AQ_CFG_TX_FRAME_MAX) {
-			++dx;
-			++ret;
-			dx->flags = 0U;
-			dx->len = AQ_CFG_TX_FRAME_MAX;
-			dx->pa = frag_pa;
-			dx->is_mapped = 1U;
+			dx = aq_ring_next_dx(ring, dx);
+			dx_buff = &ring->buff_ring[dx];
+
+			dx_buff->flags = 0U;
+			dx_buff->len = AQ_CFG_TX_FRAME_MAX;
+			dx_buff->pa = frag_pa;
+			dx_buff->is_mapped = 1U;
 
 			frag_len -= AQ_CFG_TX_FRAME_MAX;
 			frag_pa += AQ_CFG_TX_FRAME_MAX;
+			++ret;
 		}
 
-		++dx;
-		++ret;
+		dx = aq_ring_next_dx(ring, dx);
+		dx_buff = &ring->buff_ring[dx];
 
-		dx->flags = 0U;
-		dx->len = frag_len;
-		dx->pa = frag_pa;
-		dx->is_mapped = 1U;
+		dx_buff->flags = 0U;
+		dx_buff->len = frag_len;
+		dx_buff->pa = frag_pa;
+		dx_buff->is_mapped = 1U;
+		++ret;
 	}
 
-	dx->is_eop = 1U;
-	dx->skb = skb;
-
-	return ret;
-}
-
-static unsigned int aq_nic_map_skb_lso(struct aq_nic_s *self,
-				       struct sk_buff *skb,
-				       struct aq_ring_buff_s *dx)
-{
-	dx->flags = 0U;
-	dx->len_pkt = skb->len;
-	dx->len_l2 = ETH_HLEN;
-	dx->len_l3 = ip_hdrlen(skb);
-	dx->len_l4 = tcp_hdrlen(skb);
-	dx->mss = skb_shinfo(skb)->gso_size;
-	dx->is_txc = 1U;
-	return 1U;
-}
-
-static unsigned int aq_nic_map_skb(struct aq_nic_s *self, struct sk_buff *skb,
-				   struct aq_ring_buff_s *dx)
-{
-	unsigned int ret = 0U;
-
-	if (unlikely(skb_is_gso(skb))) {
-		ret = aq_nic_map_skb_lso(self, skb, dx);
-		++dx;
+	dx_buff->is_eop = 1U;
+	dx_buff->skb = skb;
+	goto exit;
+
+mapping_error:
+	for (dx = ring->sw_tail;
+	     ret > 0;
+	     --ret, dx = aq_ring_next_dx(ring, dx)) {
+		dx_buff = &ring->buff_ring[dx];
+
+		if (!dx_buff->is_txc && dx_buff->pa) {
+			if (unlikely(dx_buff->is_sop)) {
+				dma_unmap_single(aq_nic_get_dev(self),
+						 dx_buff->pa,
+						 dx_buff->len,
+						 DMA_TO_DEVICE);
+			} else {
+				dma_unmap_page(aq_nic_get_dev(self),
+					       dx_buff->pa,
+					       dx_buff->len,
+					       DMA_TO_DEVICE);
+			}
+		}
 	}
 
-	ret += aq_nic_map_skb_frag(self, skb, dx);
-
+exit:
 	return ret;
 }
 
@@ -573,7 +594,6 @@ __acquires(&ring->lock)
 	unsigned int trys = AQ_CFG_LOCK_TRYS;
 	int err = NETDEV_TX_OK;
 	bool is_nic_in_bad_state;
-	struct aq_ring_buff_s buffers[AQ_CFG_SKB_FRAGS_MAX];
 
 	frags = skb_shinfo(skb)->nr_frags + 1;
 
@@ -597,23 +617,27 @@ __acquires(&ring->lock)
 
 	do {
 		if (spin_trylock(&ring->header.lock)) {
-			frags = aq_nic_map_skb(self, skb, buffers);
-
-			aq_ring_tx_append_buffs(ring, buffers, frags);
-
-			err = self->aq_hw_ops.hw_ring_tx_xmit(self->aq_hw,
-							      ring, frags);
-			if (err >= 0) {
-				if (aq_ring_avail_dx(ring) <
-				    AQ_CFG_SKB_FRAGS_MAX + 1)
-					aq_nic_ndev_queue_stop(self, ring->idx);
+			frags = aq_nic_map_skb(self, skb, ring);
+
+			if (likely(frags)) {
+				err = self->aq_hw_ops.hw_ring_tx_xmit(
+								self->aq_hw,
+								ring, frags);
+				if (err >= 0) {
+					if (aq_ring_avail_dx(ring) <
+					    AQ_CFG_SKB_FRAGS_MAX + 1)
+						aq_nic_ndev_queue_stop(
+								self,
+								ring->idx);
+
+					++ring->stats.tx.packets;
+					ring->stats.tx.bytes += skb->len;
+				}
+			} else {
+				err = NETDEV_TX_BUSY;
 			}
-			spin_unlock(&ring->header.lock);
 
-			if (err >= 0) {
-				++ring->stats.tx.packets;
-				ring->stats.tx.bytes += skb->len;
-			}
+			spin_unlock(&ring->header.lock);
 			break;
 		}
 	} while (--trys);
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 042976d..1ba4953 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -104,25 +104,6 @@ int aq_ring_init(struct aq_ring_s *self)
 	return 0;
 }
 
-void aq_ring_tx_append_buffs(struct aq_ring_s *self,
-			     struct aq_ring_buff_s *buffer,
-			     unsigned int buffers)
-{
-	if (likely(self->sw_tail + buffers < self->size)) {
-		memcpy(&self->buff_ring[self->sw_tail], buffer,
-		       sizeof(buffer[0]) * buffers);
-	} else {
-		unsigned int first_part = self->size - self->sw_tail;
-		unsigned int second_part = buffers - first_part;
-
-		memcpy(&self->buff_ring[self->sw_tail], buffer,
-		       sizeof(buffer[0]) * first_part);
-
-		memcpy(&self->buff_ring[0], &buffer[first_part],
-		       sizeof(buffer[0]) * second_part);
-	}
-}
-
 int aq_ring_tx_clean(struct aq_ring_s *self)
 {
 	struct device *dev = aq_nic_get_dev(self->aq_nic);
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.h b/drivers/net/ethernet/aquantia/atlantic/aq_ring.h
index 0ac3f9e..0d7d47e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.h
@@ -147,9 +147,6 @@ int aq_ring_init(struct aq_ring_s *self);
 void aq_ring_tx_deinit(struct aq_ring_s *self);
 void aq_ring_rx_deinit(struct aq_ring_s *self);
 void aq_ring_free(struct aq_ring_s *self);
-void aq_ring_tx_append_buffs(struct aq_ring_s *ring,
-			     struct aq_ring_buff_s *buffer,
-			     unsigned int buffers);
 int aq_ring_tx_clean(struct aq_ring_s *self);
 int aq_ring_rx_clean(struct aq_ring_s *self, int *work_done, int budget);
 int aq_ring_rx_fill(struct aq_ring_s *self);
-- 
2.7.4

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

* Re: [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes
  2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (11 preceding siblings ...)
  2017-02-20 11:41 ` [PATCH net-next v3 12/12] net: ethernet: aquantia: Copying tx buffers is not needed Pavel Belous
@ 2017-02-20 16:26 ` David Miller
  12 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2017-02-20 16:26 UTC (permalink / raw)
  To: Pavel.Belous; +Cc: netdev, Simon.Edelhaus, Alexey.Andriyanov, LinoSanfilippo

From: Pavel Belous <Pavel.Belous@aquantia.com>
Date: Mon, 20 Feb 2017 14:41:42 +0300

> From: Pavel Belous <pavel.belous@aquantia.com>
> 
> The following patchset contains improvements and fixes for aQuantia
> AQtion ethernet driver from net-next tree.
> 
> Most fixes are based on the comments from Lino Sanfilippo.
> 
> Sanity testing was performed on real HW. No regression found.
> 
> v1->v2 :1)Removed buffers copying.
> 	2)Fixed dma error handling.
> 
> v2->v3 :1)Fixes for aq_ndev_change_mtu:
> 	-Use core MTU checking for min_mtu.
> 	-Removed extra new_mtu assigment.
> 	2)Reverse XMAS tree in aq_ring_rx_fill.

This series doesn't apply cleanly to net-next, you'll have to
respin.

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

* Re: [PATCH net-next v3 09/12] net: ethernet: aquantia: Call netdev_register after all initialized.
  2017-02-20 11:41 ` [PATCH net-next v3 09/12] net: ethernet: aquantia: Call netdev_register after all initialized Pavel Belous
@ 2017-02-20 16:28   ` David Miller
  0 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2017-02-20 16:28 UTC (permalink / raw)
  To: Pavel.Belous; +Cc: netdev, Simon.Edelhaus, Alexey.Andriyanov, LinoSanfilippo

From: Pavel Belous <Pavel.Belous@aquantia.com>
Date: Mon, 20 Feb 2017 14:41:51 +0300

> From: Pavel Belous <pavel.belous@aquantia.com>
> 
> netdev_register should be called when everything is initialized.
> 
> Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
> Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
> ---
>  drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> index a8a27c5..fdd7d72 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> @@ -261,16 +261,18 @@ int aq_nic_ndev_register(struct aq_nic_s *self)
>  		ether_addr_copy(self->ndev->dev_addr, mac_addr_permanent);
>  	}
>  #endif
> -	err = register_netdev(self->ndev);
> -	if (err < 0)
> -		goto err_exit;
>  
> -	self->is_ndev_registered = true;
>  	netif_carrier_off(self->ndev);
>  
>  	for (i = AQ_CFG_VECS_MAX; i--;)
>  		aq_nic_ndev_queue_stop(self, i);
>  
> +	err = register_netdev(self->ndev);
> +	if (err < 0)
> +		goto err_exit;
> +
> +	self->is_ndev_registered = true;
> +

Please get rid of this self->is_ndev_registered state, it is not
necessary and it is racey.

The very moment you call register_netdev(), your ->open() method
along with the rest of your driver can be invoked.

Therefore any dependency of state changes after register_netdev()
cannot be depended upon by the rest of the driver, code is going
to run immediately elsewhere in your driver as soon as that
register_netdev() is invoked, and it is going to test that
->is_ndev_registered() value, and it's going to be wrong.

Instead, test netdev->reg_state if you absolutely must have this
piece of information.

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

end of thread, other threads:[~2017-02-20 16:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-20 11:41 [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 02/12] net: ethernet: aquantia: Removed busy_count field Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 04/12] net: ethernet: aquantia: Using module_pci_driver Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 05/12] net: ethernet: aquantia: Superfluous initialization of "err" Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 06/12] net: ethernet: aquantia: Fixed missing rtnl_unlock Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 07/12] net: ethernet: aquantia: Using NETDEV_TX_OK instead 0 Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 08/12] net: ethernet: aquantia: Null pointer check for aq_nic_ndev_alloc Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 09/12] net: ethernet: aquantia: Call netdev_register after all initialized Pavel Belous
2017-02-20 16:28   ` David Miller
2017-02-20 11:41 ` [PATCH net-next v3 10/12] net: ethernet: aquantia: Fixed incorrect buff->len calculation Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 11/12] net: ethernet: aquantia: Fixed memory allocation if AQ_CFG_RX_FRAME_MAX > 1 page Pavel Belous
2017-02-20 11:41 ` [PATCH net-next v3 12/12] net: ethernet: aquantia: Copying tx buffers is not needed Pavel Belous
2017-02-20 16:26 ` [PATCH net-next v3 00/12] net: ethernet: aquantia: improvements and fixes David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).