T8242: interfaces add bidirectional support for interface adjust-mss

[jd82k]

Change summary

Add a new optional CLI leaf adjust-mss-direction under both
interfaces ... ip and interfaces ... ipv6.

Supported values:

• outbound (default, existing behavior)
• inbound
• both

Behavior:

• outbound: always installs MSS rewrite rules in postrouting using oifname
• inbound:
  • clamp-mss-to-pmtu: installs in postrouting using iifname
  • numeric MSS: installs in prerouting using iifname (so input traffic is covered)
• both: installs outbound + inbound rules according to the logic above

Implementation details:

• Extend MSS rule generation in Interface to be direction- and mode-aware
  (match key + nft chain selection).
• Add prerouting MSS chains (VYOS_TCP_MSS_PREROUTING) for IPv4/IPv6 raw tables.
• Keep backward compatibility by treating missing direction as outbound.
• Improve MSS cleanup to remove stale interface rules from both postrouting and
  prerouting chains, for both oifname and iifname matches.
• Wire direction config into the IPv4 and IPv6 interface update paths.
• Extend interface smoketests to validate:
  • direction behavior (outbound / inbound / both)
  • correct chain placement (postrouting vs prerouting)
  • both IPv4 and IPv6 MSS rule generation.

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/T8242

Related PR(s)

How to test / Smoketest result

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

[github-actions]

👍
No issues in PR Title / Commit Title

[jd82k]

Please help me change to PR title. I don't know why it doesn't meet the requirements.

[l0crian1]

Ingress would need to be applied to a new chain with a prerouting hook.

Default values are canonically not done in code, but rather using the <defaultValue> element in the XML, but a maintainer can comment more if they see that as an issue in this PR.

[jd82k]

Ingress would need to be applied to a new chain with a prerouting hook.

Default values are canonically not done in code, but rather using the <defaultValue> element in the XML, but a maintainer can comment more if they see that as an issue in this PR.

Thanks for the review. I have addressed both points in the latest update:

1. Ingress MSS is now applied in a dedicated chain with a prerouting hook (VYOS_TCP_MSS_PREROUTING), instead of using the existing postrouting-only path.

   • Outbound still uses route-based clamp (tcp option maxseg size set rt mtu) in postrouting.
   • Inbound now supports clamp-mss-to-pmtu by dynamically computing an interface-based MSS at commit time (IPv4: MTU - 40, IPv6: MTU - 60) and programming a fixed MSS rule in prerouting.

2. I removed the hardcoded default from Python.

   • The default direction is now defined canonically in XML via <defaultValue>outbound</defaultValue> under adjust-mss-direction.

[l0crian1]

Thanks for the review. I have addressed both points in the latest update:

1. Ingress MSS is now applied in a dedicated chain with a prerouting hook (VYOS_TCP_MSS_PREROUTING), instead of using the existing postrouting-only path.

   • Outbound still uses route-based clamp (tcp option maxseg size set rt mtu) in postrouting.
   • Inbound now supports clamp-mss-to-pmtu by dynamically computing an interface-based MSS at commit time (IPv4: MTU - 40, IPv6: MTU - 60) and programming a fixed MSS rule in prerouting.

2. I removed the hardcoded default from Python.

   • The default direction is now defined canonically in XML via <defaultValue>outbound</defaultValue> under adjust-mss-direction.

I want to note that I'm not a maintainer, so these are just my opinions and may not necessarily mate up with the actual VyOS maintainers.

• I don't think "dynamically computing an interface-based MSS" really helps that much. MSS often needs to be clamped due to additional encapsulation (VxLAN, GRE, IPIP, IPsec, etc...), so taking a blind guess at what the MSS should be clamped to doesn't really help. I think it's better to raise a config error when PMTUD is configured and the direction is either ingress or both.
• I think the implementation seems overly busy for what is actually being done. For instance, you can just extend the existing functions to use something like this and it would achieve the same thing:

direction_map = {
    'inbound': [["VYOS_TCP_MSS_PREROUTING", "iifname"]],
    'outbound': [["VYOS_TCP_MSS", "oifname"]],
    'both': [["VYOS_TCP_MSS_PREROUTING", "iifname"], ["VYOS_TCP_MSS", "oifname"]]
}

for dir_list in direction_map[direction]:
    chain, ifdirection = dir_list

[jd82k]

Thanks for the review. I have addressed both points in the latest update:

1. Ingress MSS is now applied in a dedicated chain with a prerouting hook (VYOS_TCP_MSS_PREROUTING), instead of using the existing postrouting-only path.

   • Outbound still uses route-based clamp (tcp option maxseg size set rt mtu) in postrouting.
   • Inbound now supports clamp-mss-to-pmtu by dynamically computing an interface-based MSS at commit time (IPv4: MTU - 40, IPv6: MTU - 60) and programming a fixed MSS rule in prerouting.

2. I removed the hardcoded default from Python.

   • The default direction is now defined canonically in XML via <defaultValue>outbound</defaultValue> under adjust-mss-direction.

I want to note that I'm not a maintainer, so these are just my opinions and may not necessarily mate up with the actual VyOS maintainers.

• I don't think "dynamically computing an interface-based MSS" really helps that much. MSS often needs to be clamped due to additional encapsulation (VxLAN, GRE, IPIP, IPsec, etc...), so taking a blind guess at what the MSS should be clamped to doesn't really help. I think it's better to raise a config error when PMTUD is configured and the direction is either ingress or both.
• I think the implementation seems overly busy for what is actually being done. For instance, you can just extend the existing functions to use something like this and it would achieve the same thing:

direction_map = {
    'inbound': [["VYOS_TCP_MSS_PREROUTING", "iifname"]],
    'outbound': [["VYOS_TCP_MSS", "oifname"]],
    'both': [["VYOS_TCP_MSS_PREROUTING", "iifname"], ["VYOS_TCP_MSS", "oifname"]]
}

for dir_list in direction_map[direction]:
    chain, ifdirection = dir_list

Thanks for the feedback. I agree that MSS tuning can be affected by multiple encapsulation layers (VXLAN/GRE/IPIP/IPsec), and a static guess is not universally correct.

That said, for interface-scoped MSS adjustment, there is still practical value in deriving ingress MSS from the interface MTU when clamp-mss-to-pmtu is requested:

A common real-world case is tunnel interfaces (for example WireGuard), where operators explicitly set interface MTU (e.g. 1420).
In that case, deriving MSS from that MTU provides predictable behavior and avoids forcing users to manually duplicate the same arithmetic in config.
Conceptually, each interface with MTU < 1500 should own its MSS behavior, especially when the feature is configured under that interface node.
So the intent is not to “blindly guess” globally, but to honor the operator’s interface MTU choice as the closest available source of truth for ingress-side clamping semantics.

If maintainers still prefer strict behavior, I can switch to validation that rejects clamp-mss-to-pmtu for inbound/both and requires an explicit numeric MSS instead.

[l0crian1]

That said, for interface-scoped MSS adjustment, there is still practical value in deriving ingress MSS from the interface MTU when clamp-mss-to-pmtu is requested:

PMTUD has a well-defined, RFC based behavior, so I think trying to create a non-standards based behavior when configuring PMTUD is ultimately wrong. That's not to say you couldn't do something like clamp-mss-to-imtu for "Clamp MSS to interface MTU", but as implemented in this PR, deviating from the established behavior could get confusing.

I think clamp-mss-to-pmtu should only be contextually valid when doing PTMUD outbound. That's just my 2¢!

[jd82k]

I reverted the ingress implementation back to the existing postrouting MSS chain.

Reasoning:

• clamp-mss-to-pmtu is route-PMTU semantics (rt mtu), which is fundamentally route-context based.
• Implementing ingress via a dedicated prerouting path required non-standard behavior (deriving a fixed MSS from interface MTU), which diverges from PMTUD semantics and can be confusing.
• Keeping everything in postrouting preserves the established VyOS behavior and keeps clamp-mss-to-pmtu standards-aligned.

What remains:

• adjust-mss-direction (outbound|inbound|both) is still supported.
• Default direction is still defined in XML (<defaultValue>outbound</defaultValue>), not hardcoded in Python.

So this change prioritizes semantic correctness, predictability, and lower maintenance risk.

[l0crian1]

I reverted the ingress implementation back to the existing postrouting MSS chain.

iifname is not valid within the postrouting hook for local traffic. This is why I mentioned the prerouting hook. Currently, clamping MSS works for both local as well as forwarded traffic, so this would have different behavior between the existing feature and the new feature, depending on ingress (forward only) and egress (forward and local).

[jd82k]

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

[jd82k]

I reverted the ingress implementation back to the existing postrouting MSS chain.

iifname is not valid within the postrouting hook for local traffic. This is why I mentioned the prerouting hook. Currently, clamping MSS works for both local as well as forwarded traffic, so this would have different behavior between the existing feature and the new feature, depending on ingress (forward only) and egress (forward and local).

Thanks for the clarification. You are correct that iifname in postrouting does not apply to locally originated traffic.

My intent here is:

• outbound: keep existing behavior (applies to both forwarded and local egress traffic).
• inbound: target transit/forwarded traffic only.

For traffic terminated on the router (input), MSS clamping is usually not needed. In TCP, each endpoint advertises MSS during SYN/SYN-ACK based on its local egress path. The sender then uses:

effective_send_mss = min(local_send_mss, peer_advertised_mss)

So for local services, the router TCP stack already advertises an MSS consistent with its own MTU/path, and the peer will limit segment size accordingly.

So the behavior difference is intentional: ingress (iifname) is forward-only, while egress keeps the existing forward+local behavior.

[l0crian1]

So for local services, the router TCP stack already advertises an MSS consistent with its own MTU/path, and the peer will limit segment size accordingly.

That isn't quite accurate. It will advertise an MSS consistent with it's MTU, but the path relies on PMTUD working end-to-end, where it's not uncommon for unreachables being blocked somewhere along the path (especially with many security requirements/guides recommending them to be disabled).

One major issue with ignoring local traffic is if someone wanted a single touch point to clamp BGP sessions. They would be left troubleshooting why it's not working.

I don't personally see any sane reason to not keep parity with the existing behavior (outbound), when adding the new behavior (inbound). Do you have a specific reason you don't want to keep parity in the behaviors?

[jd82k]

So for local services, the router TCP stack already advertises an MSS consistent with its own MTU/path, and the peer will limit segment size accordingly.

That isn't quite accurate. It will advertise an MSS consistent with it's MTU, but the path relies on PMTUD working end-to-end, where it's not uncommon for unreachables being blocked somewhere along the path (especially with many security requirements/guides recommending them to be disabled).

One major issue with ignoring local traffic is if someone wanted a single touch point to clamp BGP sessions. They would be left troubleshooting why it's not working.

I don't personally see any sane reason to not keep parity with the existing behavior (outbound), when adding the new behavior (inbound). Do you have a specific reason you don't want to keep parity in the behaviors?

Thanks, this is a fair point, especially for local services (for example BGP sessions) where operators expect a single control point.

I’ve updated the implementation accordingly:

• For numeric MSS values, inbound rules are now installed in a prerouting MSS chain (iifname), so inbound handling also covers local input traffic.
• For clamp-mss-to-pmtu, inbound remains in the existing postrouting MSS chain, aligned with the current clamp behavior and kernel-supported rt mtu usage.

So the behavior is now:

• outbound: postrouting (oifname)
• inbound:
  • numeric MSS -> prerouting (iifname)
  • clamp-mss-to-pmtu -> postrouting (iifname)
• both: combines outbound plus inbound behavior above.

This keeps backward compatibility for clamp while making fixed-MSS inbound behavior useful for local services.

[github-actions]

CI integration ❌ failed!

Details

CI logs

• CLI Smoketests ❌ failed
• CLI Smoketests (interfaces only) 👍 passed
• Config tests 👍 passed
• RAID1 tests 👍 passed
• CLI Smoketests VPP 👍 passed
• Config tests VPP 👍 passed
• TPM tests 👍 passed

[jd82k]

Please review this PR. @c-po @dmbaturin @sarthurdev @sever-sever

[jd82k]

Please assign this PR to me. Thanks!

[sarthurdev]

Much improved approach, code looks good, adds smoketest coverage and passes.