Skip to content

Fix ip4 frag bug #64

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

kracsta11
Copy link

Summary

When the MTU is set to a value other than 1500, the IPv4 fragmentation function may not work correctly. As a result, pinging packets of certain lengths may fail.

Changes

Changed the fragmentation condition from using the interface MTU minus the IP header length to using the actual maximum fragment size as the basis for determining whether fragmentation is needed.

Rationale

Using MTU - IP header size as a fragmentation condition can lead to incorrect behavior, especially when MTU is changed dynamically or set to non-default values. This change ensures that the fragmentation process uses the correct limit, avoiding oversized fragments and improving reliability across various transport scenarios.
In the current fragmentation logic, the calculations for nfb and fragsize are as follows:
const u16_t nfb = (u16_t)((netif->mtu - IP_HLEN) / 8);
fragsize = LWIP_MIN(left, (u16_t)(nfb * 8));
Since nfb is a u16_t type variable, the fragsize does not yield the exact value of netif->mtu - IP_HLEN, but instead a slightly smaller value. This slight discrepancy affects the subsequent fragmentation calculations, particularly when the remaining data length is small but still requires further fragmentation, leading to incorrect behavior.
For example, when a 3524-byte packet is fragmented with a 1176-byte fragment size, the second fragment will leave 1180 bytes, which should be further fragmented into 1176 bytes and 4 bytes (at this point, fragsize is 1176, and netif->mtu - IP_HLEN is 1180). However, due to the incorrect fragmentation condition (left < netif->mtu - IP_HLEN), the second-to-last fragment incorrectly has the last flag set to 1, leading to a fragmentation error.
image
This fix updates the fragmentation condition to use the actual maximum fragment size rather than netif->mtu - IP_HLEN, ensuring that the fragmentation logic correctly handles cases where remaining data still requires further fragmentation.

Testing

  • Environment: Zynq7000 + lwIP 2.2.1 + RTOS
  • Method: Used ping command to test specific packet sizes (3524 bytes) under different MTU settings.
  • MTU settings tested:
    • MTU set to 1200, with original fragmentation logic: ping packets of 3524 bytes consistently dropped.
      image
    • MTU set to 1200, with modified fragmentation logic: ping packets of 3524 bytes no longer dropped, successfully pinged through.
      image
    • MTU restored to 1500, with modified fragmentation logic: No packet loss observed, normal ping behavior.

@yarrick
Copy link
Member

yarrick commented May 26, 2025

Can you write a unit test that fails before the bugfix and works after?

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.

2 participants