Skip to content

Add interface-id option - #8

Merged
yxieca merged 2 commits into
sonic-net:masterfrom
kellyyeh:interface-id
Sep 1, 2022
Merged

Add interface-id option#8
yxieca merged 2 commits into
sonic-net:masterfrom
kellyyeh:interface-id

Conversation

@kellyyeh

@kellyyeh kellyyeh commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

Why I did it

Support interface-id option with opaque value carrying link-address which will be used in dual-tor

How I did it

Add interface-id option in DHCP_RELAY configuration table, copy link-address to interface-id value

Comment thread src/relay.cpp
@yxieca

yxieca commented Aug 5, 2022

Copy link
Copy Markdown
Contributor

Is there unit test to cover this change?

@saiarcot895 saiarcot895 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved pending unit tests.

@saiarcot895

Copy link
Copy Markdown
Collaborator

@yxieca the unit test PR (#5) is still open, should we merge that in first, then this?

@kellyyeh

kellyyeh commented Aug 9, 2022

Copy link
Copy Markdown
Collaborator Author

@yxieca the unit test PR (#5) is still open, should we merge that in first, then this?

Working on cleaning up unittest PR. Will merge unittest first

@yxieca

yxieca commented Aug 10, 2022

Copy link
Copy Markdown
Contributor

@yxieca the unit test PR (#5) is still open, should we merge that in first, then this?

Working on cleaning up unittest PR. Will merge unittest first

Sure. when will #5 ready for review/merge?

@yxieca
yxieca merged commit e2cd585 into sonic-net:master Sep 1, 2022
@kellyyeh
kellyyeh deleted the interface-id branch September 1, 2022 00:30
kellyyeh added a commit to kellyyeh/sonic-dhcp-relay that referenced this pull request Oct 8, 2022
* Add interface-id option

* Add out of bound check
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.

3 participants