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.

Sign up for GitHub

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

Jump to bottom

network: route - add support to configure tcp advmss #18131

Merged
merged 1 commit into from
Jan 8, 2021
Merged

network: route - add support to configure tcp advmss #18131

merged 1 commit into from
Jan 8, 2021

Conversation

ssahani
Copy link
Contributor

@ssahani ssahani commented Jan 4, 2021

closes #18055

bluca
bluca approved these changes Jan 5, 2021
View reviewed changes
Copy link
Member

@bluca bluca left a comment

Choose a reason for hiding this comment

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

LGTM. Let's wait for @yuwata as well.

poettering
poettering requested changes Jan 6, 2021
View reviewed changes
man/systemd.network.xml Show resolved Hide resolved
src/network/networkd-route.c Outdated Show resolved Hide resolved
@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed needs-review labels Jan 6, 2021
@ssahani
Copy link
Contributor Author

ssahani commented Jan 7, 2021

Updated thanks for the review!

@bluca
Copy link
Member

bluca commented Jan 7, 2021

Updated thanks for the review!

I don't see the changes to the manpage?

@ssahani
Copy link
Contributor Author

ssahani commented Jan 7, 2021

Updated thanks for the review!

I don't see the changes to the manpage?

I just added (in bytes) and k m g etc. 63a75cf#diff-d81a981ca75657b8fc8f6ad6871cb9f8d7705944919b67dad745680ebeec1b4eR1474

Not sure why it's not showing. on the main page

@bluca
Copy link
Member

bluca commented Jan 7, 2021

Updated thanks for the review!

I don't see the changes to the manpage?

I just added (in bytes) and k m g etc. 63a75cf#diff-d81a981ca75657b8fc8f6ad6871cb9f8d7705944919b67dad745680ebeec1b4eR1474

Not sure why it's not showing. on the main page

It's there now, probably some gui issues.

The manpage still says MTU though, it should be changed to MSS as Lennart mentioned.

@ssahani
Copy link
Contributor Author

ssahani commented Jan 7, 2021

done thanks !

@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed ci-failure-appears-unrelated reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jan 7, 2021
poettering
poettering requested changes Jan 7, 2021
View reviewed changes
src/network/networkd-route.c Show resolved Hide resolved
src/network/networkd-route.c Outdated Show resolved Hide resolved
@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Jan 7, 2021
@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jan 8, 2021
@bluca
Copy link
Member

bluca commented Jan 8, 2021

Comments addressed, so flipping the labels again. Waiting for the reviewer to have a chance to look again.

@poettering
Copy link
Member

lgtm

@bluca bluca added ci-failure-appears-unrelated and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Jan 8, 2021
@bluca bluca merged commit 007cac0 into systemd:master Jan 8, 2021
@yuwata yuwata mentioned this pull request Jan 8, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bluca bluca bluca approved these changes

@poettering poettering poettering requested changes

Assignees
No one assigned
Labels
ci-failure-appears-unrelated network
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

networkd support for advmss setting on routes
3 participants
@ssahani @bluca @poettering