All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes
@ 2017-02-20 19:36 Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev Pavel Belous
                   ` (12 more replies)
  0 siblings, 13 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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.
v3->v4: 1)Use ndev->reg_state instead "is_ndev_registered" flag.

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 +++++++++++----------
 .../ethernet/aquantia/atlantic/aq_nic_internal.h   |   1 -
 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 +-
 8 files changed, 121 insertions(+), 162 deletions(-)

-- 
2.7.4

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

* [PATCH net-next v4 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 02/12] net: ethernet: aquantia: Removed busy_count field Pavel Belous
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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 fed6ac5..22bb75e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -209,7 +209,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] 17+ messages in thread

* [PATCH net-next v4 02/12] net: ethernet: aquantia: Removed busy_count field.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu Pavel Belous
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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] 17+ messages in thread

* [PATCH net-next v4 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 01/12] net: ethernet: aquantia: Removed extra assignment for skb->dev Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 02/12] net: ethernet: aquantia: Removed busy_count field Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 22:31   ` Lino Sanfilippo
  2017-02-20 19:36 ` [PATCH net-next v4 04/12] net: ethernet: aquantia: Using module_pci_driver Pavel Belous
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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] 17+ messages in thread

* [PATCH net-next v4 04/12] net: ethernet: aquantia: Using module_pci_driver.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (2 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 05/12] net: ethernet: aquantia: Superfluous initialization of "err" Pavel Belous
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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] 17+ messages in thread

* [PATCH net-next v4 05/12] net: ethernet: aquantia: Superfluous initialization of "err".
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (3 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 04/12] net: ethernet: aquantia: Using module_pci_driver Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 06/12] net: ethernet: aquantia: Fixed missing rtnl_unlock Pavel Belous
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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] 17+ messages in thread

* [PATCH net-next v4 06/12] net: ethernet: aquantia: Fixed missing rtnl_unlock.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (4 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 05/12] net: ethernet: aquantia: Superfluous initialization of "err" Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 07/12] net: ethernet: aquantia: Using NETDEV_TX_OK instead 0 Pavel Belous
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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] 17+ messages in thread

* [PATCH net-next v4 07/12] net: ethernet: aquantia: Using NETDEV_TX_OK instead 0.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (5 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 06/12] net: ethernet: aquantia: Fixed missing rtnl_unlock Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 08/12] net: ethernet: aquantia: Null pointer check for aq_nic_ndev_alloc Pavel Belous
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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] 17+ messages in thread

* [PATCH net-next v4 08/12] net: ethernet: aquantia: Null pointer check for aq_nic_ndev_alloc.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (6 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 07/12] net: ethernet: aquantia: Using NETDEV_TX_OK instead 0 Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 09/12] net: ethernet: aquantia: Call netdev_register after all initialized Pavel Belous
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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] 17+ messages in thread

* [PATCH net-next v4 09/12] net: ethernet: aquantia: Call netdev_register after all initialized.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (7 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 08/12] net: ethernet: aquantia: Null pointer check for aq_nic_ndev_alloc Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 22:48   ` Lino Sanfilippo
  2017-02-20 19:36 ` [PATCH net-next v4 10/12] net: ethernet: aquantia: Fixed incorrect buff->len calculation Pavel Belous
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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.
Also we should use net_device->reg_state field instead own
"is_ndev_registered" flag to avoid any race.

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

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index a8a27c5..bce312a 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -261,16 +261,16 @@ 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;
+
 err_exit:
 	return err;
 }
@@ -293,7 +293,7 @@ void aq_nic_ndev_free(struct aq_nic_s *self)
 	if (!self->ndev)
 		goto err_exit;
 
-	if (self->is_ndev_registered)
+	if (self->ndev->reg_state == NETREG_REGISTERED)
 		unregister_netdev(self->ndev);
 
 	if (self->aq_hw)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic_internal.h b/drivers/net/ethernet/aquantia/atlantic/aq_nic_internal.h
index f81738a..e7d2711 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic_internal.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic_internal.h
@@ -22,7 +22,6 @@ struct aq_nic_s {
 	unsigned int aq_vecs;
 	unsigned int packet_filter;
 	unsigned int power_state;
-	bool is_ndev_registered;
 	u8 port;
 	struct aq_hw_ops aq_hw_ops;
 	struct aq_hw_caps_s aq_hw_caps;
-- 
2.7.4

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

* [PATCH net-next v4 10/12] net: ethernet: aquantia: Fixed incorrect buff->len calculation.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (8 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 09/12] net: ethernet: aquantia: Call netdev_register after all initialized Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 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; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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] 17+ messages in thread

* [PATCH net-next v4 11/12] net: ethernet: aquantia: Fixed memory allocation if AQ_CFG_RX_FRAME_MAX > 1 page.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (9 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 10/12] net: ethernet: aquantia: Fixed incorrect buff->len calculation Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 19:36 ` [PATCH net-next v4 12/12] net: ethernet: aquantia: Copying tx buffers is not needed Pavel Belous
  2017-02-20 22:12 ` [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes David Miller
  12 siblings, 0 replies; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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_ring.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 22bb75e..51f4e7f 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -270,6 +270,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;
@@ -282,7 +284,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] 17+ messages in thread

* [PATCH net-next v4 12/12] net: ethernet: aquantia: Copying tx buffers is not needed.
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (10 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 11/12] net: ethernet: aquantia: Fixed memory allocation if AQ_CFG_RX_FRAME_MAX > 1 page Pavel Belous
@ 2017-02-20 19:36 ` Pavel Belous
  2017-02-20 22:51   ` Lino Sanfilippo
  2017-02-20 22:12 ` [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes David Miller
  12 siblings, 1 reply; 17+ messages in thread
From: Pavel Belous @ 2017-02-20 19:36 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 bce312a..ee78444 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -468,95 +468,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;
 }
 
@@ -571,7 +592,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;
 
@@ -595,23 +615,27 @@ __acquires(&ring->lock)
 
 	do {
 		if (spin_trylock(&ring->header.lock)) {
-			frags = aq_nic_map_skb(self, skb, &buffers[0]);
-
-			aq_ring_tx_append_buffs(ring, &buffers[0], 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 51f4e7f..0358e607 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);
-	}
-}
-
 void 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 fb296b3..2572546 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.h
@@ -146,9 +146,6 @@ struct aq_ring_s *aq_ring_rx_alloc(struct aq_ring_s *self,
 int aq_ring_init(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);
 void 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] 17+ messages in thread

* Re: [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes
  2017-02-20 19:36 [PATCH net-next v4 00/12] net: ethernet: aquantia: improvements and fixes Pavel Belous
                   ` (11 preceding siblings ...)
  2017-02-20 19:36 ` [PATCH net-next v4 12/12] net: ethernet: aquantia: Copying tx buffers is not needed Pavel Belous
@ 2017-02-20 22:12 ` David Miller
  12 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2017-02-20 22:12 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 22:36:38 +0300

> 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.
> v3->v4: 1)Use ndev->reg_state instead "is_ndev_registered" flag.

Series applied.

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

* Re: [PATCH net-next v4 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu
  2017-02-20 19:36 ` [PATCH net-next v4 03/12] net: ethernet: aquantia: Fixes for aq_ndev_change_mtu Pavel Belous
@ 2017-02-20 22:31   ` Lino Sanfilippo
  0 siblings, 0 replies; 17+ messages in thread
From: Lino Sanfilippo @ 2017-02-20 22:31 UTC (permalink / raw)
  To: Pavel Belous, David S . Miller; +Cc: netdev, Simon Edelhaus, Alexey Andriyanov

Hi,

On 20.02.2017 20:36, Pavel Belous wrote:
> 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;

ether_setup() (which is called by alloc_etherdev_mq()) now already sets the network devices min mtu 
to ETH_MIN_MTU. So the instruction above can be omitted.

Regards,
Lino

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

* Re: [PATCH net-next v4 09/12] net: ethernet: aquantia: Call netdev_register after all initialized.
  2017-02-20 19:36 ` [PATCH net-next v4 09/12] net: ethernet: aquantia: Call netdev_register after all initialized Pavel Belous
@ 2017-02-20 22:48   ` Lino Sanfilippo
  0 siblings, 0 replies; 17+ messages in thread
From: Lino Sanfilippo @ 2017-02-20 22:48 UTC (permalink / raw)
  To: Pavel Belous, David S . Miller; +Cc: netdev, Simon Edelhaus, Alexey Andriyanov

On 20.02.2017 20:36, Pavel Belous wrote:
> From: Pavel Belous <pavel.belous@aquantia.com>
> 
> netdev_register should be called when everything is initialized.
> Also we should use net_device->reg_state field instead own
> "is_ndev_registered" flag to avoid any race.
> 
> Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
> ---
>  drivers/net/ethernet/aquantia/atlantic/aq_nic.c          | 10 +++++-----
>  drivers/net/ethernet/aquantia/atlantic/aq_nic_internal.h |  1 -
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> index a8a27c5..bce312a 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> @@ -261,16 +261,16 @@ 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;
> +
>  err_exit:
>  	return err;
>  }
> @@ -293,7 +293,7 @@ void aq_nic_ndev_free(struct aq_nic_s *self)
>  	if (!self->ndev)
>  		goto err_exit;
>  
> -	if (self->is_ndev_registered)
> +	if (self->ndev->reg_state == NETREG_REGISTERED)
>  		unregister_netdev(self->ndev);

Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>

A suggestion for a future patch (not necessarily part of this series):
Get rid of the above check at all. AFAICS aq_nic_ndev_free() is only
called in remove() and in probe(). In remove() the check is not needed, since it
always evaluates to true (device is always registered). In probe() it is only
needed for a clean unwind in case of error. The latter can also be done without
 the check in aq_nic_ndev_free().

Regards,
Lino

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

* Re: [PATCH net-next v4 12/12] net: ethernet: aquantia: Copying tx buffers is not needed.
  2017-02-20 19:36 ` [PATCH net-next v4 12/12] net: ethernet: aquantia: Copying tx buffers is not needed Pavel Belous
@ 2017-02-20 22:51   ` Lino Sanfilippo
  0 siblings, 0 replies; 17+ messages in thread
From: Lino Sanfilippo @ 2017-02-20 22:51 UTC (permalink / raw)
  To: Pavel Belous, David S . Miller; +Cc: netdev, Simon Edelhaus, Alexey Andriyanov

On 20.02.2017 20:36, Pavel Belous wrote:
> From: Pavel Belous <pavel.belous@aquantia.com>
> 
> This fix removes copying of tx biffers.

Typo: biffers -> buffers

> 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 bce312a..ee78444 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> @@ -468,95 +468,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;
>  }
>  
> @@ -571,7 +592,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;
>  
> @@ -595,23 +615,27 @@ __acquires(&ring->lock)
>  
>  	do {
>  		if (spin_trylock(&ring->header.lock)) {
> -			frags = aq_nic_map_skb(self, skb, &buffers[0]);
> -
> -			aq_ring_tx_append_buffs(ring, &buffers[0], 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 51f4e7f..0358e607 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);
> -	}
> -}
> -
>  void 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 fb296b3..2572546 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.h
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.h
> @@ -146,9 +146,6 @@ struct aq_ring_s *aq_ring_rx_alloc(struct aq_ring_s *self,
>  int aq_ring_init(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);
>  void 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);
> 

Looks good.

Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>

Regards,
Lino

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

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

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