Skip to content

Commit dd5492b

Browse files
jfischer-nokartben
authored andcommitted
usb: device_next: rework wSequence check in CDC NCM implementation
The consequences of a wSequence mismatch are unspecified and are primarily for debugging purposes. Our checks are a bit too strict, and if the header check fails, the sequence is not updated, and the header check subsequently fails. Rework the code to just log a warning on mismatch and also reset OUT sequence counter the same way as the IN sequence counter. Signed-off-by: Johann Fischer <[email protected]>
1 parent 82891d3 commit dd5492b

File tree

1 file changed

+12
-11
lines changed

1 file changed

+12
-11
lines changed

subsys/usb/device_next/class/usbd_cdc_ncm.c

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,8 @@ static int cdc_ncm_out_start(struct usbd_class_data *const c_data)
340340
return ret;
341341
}
342342

343-
static int verify_nth16(const union recv_ntb *ntb, uint16_t len, uint16_t seq)
343+
static int verify_nth16(struct cdc_ncm_eth_data *const data,
344+
const union recv_ntb *const ntb, const uint16_t len)
344345
{
345346
const struct nth16 *nthdr16 = &ntb->nth;
346347
const struct ndp16 *ndphdr16;
@@ -362,6 +363,14 @@ static int verify_nth16(const union recv_ntb *ntb, uint16_t len, uint16_t seq)
362363
return -EINVAL;
363364
}
364365

366+
if (sys_le16_to_cpu(nthdr16->wSequence) != data->rx_seq) {
367+
LOG_WRN("OUT NTH wSequence %u mismatch expected %u",
368+
sys_le16_to_cpu(nthdr16->wSequence), data->rx_seq);
369+
data->rx_seq = sys_le16_to_cpu(nthdr16->wSequence);
370+
}
371+
372+
data->rx_seq++;
373+
365374
if (len < (sizeof(struct nth16) + sizeof(struct ndp16) +
366375
2U * sizeof(struct ndp16_datagram))) {
367376
LOG_DBG("DROP: %s len %d", "min", len);
@@ -388,13 +397,6 @@ static int verify_nth16(const union recv_ntb *ntb, uint16_t len, uint16_t seq)
388397
return -EINVAL;
389398
}
390399

391-
if (sys_le16_to_cpu(nthdr16->wSequence) != 0 &&
392-
sys_le16_to_cpu(nthdr16->wSequence) != (seq + 1)) {
393-
LOG_DBG("DROP: seq %d %d",
394-
seq, sys_le16_to_cpu(nthdr16->wSequence));
395-
return -EINVAL;
396-
}
397-
398400
ndphdr16 = (const struct ndp16 *)(ntb->data +
399401
sys_le16_to_cpu(nthdr16->wNdpIndex));
400402

@@ -433,7 +435,7 @@ static int check_frame(struct cdc_ncm_eth_data *data, struct net_buf *const buf)
433435
int ret;
434436

435437
/* TODO: support nth32 */
436-
ret = verify_nth16(ntb, len, data->rx_seq);
438+
ret = verify_nth16(data, ntb, len);
437439
if (ret < 0) {
438440
LOG_ERR("Failed to verify NTH16");
439441
return ret;
@@ -488,8 +490,6 @@ static int check_frame(struct cdc_ncm_eth_data *data, struct net_buf *const buf)
488490
ndx++;
489491
}
490492

491-
data->rx_seq = sys_le16_to_cpu(nthdr16->wSequence);
492-
493493
if (DUMP_PKT) {
494494
LOG_HEXDUMP_DBG(ntb->data, len, "NTB");
495495
}
@@ -837,6 +837,7 @@ static void usbd_cdc_ncm_update(struct usbd_class_data *const c_data,
837837
if (data_iface == iface && alternate == 0) {
838838
atomic_clear_bit(&data->state, CDC_NCM_DATA_IFACE_ENABLED);
839839
data->tx_seq = 0;
840+
data->rx_seq = 0;
840841
}
841842

842843
if (data_iface == iface && alternate == 1) {

0 commit comments

Comments
 (0)