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

Add MTU to CNI result #1060

Merged
merged 1 commit into from
Mar 15, 2024
Merged

Add MTU to CNI result #1060

merged 1 commit into from
Mar 15, 2024

Conversation

SchSeba
Copy link
Contributor

@SchSeba SchSeba commented Feb 1, 2024

This commit allow CNIs to expose the MTU of interface

@SchSeba
Copy link
Contributor Author

SchSeba commented Feb 1, 2024

Hi @squeed as we spoke in the meeting this week here is the small PR let me know if there is something else needed.

@coveralls
Copy link

coveralls commented Feb 1, 2024

Coverage Status

coverage: 69.963%. remained the same
when pulling c45adb6 on SchSeba:add_mtu
into 563cf56 on containernetworking:main.

Copy link
Contributor

@bleggett bleggett left a comment

Choose a reason for hiding this comment

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

There was a good point brought up that we could put this in a metadata struct, but I think MTU is pretty unambiguously a core property of the interface so it makes sense here to me.

Another good point was brought up that if MTU is altered out-of-band, this value could be stale, but I think that's generally true of most things here.

@SchSeba SchSeba force-pushed the add_mtu branch 3 times, most recently from b819be1 to d9a31ca Compare March 5, 2024 20:30
@aojea
Copy link

aojea commented Mar 6, 2024

/lgtm

SPEC.md Outdated Show resolved Hide resolved
This commit allow CNIs to expose the MTU of interface

Signed-off-by: Sebastian Sch <[email protected]>
Copy link
Contributor

@s1061123 s1061123 left a comment

Choose a reason for hiding this comment

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

/lgtm

@squeed squeed merged commit 1c8da13 into containernetworking:main Mar 15, 2024
5 checks passed
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.

6 participants