Skip to content

T5797: Adjust MSS clamping from forward to postrouting hook #4609

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

Merged
merged 1 commit into from
Jul 24, 2025

Conversation

giga1699
Copy link
Contributor

@giga1699 giga1699 commented Jul 17, 2025

Change summary

Adjust the VYOS_TCP_MSS chain to hook on postrouting instead of forward in order to fix MSS clamping not being adequately applied when MPLS/VRF is used.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T5797

Related PR(s)

How to test / Smoketest result

sudo nft list ruleset

You should see

table ip raw {
    chain VYOS_TCP_MSS {
        type filter hook postrouting priority raw; policy accept;
    }
}

You should no longer see

table ip raw {
    chain VYOS_TCP_MSS {
        type filter hook forward priority raw; policy accept;
    }
}

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Copy link

github-actions bot commented Jul 17, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link

github-actions bot commented Jul 17, 2025

👍
No issues in PR Title / Commit Title

@giga1699
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

lemeshovich added a commit to vyos/vyos-cla-signatures that referenced this pull request Jul 17, 2025
@giga1699
Copy link
Contributor Author

If possible, could this be backported to sagitta and/or circinus?

I've tested this on 1.4.2 and it's working okay on a MPLS PE router currently.

@giga1699 giga1699 force-pushed the T5797 branch 2 times, most recently from fb8f9c9 to 2fe7fa0 Compare July 17, 2025 03:36
Copy link

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@c-po c-po added bp/sagitta Create automatic backport for sagitta LTS version bp/circinus Create automatic backport for circinus labels Jul 17, 2025
Copy link
Member

@sever-sever sever-sever left a comment

Choose a reason for hiding this comment

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

Add the TCP-MSS option for forwarded and local generated traffic from the router itself.
I do not see any bad things with this implementation.
We'll probably separate this in the future if we face any harm.
Like:

set interfaces ethernet ethX tcp-mss hook <forward | postrouting>

Copy link
Member

@sarthurdev sarthurdev left a comment

Choose a reason for hiding this comment

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

See no issues, tested locally and works same as previously.

@dmbaturin dmbaturin merged commit e1cf4d1 into vyos:current Jul 24, 2025
17 checks passed
@vyosbot vyosbot added mirror-initiated This PR initiated for mirror sync workflow mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Jul 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bp/circinus Create automatic backport for circinus bp/sagitta Create automatic backport for sagitta LTS version current mirror-completed
Development

Successfully merging this pull request may close these issues.

6 participants