Skip to content

Skip TCP MTU checks when TCP is disabled - #1358

Open
HTRamsey wants to merge 1 commit into
FreeRTOS:mainfrom
HTRamsey:fix/skip-tcp-mtu-checks-when-disabled
Open

Skip TCP MTU checks when TCP is disabled#1358
HTRamsey wants to merge 1 commit into
FreeRTOS:mainfrom
HTRamsey:fix/skip-tcp-mtu-checks-when-disabled

Conversation

@HTRamsey

Copy link
Copy Markdown
Contributor

Description

When TCP is disabled, vPreCheckConfigs() still asserted that the configured MTU could hold a TCP header and ipconfigTCP_MSS. FreeRTOSIPConfigDefaults.h also enforced the TCP MSS minimum and overflow checks
unconditionally. These TCP-only constraints could reject otherwise valid UDP-only configurations.

This change:

  • Moves the MTU/MSS assertion into the existing ipconfigUSE_TCP guard.
  • Applies the MSS minimum and overflow validation only when TCP is enabled.
  • Changes the DISABLE_ALL build configuration to use the minimum valid 46-byte MTU, providing compile coverage for TCP-disabled, small-MTU configurations.

TCP-enabled configurations retain the existing MSS and MTU protections.

Test Steps

  • Built the complete freertos_plus_tcp core library with FREERTOS_PLUS_TCP_TEST_CONFIGURATION=DISABLE_ALL and ipconfigNETWORK_MTU=46.
  • Built the complete freertos_plus_tcp core library with FREERTOS_PLUS_TCP_TEST_CONFIGURATION=ENABLE_ALL_IPV4_TCP.
  • Confirmed that a TCP-enabled syntax check with ipconfigTCP_MSS=535 still fails with the expected minimum-MSS configuration error.
  • Ran git diff --check.

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.

The DISABLE_ALL build-combination configuration was updated to cover the TCP-disabled, minimum-MTU case.

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.

@htibosch htibosch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you @HTRamsey for this PR. I approve it.

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