Skip to content

Conversation

@kar-rahul-aws
Copy link
Member

Added changes in WinPCap Port for IPV4 Endpoint changes

Network Interface changes for WinPCap Port

Description

Added Endpoint changes and Network Interface structure changes

Test Steps

Related Issue

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

Added changes in WinpCap Port for IPV4 Endpoint changes
@kar-rahul-aws kar-rahul-aws requested a review from a team as a code owner December 9, 2022 15:34
Copy link
Contributor

@htibosch htibosch left a comment

Choose a reason for hiding this comment

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

Thank you for this PR. I will check it later in a real project.
Please make the change suggested below.

break;
}

return xResult;
Copy link
Contributor

Choose a reason for hiding this comment

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

Very important: the return statement should be placed at the end of the function, not within the for-loop.
Can you please change that?

static BaseType_t xHasWarned = pdFALSE;
EthernetHeader_t * pxEtherHeader;
NetworkEndPoint_t * pxEndPoint;
BaseType_t xResult;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking more closely, I also see that xResult was not initialised, which is entirely my fault.
I suggest this change:

 static BaseType_t xPacketBouncedBack( const uint8_t * pucBuffer )
 {
-    BaseType_t xResult;
+    BaseType_t xResult = pdFALSE;

     for( pxEndPoint = FreeRTOS_FirstEndPoint( NULL );
          pxEndPoint != NULL;
          pxEndPoint = FreeRTOS_NextEndPoint( NULL, pxEndPoint ) )
     {
         if( memcmp( pxEndPoint->xMACAddress.ucBytes, pxEtherHeader->xSourceAddress.ucBytes, ipMAC_ADDRESS_LENGTH_BYTES ) == 0 )
         {
             xResult = pdTRUE;
             break;
         }
 
-        return xResult;
     }
+    return xResult;
 }

Copy link
Contributor

@htibosch htibosch left a comment

Choose a reason for hiding this comment

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

Hello @kar-rahul-aws , I compiled WinPCap/NetworkInterface.c in a demo project. It had a few warnings which can be solved as follows:

NetworkInterface.c :

-    BaseType_t xNetworkInterfaceInitialise( NetworkInterface_t * pxInterface )
+    static BaseType_t xNetworkInterfaceInitialise( NetworkInterface_t * pxInterface )

and please add a formal declaration as well at the top of the file.

-    BaseType_t xGetPhyLinkStatus( NetworkInterface_t * pxInterface )
+    static BaseType_t xGetPhyLinkStatus( NetworkInterface_t * pxInterface )

and please add a formal declaration as well at the top of the file.

NetworkInterface.h :

The following declarations can be removed:

-    /* INTERNAL API FUNCTIONS. */
-    BaseType_t xNetworkInterfaceInitialise( void );
-    BaseType_t xNetworkInterfaceOutput( NetworkBufferDescriptor_t * const pxNetworkBuffer,
-                                        BaseType_t xReleaseAfterSend );

xGetPhyLinkStatus() will also become a static function:

-    /* The following function is defined only when BufferAllocation_1.c is linked in the project. */
-    BaseType_t xGetPhyLinkStatus( void );

Thank you,
Hein

@htibosch
Copy link
Contributor

At this moment, xNetworkInterfaceOutput() is still called directly (in the "/single" mode).

Eventually this function will also be coupled to an interface:

pxInterface->pfInitialise = xNetworkInterfaceInitialise;
pxInterface->pfOutput = xNetworkInterfaceOutput;
pxInterface->pfGetPhyLinkStatus = xGetPhyLinkStatus;

and the code will call pxInterface->pfOutput() in stead.

That is also why the above functions must be declared static, so that multiple interfaces can be used without a conflict.

@kar-rahul-aws kar-rahul-aws changed the title Update Portable\WinPCap\NetworkInterface.c Update Portable Folder for End Point changes Dec 15, 2022
Copy link
Member

@moninom1 moninom1 left a comment

Choose a reason for hiding this comment

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

/bot run uncrustify

@tony-josi-aws
Copy link
Member

/bot run uncrustify

@moninom1 moninom1 merged commit 3064455 into FreeRTOS:dev/IPv6_integration Dec 16, 2022
@bjsowa bjsowa mentioned this pull request Jul 6, 2023
2 tasks
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.

5 participants