sam0_eth: wait until the current tx buffer is no longer in use#20534
Closed
benpicco wants to merge 1 commit intoRIOT-OS:masterfrom
Closed
sam0_eth: wait until the current tx buffer is no longer in use#20534benpicco wants to merge 1 commit intoRIOT-OS:masterfrom
benpicco wants to merge 1 commit intoRIOT-OS:masterfrom
Conversation
benpicco
added
Type: bug
github-actions
bot
added
Platform: ARM
dylad
reviewed
Apr 3, 2024
| /* Initialize TX buffer descriptors */ | ||
| for (i=0; i < ETH_TX_BUFFER_COUNT; i++) { | ||
| tx_desc[i].address = (uint32_t) tx_buf[i]; | ||
| tx_desc[i].status = DESC_TX_STATUS_USED; |
Member
There was a problem hiding this comment.
Why setting TX buffers to one here ? This is suppose to be done by the IP. Software should only clears this bit.
Contributor
Author
There was a problem hiding this comment.
The bit gets set by the hardware when it's done with the buffer.
So this will mark all buffers as unused to the driver.
Member
There was a problem hiding this comment.
But GMAC IP will not find an available buffer when it want to send data if they are all marked as used already.
GMAC IP is supposed to take an unused buffer (mark == 0) then take it (mark it to one to tell software this buffer is being used)
This is not software job to set it.
Contributor
Author
There was a problem hiding this comment.
The bit is set to 0 before starting the transmission.
Member
There was a problem hiding this comment.
Alright, I missed the inversion in the if statement for the thread_yield()
That's a bit weird but it makes sense.
So do we want really want to block this or drop the new frames (like this is the case for RX) ?
Member
|
@benpicco could you describe a bit more your setup ? |
Contributor
Author
|
Huh, my setup is just like that. When I send a 16384 byte UDP packet on
|
Member
|
On my side with same54_xpro (lastest master) with With your linux app: Am I missing something ? |
Member
|
Same thing if I do not use fe80:: addr |
Contributor
Author
|
Huh that's odd - what does this print for you? void cpu_print_info(void)
{
const char *cpu[] = {
"?",
"?",
"?",
"?",
"?",
"?",
"M4F",
"?",
};
const char *series[] = {
"?",
"E51",
"E52",
"E53",
"E54",
"?",
"D51",
};
printf("CPU: Cortex-%s\n", cpu[DSU->DID.bit.PROCESSOR]);
printf("Series: SAM %s\n", series[DSU->DID.bit.SERIES]);
printf("Die: %x\n", DSU->DID.bit.DIE);
printf("Revision: %c\n", 'A' + DSU->DID.bit.REVISION);
} |
Member
|
On my |
Contributor
Author
|
I'm on Revision D 🤯 |
Member
|
Usually, hardware works better on later revision :D |
Contributor
Author
|
Superseded by #20666 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
When many (>4) packets are sent in rapid succession, the 5th packet will overwrite the first packet in the transmit buffer before it can be sent out.
This happens e.g. when sending large (> 4500 byte) fragmented packets.
As a solution, always set the USED bit:
This however is an incomplete fix: We clear the bit on each frame to be sent, if we sent the fifth frame before any of the frames has been sent, we still overwrite the first frame.
Strangely this instead disrupts transmission of non-fragmented packets afterwards, they are stuck in the tx queue.
Testing procedure
UDP Command
Compile Flags
Receiver App (Linux)
normal operation before fragmented packet
fragmented packet is transmitted
sending is disrupted afterwards
This is still an improvement over
masterwhere the fragmented packet is never received on the destination.Issues/PRs references