Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ncm): Return invalid NTBs to free list #2950

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

tore-espressif
Copy link
Contributor

@tore-espressif tore-espressif commented Jan 15, 2025

In case we received invalid datagram, we silently fail and the buffer was not returned to empty list -> it was lost.

If this happened more than CFG_TUD_NCM_OUT_NTB_N times, we run out of NTBs and all OUT transfers are NACKed.

Closes espressif/esp-usb#107

In case we received invalid datagram, we silently fail
a the buffer was not returned to empty list -> it was lost.
If this happened more than CFG_TUD_NCM_OUT_NTB_N times, we run out of
NTBs and all OUT transfers are NACKed.

Closes espressif/esp-usb#107
@tore-espressif
Copy link
Contributor Author

@HiFiPhile @rgrr Could you please have a look?

@rgrr
Copy link
Contributor

rgrr commented Jan 16, 2025

@HiFiPhile @rgrr Could you please have a look?

Seems reasonable to me. My memory is getting darker there, but AFAIR the buffer had been fetched before via recv_get_free_ntb(). And either it should go then into the ready list or back into the free list.

What I find a little surprising is, that bad blocks are arriving at that point. Never had that.

@tore-espressif
Copy link
Contributor Author

Thanks for the confirmation!

What I find a little surprising is, that bad blocks are arriving at that point. Never had that.

I did not investigate why the datagrams are invalid; this is only a hot-fix. During upload throughput test (that is, NCM device is receiving with OUT transfers) I got several invalid datagrams per second...

@rgrr
Copy link
Contributor

rgrr commented Jan 17, 2025

:

I did not investigate why the datagrams are invalid; this is only a hot-fix. During upload throughput test (that is, NCM device is receiving with OUT transfers) I got several invalid datagrams per second...

Out of curiosity: what platform did you use for emitting the test datagrams?

@tore-espressif
Copy link
Contributor Author

tore-espressif commented Jan 20, 2025

As I said, I did not dig too deep; this was my setup

| Host PC (Win10) | <- USB -> | ESP32-S3| <- WiFi -> | Local network with internet access |

When I run speed test at speedtest.net I got this error often

@HiFiPhile
Copy link
Collaborator

Thanks for fixing it !

@HiFiPhile HiFiPhile merged commit feb41ee into hathach:master Jan 22, 2025
108 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USB-NCM driver 0.17 hangs after a period of sending packets (IEC-243)
3 participants