Skip to content

Commit ef71f1e

Browse files
metze-sambasmfrench
authored andcommitted
smb: client: fix sending the iwrap custom IRD/ORD negotiation messages
Do a real negotiation and check the servers initiator_depth and responder_resources. This should use big endian in order to be useful. I have captures of windows clients showing this. The fact that we used little endian up to now means that we sent very large numbers and the negotiation with the server truncated them to the server limits. Note the reason why this uses u8 for initiator_depth and responder_resources is that the rdma layer also uses it. The inconsitency regarding the initiator_depth and responder_resources values being reversed for iwarp devices in RDMA_CM_EVENT_ESTABLISHED should also be fixed later, but for now we should fix it. Cc: Steve French <[email protected]> Cc: Tom Talpey <[email protected]> Cc: Long Li <[email protected]> Acked-by: Namjae Jeon <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Fixes: c739858 ("CIFS: SMBD: Implement RDMA memory registration") Signed-off-by: Stefan Metzmacher <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 6920b4a commit ef71f1e

File tree

2 files changed

+103
-11
lines changed

2 files changed

+103
-11
lines changed

fs/smb/client/smbdirect.c

Lines changed: 100 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ static int smbd_conn_upcall(
179179
struct smbd_connection *info = id->context;
180180
struct smbdirect_socket *sc = &info->socket;
181181
const char *event_name = rdma_event_msg(event->event);
182+
u8 peer_initiator_depth;
183+
u8 peer_responder_resources;
182184

183185
log_rdma_event(INFO, "event=%s status=%d\n",
184186
event_name, event->status);
@@ -204,6 +206,85 @@ static int smbd_conn_upcall(
204206

205207
case RDMA_CM_EVENT_ESTABLISHED:
206208
log_rdma_event(INFO, "connected event=%s\n", event_name);
209+
210+
/*
211+
* Here we work around an inconsistency between
212+
* iWarp and other devices (at least rxe and irdma using RoCEv2)
213+
*/
214+
if (rdma_protocol_iwarp(id->device, id->port_num)) {
215+
/*
216+
* iWarp devices report the peer's values
217+
* with the perspective of the peer here.
218+
* Tested with siw and irdma (in iwarp mode)
219+
* We need to change to our perspective here,
220+
* so we need to switch the values.
221+
*/
222+
peer_initiator_depth = event->param.conn.responder_resources;
223+
peer_responder_resources = event->param.conn.initiator_depth;
224+
} else {
225+
/*
226+
* Non iWarp devices report the peer's values
227+
* already changed to our perspective here.
228+
* Tested with rxe and irdma (in roce mode).
229+
*/
230+
peer_initiator_depth = event->param.conn.initiator_depth;
231+
peer_responder_resources = event->param.conn.responder_resources;
232+
}
233+
if (rdma_protocol_iwarp(id->device, id->port_num) &&
234+
event->param.conn.private_data_len == 8) {
235+
/*
236+
* Legacy clients with only iWarp MPA v1 support
237+
* need a private blob in order to negotiate
238+
* the IRD/ORD values.
239+
*/
240+
const __be32 *ird_ord_hdr = event->param.conn.private_data;
241+
u32 ird32 = be32_to_cpu(ird_ord_hdr[0]);
242+
u32 ord32 = be32_to_cpu(ird_ord_hdr[1]);
243+
244+
/*
245+
* cifs.ko sends the legacy IRD/ORD negotiation
246+
* event if iWarp MPA v2 was used.
247+
*
248+
* Here we check that the values match and only
249+
* mark the client as legacy if they don't match.
250+
*/
251+
if ((u32)event->param.conn.initiator_depth != ird32 ||
252+
(u32)event->param.conn.responder_resources != ord32) {
253+
/*
254+
* There are broken clients (old cifs.ko)
255+
* using little endian and also
256+
* struct rdma_conn_param only uses u8
257+
* for initiator_depth and responder_resources,
258+
* so we truncate the value to U8_MAX.
259+
*
260+
* smb_direct_accept_client() will then
261+
* do the real negotiation in order to
262+
* select the minimum between client and
263+
* server.
264+
*/
265+
ird32 = min_t(u32, ird32, U8_MAX);
266+
ord32 = min_t(u32, ord32, U8_MAX);
267+
268+
info->legacy_iwarp = true;
269+
peer_initiator_depth = (u8)ird32;
270+
peer_responder_resources = (u8)ord32;
271+
}
272+
}
273+
274+
/*
275+
* negotiate the value by using the minimum
276+
* between client and server if the client provided
277+
* non 0 values.
278+
*/
279+
if (peer_initiator_depth != 0)
280+
info->initiator_depth =
281+
min_t(u8, info->initiator_depth,
282+
peer_initiator_depth);
283+
if (peer_responder_resources != 0)
284+
info->responder_resources =
285+
min_t(u8, info->responder_resources,
286+
peer_responder_resources);
287+
207288
sc->status = SMBDIRECT_SOCKET_CONNECTED;
208289
wake_up_interruptible(&info->status_wait);
209290
break;
@@ -1551,14 +1632,17 @@ static struct smbd_connection *_smbd_get_connection(
15511632
struct ib_qp_init_attr qp_attr;
15521633
struct sockaddr_in *addr_in = (struct sockaddr_in *) dstaddr;
15531634
struct ib_port_immutable port_immutable;
1554-
u32 ird_ord_hdr[2];
1635+
__be32 ird_ord_hdr[2];
15551636

15561637
info = kzalloc(sizeof(struct smbd_connection), GFP_KERNEL);
15571638
if (!info)
15581639
return NULL;
15591640
sc = &info->socket;
15601641
sp = &sc->parameters;
15611642

1643+
info->initiator_depth = 1;
1644+
info->responder_resources = SMBD_CM_RESPONDER_RESOURCES;
1645+
15621646
sc->status = SMBDIRECT_SOCKET_CONNECTING;
15631647
rc = smbd_ia_open(info, dstaddr, port);
15641648
if (rc) {
@@ -1639,22 +1723,22 @@ static struct smbd_connection *_smbd_get_connection(
16391723
}
16401724
sc->ib.qp = sc->rdma.cm_id->qp;
16411725

1642-
memset(&conn_param, 0, sizeof(conn_param));
1643-
conn_param.initiator_depth = 0;
1644-
1645-
conn_param.responder_resources =
1646-
min(sc->ib.dev->attrs.max_qp_rd_atom,
1647-
SMBD_CM_RESPONDER_RESOURCES);
1648-
info->responder_resources = conn_param.responder_resources;
1726+
info->responder_resources =
1727+
min_t(u8, info->responder_resources,
1728+
sc->ib.dev->attrs.max_qp_rd_atom);
16491729
log_rdma_mr(INFO, "responder_resources=%d\n",
16501730
info->responder_resources);
16511731

1732+
memset(&conn_param, 0, sizeof(conn_param));
1733+
conn_param.initiator_depth = info->initiator_depth;
1734+
conn_param.responder_resources = info->responder_resources;
1735+
16521736
/* Need to send IRD/ORD in private data for iWARP */
16531737
sc->ib.dev->ops.get_port_immutable(
16541738
sc->ib.dev, sc->rdma.cm_id->port_num, &port_immutable);
16551739
if (port_immutable.core_cap_flags & RDMA_CORE_PORT_IWARP) {
1656-
ird_ord_hdr[0] = info->responder_resources;
1657-
ird_ord_hdr[1] = 1;
1740+
ird_ord_hdr[0] = cpu_to_be32(conn_param.responder_resources);
1741+
ird_ord_hdr[1] = cpu_to_be32(conn_param.initiator_depth);
16581742
conn_param.private_data = ird_ord_hdr;
16591743
conn_param.private_data_len = sizeof(ird_ord_hdr);
16601744
} else {
@@ -2121,6 +2205,12 @@ static int allocate_mr_list(struct smbd_connection *info)
21212205
atomic_set(&info->mr_used_count, 0);
21222206
init_waitqueue_head(&info->wait_for_mr_cleanup);
21232207
INIT_WORK(&info->mr_recovery_work, smbd_mr_recovery_work);
2208+
2209+
if (info->responder_resources == 0) {
2210+
log_rdma_mr(ERR, "responder_resources negotiated as 0\n");
2211+
return -EINVAL;
2212+
}
2213+
21242214
/* Allocate more MRs (2x) than hardware responder_resources */
21252215
for (i = 0; i < info->responder_resources * 2; i++) {
21262216
smbdirect_mr = kzalloc(sizeof(*smbdirect_mr), GFP_KERNEL);

fs/smb/client/smbdirect.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ struct smbd_connection {
6767

6868
/* Memory registrations */
6969
/* Maximum number of RDMA read/write outstanding on this connection */
70-
int responder_resources;
70+
bool legacy_iwarp;
71+
u8 initiator_depth;
72+
u8 responder_resources;
7173
/* Maximum number of pages in a single RDMA write/read on this connection */
7274
int max_frmr_depth;
7375
/*

0 commit comments

Comments
 (0)