zebra: Modify how we display/store os description #4220

donaldsharp · 2019-04-28T00:00:28Z

The alias/description of an interface in linux was being
used to override the internal description. As such let's
fix the display to keep track of both if we have it.

Config in FRR:

!
interface docker0
 description another combination
!
interface enp3s0
 description BAMBOOZLE ME WILL YOU
!

Config in linux:

sharpd@robot ~/f/zebra> ip link show
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    alias This is the loopback you cabbage
2: enp3s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
    link/ether 74:d0:2b:9c:16:eb brd ff:ff:ff:ff:ff:ff
    alias HI HI HI

Now the 'show int descr' command:

robot# show int description
Interface       Status  Protocol  Description
docker0         up      down      another combination
enp3s0          up      up        BAMBOOZLE ME WILL YOU
                                  HI HI HI
lo              up      up        This is the loopback you cabbage

Fixes: #4191
Signed-off-by: Donald Sharp sharpd@cumulusnetworks.com

The alias/description of an interface in linux was being used to override the internal description. As such let's fix the display to keep track of both if we have it. Config in FRR: ! interface docker0 description another combination ! interface enp3s0 description BAMBOOZLE ME WILL YOU ! Config in linux: sharpd@robot ~/f/zebra> ip link show 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000 link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 alias This is the loopback you cabbage 2: enp3s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000 link/ether 74:d0:2b:9c:16:eb brd ff:ff:ff:ff:ff:ff alias HI HI HI Now the 'show int descr' command: robot# show int description Interface Status Protocol Description docker0 up down another combination enp3s0 up up BAMBOOZLE ME WILL YOU HI HI HI lo up up This is the loopback you cabbage Fixes: FRRouting#4191 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

LabN-CI · 2019-04-28T19:04:21Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/4220 `c74147d`
Date	04/28/2019
Start	14:40:20
Finish	15:04:08
Run-Time	23:48
Total	1816
Pass	1816
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-04-28-14:40:20.txt
Log	autoscript-2019-04-28-14:41:06.log.bz2
Memory	440 443 374

For details, please contact louberger

NetDEF-CI · 2019-04-28T19:38:56Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7397/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

CLANG Static Analyzer Summary

Github Pull Request 4220, comparing to Git base SHA 9f9e9ef

No Changes in Static Analysis warnings compared to base

12 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7397/artifact/shared/static_analysis/index.html

qlyoung

Some nits, particularly the null checks before free aren't necessary

qlyoung · 2019-04-29T16:08:49Z

zebra/if_netlink.c

@@ -707,6 +704,12 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup)
 	zif = (struct zebra_if *)ifp->info;
 	zif->link_ifindex = link_ifindex;

+	if (desc) {
+		if (zif->desc)
+			XFREE(MTYPE_TMP, zif->desc);


No need for the null check here.

qlyoung · 2019-04-29T16:09:18Z

zebra/if_netlink.c

+	zif = ifp->info;
+	if (zif) {
+		if (zif->desc)
+			XFREE(MTYPE_TMP, zif->desc);


qlyoung · 2019-04-29T16:09:30Z

zebra/interface.c

@@ -181,6 +181,8 @@ static int if_zebra_delete_hook(struct interface *ifp)
 		list_delete(&rtadv->AdvDNSSLList);
 #endif /* HAVE_RTADV */

+		if (zebra_if->desc)
+			XFREE(MTYPE_TMP, zebra_if->desc);


qlyoung · 2019-04-29T16:10:44Z

zebra/interface.c

+		struct zebra_if *zif;
+		bool intf_desc;
+
+		intf_desc = false;


bool intf_desc = false ?

intf_desc was being set to false once with the bool intf_desc = false why? I am not sure as that was a bit surprising too me. So I just moved it down one line to ensure it was reset at the top of the loop every time.

LabN-CI · 2019-04-30T01:04:22Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/4220 `ba5165e`
Date	04/29/2019
Start	20:40:22
Finish	21:04:08
Run-Time	23:46
Total	1816
Pass	1816
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-04-29-20:40:22.txt
Log	autoscript-2019-04-29-20:41:08.log.bz2
Memory	449 442 374

For details, please contact louberger

NetDEF-CI · 2019-04-30T02:13:51Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7406/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

CLANG Static Analyzer Summary

Github Pull Request 4220, comparing to Git base SHA e07daf5

No Changes in Static Analysis warnings compared to base

12 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7406/artifact/shared/static_analysis/index.html

riw777

looks like everything is fixed on this one, and the code is fine

polychaeta added the zebra label Apr 28, 2019

donaldsharp force-pushed the fix_linux_alias branch 2 times, most recently from e8df4f5 to c74147d Compare April 28, 2019 18:35

qlyoung added the bugfix label Apr 28, 2019

FRRouting deleted a comment from NetDEF-CI Apr 28, 2019

FRRouting deleted a comment from LabN-CI Apr 28, 2019

FRRouting deleted a comment from NetDEF-CI Apr 28, 2019

FRRouting deleted a comment from LabN-CI Apr 28, 2019

qlyoung reviewed Apr 29, 2019

View reviewed changes

donaldsharp force-pushed the fix_linux_alias branch from c74147d to ba5165e Compare April 30, 2019 00:34

donaldsharp closed this Apr 30, 2019

donaldsharp deleted the fix_linux_alias branch April 30, 2019 12:17

donaldsharp restored the fix_linux_alias branch April 30, 2019 12:18

donaldsharp reopened this Apr 30, 2019

riw777 approved these changes May 1, 2019

View reviewed changes

riw777 merged commit e5f4e84 into FRRouting:master May 1, 2019

donaldsharp deleted the fix_linux_alias branch December 9, 2019 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

zebra: Modify how we display/store os description #4220

zebra: Modify how we display/store os description #4220

donaldsharp commented Apr 28, 2019 •

edited by qlyoung

Loading

LabN-CI commented Apr 28, 2019

NetDEF-CI commented Apr 28, 2019

qlyoung left a comment

qlyoung Apr 29, 2019

donaldsharp Apr 30, 2019

qlyoung Apr 29, 2019

donaldsharp Apr 30, 2019

qlyoung Apr 29, 2019

donaldsharp Apr 30, 2019

qlyoung Apr 29, 2019

donaldsharp Apr 30, 2019

LabN-CI commented Apr 30, 2019

NetDEF-CI commented Apr 30, 2019

riw777 left a comment

zebra: Modify how we display/store os description #4220

zebra: Modify how we display/store os description #4220

Conversation

donaldsharp commented Apr 28, 2019 • edited by qlyoung Loading

LabN-CI commented Apr 28, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Apr 28, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

12 Static Analyzer issues remaining.

qlyoung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LabN-CI commented Apr 30, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Apr 30, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

12 Static Analyzer issues remaining.

riw777 left a comment

Choose a reason for hiding this comment

donaldsharp commented Apr 28, 2019 •

edited by qlyoung

Loading