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

lib, bgpd, tests, zebra: prefix_sg changes for V6 VTEP #15900

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

lib, bgpd, tests, zebra: prefix_sg changes for V6 VTEP #15900

wants to merge 7 commits into from

Conversation

mikemallin
Copy link

@mikemallin mikemallin commented May 1, 2024 •
edited

This changeset introduces changes in lib for prefix_sg (and associated downstream consumers in bgpd/zebra)

  • src address in prefix_sg is now v6 aware (struct ipaddr)
  • %pSG4 now prints a v6 src address
  • Places where zebra uses prefix_sg src address are now V6 aware
  • ES_VTEP_LIST_STR_SZ can now support a v6 VTEP address
    • This has impacts in bgpd and zebra as uint8_t is not enough for printing (size_t changes)

This is a part of the upstreaming process for #5885.

@mikemallin mikemallin force-pushed the v6-vtep-lib-upstream branch 2 times, most recently from beba123 to 2ecc114 Compare May 1, 2024 19:36
ton31337
ton31337 reviewed May 2, 2024
View reviewed changes
lib/ipaddr.h Outdated Show resolved Hide resolved
@mikemallin mikemallin force-pushed the v6-vtep-lib-upstream branch 2 times, most recently from a512a2f to 39c8cfb Compare May 2, 2024 16:55
ton31337
ton31337 approved these changes May 3, 2024
View reviewed changes
Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

Other than whitespace changes, LGTM.

lib/ipaddr.h Outdated Show resolved Hide resolved
@mikemallin
Copy link
Author

ci:rerun

4 similar comments
@mikemallin
Copy link
Author

ci:rerun

@mikemallin
Copy link
Author

ci:rerun

@mikemallin
Copy link
Author

ci:rerun

@mikemallin
Copy link
Author

ci:rerun

donaldsharp
donaldsharp reviewed May 8, 2024
View reviewed changes
lib/ipaddr.h Outdated Show resolved Hide resolved
donaldsharp
donaldsharp reviewed May 8, 2024
View reviewed changes
Copy link
Member

@donaldsharp donaldsharp left a comment

Choose a reason for hiding this comment

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

imo -> This needs to be broken up into a bunch of commits. There are a bunch of unrelated changes thrown together that would be better served to be broken up.

@github-actions github-actions bot added the rebase PR needs rebase label May 8, 2024
@mikemallin
Copy link
Author

imo -> This needs to be broken up into a bunch of commits. There are a bunch of unrelated changes thrown together that would be better served to be broken up.

Would the following be an acceptable split?

  1. prefix_sg (structure, formatting, UT)
  2. VTEP string length changes

@mikemallin mikemallin closed this May 8, 2024
@github-actions github-actions bot added size/XS and removed size/L labels May 8, 2024
@mikemallin mikemallin reopened this May 8, 2024
@github-actions github-actions bot added size/L and removed size/XS labels May 8, 2024
ton31337
ton31337 requested changes May 13, 2024
View reviewed changes
Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

Yeah, this needs to be segregated into separate commits as Donald asked, because it's now mixed a bit.

lib/ipaddr.h Outdated Show resolved Hide resolved
lib/prefix.c Show resolved Hide resolved
lib/prefix.h Outdated Show resolved Hide resolved
lib/prefix.h Outdated Show resolved Hide resolved
lib/stream.h Show resolved Hide resolved
tests/lib/test_printfrr.c Show resolved Hide resolved
@frrbot frrbot bot added the tests Topotests, make check, etc label May 21, 2024
@mikemallin mikemallin changed the title lib, bgpd, zebra: prefix_sg changes for V6 VTEP lib, bgpd, tests, zebra: prefix_sg changes for V6 VTEP May 21, 2024
@mikemallin
Copy link
Author

ci:rerun

@mikemallin
Copy link
Author

ci:rerun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@donaldsharp donaldsharp donaldsharp left review comments

@ton31337 ton31337 ton31337 requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
bgp libfrr master rebase PR needs rebase size/L tests Topotests, make check, etc zebra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mikemallin @ton31337 @donaldsharp