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

Misc interface dhcp #761

Merged
merged 7 commits into from
Oct 28, 2024
Merged

Misc interface dhcp #761

merged 7 commits into from
Oct 28, 2024

Conversation

troglobit
Copy link
Contributor

@troglobit troglobit commented Oct 24, 2024 •
edited
Loading

Description

Mixed bag of fixes and changes.

  • Clarify relationship between DHCP option 3 and 121 in test and doc (@jovatn + @axkar)
  • Add support for setting interface ifAlias using the YANG interface description (@ all)
  • Sync developer's guide and contributing guidelines (@jovatn)
  • Fix regression in RAUC compatible string (@wkz)

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Test changes
    • Checked in changed Readme.adoc (make test-spec)
    • Added new test to group Readme.adoc and yaml file
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
    • ChangeLog updated (for major changes)
  • Other (please describe):

axkar
axkar approved these changes Oct 24, 2024
View reviewed changes
Copy link
Collaborator

@axkar axkar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nothing to comment on the code. Looks great!

jovatn
jovatn approved these changes Oct 25, 2024
View reviewed changes
Copy link
Contributor

@jovatn jovatn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great!
I added some comments to the DHCP Routes test.
Feel free to ignore them, but check specifically one of the test steps if "has a" should be changed to "does not have a".

test/case/infix_dhcp/dhcp_routes/test.py Outdated Show resolved Hide resolved
test/case/infix_dhcp/dhcp_routes/test.py Outdated Show resolved Hide resolved
test/case/infix_dhcp/dhcp_routes/test.py Outdated Show resolved Hide resolved
@troglobit
Copy link
Contributor Author

Great! I added some comments to the DHCP Routes test. Feel free to ignore them, but check specifically one of the test steps if "has a" should be changed to "does not have a".

Thanks! I'd already started revising them after our chat yesterday, but I'll definitely have a look and see what I can incorporate. 😃

@troglobit troglobit requested review from wkz and jovatn and removed request for mattiaswal October 27, 2024 16:45
@troglobit
Copy link
Contributor Author

Great! I added some comments to the DHCP Routes test. Feel free to ignore them, but check specifically one of the test steps if "has a" should be changed to "does not have a".

Thanks! I'd already started revising them after our chat yesterday, but I'll definitely have a look and see what I can incorporate. 😃

There, maybe you can have a look again, @jovatn?

@troglobit
Copy link
Contributor Author

Ping @wkz, re: another compatible string fix, third time's the charm ... I hope.

@troglobit troglobit requested review from mattiaswal and removed request for wkz October 28, 2024 08:18
mattiaswal
mattiaswal requested changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@mattiaswal mattiaswal left a comment

Choose a reason for hiding this comment

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

Great cleanup, some comments on ifalias.

src/confd/src/ietf-interfaces.c Show resolved Hide resolved
jovatn
jovatn approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@jovatn jovatn left a comment

Choose a reason for hiding this comment

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

Looks great to me.
Perhaps the dhcp_routes/Readme.adoc file is not updated?
Anyhow, when I run "make test-spec" I get a pdf which looks correct for the DHCP Routes test.

@troglobit
Copy link
Contributor Author

Perhaps the dhcp_routes/Readme.adoc file is not updated?

Ah, great catch, thanks!

@troglobit
Copy link
Contributor Author

Great cleanup, some comments on ifalias.

Awesome, I'll have a look!

@troglobit
board/common: fix rauc compatible regression
dbf0fd2 
The new variable INFIX_COMPATIBLE, used for the RAUC compatible string,
is not properly evaluated on defconfigs where it is composed of another
variable, e.g.

    INFIX_COMPATIBLE="${INFIX_IMAGE_ID}"

Unfortunately, this is the default value for INFIX_COMPATIBLE, and so it
breaks all tier one defconfigs on the mainline branch since a1771ff.

Signed-off-by: Joachim Wiberg <[email protected]>
@troglobit
doc,test: clarify relationship between DHCP option 3 and 121
710616f 
Signed-off-by: Joachim Wiberg <[email protected]>
@troglobit
confd: allow interface description as ifAlias
0284e2c 
This patch adds support for setting the free form description string as
the interface alias.  This can be read from /sys/class/net/*/ifalias, or
using the 'ip link show' command.  The ifalias is reported by SNMP and
LLDP by default, and now also in the operational datastore.

Linux supports 250 characters in ifalias, but the IF-MIB is restricted
to 64 characters, so we add a local deviation to limit the type.

Signed-off-by: Joachim Wiberg <[email protected]>
@troglobit
test: new test, verify ifAlias
967bfc1 
Signed-off-by: Joachim Wiberg <[email protected]>
@troglobit
doc: add description of description setting and update ChangeLog
0112113 
Signed-off-by: Joachim Wiberg <[email protected]>
@troglobit
doc: sync dev-guide and contributing guidelines
56d1964 
Signed-off-by: Joachim Wiberg <[email protected]>
@troglobit
package/finit: backport fix to clear background color from GRUB menu
3e5e89b 
Signed-off-by: Joachim Wiberg <[email protected]>
mattiaswal
mattiaswal approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@mattiaswal mattiaswal left a comment

Choose a reason for hiding this comment

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

Great work! 💯

@troglobit troglobit merged commit 00cdff1 into main Oct 28, 2024
6 checks passed
@troglobit troglobit deleted the misc-interface-dhcp branch October 28, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axkar axkar axkar approved these changes

@jovatn jovatn jovatn approved these changes

@mattiaswal mattiaswal mattiaswal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@troglobit @mattiaswal @jovatn @axkar