Skip to content

Commit 81fe16e

Browse files
davem330davem330
committed
Merge branch 'qed-Slowpath-Queue-bug-fixes'
Denis Bolotin says: ==================== qed: Slowpath Queue bug fixes This patch series fixes several bugs in the SPQ mechanism. It deals with SPQ entries management, preventing resource leaks, memory corruptions and handles error cases throughout the driver. Please consider applying to net. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 0d5b931 + fa5c448 commit 81fe16e

File tree

9 files changed

+89
-43
lines changed

9 files changed

+89
-43
lines changed

‎drivers/net/ethernet/qlogic/qed/qed_fcoe.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ qed_sp_fcoe_func_start(struct qed_hwfn *p_hwfn,
147147
"Cannot satisfy CQ amount. CQs requested %d, CQs available %d. Aborting function start\n",
148148
fcoe_pf_params->num_cqs,
149149
p_hwfn->hw_info.feat_num[QED_FCOE_CQ]);
150-
return -EINVAL;
150+
rc = -EINVAL;
151+
goto err;
151152
}
152153

153154
p_data->mtu = cpu_to_le16(fcoe_pf_params->mtu);
@@ -156,14 +157,14 @@ qed_sp_fcoe_func_start(struct qed_hwfn *p_hwfn,
156157

157158
rc = qed_cxt_acquire_cid(p_hwfn, PROTOCOLID_FCOE, &dummy_cid);
158159
if (rc)
159-
return rc;
160+
goto err;
160161

161162
cxt_info.iid = dummy_cid;
162163
rc = qed_cxt_get_cid_info(p_hwfn, &cxt_info);
163164
if (rc) {
164165
DP_NOTICE(p_hwfn, "Cannot find context info for dummy cid=%d\n",
165166
dummy_cid);
166-
return rc;
167+
goto err;
167168
}
168169
p_cxt = cxt_info.p_cxt;
169170
SET_FIELD(p_cxt->tstorm_ag_context.flags3,
@@ -240,6 +241,10 @@ qed_sp_fcoe_func_start(struct qed_hwfn *p_hwfn,
240241
rc = qed_spq_post(p_hwfn, p_ent, NULL);
241242

242243
return rc;
244+
245+
err:
246+
qed_sp_destroy_request(p_hwfn, p_ent);
247+
return rc;
243248
}
244249

245250
static int

‎drivers/net/ethernet/qlogic/qed/qed_iscsi.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ qed_sp_iscsi_func_start(struct qed_hwfn *p_hwfn,
200200
"Cannot satisfy CQ amount. Queues requested %d, CQs available %d. Aborting function start\n",
201201
p_params->num_queues,
202202
p_hwfn->hw_info.feat_num[QED_ISCSI_CQ]);
203+
qed_sp_destroy_request(p_hwfn, p_ent);
203204
return -EINVAL;
204205
}
205206

‎drivers/net/ethernet/qlogic/qed/qed_l2.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -740,8 +740,7 @@ int qed_sp_vport_update(struct qed_hwfn *p_hwfn,
740740

741741
rc = qed_sp_vport_update_rss(p_hwfn, p_ramrod, p_rss_params);
742742
if (rc) {
743-
/* Return spq entry which is taken in qed_sp_init_request()*/
744-
qed_spq_return_entry(p_hwfn, p_ent);
743+
qed_sp_destroy_request(p_hwfn, p_ent);
745744
return rc;
746745
}
747746

@@ -1355,6 +1354,7 @@ qed_filter_ucast_common(struct qed_hwfn *p_hwfn,
13551354
DP_NOTICE(p_hwfn,
13561355
"%d is not supported yet\n",
13571356
p_filter_cmd->opcode);
1357+
qed_sp_destroy_request(p_hwfn, *pp_ent);
13581358
return -EINVAL;
13591359
}
13601360

@@ -2056,13 +2056,13 @@ qed_configure_rfs_ntuple_filter(struct qed_hwfn *p_hwfn,
20562056
} else {
20572057
rc = qed_fw_vport(p_hwfn, p_params->vport_id, &abs_vport_id);
20582058
if (rc)
2059-
return rc;
2059+
goto err;
20602060

20612061
if (p_params->qid != QED_RFS_NTUPLE_QID_RSS) {
20622062
rc = qed_fw_l2_queue(p_hwfn, p_params->qid,
20632063
&abs_rx_q_id);
20642064
if (rc)
2065-
return rc;
2065+
goto err;
20662066

20672067
p_ramrod->rx_qid_valid = 1;
20682068
p_ramrod->rx_qid = cpu_to_le16(abs_rx_q_id);
@@ -2083,6 +2083,10 @@ qed_configure_rfs_ntuple_filter(struct qed_hwfn *p_hwfn,
20832083
(u64)p_params->addr, p_params->length);
20842084

20852085
return qed_spq_post(p_hwfn, p_ent, NULL);
2086+
2087+
err:
2088+
qed_sp_destroy_request(p_hwfn, p_ent);
2089+
return rc;
20862090
}
20872091

20882092
int qed_get_rxq_coalesce(struct qed_hwfn *p_hwfn,

‎drivers/net/ethernet/qlogic/qed/qed_rdma.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,6 +1514,7 @@ qed_rdma_register_tid(void *rdma_cxt,
15141514
default:
15151515
rc = -EINVAL;
15161516
DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "rc = %d\n", rc);
1517+
qed_sp_destroy_request(p_hwfn, p_ent);
15171518
return rc;
15181519
}
15191520
SET_FIELD(p_ramrod->flags1,

‎drivers/net/ethernet/qlogic/qed/qed_roce.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,7 @@ static int qed_roce_sp_destroy_qp_responder(struct qed_hwfn *p_hwfn,
745745
DP_NOTICE(p_hwfn,
746746
"qed destroy responder failed: cannot allocate memory (ramrod). rc = %d\n",
747747
rc);
748+
qed_sp_destroy_request(p_hwfn, p_ent);
748749
return rc;
749750
}
750751

‎drivers/net/ethernet/qlogic/qed/qed_sp.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ struct qed_spq_entry {
167167
enum spq_mode comp_mode;
168168
struct qed_spq_comp_cb comp_cb;
169169
struct qed_spq_comp_done comp_done; /* SPQ_MODE_EBLOCK */
170+
171+
/* Posted entry for unlimited list entry in EBLOCK mode */
172+
struct qed_spq_entry *post_ent;
170173
};
171174

172175
struct qed_eq {
@@ -396,6 +399,17 @@ struct qed_sp_init_data {
396399
struct qed_spq_comp_cb *p_comp_data;
397400
};
398401

402+
/**
403+
* @brief Returns a SPQ entry to the pool / frees the entry if allocated.
404+
* Should be called on in error flows after initializing the SPQ entry
405+
* and before posting it.
406+
*
407+
* @param p_hwfn
408+
* @param p_ent
409+
*/
410+
void qed_sp_destroy_request(struct qed_hwfn *p_hwfn,
411+
struct qed_spq_entry *p_ent);
412+
399413
int qed_sp_init_request(struct qed_hwfn *p_hwfn,
400414
struct qed_spq_entry **pp_ent,
401415
u8 cmd,

‎drivers/net/ethernet/qlogic/qed/qed_sp_commands.c

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,19 @@
4747
#include "qed_sp.h"
4848
#include "qed_sriov.h"
4949

50+
void qed_sp_destroy_request(struct qed_hwfn *p_hwfn,
51+
struct qed_spq_entry *p_ent)
52+
{
53+
/* qed_spq_get_entry() can either get an entry from the free_pool,
54+
* or, if no entries are left, allocate a new entry and add it to
55+
* the unlimited_pending list.
56+
*/
57+
if (p_ent->queue == &p_hwfn->p_spq->unlimited_pending)
58+
kfree(p_ent);
59+
else
60+
qed_spq_return_entry(p_hwfn, p_ent);
61+
}
62+
5063
int qed_sp_init_request(struct qed_hwfn *p_hwfn,
5164
struct qed_spq_entry **pp_ent,
5265
u8 cmd, u8 protocol, struct qed_sp_init_data *p_data)
@@ -80,7 +93,7 @@ int qed_sp_init_request(struct qed_hwfn *p_hwfn,
8093

8194
case QED_SPQ_MODE_BLOCK:
8295
if (!p_data->p_comp_data)
83-
return -EINVAL;
96+
goto err;
8497

8598
p_ent->comp_cb.cookie = p_data->p_comp_data->cookie;
8699
break;
@@ -95,7 +108,7 @@ int qed_sp_init_request(struct qed_hwfn *p_hwfn,
95108
default:
96109
DP_NOTICE(p_hwfn, "Unknown SPQE completion mode %d\n",
97110
p_ent->comp_mode);
98-
return -EINVAL;
111+
goto err;
99112
}
100113

101114
DP_VERBOSE(p_hwfn, QED_MSG_SPQ,
@@ -109,6 +122,11 @@ int qed_sp_init_request(struct qed_hwfn *p_hwfn,
109122
memset(&p_ent->ramrod, 0, sizeof(p_ent->ramrod));
110123

111124
return 0;
125+
126+
err:
127+
qed_sp_destroy_request(p_hwfn, p_ent);
128+
129+
return -EINVAL;
112130
}
113131

114132
static enum tunnel_clss qed_tunn_clss_to_fw_clss(u8 type)

‎drivers/net/ethernet/qlogic/qed/qed_spq.c

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ static int qed_spq_block(struct qed_hwfn *p_hwfn,
142142

143143
DP_INFO(p_hwfn, "Ramrod is stuck, requesting MCP drain\n");
144144
rc = qed_mcp_drain(p_hwfn, p_ptt);
145+
qed_ptt_release(p_hwfn, p_ptt);
145146
if (rc) {
146147
DP_NOTICE(p_hwfn, "MCP drain failed\n");
147148
goto err;
@@ -150,18 +151,15 @@ static int qed_spq_block(struct qed_hwfn *p_hwfn,
150151
/* Retry after drain */
151152
rc = __qed_spq_block(p_hwfn, p_ent, p_fw_ret, true);
152153
if (!rc)
153-
goto out;
154+
return 0;
154155

155156
comp_done = (struct qed_spq_comp_done *)p_ent->comp_cb.cookie;
156-
if (comp_done->done == 1)
157+
if (comp_done->done == 1) {
157158
if (p_fw_ret)
158159
*p_fw_ret = comp_done->fw_return_code;
159-
out:
160-
qed_ptt_release(p_hwfn, p_ptt);
161-
return 0;
162-
160+
return 0;
161+
}
163162
err:
164-
qed_ptt_release(p_hwfn, p_ptt);
165163
DP_NOTICE(p_hwfn,
166164
"Ramrod is stuck [CID %08x cmd %02x protocol %02x echo %04x]\n",
167165
le32_to_cpu(p_ent->elem.hdr.cid),
@@ -685,6 +683,8 @@ static int qed_spq_add_entry(struct qed_hwfn *p_hwfn,
685683
/* EBLOCK responsible to free the allocated p_ent */
686684
if (p_ent->comp_mode != QED_SPQ_MODE_EBLOCK)
687685
kfree(p_ent);
686+
else
687+
p_ent->post_ent = p_en2;
688688

689689
p_ent = p_en2;
690690
}
@@ -767,6 +767,25 @@ static int qed_spq_pend_post(struct qed_hwfn *p_hwfn)
767767
SPQ_HIGH_PRI_RESERVE_DEFAULT);
768768
}
769769

770+
/* Avoid overriding of SPQ entries when getting out-of-order completions, by
771+
* marking the completions in a bitmap and increasing the chain consumer only
772+
* for the first successive completed entries.
773+
*/
774+
static void qed_spq_comp_bmap_update(struct qed_hwfn *p_hwfn, __le16 echo)
775+
{
776+
u16 pos = le16_to_cpu(echo) % SPQ_RING_SIZE;
777+
struct qed_spq *p_spq = p_hwfn->p_spq;
778+
779+
__set_bit(pos, p_spq->p_comp_bitmap);
780+
while (test_bit(p_spq->comp_bitmap_idx,
781+
p_spq->p_comp_bitmap)) {
782+
__clear_bit(p_spq->comp_bitmap_idx,
783+
p_spq->p_comp_bitmap);
784+
p_spq->comp_bitmap_idx++;
785+
qed_chain_return_produced(&p_spq->chain);
786+
}
787+
}
788+
770789
int qed_spq_post(struct qed_hwfn *p_hwfn,
771790
struct qed_spq_entry *p_ent, u8 *fw_return_code)
772791
{
@@ -824,11 +843,12 @@ int qed_spq_post(struct qed_hwfn *p_hwfn,
824843
p_ent->queue == &p_spq->unlimited_pending);
825844

826845
if (p_ent->queue == &p_spq->unlimited_pending) {
827-
/* This is an allocated p_ent which does not need to
828-
* return to pool.
829-
*/
846+
struct qed_spq_entry *p_post_ent = p_ent->post_ent;
847+
830848
kfree(p_ent);
831-
return rc;
849+
850+
/* Return the entry which was actually posted */
851+
p_ent = p_post_ent;
832852
}
833853

834854
if (rc)
@@ -842,7 +862,7 @@ int qed_spq_post(struct qed_hwfn *p_hwfn,
842862
spq_post_fail2:
843863
spin_lock_bh(&p_spq->lock);
844864
list_del(&p_ent->list);
845-
qed_chain_return_produced(&p_spq->chain);
865+
qed_spq_comp_bmap_update(p_hwfn, p_ent->elem.hdr.echo);
846866

847867
spq_post_fail:
848868
/* return to the free pool */
@@ -874,25 +894,8 @@ int qed_spq_completion(struct qed_hwfn *p_hwfn,
874894
spin_lock_bh(&p_spq->lock);
875895
list_for_each_entry_safe(p_ent, tmp, &p_spq->completion_pending, list) {
876896
if (p_ent->elem.hdr.echo == echo) {
877-
u16 pos = le16_to_cpu(echo) % SPQ_RING_SIZE;
878-
879897
list_del(&p_ent->list);
880-
881-
/* Avoid overriding of SPQ entries when getting
882-
* out-of-order completions, by marking the completions
883-
* in a bitmap and increasing the chain consumer only
884-
* for the first successive completed entries.
885-
*/
886-
__set_bit(pos, p_spq->p_comp_bitmap);
887-
888-
while (test_bit(p_spq->comp_bitmap_idx,
889-
p_spq->p_comp_bitmap)) {
890-
__clear_bit(p_spq->comp_bitmap_idx,
891-
p_spq->p_comp_bitmap);
892-
p_spq->comp_bitmap_idx++;
893-
qed_chain_return_produced(&p_spq->chain);
894-
}
895-
898+
qed_spq_comp_bmap_update(p_hwfn, echo);
896899
p_spq->comp_count++;
897900
found = p_ent;
898901
break;
@@ -931,11 +934,9 @@ int qed_spq_completion(struct qed_hwfn *p_hwfn,
931934
QED_MSG_SPQ,
932935
"Got a completion without a callback function\n");
933936

934-
if ((found->comp_mode != QED_SPQ_MODE_EBLOCK) ||
935-
(found->queue == &p_spq->unlimited_pending))
937+
if (found->comp_mode != QED_SPQ_MODE_EBLOCK)
936938
/* EBLOCK is responsible for returning its own entry into the
937-
* free list, unless it originally added the entry into the
938-
* unlimited pending list.
939+
* free list.
939940
*/
940941
qed_spq_return_entry(p_hwfn, found);
941942

‎drivers/net/ethernet/qlogic/qed/qed_sriov.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ static int qed_sp_vf_start(struct qed_hwfn *p_hwfn, struct qed_vf_info *p_vf)
101101
default:
102102
DP_NOTICE(p_hwfn, "Unknown VF personality %d\n",
103103
p_hwfn->hw_info.personality);
104+
qed_sp_destroy_request(p_hwfn, p_ent);
104105
return -EINVAL;
105106
}
106107

0 commit comments

Comments
 (0)