[PATCH net-next v2 00/12] net: ethernet: aquantia: improvements and fixes
STALE3480d
Revision v2 of 4 in this series.
20 messages,
3 authors,
2017-02-18 · open the first message on its own page
From: Pavel Belous <redacted>
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 :Removed buffers copying.
Fixed dma error handling.
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: Removed unnecessary comparsion "old_mtu ==
new_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 | 34 +---
drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 205 +++++++++++----------
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(+), 153 deletions(-)
--
2.7.4
From: Pavel Belous <redacted>
This assignment is not needed.
Signed-off-by: Pavel Belous <redacted>
---
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
From: Pavel Belous <redacted>
busy_count field and is_busy flag is not needed at all.
Signed-off-by: Pavel Belous <redacted>
---
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
From: Pavel Belous <redacted>
This check is not needed. Function aq_ndev_change_mtu wont be called
if mtu has not changed.
Signed-off-by: Pavel Belous <redacted>
---
drivers/net/ethernet/aquantia/atlantic/aq_main.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index c17c70a..e5539c8 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c @@ -102,17 +102,15 @@ 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 ;
- 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 )) { --
2.7.4
From: Pavel Belous <redacted>
Remove boilerplate code by using macro module_pci_driver.
Signed-off-by: Pavel Belous <redacted>
---
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 e5539c8..f7513cb 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c @@ -250,22 +250,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
From: Pavel Belous <redacted>
Fixed superfluous initialization of err.
Signed-off-by: Pavel Belous <redacted>
---
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 f7513cb..68c19d3 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
From: Pavel Belous <redacted>
rtnl_unlock should be called if error occurred.
Signed-off-by: Pavel Belous <redacted>
---
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 a153750..1bf5975 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c @@ -931,7 +931,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 ) { @@ -956,8 +956,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
From: Pavel Belous <redacted>
Use NETDEV_TX_OK as the return value for successful transmission.
Signed-off-by: Pavel Belous <redacted>
---
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 1bf5975..4cf633c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c @@ -567,7 +567,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
From: Pavel Belous <redacted>
We should check for a null pointer for aq_nic_ndev_alloc
instead netdev_priv.
Signed-off-by: Pavel Belous <redacted>
---
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 4cf633c..e50fba2 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
From: Pavel Belous <redacted>
netdev_register should be called when everything is initialized.
Signed-off-by: Pavel Belous <redacted>
---
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 e50fba2..daed4c1 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c @@ -260,16 +260,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
From: Pavel Belous <redacted>
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 <redacted>
---
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
From: Pavel Belous <redacted>
We should allocate the number of pages based on the config parameter
AQ_CFG_RX_FRAME_MAX.
Signed-off-by: Pavel Belous <redacted>
---
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 daed4c1..25dc9b4 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c @@ -596,9 +596,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..0877625 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c @@ -278,6 +278,8 @@ int aq_ring_rx_fill(struct aq_ring_s *self)
struct aq_ring_buff_s * buff = NULL ;
int err = 0 ;
int i = 0 ;
+ unsigned int pages_order = fls ( AQ_CFG_RX_FRAME_MAX / PAGE_SIZE +
+ ( AQ_CFG_RX_FRAME_MAX % PAGE_SIZE ? 1 : 0 )) - 1 ;
for ( i = aq_ring_avail_dx ( self ); i -- ;
self -> sw_tail = aq_ring_next_dx ( self , self -> sw_tail )) { @@ -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
From: Pavel Belous <redacted>
This fix removes copying of tx biffers.
Now we use ring->buff_fing directly.
Signed-off-by: Pavel Belous <redacted>
---
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 25dc9b4..46d43c4 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c @@ -469,95 +469,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 ;
}
@@ -572,7 +593,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 ;
@@ -596,23 +616,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 0877625..0f65e0c 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
On Sat, Feb 18, 2017 at 12:07:25AM +0300, Pavel Belous wrote: quoted hunk From: Pavel Belous <redacted>
This check is not needed. Function aq_ndev_change_mtu wont be called
if mtu has not changed.
Signed-off-by: Pavel Belous <redacted>
---
drivers/net/ethernet/aquantia/atlantic/aq_main.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index c17c70a..e5539c8 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c @@ -102,17 +102,15 @@ 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 ;
- if ( new_mtu == ndev -> mtu ) {
- err = 0 ;
- goto err_exit ;
- }
if ( new_mtu < 68 ) {
err = - EINVAL ;
goto err_exit ;
}
You can set ndev->min_mtu and the core will do this check for you.
+
err = aq_nic_set_mtu(aq_nic, new_mtu + ETH_HLEN);
if (err < 0)
goto err_exit;
+
White space changes should be in a separate patch.
ndev->mtu = new_mtu;
I think __dev_set_mtu() takes care of this for you.
Andrew
quoted hunk diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 4c40644..0877625 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c @@ -278,6 +278,8 @@ int aq_ring_rx_fill(struct aq_ring_s *self)
struct aq_ring_buff_s * buff = NULL ;
int err = 0 ;
int i = 0 ;
+ unsigned int pages_order = fls ( AQ_CFG_RX_FRAME_MAX / PAGE_SIZE +
+ ( AQ_CFG_RX_FRAME_MAX % PAGE_SIZE ? 1 : 0 )) - 1 ;
Reverse Christmas tree?
Andrew
Hi,
On 17.02.2017 22:07, Pavel Belous wrote: From: Pavel Belous <redacted>
We should allocate the number of pages based on the config parameter
AQ_CFG_RX_FRAME_MAX.
Signed-off-by: Pavel Belous <redacted> 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);
This change has nothing to do with what the commit message claims that the
patch is about. Please dont mix fixes and totally unrelated cleanups in one
patch.
Regards,
Lino
Hi,
On 17.02.2017 22:07, Pavel Belous wrote: From: Pavel Belous <redacted>
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 :Removed buffers copying.
Fixed dma error handling.
Please review.
You could have added all "reviewed-by" tags that you have received so
far for patches in the former version of this series. I think otherwise this
information will get lost.
Regards,
Lino
On 02/18/2017 01:43 AM, Andrew Lunn wrote: quoted diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 4c40644..0877625 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c @@ -278,6 +278,8 @@ int aq_ring_rx_fill(struct aq_ring_s *self)
struct aq_ring_buff_s * buff = NULL ;
int err = 0 ;
int i = 0 ;
+ unsigned int pages_order = fls ( AQ_CFG_RX_FRAME_MAX / PAGE_SIZE +
+ ( AQ_CFG_RX_FRAME_MAX % PAGE_SIZE ? 1 : 0 )) - 1 ;
Reverse Christmas tree?
Andrew
Thank you.
I will fix it in v3.
Regards,
Pavel
On 02/18/2017 02:56 PM, Lino Sanfilippo wrote: Hi,
On 17.02.2017 22:07, Pavel Belous wrote: quoted From: Pavel Belous <redacted>
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 :Removed buffers copying.
Fixed dma error handling.
Please review.
You could have added all "reviewed-by" tags that you have received so
far for patches in the former version of this series. I think otherwise this
information will get lost.
Regards,
Lino
I was thinking to add this tag, but i was unsure.
Thank you, I will add "Reviewed-by" tags for reviewed patches from v1
(not changed) in patches v3.
Regards,
Pavel
On 02/18/2017 02:50 PM, Lino Sanfilippo wrote: Hi,
On 17.02.2017 22:07, Pavel Belous wrote: quoted From: Pavel Belous <redacted>
We should allocate the number of pages based on the config parameter
AQ_CFG_RX_FRAME_MAX.
Signed-off-by: Pavel Belous <redacted> quoted 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);
This change has nothing to do with what the commit message claims that the
patch is about. Please dont mix fixes and totally unrelated cleanups in one
patch.
Regards,
Lino
Sorry, its just small fix for readability.
I will remove it or put in separate patch in v3.
Regards,
Pavel