Skip to content

Conversation

@evpopov
Copy link
Contributor

@evpopov evpopov commented Jan 24, 2025

Description

The function xIPv6_GetIPType() misidentifies all unknown addresses that start with 0x0000 as eIPv6_Loopback.
The problem is that on the last cycle of the for(;;) loop, the code is allowed to do the normal mask-type check even if the loopback test fails.

Test Steps

IP_Address_t xTestAddress;
// Set the test address to the "::" unspecified IPv6 address
memset( xTestAddress.xIP_IPv6.ucBytes, 0x00, ipSIZE_OF_IPv6_ADDRESS );
IPv6_Type_t xAddressType = xIPv6_GetIPType( &( xTestAddress.xIP_IPv6 ) );
if( xAddressType == eIPv6_Unknown )
{
	FreeRTOS_printf("xTestAddress identified as eIPv6_Unknown" );
}
else
{
	FreeRTOS_printf("xTestAddress misidentified as type %u", xAddressType );
}

Note:
This fix could have been done in a few other way. One alternative way is to perform the mask test first and only if it succeeds, do the loopback test if needed.
Let me know it you want me to revise the PR or feel free to include a similar fix to a different PR and I can close this one.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@evpopov evpopov requested a review from a team as a code owner January 24, 2025 13:54
@evpopov evpopov changed the title Fixes IPv6 loopback type detection Fixes IPv6 loopback address type check Jan 24, 2025
Copy link
Member

@AniruddhaKanhere AniruddhaKanhere left a comment

Choose a reason for hiding this comment

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

These changes look good to me. Thank you @evpopov.

I will wait for @tony-josi-aws to comment as well to see what he thinks as he is well versed with the code.

Thanks

@tony-josi-aws
Copy link
Member

The change looks good to me
Thanks, @evpopov for this contribution.

@AniruddhaKanhere AniruddhaKanhere merged commit df25226 into FreeRTOS:main Jan 29, 2025
10 checks passed
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.

3 participants