Skip to content

Commit 3aec641

Browse files
caildavem330
cail
authored and
davem330
committed
aquantia: Fix Tx queue hangups
Driver did a poor job in managing its Tx queues: Sometimes it could stop tx queues due to link down condition in aq_nic_xmit - but never waked up them. That led to Tx path total suspend. This patch fixes this and improves generic queue management: - introduces queue restart counter - uses generic netif_ interface to disable and enable tx path - refactors link up/down condition and introduces dmesg log event when link changes. - introduces new constant for minimum descriptors count required for queue wakeup Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent d85fc17 commit 3aec641

File tree

6 files changed

+76
-59
lines changed

6 files changed

+76
-59
lines changed

‎drivers/net/ethernet/aquantia/atlantic/aq_cfg.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@
5151

5252
#define AQ_CFG_SKB_FRAGS_MAX 32U
5353

54+
/* Number of descriptors available in one ring to resume this ring queue
55+
*/
56+
#define AQ_CFG_RESTART_DESC_THRES (AQ_CFG_SKB_FRAGS_MAX * 2)
57+
5458
#define AQ_CFG_NAPI_WEIGHT 64U
5559

5660
#define AQ_CFG_MULTICAST_ADDRESS_MAX 32U

‎drivers/net/ethernet/aquantia/atlantic/aq_nic.c

Lines changed: 40 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,35 @@ int aq_nic_cfg_start(struct aq_nic_s *self)
119119
return 0;
120120
}
121121

122+
static int aq_nic_update_link_status(struct aq_nic_s *self)
123+
{
124+
int err = self->aq_hw_ops.hw_get_link_status(self->aq_hw);
125+
126+
if (err)
127+
return err;
128+
129+
if (self->link_status.mbps != self->aq_hw->aq_link_status.mbps)
130+
pr_info("%s: link change old %d new %d\n",
131+
AQ_CFG_DRV_NAME, self->link_status.mbps,
132+
self->aq_hw->aq_link_status.mbps);
133+
134+
self->link_status = self->aq_hw->aq_link_status;
135+
if (!netif_carrier_ok(self->ndev) && self->link_status.mbps) {
136+
aq_utils_obj_set(&self->header.flags,
137+
AQ_NIC_FLAG_STARTED);
138+
aq_utils_obj_clear(&self->header.flags,
139+
AQ_NIC_LINK_DOWN);
140+
netif_carrier_on(self->ndev);
141+
netif_tx_wake_all_queues(self->ndev);
142+
}
143+
if (netif_carrier_ok(self->ndev) && !self->link_status.mbps) {
144+
netif_carrier_off(self->ndev);
145+
netif_tx_disable(self->ndev);
146+
aq_utils_obj_set(&self->header.flags, AQ_NIC_LINK_DOWN);
147+
}
148+
return 0;
149+
}
150+
122151
static void aq_nic_service_timer_cb(unsigned long param)
123152
{
124153
struct aq_nic_s *self = (struct aq_nic_s *)param;
@@ -131,26 +160,13 @@ static void aq_nic_service_timer_cb(unsigned long param)
131160
if (aq_utils_obj_test(&self->header.flags, AQ_NIC_FLAGS_IS_NOT_READY))
132161
goto err_exit;
133162

134-
err = self->aq_hw_ops.hw_get_link_status(self->aq_hw);
135-
if (err < 0)
163+
err = aq_nic_update_link_status(self);
164+
if (err)
136165
goto err_exit;
137166

138-
self->link_status = self->aq_hw->aq_link_status;
139-
140167
self->aq_hw_ops.hw_interrupt_moderation_set(self->aq_hw,
141168
self->aq_nic_cfg.is_interrupt_moderation);
142169

143-
if (self->link_status.mbps) {
144-
aq_utils_obj_set(&self->header.flags,
145-
AQ_NIC_FLAG_STARTED);
146-
aq_utils_obj_clear(&self->header.flags,
147-
AQ_NIC_LINK_DOWN);
148-
netif_carrier_on(self->ndev);
149-
} else {
150-
netif_carrier_off(self->ndev);
151-
aq_utils_obj_set(&self->header.flags, AQ_NIC_LINK_DOWN);
152-
}
153-
154170
memset(&stats_rx, 0U, sizeof(struct aq_ring_stats_rx_s));
155171
memset(&stats_tx, 0U, sizeof(struct aq_ring_stats_tx_s));
156172
for (i = AQ_DIMOF(self->aq_vec); i--;) {
@@ -240,7 +256,6 @@ struct aq_nic_s *aq_nic_alloc_cold(const struct net_device_ops *ndev_ops,
240256
int aq_nic_ndev_register(struct aq_nic_s *self)
241257
{
242258
int err = 0;
243-
unsigned int i = 0U;
244259

245260
if (!self->ndev) {
246261
err = -EINVAL;
@@ -262,8 +277,7 @@ int aq_nic_ndev_register(struct aq_nic_s *self)
262277

263278
netif_carrier_off(self->ndev);
264279

265-
for (i = AQ_CFG_VECS_MAX; i--;)
266-
aq_nic_ndev_queue_stop(self, i);
280+
netif_tx_disable(self->ndev);
267281

268282
err = register_netdev(self->ndev);
269283
if (err < 0)
@@ -318,12 +332,8 @@ struct aq_nic_s *aq_nic_alloc_hot(struct net_device *ndev)
318332
err = -EINVAL;
319333
goto err_exit;
320334
}
321-
if (netif_running(ndev)) {
322-
unsigned int i;
323-
324-
for (i = AQ_CFG_VECS_MAX; i--;)
325-
netif_stop_subqueue(ndev, i);
326-
}
335+
if (netif_running(ndev))
336+
netif_tx_disable(ndev);
327337

328338
for (self->aq_vecs = 0; self->aq_vecs < self->aq_nic_cfg.vecs;
329339
self->aq_vecs++) {
@@ -383,16 +393,6 @@ int aq_nic_init(struct aq_nic_s *self)
383393
return err;
384394
}
385395

386-
void aq_nic_ndev_queue_start(struct aq_nic_s *self, unsigned int idx)
387-
{
388-
netif_start_subqueue(self->ndev, idx);
389-
}
390-
391-
void aq_nic_ndev_queue_stop(struct aq_nic_s *self, unsigned int idx)
392-
{
393-
netif_stop_subqueue(self->ndev, idx);
394-
}
395-
396396
int aq_nic_start(struct aq_nic_s *self)
397397
{
398398
struct aq_vec_s *aq_vec = NULL;
@@ -451,10 +451,6 @@ int aq_nic_start(struct aq_nic_s *self)
451451
goto err_exit;
452452
}
453453

454-
for (i = 0U, aq_vec = self->aq_vec[0];
455-
self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i])
456-
aq_nic_ndev_queue_start(self, i);
457-
458454
err = netif_set_real_num_tx_queues(self->ndev, self->aq_vecs);
459455
if (err < 0)
460456
goto err_exit;
@@ -463,6 +459,8 @@ int aq_nic_start(struct aq_nic_s *self)
463459
if (err < 0)
464460
goto err_exit;
465461

462+
netif_tx_start_all_queues(self->ndev);
463+
466464
err_exit:
467465
return err;
468466
}
@@ -602,7 +600,6 @@ int aq_nic_xmit(struct aq_nic_s *self, struct sk_buff *skb)
602600
unsigned int vec = skb->queue_mapping % self->aq_nic_cfg.vecs;
603601
unsigned int tc = 0U;
604602
int err = NETDEV_TX_OK;
605-
bool is_nic_in_bad_state;
606603

607604
frags = skb_shinfo(skb)->nr_frags + 1;
608605

@@ -613,13 +610,10 @@ int aq_nic_xmit(struct aq_nic_s *self, struct sk_buff *skb)
613610
goto err_exit;
614611
}
615612

616-
is_nic_in_bad_state = aq_utils_obj_test(&self->header.flags,
617-
AQ_NIC_FLAGS_IS_NOT_TX_READY) ||
618-
(aq_ring_avail_dx(ring) <
619-
AQ_CFG_SKB_FRAGS_MAX);
613+
aq_ring_update_queue_state(ring);
620614

621-
if (is_nic_in_bad_state) {
622-
aq_nic_ndev_queue_stop(self, ring->idx);
615+
/* Above status update may stop the queue. Check this. */
616+
if (__netif_subqueue_stopped(self->ndev, ring->idx)) {
623617
err = NETDEV_TX_BUSY;
624618
goto err_exit;
625619
}
@@ -631,9 +625,6 @@ int aq_nic_xmit(struct aq_nic_s *self, struct sk_buff *skb)
631625
ring,
632626
frags);
633627
if (err >= 0) {
634-
if (aq_ring_avail_dx(ring) < AQ_CFG_SKB_FRAGS_MAX + 1)
635-
aq_nic_ndev_queue_stop(self, ring->idx);
636-
637628
++ring->stats.tx.packets;
638629
ring->stats.tx.bytes += skb->len;
639630
}
@@ -898,9 +889,7 @@ int aq_nic_stop(struct aq_nic_s *self)
898889
struct aq_vec_s *aq_vec = NULL;
899890
unsigned int i = 0U;
900891

901-
for (i = 0U, aq_vec = self->aq_vec[0];
902-
self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i])
903-
aq_nic_ndev_queue_stop(self, i);
892+
netif_tx_disable(self->ndev);
904893

905894
del_timer_sync(&self->service_timer);
906895

‎drivers/net/ethernet/aquantia/atlantic/aq_nic.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ struct net_device *aq_nic_get_ndev(struct aq_nic_s *self);
8383
int aq_nic_init(struct aq_nic_s *self);
8484
int aq_nic_cfg_start(struct aq_nic_s *self);
8585
int aq_nic_ndev_register(struct aq_nic_s *self);
86-
void aq_nic_ndev_queue_start(struct aq_nic_s *self, unsigned int idx);
87-
void aq_nic_ndev_queue_stop(struct aq_nic_s *self, unsigned int idx);
8886
void aq_nic_ndev_free(struct aq_nic_s *self);
8987
int aq_nic_start(struct aq_nic_s *self);
9088
int aq_nic_xmit(struct aq_nic_s *self, struct sk_buff *skb);

‎drivers/net/ethernet/aquantia/atlantic/aq_ring.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,32 @@ int aq_ring_init(struct aq_ring_s *self)
104104
return 0;
105105
}
106106

107+
void aq_ring_update_queue_state(struct aq_ring_s *ring)
108+
{
109+
if (aq_ring_avail_dx(ring) <= AQ_CFG_SKB_FRAGS_MAX)
110+
aq_ring_queue_stop(ring);
111+
else if (aq_ring_avail_dx(ring) > AQ_CFG_RESTART_DESC_THRES)
112+
aq_ring_queue_wake(ring);
113+
}
114+
115+
void aq_ring_queue_wake(struct aq_ring_s *ring)
116+
{
117+
struct net_device *ndev = aq_nic_get_ndev(ring->aq_nic);
118+
119+
if (__netif_subqueue_stopped(ndev, ring->idx)) {
120+
netif_wake_subqueue(ndev, ring->idx);
121+
ring->stats.tx.queue_restarts++;
122+
}
123+
}
124+
125+
void aq_ring_queue_stop(struct aq_ring_s *ring)
126+
{
127+
struct net_device *ndev = aq_nic_get_ndev(ring->aq_nic);
128+
129+
if (!__netif_subqueue_stopped(ndev, ring->idx))
130+
netif_stop_subqueue(ndev, ring->idx);
131+
}
132+
107133
void aq_ring_tx_clean(struct aq_ring_s *self)
108134
{
109135
struct device *dev = aq_nic_get_dev(self->aq_nic);

‎drivers/net/ethernet/aquantia/atlantic/aq_ring.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ struct aq_ring_stats_tx_s {
9494
u64 errors;
9595
u64 packets;
9696
u64 bytes;
97+
u64 queue_restarts;
9798
};
9899

99100
union aq_ring_stats_s {
@@ -147,6 +148,9 @@ struct aq_ring_s *aq_ring_rx_alloc(struct aq_ring_s *self,
147148
int aq_ring_init(struct aq_ring_s *self);
148149
void aq_ring_rx_deinit(struct aq_ring_s *self);
149150
void aq_ring_free(struct aq_ring_s *self);
151+
void aq_ring_update_queue_state(struct aq_ring_s *ring);
152+
void aq_ring_queue_wake(struct aq_ring_s *ring);
153+
void aq_ring_queue_stop(struct aq_ring_s *ring);
150154
void aq_ring_tx_clean(struct aq_ring_s *self);
151155
int aq_ring_rx_clean(struct aq_ring_s *self,
152156
struct napi_struct *napi,

‎drivers/net/ethernet/aquantia/atlantic/aq_vec.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,7 @@ static int aq_vec_poll(struct napi_struct *napi, int budget)
5959
if (ring[AQ_VEC_TX_ID].sw_head !=
6060
ring[AQ_VEC_TX_ID].hw_head) {
6161
aq_ring_tx_clean(&ring[AQ_VEC_TX_ID]);
62-
63-
if (aq_ring_avail_dx(&ring[AQ_VEC_TX_ID]) >
64-
AQ_CFG_SKB_FRAGS_MAX) {
65-
aq_nic_ndev_queue_start(self->aq_nic,
66-
ring[AQ_VEC_TX_ID].idx);
67-
}
62+
aq_ring_update_queue_state(&ring[AQ_VEC_TX_ID]);
6863
was_tx_cleaned = true;
6964
}
7065

@@ -364,6 +359,7 @@ void aq_vec_add_stats(struct aq_vec_s *self,
364359
stats_tx->packets += tx->packets;
365360
stats_tx->bytes += tx->bytes;
366361
stats_tx->errors += tx->errors;
362+
stats_tx->queue_restarts += tx->queue_restarts;
367363
}
368364
}
369365

0 commit comments

Comments
 (0)