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

Update stm32 emac ethernet driver #12776

Merged

Conversation

JarkkoPaso
Copy link

Summary of changes

DISCO_F769NI EMAC driver may return ethernet packet with illegal length when driver is under heavy load. In one case, the received bytes indicate frame length of 53 bytes but advertised data length was 65518 bytes. In another case EMAC driver variable EthHandle.RxFrameInfos.length contained value 0xFFFF FFFC.

As a work-around accept only 1-1500 bytes long ethernet packets.

Impact of changes

STM EMAC driver will not return long data packets to the application. Packets longer than 1500 bytes are ignored.

Migration actions required

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


Arto Kinnunen added 2 commits April 8, 2020 14:11
DISCO_F769NI EMAC driver may return ethernet packet with illegal
length when driver is under heavy load. In one case, the received
bytes indicate frame length of 53 bytes but advertised data length
was 65518 bytes. In another case EMAC driver variable
`EthHandle.RxFrameInfos.length` contained value 0xFFFF FFFC.

As a work-around accept only 1-1500 bytes long ethernet packets.
-Fix len type
-Use ETH_RX_BUF_SIZE instead of hard-coded value 1500
@jeromecoutant
Copy link
Collaborator

jeromecoutant commented Apr 8, 2020

NB: from @artokin in #12459

we have tried three times but unfortunately we are not able to reproduce the original error.

So is it really needed....

@adbridge
Copy link
Contributor

adbridge commented Apr 8, 2020

NB: from @artokin in #12459

we have tried three times but unfortunately we are not able to reproduce the original error.

So is it really needed....

@JarkkoPaso could you please chat to @artokin about whether this is actually needed ?

@JarkkoPaso
Copy link
Author

@artokin is not available before Tuesday.
@teetak01 @kjbracey-arm @mikaleppanen do you know anything about this issue?

Copy link
Contributor

@kjbracey kjbracey left a comment

Choose a reason for hiding this comment

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

Change is already present on master, and I see no reason not to backport it. If you want to argue that it should be removed from master, that's a separate discussion.

The nonsense-length error certainly has been seen in the past, and if it happens it is system fatal. Seems the right thing to do to maintain a 1-comparison safety check there until the reason for the original error is understood.

The failure to reproduce during the #12459 discussion could be down to some alignment precondition like that recent sleep crash thing.

@mergify mergify bot added the needs: CI label Apr 8, 2020
@adbridge adbridge requested a review from andypowers April 8, 2020 13:21
Copy link
Collaborator

@andypowers andypowers left a comment

Choose a reason for hiding this comment

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

Approved.

@adbridge
Copy link
Contributor

adbridge commented Apr 8, 2020

CI started

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 8, 2020

CI aborted ,another job started (will clean the statuses after the current completes so do not clear any new one)

@mbed-ci
Copy link

mbed-ci commented Apr 8, 2020

Test run: FAILED

Summary: 3 of 3 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_unittests
  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_build-GCC_ARM

@mbed-ci
Copy link

mbed-ci commented Apr 8, 2020

Test run: SUCCESS

Summary: 10 of 10 test jobs passed
Build number : 1
Build artifacts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants