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

[FRR] Enable SNMP support #2981

Merged
merged 2 commits into from
Jun 19, 2019
Merged

[FRR] Enable SNMP support #2981

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aed8289
[FRR] Enable SNMP support
MichelMoriniaux Jun 7, 2019
c1cf22c
[FRR] Enable SNMP support
MichelMoriniaux Jun 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dockers/docker-fpm-frr/Dockerfile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,6 @@ RUN rm -rf /debs ~/.cache
COPY ["bgpcfgd", "start.sh", "/usr/bin/"]
COPY ["*.j2", "/usr/share/sonic/templates/"]
COPY ["supervisord.conf", "/etc/supervisor/conf.d/"]
COPY ["snmp.conf", "/etc/snmp/frr.conf"]

ENTRYPOINT ["/usr/bin/supervisord"]
1 change: 1 addition & 0 deletions dockers/docker-fpm-frr/bgpd.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ hostname {{ DEVICE_METADATA['localhost']['hostname'] }}
password zebra
log syslog informational
log facility local4
agentx
! enable password {# {{ en_passwd }} TODO: param needed #}
{% endblock system_init %}
!
Expand Down
1 change: 1 addition & 0 deletions dockers/docker-fpm-frr/frr.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ hostname {{ DEVICE_METADATA['localhost']['hostname'] }}
password zebra
log syslog informational
log facility local4
agentx
! enable password {# {{ en_passwd }} TODO: param needed #}
{% endblock system_init %}
!
Expand Down
4 changes: 4 additions & 0 deletions dockers/docker-fpm-frr/snmp.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# this line allows the FRR docker to speak with the snmp container
# make sure this line matches the one in the snmp docker
# snmp:/etc/snmp/snmpd.conf
agentXSocket tcp:localhost:3161
4 changes: 2 additions & 2 deletions dockers/docker-fpm-frr/supervisord.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ stdout_logfile=syslog
stderr_logfile=syslog

[program:zebra]
command=/usr/lib/frr/zebra -A 127.0.0.1 -s 90000000 -M fpm
command=/usr/lib/frr/zebra -A 127.0.0.1 -s 90000000 -M fpm -M snmp
priority=4
autostart=false
autorestart=false
Expand All @@ -49,7 +49,7 @@ stdout_logfile=syslog
stderr_logfile=syslog

[program:bgpd]
command=/usr/lib/frr/bgpd -A 127.0.0.1
command=/usr/lib/frr/bgpd -A 127.0.0.1 -M snmp
priority=5
stopsignal=KILL
autostart=false
Expand Down
4 changes: 4 additions & 0 deletions dockers/docker-snmp-sv2/snmpd.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ load 12 10 5
#
# Run as an AgentX master agent
master agentx
# internal socket to allow extension to other docker containers
# Currently the othe container using this is docker-fpm-frr
Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

othe [](start = 16, length = 4)

typo #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected

# make sure this line matches bgp:/etc/snmp/frr.conf
agentxsocket tcp:localhost:3161
Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

3161 [](start = 30, length = 4)

How did you choose this port? I see it also in snmp.conf. Suggest prevent magic numbers, or add comment for reference each other so we will not forget if we want to modify it. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

standard port is 705, chose 3000+ snmp port to not run into issues for daemons that re not launched as root.
We can make this a configurable option in rules/config?

Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

rules/config is good! #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually talked to fast: this would require a nested template (dockers/docker-snmp-sv2/snmpd.conf.j2.j2 ) which would add complexity, the best thing I believe is to add it to our plan to integrate snmp config in config_db.json as per sonic-net/SONiC#231
we should start working on this in Q3 ( within a month ). what do you think?

Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

OK to me. Then please check the alternative recommendation

add comment for reference each other so we will not forget if we want to modify it. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added references to both files

Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

agentxsocket [](start = 0, length = 12)

How to test this feature? Can you add a test case in sonic-mgmt? #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you need an agentx client to be able to test this feature: we can provide an end to end test by using the bgp daemon as agentx client and doing an snmp request to a RFC1657 OID

Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure why we need an agentx client to test. I guess you can add some queries into existing sonic-mgmt snmp test. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes I'm looking into it right now, I'm checking to see if the ansible snmp_facts module queries the right OIDs which would make the test trivial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/Azure/sonic-mgmt/blob/master/ansible/library/snmp_facts.py does not support RFC1657 at this time, we could patch it to add a subset of OIDs but its going to take some time. I suggest we add it to the issues backlog of sonic-mgmt and we can pick it up

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you could not provide a sonic-mgmt test case right now, please add some manual test instruction here (with steps and sample output). We could not accept feature without test.

In reply to: 293172354 [](ancestors = 293172354)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK will add a link to look at the docker-fpm-frr snmp.conf file and put the test protocol there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the info to both files


#
# SysDescription pass-through
Expand Down
2 changes: 1 addition & 1 deletion rules/docker-fpm-frr.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

DOCKER_FPM_FRR = docker-fpm-frr.gz
$(DOCKER_FPM_FRR)_PATH = $(DOCKERS_PATH)/docker-fpm-frr
$(DOCKER_FPM_FRR)_DEPENDS += $(FRR) $(SWSS) $(LIBYANG)
$(DOCKER_FPM_FRR)_DEPENDS += $(FRR) $(FRR_SNMP) $(SWSS) $(LIBYANG)
$(DOCKER_FPM_FRR)_LOAD_DOCKERS += $(DOCKER_CONFIG_ENGINE_STRETCH)
SONIC_DOCKER_IMAGES += $(DOCKER_FPM_FRR)

Expand Down
8 changes: 7 additions & 1 deletion rules/frr.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,10 @@ $(eval $(call add_derived_package,$(FRR),$(FRR_PYTHONTOOLS)))
FRR_DBG = frr-dbgsym_$(FRR_VERSION)-sonic-$(FRR_SUBVERSION)_amd64.deb
$(eval $(call add_derived_package,$(FRR),$(FRR_DBG)))

export FRR FRR_PYTHONTOOLS FRR_DBG
FRR_SNMP = frr-snmp_$(FRR_VERSION)-sonic-$(FRR_SUBVERSION)_amd64.deb
$(eval $(call add_derived_package,$(FRR),$(FRR_SNMP)))

FRR_SNMP_DBG = frr-snmp-dbgsym_$(FRR_VERSION)-sonic-$(FRR_SUBVERSION)_amd64.deb
$(eval $(call add_derived_package,$(FRR),$(FRR_SNMP_DBG)))

export FRR FRR_PYTHONTOOLS FRR_DBG FRR_SNMP FRR_SNMP_DBG
lguohan marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions src/sonic-config-engine/tests/sample_output/frr.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ hostname switch-t0
password zebra
log syslog informational
log facility local4
agentx
! enable password !
! Enable link-detect (default disabled)
interface PortChannel01
Expand Down
2 changes: 1 addition & 1 deletion src/sonic-frr/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ SHELL = /bin/bash
.SHELLFLAGS += -e

MAIN_TARGET = $(FRR)
DERIVED_TARGET = $(FRR_PYTHONTOOLS) $(FRR_DBG)
DERIVED_TARGET = $(FRR_PYTHONTOOLS) $(FRR_DBG) $(FRR_SNMP) $(FRR_SNMP_DBG)

$(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :
# Build the package
Expand Down