-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[dhcp_relay] Enhance DHCP monitor application to support DHCPv6 #8060
[dhcp_relay] Enhance DHCP monitor application to support DHCPv6 #8060
Conversation
Signed-off-by: Shlomi Bitton <[email protected]>
src/dhcpmon/src/dhcp_device.c
Outdated
case DHCPv6_MESSAGE_TYPE_REPLY: | ||
case DHCPv6_MESSAGE_TYPE_RECONFIGURE: | ||
case DHCPv6_MESSAGE_TYPE_INFORMATION_REQUEST: | ||
case DHCPv6_MESSAGE_TYPE_RELAY_FORWARD: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we get rid of relay-forward and relay-reply message types and count inner message against DHCPv6 message type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tahmed-dev I changed the code to fit your suggestion, please review.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I noticed now that the parser will point to inner type of encapped packet. Can we remove the Relay Forward and Relay Reply from this case and let them fall through to the default section as this should not be expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. just a minor comment
src/dhcpmon/src/dhcp_device.c
Outdated
case DHCPv6_MESSAGE_TYPE_REPLY: | ||
case DHCPv6_MESSAGE_TYPE_RECONFIGURE: | ||
case DHCPv6_MESSAGE_TYPE_INFORMATION_REQUEST: | ||
case DHCPv6_MESSAGE_TYPE_RELAY_FORWARD: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I noticed now that the parser will point to inner type of encapped packet. Can we remove the Relay Forward and Relay Reply from this case and let them fall through to the default section as this should not be expected.
…kets will be counted. Handling of these packets type is not expected.
i saw tamer approved, but now it is not in the status. @tahmed-dev, can you approve? |
#### Why I did it Enhance DHCP monitor application following the implementation PR: #7772 #### How I did it Add the support for monitoring DHCPv6 packets. #### How to verify it Install an image with this PR and the implementation PR.
…c-net#8060) #### Why I did it Enhance DHCP monitor application following the implementation PR: sonic-net#7772 #### How I did it Add the support for monitoring DHCPv6 packets. #### How to verify it Install an image with this PR and the implementation PR.
…c-net#8060) #### Why I did it Enhance DHCP monitor application following the implementation PR: sonic-net#7772 #### How I did it Add the support for monitoring DHCPv6 packets. #### How to verify it Install an image with this PR and the implementation PR. (cherry picked from commit bef5477)
…v6 (sonic-net#8060)" This reverts commit bef5477.
Signed-off-by: Shlomi Bitton [email protected]
Why I did it
Enhance DHCP monitor application following the implementation PR: #7772
How I did it
Add the support for monitoring DHCPv6 packets.
How to verify it
Install an image with this PR and the implementation PR.
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)