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.

Sign up for GitHub

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

Jump to bottom

ARP/ND Build Separation #1175

Merged
merged 41 commits into from
Nov 11, 2024
Merged

ARP/ND Build Separation #1175

merged 41 commits into from
Nov 11, 2024

Conversation

HTRamsey
Copy link
Contributor

@HTRamsey HTRamsey commented Aug 1, 2024 •
edited
Loading

Full separation of ND & ARP for better ipv4/ipv6 configuration
Separate config macros because ARP/ND have different defaults

HTRamsey and others added 14 commits February 8, 2024 19:30
@HTRamsey
Copy link
Contributor Author

HTRamsey commented Aug 3, 2024

/bot run formatting

@HTRamsey HTRamsey force-pushed the dev-ip-build-sep branch 6 times, most recently from 29a9002 to 08ed9d5 Compare August 25, 2024 23:54
@HTRamsey HTRamsey marked this pull request as ready for review August 26, 2024 00:17
@HTRamsey HTRamsey requested a review from a team as a code owner August 26, 2024 00:17
@moninom1
Copy link
Member

moninom1 commented Aug 27, 2024 •
edited
Loading

Hi @HTRamsey , Thank you for this PR, we will review it internally.
Can you also please let us know the plan to fix Unit tests and CBMC failures. Thanks

@HTRamsey
Copy link
Contributor Author

HTRamsey commented Aug 27, 2024 •
edited
Loading

Probably by the end of the week. It's mostly just name changes & moving stuff so it should be easy to fix

@HTRamsey
Copy link
Contributor Author

Whenever I give a time estimation I guess just double or triple it

@ActoryOu
Copy link
Member

Hi @HTRamsey,
Sorry for late response. I finially find some time fixing the UT/CBMC issues.
Please help take a look when you're available to check if any change is unexpected.

Thank you.

@HTRamsey
Copy link
Contributor Author

@ActoryOu looks good as far as I can tell

tony-josi-aws
tony-josi-aws reviewed Nov 7, 2024
View reviewed changes
source/FreeRTOS_ARP.c Outdated
const IPHeader_t * pxIPHeader = &( pxIPPacket->xIPHeader );
const IPV4Parameters_t * pxIPv4Settings = &( pxNetworkBuffer->pxEndPoint->ipv4_settings );

/* configASSERT( ( pxIPPacket->xEthernetHeader.usFrameType == ipIPv4_FRAME_TYPE ) || ( pxIPPacket->xEthernetHeader.usFrameType == ipARP_FRAME_TYPE ) ); */
Copy link
Member

Choose a reason for hiding this comment

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

If this is a temporary test code, please remove this line. Or the assert can be uncommented so that it could help with debugging.
I prefer to keep the assert.

tony-josi-aws
tony-josi-aws reviewed Nov 7, 2024
View reviewed changes
source/FreeRTOS_ND.c Outdated Show resolved Hide resolved
@HTRamsey
Copy link
Contributor Author

HTRamsey commented Nov 8, 2024

I don't know how to fix that last proof thing

@tony-josi-aws
Copy link
Member

I don't know how to fix that last proof thing

@HTRamsey Thanks for taking care of the other review comments. Will take a look at the failing CBMC proof.

@HTRamsey
Copy link
Contributor Author

@tony-josi-aws this is completely unrelated, but is there any way we could get the IPerf code in the tcp_utilities?

@tony-josi-aws
Copy link
Member

tony-josi-aws commented Nov 11, 2024 •
edited
Loading

@HTRamsey That seems like a good idea to me. Tagging @htibosch to know his views as he develop/maintains the version that works for FreeRTOS+TCP.

@tony-josi-aws tony-josi-aws merged commit 1599660 into FreeRTOS:main Nov 11, 2024
10 checks passed
@HTRamsey HTRamsey deleted the dev-ip-build-sep branch November 11, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tony-josi-aws tony-josi-aws tony-josi-aws approved these changes

@aggarg aggarg aggarg approved these changes

@ActoryOu ActoryOu ActoryOu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@HTRamsey @moninom1 @ActoryOu @tony-josi-aws @aggarg @actions-user