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.

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

[config] Prevent deleting VLAN with IP addresses. #1429

Merged
merged 7 commits into from
Feb 16, 2021

Conversation

d-dashkov
Copy link
Contributor

@d-dashkov d-dashkov commented Feb 11, 2021

What I did

Prevent deleting VLAN if it has IP addresses.
fixes: sonic-net/sonic-buildimage#6769
Currently, if an IP was added to the vlan, it creates entries in the VLAN_INTERFACE table and increments the reference counter for the vlan:

VLAN_INTERFACE|Vlan22|100.0.0.20/24
VLAN_INTERFACE|Vlan22

But after deleting the vlan, they get stuck in the table and throw an error:

sonic ERR swss#orchagent: :- removeVlan: Failed to remove ref count 1 VLAN Vlan22

Therefore, to prevent this error, make sure that the entries do not exist before deleting the VLAN.

How I did it

Added check of the VLAN INTERFACE table and if there are records, the delete operation is not available and an error is displayed.

How to verify it

  1. config vlan add 22
  2. config int ip add Vlan22 10.0.0.51/24
  3. config vlan del 22
Error: Vlan22 can not be removed. First remove IP addresses assigned to this VLAN

Signed-off-by: d-dashkov [email protected]

@@ -46,6 +46,13 @@ def del_vlan(db, vid):
if clicommon.check_if_vlanid_exist(db.cfgdb, vlan) == False:
ctx.fail("{} does not exist".format(vlan))

intf_table = db.cfgdb.get_table('VLAN_INTERFACE')
Copy link
Contributor

Choose a reason for hiding this comment

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

One concern is , by removing it in background, user may not be aware that ip address association is removed. Also in the current Sonic image, you can add an ip address to a vlan interface for which vlan is non-existent.

For e.g: config interface ip add Vlan200 2.3.4.6/24 <== This would be accepted, even if Vlan200 doesn't exist.

My suggestion would be to log warning to user that Vlan still have IP address and let user remove the ip. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made an update as per your suggestion to warn the user, but it requires some changes in the tests.
Also is this the correct behavior of adding IP to non-existent entries? There is an issue on same problem with my PR.(sonic-net/sonic-buildimage#6358).

@d-dashkov d-dashkov changed the title [config] deleting VLAN_INTERFACE entries [config] Prevent deleting VLAN with IP addresses. Feb 15, 2021
Signed-off-by: d-dashkov <[email protected]>
@d-dashkov d-dashkov requested a review from prsunny February 15, 2021 19:01
@prsunny prsunny merged commit 6d1ed6c into sonic-net:master Feb 16, 2021
daall pushed a commit that referenced this pull request Feb 16, 2021
Warn user while deleting VLAN if it has IP addresses.
Signed-off-by: d-dashkov <[email protected]>
anand-kumar-subramanian pushed a commit to anand-kumar-subramanian/sonic-utilities that referenced this pull request Mar 2, 2021
Warn user while deleting VLAN if it has IP addresses.
Signed-off-by: d-dashkov <[email protected]>
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.

Orchagent errors in case of deleting a vlan that has IP addresses.
2 participants