Skip to content

lwIP: Refresh pbuf code with cherry-picked upstream#106

Merged
WinterMute merged 1 commit intodevkitPro:masterfrom
endrift:pbuf
Jan 24, 2021
Merged

lwIP: Refresh pbuf code with cherry-picked upstream#106
WinterMute merged 1 commit intodevkitPro:masterfrom
endrift:pbuf

Conversation

@endrift
Copy link
Copy Markdown
Contributor

@endrift endrift commented Dec 8, 2020

This cherry-picks some upstream code that redoes how pbuf "pool" allocation works, which (in my testing) resolves the issue @bendmorris was having in #103 without having to invasively insert interrupt disabling/enabling code. I'd appreciate it if this could get more testing before merging, however, since my own testing was pretty limited.

@trevor403
Copy link
Copy Markdown

I'd be happy to do some testing in Dolphin and on Console. If I'm understanding correctly the issue occurs when several threads are trying to allocate from pbuf concurrently before initialization is complete?

@endrift
Copy link
Copy Markdown
Contributor Author

endrift commented Dec 8, 2020

No, not quite. Since the Wii is uniprocessor the only way a race can happen is via interrupts. The BBA generates interrupts when it gets traffic, so if it gets an interrupt while allocating a pbuf from the pool (which happens when it receives ethernet frames I believe), there is a possibility the pool would be in an inconsistent state. There were already interrupt protection boundaries in the code (as seen in the conversation in the last PR) but inlining them ended up causing some of the register uses to cross these boundaries, leading to further possibility of corruption. Though #104 improved the boundaries, it wasn't sufficient, and I found other ways that the pool could get left inconsistent. I did find places to move the boundaries to fix it but I deemed that too heavy-handed and decided to see how upstream handled it. This PR removes the complicated pool pbuf setup replacing it with the stock memp allocation path (as upstream did a few months after the last time our downstream updated), which is used seemingly without issue in many other places in the code.

@bendmorris
Copy link
Copy Markdown
Contributor

I could repro the dhcp issue 100% of the time using the sockettest example program, which would show "network configuration failed" instead of an ip. This was the case on GC as well as dolphin.

I'll try this out later today.

@endrift
Copy link
Copy Markdown
Contributor Author

endrift commented Dec 8, 2020

Ah, it sounds like you were doing the same test as me then, except I only tried on hardware.

@bendmorris
Copy link
Copy Markdown
Contributor

Confirmed, this resolves it for me on both.

@trevor403
Copy link
Copy Markdown

This also worked for me on both emulator and on hardware

@WinterMute WinterMute merged commit b2a0b7a into devkitPro:master Jan 24, 2021
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.

4 participants