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.

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

Hashmap for vrf lookup by table #10821

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Hashmap for vrf lookup by table #10821

wants to merge 4 commits into from

Conversation

nathan-duddles
Copy link

Summary

Add hashmap for lookup of vrf_id by table_id to vrf_lookup_by_id().

vrf_lookup_by_id() moved from zebra/rt_netlink.c to lib/vrf.c

Related Issue

Add Mapping From Table ID to VRF #5982

Components

zebra, lib

@nathan-duddles
zebra: Remove vrf_lookup_by_table decl/def from rt_netlink
3b88af3 
vrf_lookup_by_table() function is used outside of rt_netlink.c
currently in if_netlink.c and may be useful in other files
in the future

Will add to vrf.c[h], as this currently is home to other vrf lookup
functions

Signed-off-by: Nathan Duddles <[email protected]>
@nathan-duddles nathan-duddles force-pushed the hashmap_for_vrf_lookup_by_table branch from 1c0ec9c to aaf6647 Compare March 17, 2022 21:27
@nathan-duddles
lib: Add vrf_lookup_by_table with hashmap to vrf.c[h]
399a33c 
Lookup uses hashmap with table_id key and vrf_id values.
Only for vrf backended by vrf lite. Could do similar
implementation with ns_id in future.

Hashmap local to vrf.c but must be updated when
vrf is added or modified. For updates to vrf_id,
call to helper function vrf_ids_by_table_update()
is added in the publicly used vrf_update() function.

Previously, the table_id was set by directly modifying
vrf->data.l.table_id in zclient.c. However, as no functions
in vrf.c are notified when change occurs, a helper function,
vrf_update_table_id(), is added. This both updates
vrf_data.l.table_id, and updates the local hashmap.

vrf_delete() function used for removing vrfs. Add call to
here to vrf_ids_by_table_delete() helper function
to remove entries from hashmap.

Signed-off-by: Nathan Duddles <[email protected]>
@nathan-duddles nathan-duddles force-pushed the hashmap_for_vrf_lookup_by_table branch from aaf6647 to 399a33c Compare March 17, 2022 21:30
@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 17, 2022 •
edited
Loading

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/

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.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 i386 part 3: Failed (click for details) Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO3U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 3: No useful log found
Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details) Topotests Ubuntu 18.04 arm8 part 9: No useful log found
Topotests Ubuntu 18.04 i386 part 1: Failed (click for details) Topotests Ubuntu 18.04 i386 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO1U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 1: No useful log found
Topotests Ubuntu 18.04 i386 part 6: Failed (click for details) Topotests Ubuntu 18.04 i386 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO6U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 6: No useful log found
Topotests Ubuntu 18.04 i386 part 8: Failed (click for details) Topotests Ubuntu 18.04 i386 part 8: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO8U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 8: No useful log found
Topotests Ubuntu 18.04 i386 part 5: Failed (click for details) Topotests Ubuntu 18.04 i386 part 5: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO5U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 5: No useful log found
Topotests Ubuntu 18.04 i386 part 0: Failed (click for details) Topotests Ubuntu 18.04 i386 part 0: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO0U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 0: No useful log found
Topotests Ubuntu 18.04 i386 part 9: Failed (click for details) Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found
Topotests Ubuntu 18.04 i386 part 4: Failed (click for details) Topotests Ubuntu 18.04 i386 part 4: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO4U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 4: No useful log found
Topotests Ubuntu 18.04 i386 part 2: Failed (click for details) Topotests Ubuntu 18.04 i386 part 2: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO2U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 2: No useful log found
Topotests Ubuntu 18.04 i386 part 7: Failed (click for details) Topotests Ubuntu 18.04 i386 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO7U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 7: No useful log found
Successful on other platforms/tests
  • Topotests Ubuntu 18.04 arm8 part 3
  • Topotests Ubuntu 18.04 amd64 part 2
  • Addresssanitizer topotests part 5
  • Topotests debian 10 amd64 part 4
  • Addresssanitizer topotests part 4
  • Topotests debian 10 amd64 part 8
  • Addresssanitizer topotests part 0
  • Topotests Ubuntu 18.04 arm8 part 4
  • Topotests debian 10 amd64 part 9
  • Topotests Ubuntu 18.04 amd64 part 3
  • IPv6 protocols on Ubuntu 18.04
  • Topotests debian 10 amd64 part 3
  • Topotests debian 10 amd64 part 0
  • Ubuntu 16.04 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 2
  • Ubuntu 20.04 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 7
  • Topotests Ubuntu 18.04 amd64 part 8
  • IPv4 protocols on Ubuntu 18.04
  • Topotests Ubuntu 18.04 amd64 part 4
  • Addresssanitizer topotests part 9
  • IPv4 ldp protocol on Ubuntu 18.04
  • Topotests Ubuntu 18.04 arm8 part 0
  • Topotests Ubuntu 18.04 amd64 part 9
  • Debian 10 deb pkg check
  • Addresssanitizer topotests part 7
  • Topotests Ubuntu 18.04 amd64 part 7
  • Topotests Ubuntu 18.04 amd64 part 5
  • Addresssanitizer topotests part 6
  • Addresssanitizer topotests part 3
  • Topotests debian 10 amd64 part 1
  • CentOS 7 rpm pkg check
  • Topotests Ubuntu 18.04 arm8 part 5
  • Fedora 29 rpm pkg check
  • Topotests Ubuntu 18.04 amd64 part 0
  • Topotests debian 10 amd64 part 2
  • Addresssanitizer topotests part 2
  • Static analyzer (clang)
  • Topotests Ubuntu 18.04 amd64 part 1
  • Topotests debian 10 amd64 part 5
  • Debian 9 deb pkg check
  • Ubuntu 18.04 deb pkg check
  • Addresssanitizer topotests part 1
  • Topotests debian 10 amd64 part 7
  • Addresssanitizer topotests part 8
  • Topotests Ubuntu 18.04 arm8 part 6
  • Topotests Ubuntu 18.04 arm8 part 1
  • Topotests debian 10 amd64 part 6
  • Topotests Ubuntu 18.04 arm8 part 8
  • Topotests Ubuntu 18.04 amd64 part 6

Warnings Generated during build:

Checkout code: Successful with additional warnings
Topotests Ubuntu 18.04 i386 part 3: Failed (click for details) Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO3U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 3: No useful log found
Topotests Ubuntu 18.04 arm8 part 9: Failed (click for details) Topotests Ubuntu 18.04 arm8 part 9: No useful log found
Topotests Ubuntu 18.04 i386 part 1: Failed (click for details) Topotests Ubuntu 18.04 i386 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO1U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 1: No useful log found
Topotests Ubuntu 18.04 i386 part 6: Failed (click for details) Topotests Ubuntu 18.04 i386 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO6U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 6: No useful log found
Topotests Ubuntu 18.04 i386 part 8: Failed (click for details) Topotests Ubuntu 18.04 i386 part 8: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO8U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 8: No useful log found
Topotests Ubuntu 18.04 i386 part 5: Failed (click for details) Topotests Ubuntu 18.04 i386 part 5: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO5U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 5: No useful log found
Topotests Ubuntu 18.04 i386 part 0: Failed (click for details) Topotests Ubuntu 18.04 i386 part 0: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO0U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 0: No useful log found
Topotests Ubuntu 18.04 i386 part 9: Failed (click for details) Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found
Topotests Ubuntu 18.04 i386 part 4: Failed (click for details) Topotests Ubuntu 18.04 i386 part 4: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO4U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 4: No useful log found
Topotests Ubuntu 18.04 i386 part 2: Failed (click for details) Topotests Ubuntu 18.04 i386 part 2: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO2U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 2: No useful log found
Topotests Ubuntu 18.04 i386 part 7: Failed (click for details) Topotests Ubuntu 18.04 i386 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4209/artifact/TOPO7U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 7: No useful log found 
<stdin>:28: trailing whitespace.
static int vrf_ids_by_table_cmp(const struct vrf_ids_by_table_proxy *item_a, 
<stdin>:39: trailing whitespace.
DECLARE_HASH(vrf_ids_by_table, struct vrf_ids_by_table_proxy, 
<stdin>:44: trailing whitespace.
{	
<stdin>:45: trailing whitespace.
	struct vrf_ids_by_table_proxy ref = {.table_id = table_id};	
<stdin>:56: trailing whitespace.
	struct vrf_ids_by_table_proxy ref = {.table_id = table_id, .vrf_id = new_vrf_id};	
warning: squelched 12 whitespace errors
warning: 17 lines add whitespace errors.
Report for vrf.c | 56 issues
===============================================
< WARNING: please, no spaces at the start of a line
< #65: FILE: /tmp/f1-3825/vrf.c:65:
< ERROR: trailing whitespace
< #72: FILE: /tmp/f1-3825/vrf.c:72:
< WARNING: line over 80 characters
< #73: FILE: /tmp/f1-3825/vrf.c:73:
< ERROR: trailing whitespace
< #83: FILE: /tmp/f1-3825/vrf.c:83:
< WARNING: line over 80 characters
< #84: FILE: /tmp/f1-3825/vrf.c:84:
< ERROR: trailing whitespace
< #88: FILE: /tmp/f1-3825/vrf.c:88:
< ERROR: trailing whitespace
< #89: FILE: /tmp/f1-3825/vrf.c:89:
< WARNING: line over 80 characters
< #90: FILE: /tmp/f1-3825/vrf.c:90:
< WARNING: braces {} are not necessary for single statement blocks
< #93: FILE: /tmp/f1-3825/vrf.c:93:
< ERROR: trailing whitespace
< #100: FILE: /tmp/f1-3825/vrf.c:100:
< WARNING: line over 80 characters
< #100: FILE: /tmp/f1-3825/vrf.c:100:
< ERROR: trailing whitespace
< #102: FILE: /tmp/f1-3825/vrf.c:102:
< ERROR: trailing whitespace
< #103: FILE: /tmp/f1-3825/vrf.c:103:
< WARNING: line over 80 characters
< #105: FILE: /tmp/f1-3825/vrf.c:105:
< WARNING: braces {} are not necessary for single statement blocks
< #108: FILE: /tmp/f1-3825/vrf.c:108:
< ERROR: trailing whitespace
< #298: FILE: /tmp/f1-3825/vrf.c:298:
< ERROR: trailing whitespace
< #299: FILE: /tmp/f1-3825/vrf.c:299:
< ERROR: trailing whitespace
< #305: FILE: /tmp/f1-3825/vrf.c:305:
< ERROR: trailing whitespace
< #316: FILE: /tmp/f1-3825/vrf.c:316:
< ERROR: trailing whitespace
< #379: FILE: /tmp/f1-3825/vrf.c:379:
< ERROR: trailing whitespace
< #382: FILE: /tmp/f1-3825/vrf.c:382:
< ERROR: trailing whitespace
< #389: FILE: /tmp/f1-3825/vrf.c:389:
< ERROR: trailing whitespace
< #401: FILE: /tmp/f1-3825/vrf.c:401:
< ERROR: trailing whitespace
< #405: FILE: /tmp/f1-3825/vrf.c:405:
< WARNING: line over 80 characters
< #410: FILE: /tmp/f1-3825/vrf.c:410:
< WARNING: Missing a blank line after declarations
< #411: FILE: /tmp/f1-3825/vrf.c:411:
< WARNING: braces {} are not necessary for single statement blocks
< #411: FILE: /tmp/f1-3825/vrf.c:411:
< ERROR: trailing whitespace
< #415: FILE: /tmp/f1-3825/vrf.c:415:

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 17, 2022 •
edited
Loading

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/

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.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 i386 part 3: Failed (click for details) Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO3U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 3: No useful log found
Topotests debian 10 amd64 part 3: Failed (click for details) Topotests debian 10 amd64 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO3DEB10AMD64/ErrorLog/ Topotests debian 10 amd64 part 3: No useful log found
Topotests Ubuntu 18.04 i386 part 6: Failed (click for details) Topotests Ubuntu 18.04 i386 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO6U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 6: No useful log found
Topotests Ubuntu 18.04 i386 part 1: Failed (click for details) Topotests Ubuntu 18.04 i386 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO1U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 1: No useful log found
Topotests Ubuntu 18.04 i386 part 8: Failed (click for details) Topotests Ubuntu 18.04 i386 part 8: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO8U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 8: No useful log found
Topotests Ubuntu 18.04 i386 part 5: Failed (click for details) Topotests Ubuntu 18.04 i386 part 5: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO5U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 5: No useful log found
Topotests Ubuntu 18.04 i386 part 0: Failed (click for details) Topotests Ubuntu 18.04 i386 part 0: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO0U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 0: No useful log found
Topotests Ubuntu 18.04 i386 part 9: Failed (click for details) Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found
Topotests Ubuntu 18.04 i386 part 4: Failed (click for details) Topotests Ubuntu 18.04 i386 part 4: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO4U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 4: No useful log found
Topotests Ubuntu 18.04 i386 part 7: Failed (click for details) Topotests Ubuntu 18.04 i386 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO7U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 7: No useful log found
Topotests Ubuntu 18.04 i386 part 2: Failed (click for details) Topotests Ubuntu 18.04 i386 part 2: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO2U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 2: No useful log found
Successful on other platforms/tests
  • Topotests Ubuntu 18.04 amd64 part 2
  • Addresssanitizer topotests part 5
  • Topotests Ubuntu 18.04 arm8 part 3
  • Topotests debian 10 amd64 part 4
  • Addresssanitizer topotests part 4
  • Addresssanitizer topotests part 0
  • Topotests debian 10 amd64 part 8
  • Topotests debian 10 amd64 part 9
  • Topotests Ubuntu 18.04 arm8 part 4
  • Topotests Ubuntu 18.04 amd64 part 3
  • Topotests Ubuntu 18.04 arm8 part 9
  • IPv6 protocols on Ubuntu 18.04
  • Static analyzer (clang)
  • Topotests Ubuntu 18.04 arm8 part 2
  • Topotests debian 10 amd64 part 0
  • Ubuntu 16.04 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 7
  • Addresssanitizer topotests part 9
  • Topotests Ubuntu 18.04 amd64 part 8
  • Topotests debian 10 amd64 part 2
  • IPv4 protocols on Ubuntu 18.04
  • Topotests Ubuntu 18.04 arm8 part 0
  • IPv4 ldp protocol on Ubuntu 18.04
  • Topotests Ubuntu 18.04 amd64 part 9
  • Addresssanitizer topotests part 7
  • Ubuntu 20.04 deb pkg check
  • Topotests Ubuntu 18.04 amd64 part 7
  • Topotests Ubuntu 18.04 amd64 part 5
  • Debian 10 deb pkg check
  • Fedora 29 rpm pkg check
  • Addresssanitizer topotests part 6
  • Addresssanitizer topotests part 3
  • Topotests debian 10 amd64 part 1
  • Topotests Ubuntu 18.04 arm8 part 5
  • Topotests Ubuntu 18.04 amd64 part 4
  • CentOS 7 rpm pkg check
  • Topotests Ubuntu 18.04 amd64 part 0
  • Addresssanitizer topotests part 2
  • Topotests Ubuntu 18.04 amd64 part 1
  • Topotests debian 10 amd64 part 5
  • Addresssanitizer topotests part 1
  • Ubuntu 18.04 deb pkg check
  • Topotests debian 10 amd64 part 7
  • Debian 9 deb pkg check
  • Topotests Ubuntu 18.04 amd64 part 6
  • Addresssanitizer topotests part 8
  • Topotests Ubuntu 18.04 arm8 part 8
  • Topotests Ubuntu 18.04 arm8 part 1
  • Topotests Ubuntu 18.04 arm8 part 6
  • Topotests debian 10 amd64 part 6

Warnings Generated during build:

Checkout code: Successful with additional warnings
Topotests Ubuntu 18.04 i386 part 3: Failed (click for details) Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO3U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 3: No useful log found
Topotests debian 10 amd64 part 3: Failed (click for details) Topotests debian 10 amd64 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO3DEB10AMD64/ErrorLog/ Topotests debian 10 amd64 part 3: No useful log found
Topotests Ubuntu 18.04 i386 part 6: Failed (click for details) Topotests Ubuntu 18.04 i386 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO6U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 6: No useful log found
Topotests Ubuntu 18.04 i386 part 1: Failed (click for details) Topotests Ubuntu 18.04 i386 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO1U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 1: No useful log found
Topotests Ubuntu 18.04 i386 part 8: Failed (click for details) Topotests Ubuntu 18.04 i386 part 8: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO8U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 8: No useful log found
Topotests Ubuntu 18.04 i386 part 5: Failed (click for details) Topotests Ubuntu 18.04 i386 part 5: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO5U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 5: No useful log found
Topotests Ubuntu 18.04 i386 part 0: Failed (click for details) Topotests Ubuntu 18.04 i386 part 0: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO0U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 0: No useful log found
Topotests Ubuntu 18.04 i386 part 9: Failed (click for details) Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found
Topotests Ubuntu 18.04 i386 part 4: Failed (click for details) Topotests Ubuntu 18.04 i386 part 4: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO4U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 4: No useful log found
Topotests Ubuntu 18.04 i386 part 7: Failed (click for details) Topotests Ubuntu 18.04 i386 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO7U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 7: No useful log found
Topotests Ubuntu 18.04 i386 part 2: Failed (click for details) Topotests Ubuntu 18.04 i386 part 2: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4210/artifact/TOPO2U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 2: No useful log found 
<stdin>:28: trailing whitespace.
static int vrf_ids_by_table_cmp(const struct vrf_ids_by_table_proxy *item_a, 
<stdin>:39: trailing whitespace.
DECLARE_HASH(vrf_ids_by_table, struct vrf_ids_by_table_proxy, 
<stdin>:44: trailing whitespace.
{	
<stdin>:45: trailing whitespace.
	struct vrf_ids_by_table_proxy ref = {.table_id = table_id};	
<stdin>:56: trailing whitespace.
	struct vrf_ids_by_table_proxy ref = {.table_id = table_id, .vrf_id = new_vrf_id};	
warning: squelched 12 whitespace errors
warning: 17 lines add whitespace errors.
Report for vrf.c | 56 issues
===============================================
< WARNING: please, no spaces at the start of a line
< #65: FILE: /tmp/f1-4847/vrf.c:65:
< ERROR: trailing whitespace
< #72: FILE: /tmp/f1-4847/vrf.c:72:
< WARNING: line over 80 characters
< #73: FILE: /tmp/f1-4847/vrf.c:73:
< ERROR: trailing whitespace
< #83: FILE: /tmp/f1-4847/vrf.c:83:
< WARNING: line over 80 characters
< #84: FILE: /tmp/f1-4847/vrf.c:84:
< ERROR: trailing whitespace
< #88: FILE: /tmp/f1-4847/vrf.c:88:
< ERROR: trailing whitespace
< #89: FILE: /tmp/f1-4847/vrf.c:89:
< WARNING: line over 80 characters
< #90: FILE: /tmp/f1-4847/vrf.c:90:
< WARNING: braces {} are not necessary for single statement blocks
< #93: FILE: /tmp/f1-4847/vrf.c:93:
< ERROR: trailing whitespace
< #100: FILE: /tmp/f1-4847/vrf.c:100:
< WARNING: line over 80 characters
< #100: FILE: /tmp/f1-4847/vrf.c:100:
< ERROR: trailing whitespace
< #102: FILE: /tmp/f1-4847/vrf.c:102:
< ERROR: trailing whitespace
< #103: FILE: /tmp/f1-4847/vrf.c:103:
< WARNING: line over 80 characters
< #105: FILE: /tmp/f1-4847/vrf.c:105:
< WARNING: braces {} are not necessary for single statement blocks
< #108: FILE: /tmp/f1-4847/vrf.c:108:
< ERROR: trailing whitespace
< #298: FILE: /tmp/f1-4847/vrf.c:298:
< ERROR: trailing whitespace
< #299: FILE: /tmp/f1-4847/vrf.c:299:
< ERROR: trailing whitespace
< #305: FILE: /tmp/f1-4847/vrf.c:305:
< ERROR: trailing whitespace
< #316: FILE: /tmp/f1-4847/vrf.c:316:
< ERROR: trailing whitespace
< #379: FILE: /tmp/f1-4847/vrf.c:379:
< ERROR: trailing whitespace
< #382: FILE: /tmp/f1-4847/vrf.c:382:
< ERROR: trailing whitespace
< #389: FILE: /tmp/f1-4847/vrf.c:389:
< ERROR: trailing whitespace
< #401: FILE: /tmp/f1-4847/vrf.c:401:
< ERROR: trailing whitespace
< #405: FILE: /tmp/f1-4847/vrf.c:405:
< WARNING: line over 80 characters
< #410: FILE: /tmp/f1-4847/vrf.c:410:
< WARNING: Missing a blank line after declarations
< #411: FILE: /tmp/f1-4847/vrf.c:411:
< WARNING: braces {} are not necessary for single statement blocks
< #411: FILE: /tmp/f1-4847/vrf.c:411:
< ERROR: trailing whitespace
< #415: FILE: /tmp/f1-4847/vrf.c:415:

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 18, 2022 •
edited
Loading

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/

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.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 i386 part 3: Failed (click for details) Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO3U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 3: No useful log found
Topotests Ubuntu 18.04 i386 part 6: Failed (click for details) Topotests Ubuntu 18.04 i386 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO6U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 6: No useful log found
Topotests Ubuntu 18.04 i386 part 8: Failed (click for details) Topotests Ubuntu 18.04 i386 part 8: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO8U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 8: No useful log found
Topotests Ubuntu 18.04 i386 part 1: Failed (click for details) Topotests Ubuntu 18.04 i386 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO1U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 1: No useful log found
Topotests Ubuntu 18.04 i386 part 5: Failed (click for details) Topotests Ubuntu 18.04 i386 part 5: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO5U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 5: No useful log found
Topotests Ubuntu 18.04 i386 part 0: Failed (click for details) Topotests Ubuntu 18.04 i386 part 0: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO0U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 0: No useful log found
Topotests Ubuntu 18.04 i386 part 9: Failed (click for details) Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found
Topotests Ubuntu 18.04 i386 part 4: Failed (click for details) Topotests Ubuntu 18.04 i386 part 4: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO4U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 4: No useful log found
Topotests Ubuntu 18.04 i386 part 2: Failed (click for details) Topotests Ubuntu 18.04 i386 part 2: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO2U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 2: No useful log found
Topotests Ubuntu 18.04 i386 part 7: Failed (click for details) Topotests Ubuntu 18.04 i386 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO7U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 7: No useful log found
Successful on other platforms/tests
  • Topotests Ubuntu 18.04 amd64 part 2
  • Topotests Ubuntu 18.04 arm8 part 3
  • Addresssanitizer topotests part 5
  • Topotests debian 10 amd64 part 4
  • Addresssanitizer topotests part 4
  • Topotests debian 10 amd64 part 3
  • Addresssanitizer topotests part 0
  • Topotests debian 10 amd64 part 8
  • Topotests debian 10 amd64 part 9
  • Topotests Ubuntu 18.04 arm8 part 9
  • Topotests Ubuntu 18.04 arm8 part 4
  • IPv6 protocols on Ubuntu 18.04
  • Topotests Ubuntu 18.04 amd64 part 3
  • Static analyzer (clang)
  • IPv4 protocols on Ubuntu 18.04
  • Topotests Ubuntu 18.04 arm8 part 2
  • Topotests debian 10 amd64 part 0
  • Topotests Ubuntu 18.04 arm8 part 7
  • Topotests Ubuntu 18.04 amd64 part 8
  • Topotests debian 10 amd64 part 2
  • Addresssanitizer topotests part 9
  • IPv4 ldp protocol on Ubuntu 18.04
  • Topotests Ubuntu 18.04 arm8 part 0
  • Ubuntu 16.04 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 5
  • Topotests Ubuntu 18.04 amd64 part 9
  • Addresssanitizer topotests part 7
  • Ubuntu 20.04 deb pkg check
  • Topotests Ubuntu 18.04 amd64 part 5
  • Debian 10 deb pkg check
  • Fedora 29 rpm pkg check
  • Topotests Ubuntu 18.04 amd64 part 7
  • Topotests debian 10 amd64 part 1
  • Addresssanitizer topotests part 3
  • Topotests Ubuntu 18.04 amd64 part 4
  • CentOS 7 rpm pkg check
  • Addresssanitizer topotests part 2
  • Topotests Ubuntu 18.04 amd64 part 0
  • Topotests debian 10 amd64 part 5
  • Addresssanitizer topotests part 1
  • Topotests Ubuntu 18.04 amd64 part 1
  • Topotests debian 10 amd64 part 7
  • Debian 9 deb pkg check
  • Addresssanitizer topotests part 8
  • Topotests Ubuntu 18.04 amd64 part 6
  • Topotests Ubuntu 18.04 arm8 part 8
  • Ubuntu 18.04 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 6
  • Addresssanitizer topotests part 6
  • Topotests Ubuntu 18.04 arm8 part 1
  • Topotests debian 10 amd64 part 6

Warnings Generated during build:

Checkout code: Successful with additional warnings
Topotests Ubuntu 18.04 i386 part 3: Failed (click for details) Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO3U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 3: No useful log found
Topotests Ubuntu 18.04 i386 part 6: Failed (click for details) Topotests Ubuntu 18.04 i386 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO6U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 6: No useful log found
Topotests Ubuntu 18.04 i386 part 8: Failed (click for details) Topotests Ubuntu 18.04 i386 part 8: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO8U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 8: No useful log found
Topotests Ubuntu 18.04 i386 part 1: Failed (click for details) Topotests Ubuntu 18.04 i386 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO1U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 1: No useful log found
Topotests Ubuntu 18.04 i386 part 5: Failed (click for details) Topotests Ubuntu 18.04 i386 part 5: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO5U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 5: No useful log found
Topotests Ubuntu 18.04 i386 part 0: Failed (click for details) Topotests Ubuntu 18.04 i386 part 0: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO0U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 0: No useful log found
Topotests Ubuntu 18.04 i386 part 9: Failed (click for details) Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found
Topotests Ubuntu 18.04 i386 part 4: Failed (click for details) Topotests Ubuntu 18.04 i386 part 4: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO4U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 4: No useful log found
Topotests Ubuntu 18.04 i386 part 2: Failed (click for details) Topotests Ubuntu 18.04 i386 part 2: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO2U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 2: No useful log found
Topotests Ubuntu 18.04 i386 part 7: Failed (click for details) Topotests Ubuntu 18.04 i386 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4211/artifact/TOPO7U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 7: No useful log found 
Report for vrf.c | 6 issues
===============================================
< WARNING: braces {} are not necessary for single statement blocks
< #95: FILE: /tmp/f1-12615/vrf.c:95:
< WARNING: braces {} are not necessary for single statement blocks
< #112: FILE: /tmp/f1-12615/vrf.c:112:
< WARNING: braces {} are not necessary for single statement blocks
< #416: FILE: /tmp/f1-12615/vrf.c:416:

idryzhov
idryzhov requested changes Mar 18, 2022
View reviewed changes
Copy link
Contributor

@idryzhov idryzhov left a comment

Choose a reason for hiding this comment

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

I don't see any reason to make this part of the library. vrf_lookup_by_table is only ever used in zebra.

@nathan-duddles
Copy link
Author

Sorry, I'm not sure how to interpret these test failures. It looks like all TopoTests Ubuntu 18.04 i386 failed.

Running topotests locally in an Ubuntu 20.04 x86_64 VM, I'm only getting a single failing test, which occurs when run on both FRRouting:master and nathan-duddles:hashmap_for_vrf_lookup_by_table. Failing test is:
ospf_basic_functionality/test_ospf_chaos.py::test_ospf_chaos_tc34_p1

Also, it like some warnings were generated for formatting (e.g. trailing white spaces). I did see a linter test failing when initially creating the pull request and I made the recommended changes, amended the commit and force pushed. I guess these warnings reflect the code before those changes.

@nathan-duddles
Copy link
Author

nathan-duddles commented Mar 18, 2022 •
edited
Loading

Thank you for the comment @idryzhov . I had moved it to lib/vrf.c[h] based on the recommendation to do so in the GH issue: #5982 but I am new to contributing to FRR, so I will defer to others where it is best located. I'll just note that lib/vrf.c[h] is where similar functions are defined: vrf_lookup_by_id, vrf_lookup_by_name, vrf_id_to_name.

I believe it was originally added in zebra/rt_netlink.c because it is used there. It is also used in zebra/if_netlink.c. As I mentioned in the comments of issue, I think this function could also be called at pimd/pim_iface.c at line 1714, where lookup of vrf by table_id is currently done using the inefficient RB_FOREACH method.

@donaldsharp
Copy link
Member

bgpd is crashing. Here is some valgrind hints as to what is going wrong:

==884032== Conditional jump or move depends on uninitialised value(s)
==884032== at 0x4991A76: vrf_ids_by_table_const_find (vrf.c:83)
==884032== by 0x4991AEE: vrf_ids_by_table_find (vrf.c:83)
==884032== by 0x4991C6D: vrf_ids_by_table_delete (vrf.c:92)
==884032== by 0x4992369: vrf_delete (vrf.c:361)
==884032== by 0x4992A94: vrf_terminate_single (vrf.c:675)
==884032== by 0x4992B83: vrf_terminate (vrf.c:703)
==884032== by 0x1EB633: bgp_vrf_terminate (bgp_main.c:362)
==884032== by 0x1EB20C: bgp_exit (bgp_main.c:253)
==884032== by 0x1EB020: sigint (bgp_main.c:170)
==884032== by 0x4974A74: frr_sigevent_process (sigevent.c:130)
==884032== by 0x498DA98: thread_fetch (thread.c:1771)
==884032== by 0x49242CF: frr_run (libfrr.c:1195)
==884032== by 0x1EBB00: main (bgp_main.c:519)
==884032==
==884032== Conditional jump or move depends on uninitialised value(s)
==884032== at 0x4991AC3: vrf_ids_by_table_const_find (vrf.c:83)
==884032== by 0x4991AEE: vrf_ids_by_table_find (vrf.c:83)
==884032== by 0x4991C6D: vrf_ids_by_table_delete (vrf.c:92)
==884032== by 0x4992369: vrf_delete (vrf.c:361)
==884032== by 0x4992A94: vrf_terminate_single (vrf.c:675)
==884032== by 0x4992B83: vrf_terminate (vrf.c:703)
==884032== by 0x1EB633: bgp_vrf_terminate (bgp_main.c:362)
==884032== by 0x1EB20C: bgp_exit (bgp_main.c:253)
==884032== by 0x1EB020: sigint (bgp_main.c:170)
==884032== by 0x4974A74: frr_sigevent_process (sigevent.c:130)
==884032== by 0x498DA98: thread_fetch (thread.c:1771)
==884032== by 0x49242CF: frr_run (libfrr.c:1195)
==884032== by 0x1EBB00: main (bgp_main.c:519)
==884032==

@nathan-duddles
Copy link
Author

Thank you providing the hint and the Valgrind output!

It looks like I failed to call vrf_ids_by_table_init(&vrf_ids_by_table_head). I have added a flag/check on whether or not it is initialized, so that it is initialized the first time vrf_ids_by_table_update() is called. If there is a better way to do this, please do let me know!

As I'm not getting this failing case locally, is it possible to push changes to this branch and run the CI check again? Shall I amend and force push to the same commit?

@sworleys
Copy link
Member

I don't see any reason to make this part of the library. vrf_lookup_by_table is only ever used in zebra.

while currently only being used by zebra, the function makes sense to made general for VRFs and added to lib in my opinion. The API could be useful for other daemons. Pbr is one example that deals with both vrfs/table's directly.

sworleys
sworleys requested changes Mar 18, 2022
View reviewed changes
lib/vrf.c Outdated Show resolved Hide resolved
lib/vrf.c Outdated Show resolved Hide resolved
@sworleys
Copy link
Member

As I'm not getting this failing case locally, is it possible to push changes to this branch and run the CI check again? Shall I amend and force push to the same commit?

yes you can force-push and re-run CI. Do it sparingly though as it is a shared resource.

@nathan-duddles
Copy link
Author

Thank you for these changes! I made the updates, along with the check on the hash initialization, and pushed the new commit. It looks like CI is being rerun automatically.

@nathan-duddles nathan-duddles force-pushed the hashmap_for_vrf_lookup_by_table branch 2 times, most recently from 2107c09 to de68f39 Compare March 18, 2022 17:09
@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 18, 2022 •
edited
Loading

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/

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.

Get source / Pull Request: Successful

Building Stage: Failed

OpenBSD 7 amd64 build: Failed (click for details) OpenBSD 7 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/artifact/CI011BUILD/frr.xref.xz/frr.xref.xz OpenBSD 7 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/artifact/CI011BUILD/config.log/config.log.gz OpenBSD 7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/artifact/CI011BUILD/config.status/config.status

DejaGNU Unittests (make check) failed for OpenBSD 7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/artifact/CI011BUILD/ErrorLog/log_pytests.txt

Successful on other platforms/tests
  • Redhat 8 amd64 build
  • Debian 10 amd64 build
  • Fedora 29 amd64 build
  • FreeBSD 11 amd64 build
  • Debian 9 amd64 build
  • Ubuntu 20.04 amd64 build
  • Ubuntu 16.04 arm7 build
  • Ubuntu 18.04 ppc64le build
  • Ubuntu 18.04 amd64 build
  • FreeBSD 12 amd64 build
  • Ubuntu 16.04 arm8 build
  • Ubuntu 16.04 amd64 build
  • CentOS 7 amd64 build
  • Ubuntu 18.04 i386 build
  • NetBSD 9 amd64 build
  • Ubuntu 18.04 arm7 build
  • Ubuntu 18.04 arm8 build
  • Ubuntu 16.04 i386 build
  • Debian 11 amd64 build

Warnings Generated during build:

Checkout code: Successful with additional warnings
OpenBSD 7 amd64 build: Failed (click for details) OpenBSD 7 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/artifact/CI011BUILD/frr.xref.xz/frr.xref.xz OpenBSD 7 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/artifact/CI011BUILD/config.log/config.log.gz OpenBSD 7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/artifact/CI011BUILD/config.status/config.status

DejaGNU Unittests (make check) failed for OpenBSD 7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-PULLREQ2-4228/artifact/CI011BUILD/ErrorLog/log_pytests.txt

Report for vrf.c | 10 issues
===============================================
< WARNING: Block comments use a trailing */ on a separate line
< #105: FILE: /tmp/f1-5291/vrf.c:105:
< WARNING: braces {} are not necessary for single statement blocks
< #106: FILE: /tmp/f1-5291/vrf.c:106:
< WARNING: braces {} are not necessary for single statement blocks
< #130: FILE: /tmp/f1-5291/vrf.c:130:
< ERROR: do not initialise statics to 0
< #139: FILE: /tmp/f1-5291/vrf.c:139:
< WARNING: braces {} are not necessary for single statement blocks
< #434: FILE: /tmp/f1-5291/vrf.c:434:

@nathan-duddles nathan-duddles force-pushed the hashmap_for_vrf_lookup_by_table branch from de68f39 to 21577d0 Compare March 18, 2022 18:43
@nathan-duddles
Copy link
Author

Sorry, I didn't realize the issue with explicitly initializing a static variable to zero. Seems this was flagged as an ERROR by the OpenBSD 7 amd64. I've corrected this, along with other WARNINGS related to formatting and re-pushed.

@nathan-duddles nathan-duddles force-pushed the hashmap_for_vrf_lookup_by_table branch 2 times, most recently from 9dd3347 to 458100c Compare March 20, 2022 00:34
@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 20, 2022 •
edited
Loading

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4242/

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.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 i386 part 9: Failed (click for details) Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4242/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found
Successful on other platforms/tests
  • Topotests Ubuntu 18.04 amd64 part 1
  • Static analyzer (clang)
  • Addresssanitizer topotests part 1
  • Topotests Ubuntu 18.04 i386 part 4
  • Ubuntu 18.04 deb pkg check
  • Topotests Ubuntu 18.04 amd64 part 8
  • Topotests debian 10 amd64 part 2
  • Topotests debian 10 amd64 part 7
  • IPv4 protocols on Ubuntu 18.04
  • Topotests Ubuntu 18.04 arm8 part 0
  • Debian 9 deb pkg check
  • Topotests Ubuntu 18.04 amd64 part 6
  • Topotests Ubuntu 18.04 i386 part 8
  • Addresssanitizer topotests part 8
  • Topotests Ubuntu 18.04 amd64 part 9
  • Ubuntu 20.04 deb pkg check
  • Debian 10 deb pkg check
  • Addresssanitizer topotests part 5
  • Topotests Ubuntu 18.04 arm8 part 3
  • Topotests debian 10 amd64 part 1
  • Topotests debian 10 amd64 part 4
  • Addresssanitizer topotests part 4
  • Topotests Ubuntu 18.04 arm8 part 5
  • Addresssanitizer topotests part 0
  • Topotests debian 10 amd64 part 3
  • Topotests debian 10 amd64 part 5
  • Topotests Ubuntu 18.04 arm8 part 2
  • Topotests debian 10 amd64 part 0
  • Ubuntu 16.04 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 7
  • Topotests Ubuntu 18.04 i386 part 6
  • Addresssanitizer topotests part 9
  • Topotests Ubuntu 18.04 i386 part 1
  • IPv4 ldp protocol on Ubuntu 18.04
  • Addresssanitizer topotests part 7
  • Topotests Ubuntu 18.04 arm8 part 8
  • Topotests Ubuntu 18.04 arm8 part 1
  • Topotests Ubuntu 18.04 i386 part 7
  • Topotests Ubuntu 18.04 amd64 part 7
  • Topotests Ubuntu 18.04 arm8 part 6
  • Topotests Ubuntu 18.04 amd64 part 5
  • Topotests debian 10 amd64 part 6
  • Topotests Ubuntu 18.04 i386 part 2
  • Fedora 29 rpm pkg check
  • Addresssanitizer topotests part 6
  • Topotests Ubuntu 18.04 i386 part 5
  • Topotests Ubuntu 18.04 amd64 part 2
  • Addresssanitizer topotests part 3
  • Topotests Ubuntu 18.04 i386 part 0
  • Topotests Ubuntu 18.04 i386 part 3
  • Topotests Ubuntu 18.04 amd64 part 4
  • CentOS 7 rpm pkg check
  • Topotests debian 10 amd64 part 8
  • Topotests debian 10 amd64 part 9
  • Topotests Ubuntu 18.04 arm8 part 4
  • Topotests Ubuntu 18.04 amd64 part 3
  • Topotests Ubuntu 18.04 amd64 part 0
  • Topotests Ubuntu 18.04 arm8 part 9
  • Addresssanitizer topotests part 2
  • IPv6 protocols on Ubuntu 18.04

Warnings Generated during build:

Checkout code: Successful with additional warnings
Topotests Ubuntu 18.04 i386 part 9: Failed (click for details) Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4242/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found 
<stdin>:78: trailing whitespace.
	}	
<stdin>:244: trailing whitespace.
	
warning: 2 lines add whitespace errors.
Report for vrf.c | 14 issues
===============================================
< WARNING: line over 80 characters
< #114: FILE: /tmp/f1-10755/vrf.c:114:
< WARNING: Missing a blank line after declarations
< #114: FILE: /tmp/f1-10755/vrf.c:114:
< ERROR: space required before the open parenthesis '('
< #120: FILE: /tmp/f1-10755/vrf.c:120:
< WARNING: Block comments should align the * on each line
< #128: FILE: /tmp/f1-10755/vrf.c:128:
< ERROR: trailing whitespace
< #133: FILE: /tmp/f1-10755/vrf.c:133:
< WARNING: C99 // comments do not match recommendation
< #350: FILE: /tmp/f1-10755/vrf.c:350:
< ERROR: trailing whitespace
< #673: FILE: /tmp/f1-10755/vrf.c:673:

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 20, 2022 •
edited
Loading

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4243/

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.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details) Topotests Ubuntu 18.04 arm8 part 6: No useful log found
Successful on other platforms/tests
  • Static analyzer (clang)
  • IPv4 protocols on Ubuntu 18.04
  • Addresssanitizer topotests part 1
  • Topotests Ubuntu 18.04 i386 part 4
  • Topotests Ubuntu 18.04 amd64 part 1
  • Topotests Ubuntu 18.04 amd64 part 8
  • Topotests debian 10 amd64 part 2
  • Topotests debian 10 amd64 part 7
  • Topotests Ubuntu 18.04 arm8 part 0
  • Debian 9 deb pkg check
  • Addresssanitizer topotests part 8
  • Topotests Ubuntu 18.04 arm8 part 5
  • Topotests Ubuntu 18.04 amd64 part 6
  • Topotests Ubuntu 18.04 i386 part 8
  • Topotests Ubuntu 18.04 amd64 part 9
  • Ubuntu 18.04 deb pkg check
  • Ubuntu 20.04 deb pkg check
  • Addresssanitizer topotests part 6
  • Debian 10 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 3
  • Topotests debian 10 amd64 part 1
  • Addresssanitizer topotests part 5
  • Topotests debian 10 amd64 part 4
  • Addresssanitizer topotests part 4
  • Topotests debian 10 amd64 part 3
  • Addresssanitizer topotests part 0
  • Topotests Ubuntu 18.04 i386 part 9
  • Topotests debian 10 amd64 part 5
  • Topotests Ubuntu 18.04 arm8 part 2
  • Topotests debian 10 amd64 part 0
  • Topotests Ubuntu 18.04 arm8 part 7
  • Topotests Ubuntu 18.04 i386 part 6
  • Addresssanitizer topotests part 9
  • IPv4 ldp protocol on Ubuntu 18.04
  • Ubuntu 16.04 deb pkg check
  • Topotests Ubuntu 18.04 i386 part 1
  • Addresssanitizer topotests part 7
  • Topotests Ubuntu 18.04 arm8 part 8
  • Topotests Ubuntu 18.04 i386 part 2
  • Topotests Ubuntu 18.04 i386 part 7
  • Topotests Ubuntu 18.04 arm8 part 1
  • Topotests debian 10 amd64 part 6
  • Topotests Ubuntu 18.04 amd64 part 5
  • Fedora 29 rpm pkg check
  • Topotests Ubuntu 18.04 amd64 part 7
  • Topotests Ubuntu 18.04 amd64 part 2
  • Addresssanitizer topotests part 3
  • Topotests Ubuntu 18.04 i386 part 5
  • Topotests Ubuntu 18.04 i386 part 0
  • Topotests Ubuntu 18.04 i386 part 3
  • Topotests Ubuntu 18.04 amd64 part 4
  • CentOS 7 rpm pkg check
  • Topotests debian 10 amd64 part 8
  • Topotests debian 10 amd64 part 9
  • Topotests Ubuntu 18.04 arm8 part 9
  • Topotests Ubuntu 18.04 arm8 part 4
  • Addresssanitizer topotests part 2
  • IPv6 protocols on Ubuntu 18.04
  • Topotests Ubuntu 18.04 amd64 part 0
  • Topotests Ubuntu 18.04 amd64 part 3

Warnings Generated during build:

Checkout code: Successful with additional warnings
Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details) Topotests Ubuntu 18.04 arm8 part 6: No useful log found 
Report for vrf.c | 4 issues
===============================================
< WARNING: Missing a blank line after declarations
< #163: FILE: /tmp/f1-30231/vrf.c:163:
< WARNING: C99 // comments do not match recommendation
< #350: FILE: /tmp/f1-30231/vrf.c:350:

@nathan-duddles
Copy link
Author

This patch failed two tests for Ubuntu 18.04 arm8:
https://ci1.netdef.org/build/result/viewBuildResultsFailedTests.action?buildKey=FRR-PULLREQ2-TOPO6U18ARM8&buildNumber=4243
Which did pass on Ubuntu 18.04 amd64:
https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-4243/test/case/109021784
https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO6U18AMD64-4243/test/case/109021787

I'm sorry I haven't been able to figure out why these be failing on Ubuntu 18.04 arm8. I only have access to an amd64 machine and running all topotests on Ubuntu 18.04 is passing for me locally. (Running with valgrind shows possible memory leaks in unrelated modules and these same warnings appear when running on main without this patch.)

If anyone has any ideas why these tests may be failing or suggestions on how to debug this, I would really appreciate any feedback. Thanks!

@donaldsharp
Copy link
Member

those tests are not unknown to occassionally fail. Let me rerun

@donaldsharp
Copy link
Member

ci:rerun snmp test fialure?

@nathan-duddles
Copy link
Author

Ok, thanks! I also just noticed from the warning that there is a comment to myself that I should have removed. I'll fix this by amending the last commit.

@nathan-duddles nathan-duddles force-pushed the hashmap_for_vrf_lookup_by_table branch from 458100c to 15bbb2e Compare March 24, 2022 23:07
@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 25, 2022 •
edited
Loading

Continuous Integration Result: SUCCESSFUL

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-PULLREQ2-4368/

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.

Warnings Generated during build:

Checkout code: Successful with additional warnings 
Report for vrf.c | 4 issues
===============================================
< WARNING: Missing a blank line after declarations
< #163: FILE: /tmp/f1-22082/vrf.c:163:
< WARNING: C99 // comments do not match recommendation
< #350: FILE: /tmp/f1-22082/vrf.c:350:

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 25, 2022 •
edited
Loading

Continuous Integration Result: SUCCESSFUL

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-PULLREQ2-4369/

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.

Warnings Generated during build:

Checkout code: Successful with additional warnings 
<stdin>:60: trailing whitespace.
				
warning: 1 line adds whitespace errors.
Report for vrf.c | 4 issues
===============================================
< ERROR: trailing whitespace
< #115: FILE: /tmp/f1-30135/vrf.c:115:
< WARNING: Missing a blank line after declarations
< #161: FILE: /tmp/f1-30135/vrf.c:161:

@nathan-duddles
lib: Fix memory issues with hash map
24b7dc7 
Each entry in hash map must be allocated memory
on the heap.

In addition, much check that head is initialized
prior to calling _add() and _del() functions.

Also made corrections to compare and hash functions.

Fix formatting causing build warnings

Signed-off-by: Nathan Duddles <[email protected]>
@nathan-duddles nathan-duddles force-pushed the hashmap_for_vrf_lookup_by_table branch from 15bbb2e to 24b7dc7 Compare March 25, 2022 10:12
@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Mar 25, 2022 •
edited
Loading

Continuous Integration Result: SUCCESSFUL

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-PULLREQ2-4388/

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.

Warnings Generated during build:

Checkout code: Successful with additional warnings 
Report for vrf.c | 2 issues
===============================================
< WARNING: Missing a blank line after declarations
< #161: FILE: /tmp/f1-18129/vrf.c:161:

@sworleys
Copy link
Member

sworleys commented Apr 5, 2022

@nathan-duddles looks mostly good to me. I think the way you are checking for initialization is fine.

could you address the checkpatch warning?

you can recreate this locally by turning your patches into a single patch file, reverting the tree to before they are applied and doing:
./tools/checkpatch.sh ../my-patch.txt ./

it's the same script the linux kernel uses for style checking

Report for vrf.c | 2 issues
===============================================
< WARNING: Missing a blank line after declarations
< #161: FILE: /tmp/f1-18129/vrf.c:161:

idryzhov
idryzhov requested changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@idryzhov idryzhov left a comment

Choose a reason for hiding this comment

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

Once again, the functionality is daemon-specific.
And the current implementation only confirms this.
We can't use daemon-specific structs and headers in the library code.

lib/vrf.c Outdated Show resolved Hide resolved
lib/vrf.c Outdated Show resolved Hide resolved
@sworleys
Copy link
Member

sworleys commented Apr 5, 2022

Once again, the functionality is daemon-specific. And the current implementation only confirms this. We can't use daemon-specific structs and headers in the library code.

Hmm... yea @idryzhov is correct here. I didn't realize netns-backed vrfs breaks it from being shared knowledge in lib.

Let's just move the code to zebra/zebra_vrf.c[h] for now probably and leave it zebra specific to get the code merged.

@nathan-duddles
Copy link
Author

Thank you both for your review and comments on this! Good catch @idryzhov on the inclusion of zebra_vrf.

Actually, zebra_vrf functionality isn't needed; as I wasn't adding a hash method for netns-backed vrf, I just avoiding modify that, but I should have since moving it into vrf.c. The only zebra_vrf function used was zvrf_id(), which is simply returning vrf->vrf_id for the encapsulated vrf struct. I've done the refactor and removed the include of zebra_vrf and use of zvrf_id(). Am testing locally to make sure this still works and then will push a new commit.

@nathan-duddles
lib: Refactor to remove use of zebra_vrf
4c740b5 
Use of zebra_vrf was from when vrf_lookup_by_table()
was defined in rt_netlink. Use of zebra_vrf function:
zvrf_id() is not needed, as this just encapsulates
the return of vrf->vrf_id.

Also fixed formatting to pass checkpath warning
on line 161.

Signed-off-by: Nathan Duddles <[email protected]>
@NetDEF-CI
Copy link
Collaborator

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-PULLREQ2-4680/

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.

@nathan-duddles
Copy link
Author

When you have time, please take a look at the updates I've made and let me know if more changes are needed.

As I mentioned above, I think vrf_lookup_by_table() could also be called at pimd/pim_iface.c at line 1714, where lookup of vrf by table_id is currently done using the inefficient RB_FOREACH method.

If vrf_lookup_by_table() can be added to vrf.c in this pull request, I can make the update to pim_iface and submit another pull request for that.

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

This PR is stale because it has been open 180 days with no activity. Comment or remove the autoclose label in order to avoid having this PR closed.

@nathan-duddles
Copy link
Author

Hello! The autoclose tag reminded me about this pull request. I would love to get this reviewed and merged if you are still wanting the patch. Please let me know if there is anything else I can do on this. Thanks!

@github-actions github-actions bot removed the autoclose label Oct 8, 2022
@github-actions
Copy link

github-actions bot commented Apr 7, 2023

This PR is stale because it has been open 180 days with no activity. Comment or remove the autoclose label in order to avoid having this PR closed.

@github-actions github-actions bot added the rebase PR needs rebase label Sep 25, 2023
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

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

@idryzhov idryzhov Awaiting requested review from idryzhov

@sworleys sworleys Awaiting requested review from sworleys

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
conflicts libfrr rebase PR needs rebase zebra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nathan-duddles @NetDEF-CI @donaldsharp @sworleys @idryzhov @rzalamena