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

[SNMP]: Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG #17045

Merged

qiluo-msft merged 9 commits into sonic-net:master from SuvarnaMeenakshi:parser_snmp

Dec 6, 2023

Contributor

SuvarnaMeenakshi commented Oct 31, 2023 •

edited

Loading

table

Why I did it

SNMP query over IPv6 does not work due to issue in net-snmp where IPv6 query does not work on multi-nic environment.
To get around this, if snmpd listens on specific ipv4 or ipv6 address, then the issue is not seen.
We plan to configure Management IP and Loopback IP configured in minigraph.xml as SNMP_AGENT_ADDRESS in config_db., based on changes discussed in sonic-net/SONiC#1457.

Work item tracking

Microsoft ADO (number only):26091228

How I did it

Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG with management and Loopback0 IP addresses.
Modify snmpd.conf.j2 to use SNMP_AGENT_ADDRESS_CONFIG table if it is present in config_db, if not listen on any IP.
Main change:

if minigraph.xml is used to configure the device, then snmpd will listen on mgmt and loopback IP addresses,
if config_db is used to configure the device, snmpd will listen IP present in SNMP_AGENT_ADDRESS_CONFIG if that table is present, if table is not present snmpd will listen on any IP.

How to verify it

config_db.json created from minigraph.xml for single asic VS image with mgmt and Loopback IP addresses.

    "SNMP_AGENT_ADDRESS_CONFIG": {
        "10.1.0.32|161|": {},
        "10.250.0.101|161|": {},
        "FC00:1::32|161|": {},
        "fec0::ffff:afa:1|161|": {}
    },
 .....
 
 snmpd listening on the above IP addresses:
 admin@vlab-01:~$ sudo netstat -tulnp | grep 161
tcp        0      0 127.0.0.1:3161          0.0.0.0:*               LISTEN      71522/snmpd         
udp        0      0 10.250.0.101:161        0.0.0.0:*                           71522/snmpd         
udp        0      0 10.1.0.32:161           0.0.0.0:*                           71522/snmpd         
udp6       0      0 fec0::ffff:afa:1:161    :::*                                71522/snmpd         
udp6       0      0 fc00:1::32:161          :::*                                71522/snmpd

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

SuvarnaMeenakshi added 2 commits

October 31, 2023 00:19


          [SNMP]: Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG

81a9e2d

table

Signed-off-by: Suvarna Meenakshi <[email protected]>


          Use docker0 ip for multi-asic platform.

c41dd1f

Signed-off-by: Suvarna Meenakshi <[email protected]>

SuvarnaMeenakshi requested a review from qiluo-msft as a code owner

October 31, 2023 00:48


          Modify to update only for multi-asic

e044a5a

Signed-off-by: Suvarna Meenakshi <[email protected]>

SuvarnaMeenakshi mentioned this pull request

[SNMP]: SONiC SNMP Changes to support IPv6 sonic-net/SONiC#1457

Merged

SuvarnaMeenakshi requested review from dgsudharsan, venkatmahalingam, arlakshm and abdosi

October 31, 2023 22:41

Contributor Author

SuvarnaMeenakshi commented Nov 3, 2023

/azp run

azure-pipelines bot commented Nov 3, 2023

Commenter does not have sufficient privileges for PR 17045 in repo sonic-net/sonic-buildimage

arlakshm reviewed

View reviewed changes

src/sonic-config-engine/minigraph.py Outdated Show resolved Hide resolved

arlakshm previously approved these changes

View reviewed changes

Contributor

arlakshm left a comment

lgtm.

This was referenced Nov 16, 2023

SNMP_AGENT_ADDRESS_CONFIG configuration does not work for link local IP sonic-net/sonic-mgmt#10766

Open

[SNMP]: Skip test_snmp_link_local testcase sonic-net/sonic-mgmt#10768

Open


          Remove zoneid usage as interface is not known

921c685

when using SNMP_AGENT_ADDRESS_CONFIG.

Signed-off-by: Suvarna Meenakshi <[email protected]>

SuvarnaMeenakshi dismissed arlakshm’s stale review via

921c685

November 17, 2023 00:49

arlakshm previously approved these changes

View reviewed changes


          Fix as per PR comment to use all Loopback IP address

ce3a8de

and not just Loopback0

Signed-off-by: Suvarna Meenakshi <[email protected]>

SuvarnaMeenakshi dismissed arlakshm’s stale review via

ce3a8de

November 28, 2023 21:50

qiluo-msft reviewed

View reviewed changes

src/sonic-config-engine/minigraph.py

                           results['LOOPBACK_INTERFACE'][host_lo_intf[0]] = {}
                   results['MGMT_VRF_CONFIG'] = mvrf
+                  # Update SNMP_AGENT_ADDRESS_CONFIG with Management IP and Loopback IP
+                  # if available.
+                  if not is_multi_asic() and asic_name is None:

Collaborator

qiluo-msft Nov 28, 2023 •

edited

Loading

asic_name

why checking asic_name? #Closed

Contributor Author

SuvarnaMeenakshi Nov 28, 2023

For host namespace, asic_name will be None and SNMP_AGENT_ADDRESS_CONFIG is only required for host namespace in case of multi-asic devices. For single asic device asic_name will always be None, but added this check to ensure that we only generate for host namespace.
Minigraph parser generates config for host and each namespace separately.
This is done by passing the right asic_name/

qiluo-msft reviewed

View reviewed changes

src/sonic-config-engine/minigraph.py Outdated

+                          snmp_key = mgmt_if[1].split('/')[0] + '|' + str(port)  + '|'
+                          results['SNMP_AGENT_ADDRESS_CONFIG'][snmp_key] = {}
+                      # Add Loopback IP as agent address for single asic
+                      for loip in results['LOOPBACK_INTERFACE']:

Collaborator

qiluo-msft Nov 28, 2023 •

edited

Loading

results['LOOPBACK_INTERFACE']:

Could you use lo_intfs directly? No need to check len(loip) == 2. #Closed

Contributor Author

SuvarnaMeenakshi Nov 28, 2023 •

edited

Loading

the len(loip) is to make sure that the loip inside LOOPBACK_INTERFACE is actually in the format <name of interface>|<IP address> and not just the name of the IP address.
For example, we have

       "LOOPBACK_INTERFACE": {
        "Loopback0": {}, ------------------ This does not contain the IP address, so we don't need to parse this.
        "Loopback0|10.1.0.32/32": {}, ---- here the len(loip) will be 2 and we will be able to get the IP address.
        
(Pdb) loip
('Loopback0', '10.1.0.32/32')
(Pdb) len(loip)
2

Collaborator

qiluo-msft Nov 29, 2023

results['MGMT_INTERFACE'] is generated by this script itself, it is more efficient to parse from original data, not parsing from generated data.

Contributor Author

SuvarnaMeenakshi Nov 29, 2023

Makes sense, I have modified as suggested.

qiluo-msft reviewed

View reviewed changes

src/sonic-config-engine/minigraph.py Outdated Show resolved Hide resolved

qiluo-msft reviewed

View reviewed changes

src/sonic-config-engine/minigraph.py

                           results['LOOPBACK_INTERFACE'][host_lo_intf[0]] = {}
                   results['MGMT_VRF_CONFIG'] = mvrf
+                  # Update SNMP_AGENT_ADDRESS_CONFIG with Management IP and Loopback IP
+                  # if available.
+                  if not is_multi_asic() and asic_name is None:

Collaborator

qiluo-msft Nov 28, 2023 •

edited

Loading

if

If the condition does not meet, please initialize an empty results['SNMP_AGENT_ADDRESS_CONFIG']. #Closed

qiluo-msft reviewed

View reviewed changes

src/sonic-config-engine/minigraph.py

                           results['LOOPBACK_INTERFACE'][host_lo_intf[0]] = {}
                   results['MGMT_VRF_CONFIG'] = mvrf
+                  # Update SNMP_AGENT_ADDRESS_CONFIG with Management IP and Loopback IP
+                  # if available.
+                  if not is_multi_asic() and asic_name is None:

Collaborator

qiluo-msft Nov 28, 2023 •

edited

Loading

not

I guess you want to check is_multi_asic, not single asic. #Closed

Contributor Author

SuvarnaMeenakshi Nov 29, 2023

I want to check if the device is single asic, then update SNMP_AGENT_ADDRESS_CONFIG.
Hence checking "not is_multi_asic()".
No change required for multi-asic.

qiluo-msft requested a review from wen587

November 28, 2023 23:40

SuvarnaMeenakshi added 4 commits

November 29, 2023 00:19


          Fix as per review comments.

4e35e11

Signed-off-by: Suvarna Meenakshi <[email protected]>


          Use string type for port number variable to avoid conversion to str

a589bda

Signed-off-by: Suvarna Meenakshi <[email protected]>


          Modify to loop through the original parsed data as per

3f7f4fe

review comment.

Signed-off-by: Suvarna Meenakshi <[email protected]>


          Add additional new line

ea3f368

Signed-off-by: Suvarna Meenakshi <[email protected]>

Contributor

wen587 commented Nov 29, 2023

Is it a feature in 202205 branch? If so, we will also require the change in NDM side.
Also, for new table we will require related YANG model to support the YANG validation.

Contributor Author

SuvarnaMeenakshi commented Nov 29, 2023

Is it a feature in 202205 branch? If so, we will also require the change in NDM side. Also, for new table we will require related YANG model to support the YANG validation.

This change will be only in master and not in 202205/2023 branches.
We have added the yang model with this PR: #15587

qiluo-msft approved these changes

View reviewed changes

Collaborator

qiluo-msft left a comment

LGTM. Please also check with reviewers.

wen587 approved these changes

View reviewed changes

SuvarnaMeenakshi added the Request for 202311 Branch label

qiluo-msft merged commit 90dc254 into sonic-net:master

20 checks passed

vivekrnv mentioned this pull request

[snmp] Snmpd fails to start when mgmt or Loopback interface is configured with Link local IPv6 address #16001

Open

yxieca added Approved for 202311 Branch Bug 🐛 labels

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request


          [SNMP]: Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG t…

…able (sonic-net#17045)

#### Why I did it
SNMP query over IPv6 does not work due to issue in net-snmp where IPv6 query does not work on multi-nic environment.
To get around this, if snmpd listens on specific ipv4 or ipv6 address, then the issue is not seen.
We plan to configure Management IP and Loopback IP configured in minigraph.xml as SNMP_AGENT_ADDRESS in config_db., based on changes discussed in sonic-net/SONiC#1457.

##### Work item tracking
- Microsoft ADO **(number only)**:26091228

#### How I did it
Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG with management and Loopback0 IP addresses.
Modify snmpd.conf.j2 to use SNMP_AGENT_ADDRESS_CONFIG table if it is present in config_db, if not listen on any IP.
Main change:
1. if minigraph.xml is used to configure the device, then snmpd will listen on mgmt and loopback IP addresses,
2. if config_db is used to configure the device, snmpd will listen IP present in SNMP_AGENT_ADDRESS_CONFIG  if that table is present, if table is not present snmpd will listen on any IP.
#### How to verify it
config_db.json created from minigraph.xml for single asic VS image with mgmt and Loopback IP addresses.
```
    "SNMP_AGENT_ADDRESS_CONFIG": {
        "10.1.0.32|161|": {},
        "10.250.0.101|161|": {},
        "FC00:1::32|161|": {},
        "fec0::ffff:afa:1|161|": {}
    },
 .....
 
 snmpd listening on the above IP addresses:
 admin@vlab-01:~$ sudo netstat -tulnp | grep 161
tcp        0      0 127.0.0.1:3161          0.0.0.0:*               LISTEN      71522/snmpd         
udp        0      0 10.250.0.101:161        0.0.0.0:*                           71522/snmpd         
udp        0      0 10.1.0.32:161           0.0.0.0:*                           71522/snmpd         
udp6       0      0 fec0::ffff:afa:1:161    :::*                                71522/snmpd         
udp6       0      0 fc00:1::32:161          :::*                                71522/snmpd  
```

mssonicbld mentioned this pull request

[action] [PR:17045] [SNMP]: Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG table (#17045) #17986

Merged

mssonicbld added the Created PR to 202311 Branch label

Collaborator

mssonicbld commented Feb 2, 2024

Cherry-pick PR to 202311: #17986

mssonicbld pushed a commit that referenced this pull request


          [SNMP]: Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG t…

6905ab7

…able (#17045)

#### Why I did it
SNMP query over IPv6 does not work due to issue in net-snmp where IPv6 query does not work on multi-nic environment.
To get around this, if snmpd listens on specific ipv4 or ipv6 address, then the issue is not seen.
We plan to configure Management IP and Loopback IP configured in minigraph.xml as SNMP_AGENT_ADDRESS in config_db., based on changes discussed in sonic-net/SONiC#1457.

##### Work item tracking
- Microsoft ADO **(number only)**:26091228

#### How I did it
Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG with management and Loopback0 IP addresses.
Modify snmpd.conf.j2 to use SNMP_AGENT_ADDRESS_CONFIG table if it is present in config_db, if not listen on any IP.
Main change:
1. if minigraph.xml is used to configure the device, then snmpd will listen on mgmt and loopback IP addresses,
2. if config_db is used to configure the device, snmpd will listen IP present in SNMP_AGENT_ADDRESS_CONFIG  if that table is present, if table is not present snmpd will listen on any IP.
#### How to verify it
config_db.json created from minigraph.xml for single asic VS image with mgmt and Loopback IP addresses.
```
    "SNMP_AGENT_ADDRESS_CONFIG": {
        "10.1.0.32|161|": {},
        "10.250.0.101|161|": {},
        "FC00:1::32|161|": {},
        "fec0::ffff:afa:1|161|": {}
    },
 .....
 
 snmpd listening on the above IP addresses:
 admin@vlab-01:~$ sudo netstat -tulnp | grep 161
tcp        0      0 127.0.0.1:3161          0.0.0.0:*               LISTEN      71522/snmpd         
udp        0      0 10.250.0.101:161        0.0.0.0:*                           71522/snmpd         
udp        0      0 10.1.0.32:161           0.0.0.0:*                           71522/snmpd         
udp6       0      0 fec0::ffff:afa:1:161    :::*                                71522/snmpd         
udp6       0      0 fc00:1::32:161          :::*                                71522/snmpd  
```

yxieca added Included in 202311 Branch and removed Approved for 202311 Branch Created PR to 202311 Branch labels

SuvarnaMeenakshi mentioned this pull request

Fix to use IPv6 linklocal address as snmp agent address #18350

Merged

11 tasks

lguohan pushed a commit that referenced this pull request


          [snmpd]: Fix to use IPv6 linklocal address as snmp agent address (#18350

a0ff20e

)

Why I did it

1. fix [snmp] Snmpd fails to start when mgmt or Loopback interface is configured with Link local IPv6 address #16001
2. fix Timeout error while fetching response using snmpget #17807

#17045 modified minigraph parser to use management and loopback IPs to support SNMP query over IPv6. With this fix, if mgmt or loopback IP contains link local IP, that will not work as link local IP has to be appended with scope id associating the IP address to a specific interface. This PR change is to ensure that snmp works with link local IPv6 address.

How I did it
Modify minigraph parser to append the Ip address with % scope id if snmp agent address being used is link local IP address.
Modify snmpd.conf.j2 to take this change while checking if an IP address is ipv4 or ipv6.

How to verify it
Verified by configuring link local ipv6 address.
Last login: Wed Mar 13 01:45:09 2024 from 10.1.84.57
admin@<>:~$ sudo netstat -tulnp | grep 161
...
udp6 0 0 fe80::f6ee:31ff:fe9:161 :::* 70355/snmpd

Signed-off-by: Suvarna Meenakshi <[email protected]>

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

Reviewers

qiluo-msft qiluo-msft approved these changes

wen587 wen587 approved these changes

arlakshm arlakshm left review comments

dgsudharsan Awaiting requested review from dgsudharsan

venkatmahalingam Awaiting requested review from venkatmahalingam

abdosi Awaiting requested review from abdosi

Labels

Bug 🐛 Included in 202311 Branch Request for 202311 Branch