Skip to content

Check prefix length while configuring route#121

Merged
sumukhatv merged 5 commits intosonic-net:masterfrom
sumukhatv:route_ip_check
Aug 10, 2022
Merged

Check prefix length while configuring route#121
sumukhatv merged 5 commits intosonic-net:masterfrom
sumukhatv:route_ip_check

Conversation

@sumukhatv
Copy link
Copy Markdown
Collaborator

Allow only prefixes with length greater than /18 for v4 and equal to /64 for v6

@sumukhatv sumukhatv requested a review from prsunny August 1, 2022 19:34
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Aug 8, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

if adv_prefix, ok := kv["advertise_prefix"]; ok {
if adv_prefix == "true" {
prefix_len, _ := network.Mask.Size()
if isV4orV6(ip.String()) == 4 {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we use this - ip.To4()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The isV4orV6 method is calling ip.To4()

continue
}

kv, err := GetKVs(confdb.db_num, generateDBTableKey(confdb.separator, VNET_TB, vnet_id_str))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we have a logic to cache this after getting it first time? We can have significantly high number of routes and for each time, checking the DB can be expensive. So suggest a simple logic to set advertisement field for a vnet.

@prsunny
Copy link
Copy Markdown
Collaborator

prsunny commented Aug 9, 2022

please update submodule for master/202012

@sumukhatv sumukhatv merged commit bcc6f70 into sonic-net:master Aug 10, 2022
@sumukhatv sumukhatv deleted the route_ip_check branch August 10, 2022 05:34
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.

2 participants