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

Provide broadcast IP while configuring interface ip address #1007

Merged
merged 2 commits into from
Aug 3, 2019

Conversation

vasant17
Copy link
Contributor

@vasant17 vasant17 commented Jul 30, 2019

Signed-off-by: Vasant Patil [email protected]

Problem: When SONiC CLI command is used to display summary of interface(s), broadcast address is always 0.0.0.0 irrespective of prefix length

Solution: When interface ip address is added using the command "ip addr add ...", we can specify the broadcast address as well. I did NOT set broadcast addr for interface with point-to-point link addresses(/31, and /127)

Testing:
admin@lnos-x1-a-csw06:~$ show interfaces summary Vlan100
Vlan100: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet 10.1.1.0 netmask 255.255.255.0 broadcast 10.1.1.255
inet6 fe80::2e0:ecff:fe3b:d72f prefixlen 64 scopeid 0x20
ether 00:e0:ec:3b:d7:2f txqueuelen 1000 (Ethernet)
RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 5 bytes 650 (650.0 B)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0

admin@lnos-x1-a-csw06:~$

vapatil@server05:~$

=========== Interface with /31 prefix legth ================
admin@lnos-x1-a-csw06:~$ show interface summary Ethernet124
Ethernet124: flags=4099<UP,BROADCAST,MULTICAST> mtu 9100
inet 10.0.0.62 netmask 255.255.255.254 broadcast 0.0.0.0
ether 00:e0:ec:3b:d7:2f txqueuelen 1000 (Ethernet)
RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 0 bytes 0 (0.0 B)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0

=========== Interface with /24 prefix legth ================
admin@lnos-x1-a-csw06:~$ show interface summary Vlan100
Vlan100: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet 10.1.1.0 netmask 255.255.255.0 broadcast 10.1.1.255
inet6 fe80::2e0:ecff:fe3b:d72f prefixlen 64 scopeid 0x20
ether 00:e0:ec:3b:d7:2f txqueuelen 1000 (Ethernet)
RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 6 bytes 768 (768.0 B)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0

admin@lnos-x1-a-csw06:~$

What I did

Why I did it

How I verified it

Details if related

@msftclas
Copy link

msftclas commented Jul 30, 2019

CLA assistant check
All CLA requirements met.

@@ -183,7 +191,7 @@ bool IntfMgr::doIntfAddrTask(const vector<string>& keys,
SWSS_LOG_ENTER();

string alias(keys[0]);
IpPrefix ip_prefix(keys[1]);
IpPrefix ipPrefix(keys[1]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this change is irrelevant. If no specific reason, keep the original convention. I mean, is_lo and other params are not following this style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Sunny,
I thought so as well, but I do see couple of variables in this function in camelCase(appKey, fvVector). Is there any coding standard guidelines? Now, I can change those two variables as app_key and fv_vector or change is_lo to isLo. Please suggest and point me to a coding standard guidelines doc if we have one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, I just reverted my changes for ip_prefix varibale.

@lguohan
Copy link
Contributor

lguohan commented Aug 1, 2019

retest this please

1 similar comment
@lguohan
Copy link
Contributor

lguohan commented Aug 2, 2019

retest this please

@lguohan lguohan merged commit 8fcf43d into sonic-net:master Aug 3, 2019
praveen-li pushed a commit to praveen-li/sonic-swss that referenced this pull request Aug 24, 2020
…t#1007)

RB=1741922
G=lnos-reviewers
R=pchaudhary,pmao,rmolina,samaity,sfardeen,zxu
A=

Problem: When SONiC CLI command is used to display summary of interface(s), broadcast address is always 0.0.0.0 irrespective of prefix length

Solution: When interface ip address is added using the command "ip addr add ...", we can specify the broadcast address as well. I did NOT set broadcast addr for interface with point-to-point link addresses(/31, and /127)

Signed-off-by: Vasant Patil [email protected]
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants