-
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
Add DHCP Relay #38
Add DHCP Relay #38
Conversation
|
||
INTERFACES="" | ||
|
||
#-a provides option 82 circuit id information |
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.
The comment does not make sense. #Closed
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.
I think Manny is explaining that the -a option enables the relay agent to use DHCP's option82 function.
Manny, I think you need to actually specify OPTIONS="-a", right?
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.
Yes, the -a option enables the use of DHCP Option 82. I left Option 82 off by default but it might make more sense to set OPTIONS="-a"
ENV DEBIAN_FRONTEND=noninteractive | ||
|
||
RUN apt-get update && apt-get -y install \ | ||
isc-dhcp-relay |
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.
clean up apt in the same 'RUN' command to save disk space. #Closed
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.
Will do
@@ -35,6 +35,7 @@ RUN sed --in-place 's/httpredir.debian.org/debian-archive.trafficmanager.net/' / | |||
python-dev \ | |||
wget \ | |||
cmake \ | |||
isc-dhcp-server \ |
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.
- indentation not aligned.
- why install in ptf docker? #Closed
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.
DHCP server is used for DHCP testing. One option was to create a seperate Docker container for the server but after discussion with Pavel and Darren it seems ok to just install isc-dhcp-server in the ptf docker. The client (ptf code) and server run on the same docker container.
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.
I think it is better to remove this package from ptf docker since ptf docker is generic docker for testing purpose, what if some other one want to use ptf docker to test another dhcp server implementation?
It is better to move this into the ansible playbook of the test itself.
In reply to: 84995018 [](ancestors = 84995018)
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.
Okay, that's an easy change. I will install and start dhcp server from playbook instead.
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.
Leave a comment
|
||
INTERFACES="" | ||
|
||
#-a provides option 82 circuit id information |
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.
I think Manny is explaining that the -a option enables the relay agent to use DHCP's option82 function.
Manny, I think you need to actually specify OPTIONS="-a", right?
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.
added clean up in dockerfile, fixed indentation, and added option 82 "-a" flag as default for relay config
COPY isc-dhcp-relay /etc/default/isc-dhcp-relay | ||
|
||
ENTRYPOINT service isc-dhcp-relay start \ | ||
&& /bin/bash |
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.
do we need to start rsyslog in the docker? how do we provide logging functionaility?
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.
addressed in latest iteration, used rsyslog
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.
requested changes
## Clean up | ||
RUN apt-get update && apt-get -y install \ | ||
isc-dhcp-relay && \ | ||
apt-get clean -y; apt-get autoclean -y; apt-get autoremove -y |
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.
Mix tabs with spaces. Several times in this PR. #Closed
@@ -35,6 +35,7 @@ RUN sed --in-place 's/httpredir.debian.org/debian-archive.trafficmanager.net/' / | |||
python-dev \ | |||
wget \ | |||
cmake \ | |||
isc-dhcp-server \ |
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.
tab and space mixing #Closed
❓ |
b36a160
to
dc84c41
Compare
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
[rfc4363]: Add key check logic for SAI bridge port objects (#38)
* [device] Update arista driver submodule * Set Arista scd driver config Set ARISTA_SCD_DRIVER_CONFIG config to build and include scd driver to resulting Arista deb packages
* [device] Update arista driver submodule * Set Arista scd driver config Set ARISTA_SCD_DRIVER_CONFIG config to build and include scd driver to resulting Arista deb packages
improve app/sai/counter db dump performance get bgp neighbor received routes
* [_sonic_yang_ext.py]: Parse multilist in YANG Container. This is needed to support VRF feature in SONiC. Signed-off-by: Praveen Chaudhary [email protected] * [sonic-vlan.yang]: Change Vlan yang models to remove admin-status as mandatory attribute. Signed-off-by: Praveen Chaudhary [email protected] * [_sonic_yang_ext.py]: Minor fix in _sonic_yang_ext.py Signed-off-by: Praveen Chaudhary [email protected]
* src/sonic-telemetry fa8d498...3bd7ca3 (4): > Update gnmi deps (#40) > [testdata] Update SFP keys to align with new standard (#39) > Fixed the parameters for subscribe APIs (#38) > Azure ro mode (#34) * src/sonic-mgmt-common 444aa9a...cc01ce4 (4): > Make gnmi dep version the same as in telemetry repo (#17) > Cleanup translib and cvl go test cases (#13) > Package update and enhancements/fixes in YGOT, and Request Binder (#12) > Translib phase I changes (#11) Note: sonic-telemetry submodule update is dependent upon sonic-mgmt-common submodule update, thus updating both in this patch
1. add parsing for: dom_control_bytes_masks, dom_option_value_masks and dom_module_monitor 2. adjust indent
Signed-off-by: Shu0T1an ChenG <[email protected]>
df46ed418e661a9bccdb2639d8873def356f8ba0 (HEAD -> master, origin/master, origin/HEAD) Fix the LLDP_LOC_CHASSIS not getting populated if no remote neighbors are present (sonic-net#39) e487532e11cc0e97cfce573b6b997fdd0beeb660 [CI] Set up CI&PR with Azure Pipelines (sonic-net#38) 3c9f488490a1dbded20dbf2d8a88a5ab4dbda8df Replace swsssdk's SonicV2Connector with swsscommon's implementation (sonic-net#35) Signed-off-by: Abhishek Dosi <[email protected]>
df46ed418e661a9bccdb2639d8873def356f8ba0 (HEAD -> master, origin/master, origin/HEAD) Fix the LLDP_LOC_CHASSIS not getting populated if no remote neighbors are present (#39) e487532e11cc0e97cfce573b6b997fdd0beeb660 [CI] Set up CI&PR with Azure Pipelines (#38) 3c9f488490a1dbded20dbf2d8a88a5ab4dbda8df Replace swsssdk's SonicV2Connector with swsscommon's implementation (#35) Signed-off-by: Abhishek Dosi <[email protected]>
df46ed418e661a9bccdb2639d8873def356f8ba0 (HEAD -> master, origin/master, origin/HEAD) Fix the LLDP_LOC_CHASSIS not getting populated if no remote neighbors are present (sonic-net#39) e487532e11cc0e97cfce573b6b997fdd0beeb660 [CI] Set up CI&PR with Azure Pipelines (sonic-net#38) 3c9f488490a1dbded20dbf2d8a88a5ab4dbda8df Replace swsssdk's SonicV2Connector with swsscommon's implementation (sonic-net#35) Signed-off-by: Abhishek Dosi <[email protected]>
Signed-off-by: Mykola Faryma <[email protected]>
df46ed418e661a9bccdb2639d8873def356f8ba0 (HEAD -> master, origin/master, origin/HEAD) Fix the LLDP_LOC_CHASSIS not getting populated if no remote neighbors are present (sonic-net#39) e487532e11cc0e97cfce573b6b997fdd0beeb660 [CI] Set up CI&PR with Azure Pipelines (sonic-net#38) 3c9f488490a1dbded20dbf2d8a88a5ab4dbda8df Replace swsssdk's SonicV2Connector with swsscommon's implementation (sonic-net#35) Signed-off-by: Abhishek Dosi <[email protected]>
…-net#38) 1. wrap the apis that are supported by new platform api in a way that to call npapi first and to call sfputil if the former returns NotImplementError. 2. for the functions that only available in sfputil, a helper module is introduced to replace sfputil.
f00efef Longxiang Lyu Wed Mar 16 09:12:46 2022 +0800 Add a command line option to store logs into a separate file (#41) ff2e67d Longxiang Lyu Tue Mar 15 09:10:59 2022 +0800 Add default port cable type (#39) ebbb4d8 Jing Zhang Mon Mar 14 15:41:11 2022 -0700 Prevent switching MUX to "Unknown" (#36) c779b8f Longxiang Lyu Thu Mar 10 21:35:11 2022 +0800 [nonfunctional] Use LinkProberStateMachineBase (#38) b9fedd0 Longxiang Lyu Wed Mar 9 13:03:58 2022 +0800 [NONFUNCTIONAL] Add LinkProberStateMachineBase (#37) bedd42b Longxiang Lyu Wed Mar 9 10:03:00 2022 +0800 Add .clang-format file to format code (#28) 9fe4fc6 Guohan Lu Thu Mar 3 17:51:43 2022 -0800 [doc]: add lgtm badge in README.md c1249d9 Longxiang Lyu Wed Mar 2 18:05:18 2022 +0800 Enable lgtm (#33) b8514c6 Longxiang Lyu Wed Mar 2 13:34:39 2022 +0800 Collect port cable type to use corresponding state machine (#31) 9b59ef9 Longxiang Lyu Wed Mar 2 07:19:33 2022 +0800 Improve make clean (#32)
IP SLA HLD v1
* 3beaadc (HEAD, origin/master, origin/HEAD, master) Improve unit test for gnmi (#38) * a12fcd8 Merge pull request #13 from renukamanavalan/statistics * | 28b2f27 Enable unit test * | 65e3c8f Add log for authentication and add unit test. #12 * | 1b3b838 Merge pull request #8 from ganglyu/fix_pipeline
This reverts commit 617559c.
…tches (sonic-net#38) Signed-off-by: Qi Luo <[email protected]>
[code sync] Merge code from sonic-net/sonic-buildimage:202205 to 202205
-added Dhcp Relay Docker
-added isc-dhcp-server package to ptf docker to avoid manually installing server for testing