Skip to content
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

Allow setting TCP MSS clamping value also for non GN #2087

Merged
merged 1 commit into from Oct 28, 2022

Conversation

yboaron
Copy link
Contributor

@yboaron yboaron commented Oct 27, 2022

We noticed in some cases MTU issues even when
GN was disabled.

This PR allows the user to force a particular MSS clamping value
also for non GN.

To force a particular MSS clamping value use the submariner.io/tcp-clamp-mss
node annotation on Gateway nodes.

e.g. kubectl annotate node <node_name> submariner.io/tcp-clamp-mss=<value>

Signed-off-by: Yossi Boaron yboaron@redhat.com

@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr2087/yboaron/clamp_non_gn
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@nyechiel nyechiel added this to In progress in Submariner Project 0.14 via automation Oct 27, 2022
@nyechiel nyechiel moved this from In progress to In Review in Submariner Project 0.14 Oct 27, 2022
@nyechiel nyechiel added backport This change requires a backport to eligible release branches ready-to-test When a PR is ready for full E2E testing labels Oct 27, 2022
@@ -62,7 +62,7 @@ var logger = log.Logger{Logger: logf.Log.WithName("MTU")}

func NewMTUHandler(localClusterCidr []string, isGlobalnet bool, tcpMssValue int) event.Handler {
forceMss := notNeeded
if isGlobalnet {
if isGlobalnet || tcpMssValue != 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Not in the scope of this PR, but we should probably add some validation on the MTU value that is configured by the user (i.e., to ensure that its in the valid reasonable range)

@nyechiel
Copy link
Member

This would need a backport to both release-0.14 and release-0.13.

We noticed in some cases MTU issues even when
GN was disabled.

This PR allows the user to force a
particular MSS clamping value also for non GN.

To force a particular MSS clamping value use the
submariner.io/tcp-clamp-mss node annotation on
Gateway nodes.

e.g. kubectl annotate node <node_name> submariner.io/tcp-clamp-mss=<value>

Signed-off-by: Yossi Boaron <yboaron@redhat.com>
@sridhargaddam sridhargaddam enabled auto-merge (squash) October 28, 2022 06:00
@sridhargaddam sridhargaddam merged commit 9e47bd0 into submariner-io:devel Oct 28, 2022
34 of 35 checks passed
Submariner Project 0.14 automation moved this from In Review to Done Oct 28, 2022
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr2087/yboaron/clamp_non_gn]

sridhargaddam pushed a commit to sridhargaddam/submariner that referenced this pull request Oct 28, 2022
)

We noticed in some cases MTU issues even when
GN was disabled.

This PR allows the user to force a
particular MSS clamping value also for non GN.

To force a particular MSS clamping value use the
submariner.io/tcp-clamp-mss node annotation on
Gateway nodes.

e.g. kubectl annotate node <node_name> submariner.io/tcp-clamp-mss=<value>

Signed-off-by: Yossi Boaron <yboaron@redhat.com>
(cherry picked from commit 9e47bd0)
@sridhargaddam
Copy link
Member

sridhargaddam commented Oct 28, 2022

Backport PRs:
Release-0.14 branch: #2091
Release-0.13 branch: #2092

sridhargaddam pushed a commit to sridhargaddam/submariner that referenced this pull request Oct 28, 2022
)

We noticed in some cases MTU issues even when
GN was disabled.

This PR allows the user to force a
particular MSS clamping value also for non GN.

To force a particular MSS clamping value use the
submariner.io/tcp-clamp-mss node annotation on
Gateway nodes.

e.g. kubectl annotate node <node_name> submariner.io/tcp-clamp-mss=<value>

Signed-off-by: Yossi Boaron <yboaron@redhat.com>
(cherry picked from commit 9e47bd0)
tpantelis pushed a commit that referenced this pull request Oct 28, 2022
We noticed in some cases MTU issues even when
GN was disabled.

This PR allows the user to force a
particular MSS clamping value also for non GN.

To force a particular MSS clamping value use the
submariner.io/tcp-clamp-mss node annotation on
Gateway nodes.

e.g. kubectl annotate node <node_name> submariner.io/tcp-clamp-mss=<value>

Signed-off-by: Yossi Boaron <yboaron@redhat.com>
(cherry picked from commit 9e47bd0)
tpantelis pushed a commit that referenced this pull request Oct 28, 2022
We noticed in some cases MTU issues even when
GN was disabled.

This PR allows the user to force a
particular MSS clamping value also for non GN.

To force a particular MSS clamping value use the
submariner.io/tcp-clamp-mss node annotation on
Gateway nodes.

e.g. kubectl annotate node <node_name> submariner.io/tcp-clamp-mss=<value>

Signed-off-by: Yossi Boaron <yboaron@redhat.com>
(cherry picked from commit 9e47bd0)
@sridhargaddam sridhargaddam added the release-note-needed Should be mentioned in the release notes label Nov 3, 2022
yboaron added a commit to yboaron/submariner that referenced this pull request Nov 3, 2022
yboaron added a commit to yboaron/submariner that referenced this pull request Nov 3, 2022
yboaron added a commit to yboaron/submariner that referenced this pull request Nov 6, 2022
Signed-off-by: Yossi Boaron <yboaron@redhat.com>
tpantelis pushed a commit that referenced this pull request Nov 7, 2022
Signed-off-by: Yossi Boaron <yboaron@redhat.com>
@dfarrell07 dfarrell07 removed the release-note-needed Should be mentioned in the release notes label May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport This change requires a backport to eligible release branches backport-handled ready-to-test When a PR is ready for full E2E testing release-note-handled
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants