Skip to content

Atmel SAM driver: let gmac_dev_read() return the proper length #950

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.

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

Closed

Conversation

htibosch
Copy link
Contributor

@htibosch htibosch commented Jun 30, 2023

This PR is now closed and replaced with PR #1000

Description

When receiving data, the driver for SAM will increase the number of bytes:

    /* Read +2 bytes because buffers are aligned at -2 bytes */
    bytesLeft = min( bytesLeft + 2, ( int32_t ) ul_frame_size );

This worked OK until PR #941, which is stricter on checking the received length:

-    if( uxBufferLength < ( size_t ) ( ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + ( size_t ) usPayloadLength ) )
+    if( uxBufferLength != ( size_t ) ( ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + ( size_t ) usPayloadLength ) )

Test Steps

Compile an application for SAM with the latest release ( after PR 941 ). Compare the behaviour before and after this PR.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

As I am not in my office, I am not able to test the change in real hardware.

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

@kstribrnAmzn
Copy link
Member

/bot run uncrustify

@@ -621,6 +621,9 @@ uint32_t gmac_dev_read( gmac_device_t * p_gmac_dev,
return GMAC_RX_NO_DATA;
}

/* Return the number of bytes received. */
*p_rcv_size = bytesLeft;
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to compare with frame size?
The *p_rcv_size might be different when ul_frame_size is smaller than bytesLeft + 2.

if( bytesLeft + 2 <= ul_frame_size )
{
    *p_rcv_size = bytesLeft;
}
else
{
    *p_rcv_size = ul_frame_size;
}

@@ -621,6 +621,9 @@ uint32_t gmac_dev_read( gmac_device_t * p_gmac_dev,
return GMAC_RX_NO_DATA;
}

/* Return the number of bytes received. */
*p_rcv_size = bytesLeft;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*p_rcv_size = bytesLeft;
*p_rcv_size = min( bytesLeft + 2, ( int32_t ) ul_frame_size );

Copy link
Member

Choose a reason for hiding this comment

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

@ActoryOu and @htibosch this should be the fix. Am I correct?

Copy link
Member

Choose a reason for hiding this comment

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

Hi @AniruddhaKanhere,
It can't be bytesLeft + 2 because the stack compares packet length with buffer length.
It's only allowed when buffer length is exactly equal to packet length, which is bytesLeft.
Take #941 as reference.

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