Skip to content

Conversation

@bergzand
Copy link
Member

@bergzand bergzand commented Nov 15, 2021

Contribution description

This truncates the incomming frames to ETHERNET_FRAME_LEN and silently
discards the rest of the frame until the end of the frame. This should
be modified to an endpoint halt condition after #17090 is merged, but
for now this should be good enough.

Stalling the endpoint with the current stall implementation could cause
a ping of death scenario, so for now the data is truncated until the
above solution can be implemented.

(I've also removed an duplicate USBOPT_EP_AVAILABLE call)

Testing procedure

The tests/usbus_cdc_ecm application should still work on a platform of choice.

Issues/PRs references

Reported in detail by @szymonh

This truncates the incomming frames to ETHERNET_FRAME_LEN and silently
discards the rest of the frame until the end of the frame. This should
be modified to an endpoint halt condition after RIOT-OS#17090 is merged, but
for now this should be good enough.

Stalling the endpoint with the current stall implementation could cause
a ping of death scenario, so for now the data is truncated until the
above solution can be implemented.
@bergzand bergzand added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: USB Area: Universal Serial Bus labels Nov 15, 2021
@bergzand bergzand requested a review from dylad November 15, 2021 15:08
@bergzand bergzand requested a review from aabadie as a code owner November 15, 2021 15:08
@github-actions github-actions bot added the Area: sys Area: System label Nov 15, 2021
@dylad dylad added this to the Release 2022.01 milestone Nov 15, 2021
@dylad dylad self-assigned this Nov 15, 2021
@dylad dylad added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Nov 15, 2021
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Tested it and there is no regression so far with tests/usbus_cdc_ecm

@dylad dylad added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 3-testing The PR was tested according to the maintainer guidelines labels Nov 15, 2021
@fjmolinas fjmolinas merged commit 5f119e3 into RIOT-OS:master Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: sys Area: System Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants