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

speculative fix to 32bit failures: set vrf_id_t to 64 bits #1876

Closed

Conversation

louberger
Copy link
Member

No description provided.

@LabN-CI
Copy link
Collaborator

LabN-CI commented Mar 13, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/1876 a44db78
Date 03/12/2018
Start 20:17:15
Finish 20:40:25
Run-Time 23:10
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-03-12-20:17:15.txt
Log autoscript-2018-03-12-20:17:55.log.bz2

For details, please contact louberger

@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-FRRPULLREQ-2859/

This is a comment from an EXPERIMENTAL 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 bgp_zebra.c
===============================================
< WARNING: line over 80 characters
< #419: FILE: /tmp/f1/bgp_zebra.c:419:
< +		zlog_debug("Rx Intf neighbor add VRF %llu IF %s addr %s", vrf_id,
< 
--
< WARNING: line over 80 characters
< #448: FILE: /tmp/f1/bgp_zebra.c:448:
< +		zlog_debug("Rx Intf neighbor del VRF %llu IF %s addr %s", vrf_id,
< 
< WARNING: line over 80 characters
< #480: FILE: /tmp/f1/bgp_zebra.c:480:
< +		zlog_debug("Rx Intf VRF change VRF %llu IF %s NewVRF %llu", vrf_id,
< 
--
< WARNING: line over 80 characters
< #1184: FILE: /tmp/f1/bgp_zebra.c:1184:
< +		zlog_debug("Tx route %s VRF %llu %s metric %u tag %" ROUTE_TAG_PRI
< 
< WARNING: line over 80 characters
< #1281: FILE: /tmp/f1/bgp_zebra.c:1281:
< +		zlog_debug("Tx route delete VRF %llu %s", peer->bgp->vrf_id, buf);
< 
Report for if.h
===============================================
< WARNING: line over 80 characters
< #302: FILE: /tmp/f1/if.h:302:
< +			"name exists already in VRF %llu!",                      \
< 
--
< WARNING: line over 80 characters
< #309: FILE: /tmp/f1/if.h:309:
< +			"name doesn't exist in VRF %llu!",                       \
< 
--
< WARNING: line over 80 characters
< #316: FILE: /tmp/f1/if.h:316:
< +			"ifindex exists already in VRF %llu!",                   \
< 
--
< WARNING: line over 80 characters
< #323: FILE: /tmp/f1/if.h:323:
< +			"ifindex doesn't exist in VRF %llu!",                    \
< 
Report for ospf_zebra.c
===============================================
< WARNING: line over 80 characters
< #78: FILE: /tmp/f1/ospf_zebra.c:78:
< +		zlog_debug("Zebra rcvd: router id update %s vrf %s id %llu", buf,
< 
Report for zclient.c
===============================================
< WARNING: line over 80 characters
< #488: FILE: /tmp/f1/zclient.c:488:
---
> #488: FILE: /tmp/f2/zclient.c:488:
Report for zebra.h
===============================================
< WARNING: do not add new typedefs
< #499: FILE: /tmp/f1/zebra.h:499:
< +typedef unsigned long long vrf_id_t;
< 
Report for zebra_pw.c
===============================================
< WARNING: line over 80 characters
< #93: FILE: /tmp/f1/zebra_pw.c:93:
< +		zlog_debug("%llu: deleting pseudowire %s protocol %s", pw->vrf_id,
< 
Report for zebra_rnh.c
===============================================
< WARNING: line over 80 characters
< #193: FILE: /tmp/f1/zebra_rnh.c:193:
< +		zlog_debug("%llu: Client %s registers for RNH %s type %d", vrf_id,
---
--
< WARNING: line over 80 characters
< #771: FILE: /tmp/f1/zebra_rnh.c:771:
< +		zlog_debug("%llu:%s: Evaluate RNH, type %d %s", vrfid, bufn, type,
< 
Report for zebra_vxlan.c
===============================================
< WARNING: line over 80 characters
< #6697: FILE: /tmp/f1/zebra_vxlan.c:6697:
< +		zlog_err("EVPN VNI Adv for non-default VRF %llu", zvrf_id(zvrf));
< 

CLANG Static Analyzer Summary

  • Github Pull Request 1876, comparing to Git base SHA 9d16566

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

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

Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: Lou Berger <lberger@labn.net>
Signed-off-by: Lou Berger <lberger@labn.net>
@louberger louberger force-pushed the working/master/fix-32bit-vrfs branch from a44db78 to 5255858 Compare March 13, 2018 01:28
@louberger louberger requested a review from donaldsharp March 13, 2018 01:28
@FRRouting FRRouting deleted a comment from NetDEF-CI Mar 13, 2018
@FRRouting FRRouting deleted a comment from LabN-CI Mar 13, 2018
@louberger
Copy link
Member Author

@mwinter-osr @rwestphal this seems to fix the 32 bit issues.

@louberger
Copy link
Member Author

louberger commented Mar 13, 2018

@donaldsharp this is a bit of a brute force solution to this issue. I have no idea if 64bit vrf_id_t is really needed or not... but it works

@donaldsharp
Copy link
Member

Can we get a bit more explanation about what is going on? It's not clear at all.

Additionally this change will clearly break passing of the vrf id through zapi. And this should be fixed.

@LabN-CI
Copy link
Collaborator

LabN-CI commented Mar 13, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/1876 5255858
Date 03/12/2018
Start 21:32:14
Finish 21:55:30
Run-Time 23:16
Total 1816
Pass 1816
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-03-12-21:32:14.txt
Log autoscript-2018-03-12-21:32:53.log.bz2

For details, please contact louberger

@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-FRRPULLREQ-2860/

This is a comment from an EXPERIMENTAL 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 zebra.h
===============================================
< WARNING: do not add new typedefs
< #499: FILE: /tmp/f1/zebra.h:499:
< +typedef unsigned long long vrf_id_t;
< 

CLANG Static Analyzer Summary

  • Github Pull Request 1876, comparing to Git base SHA fac615f

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

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

@pguibert6WIND pguibert6WIND self-requested a review March 13, 2018 09:16
@pguibert6WIND
Copy link
Member

I am also against that solution.
I will look at the i386 issue

@pguibert6WIND
Copy link
Member

from first observations, it seems netns from kernel behaves strangely on i386.
Don't know yet if this is due to my setup or not, but what is dumped here looks similar to what I observe.

to make it simple, setns() is not handled.
consequently: zebra_vrf is not enabled , but vrf library sees it as enabled.

as first patch, I will try to enforce the vrf library by enabling it only if setns() returns ok.

@louberger louberger deleted the working/master/fix-32bit-vrfs branch July 30, 2018 14:41
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.

5 participants