Firstly, *keep no change* for `hash_get()` with NULL
`alloc_func`.
Only focus on cases with non-NULL `alloc_func` of
`hash_get()`.
Since `hash_get()` with non-NULL `alloc_func` parameter
shall not fail, just ignore the returned value of it.
The returned value must not be NULL.
So in this case, remove the unnecessary checking NULL
or not for the returned value and add `void` in front
of it.
Importantly, also *keep no change* for the two cases with
non-NULL `alloc_func` -
1) Use `assert(<returned_data> == <searching_data>)` to
ensure it is a created node, not a found node.
Refer to `isis_vertex_queue_insert()` of isisd, there
are many examples of this case in isid.
2) Use `<returned_data> != <searching_data>` to judge it
is a found node, then free <searching_data>.
Refer to `aspath_intern()` of bgpd, there are many
examples of this case in bgpd.
Here, <returned_data> is the returned value from `hash_get()`,
and <searching_data> is the data, which is to be put into
hash table.
Signed-off-by: anlan_cs <vic.lan@pica8.com>
Don't rely on the OS interface name length definition and use the FRR
definition instead.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Passing NULL for a `%pTVMs` would result in `(null)Ms`, i.e. the `Ms`
flags not eaten up. Change to eat those up, and print `-` instead for
NULL times.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
There's a common pattern of "get VRF context for CLI node" here, which
first got a helper macro in zebra that then permeated into pimd.
Unfortunately the pimd copy wasn't quite adjusted correctly and thus
caused two coverity warnings (CID 1517453, CID 1517454).
Fix the PIM one, and clean up by providing a common base macro in
`lib/vty.h`.
Also rename the macros (add `_VRF`) to make more clear what they do.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
By changing this API call to use a `struct ipaddr`, which encodes the
type of IP address with it. (And rename/remove the `IPV4` from the
command name.)
Also add a comment explaining that this function call is going to be
obsolete in the long run since pimd needs to move to proper MRIB NHT.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
If duplicate value is entered, the whole plist/alist just dropped.
Before:
```
$ grep prefix-list /etc/frr/frr.conf
ip prefix-list test seq 5 permit 1.1.1.1/32
ip prefix-list test seq 10 permit 1.1.1.1/32
$ systemctl restart frr
$ vtysh -c 'show run | include prefix-list'
$
```
After:
```
$ grep prefix-list /etc/frr/frr.conf
ip prefix-list test seq 5 permit 1.1.1.1/32
ip prefix-list test seq 10 permit 1.1.1.1/32
$ systemctl restart frr
$ vtysh -c 'show run | include prefix-list'
ip prefix-list test seq 5 permit 1.1.1.1/32
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
End operator is showing:
!
frr version 8.0.1
frr defaults traditional
hostname test.example.com
domainname
domainname should not be printed in this case at all. I do not
see any mechanism in current code that this could happen, but
what do I know? Put some extra stupid insurance in place
to prevent bad config from being generated.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Recent commit e92508a741 changed
the prefix_master->str to a RB tree. This introduced a condition
whnere on shutdown the prefix list was removed from the master list
and then operated on by passing around a name. Which was then used
to lookup the prefix list again when we operated on the code.
This change to a RB Tree first deleted the item from the RB tree
first thus introducing this crash
Crash:
(gdb) bt
index=0x556c07d59650, pentry=0x556c07d29380) at lib/routemap.c:2397
arg=0x7ffdbf84bc60) at lib/hash.c:267
event=RMAP_EVENT_PLIST_DELETED) at lib/routemap.c:2489
Grab the first item on the list, clean it and then remove it.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Just simple helpers to get a scope value, never-forward, and is-SSM for
a given address.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This has already been a requirement for Solaris, it is still a
requirement for some of the autoconf feature checks to work correctly,
and it will be a requirement for `-fms-extensions`.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The commands:
router isis 1
mpls-te on
no mpls-te on
mpls-te on
no mpls-te on
!
Will crash
Valgrind gives us this:
==652336== Invalid read of size 8
==652336== at 0x49AB25C: typed_rb_min (typerb.c:495)
==652336== by 0x4943B54: vertices_const_first (link_state.h:424)
==652336== by 0x493DCE4: vertices_first (link_state.h:424)
==652336== by 0x493DADC: ls_ted_del_all (link_state.c:1010)
==652336== by 0x47E77B: isis_instance_mpls_te_destroy (isis_nb_config.c:1871)
==652336== by 0x495BE20: nb_callback_destroy (northbound.c:1131)
==652336== by 0x495B5AC: nb_callback_configuration (northbound.c:1356)
==652336== by 0x4958127: nb_transaction_process (northbound.c:1473)
==652336== by 0x4958275: nb_candidate_commit_apply (northbound.c:906)
==652336== by 0x49585B8: nb_candidate_commit (northbound.c:938)
==652336== by 0x495CE4A: nb_cli_classic_commit (northbound_cli.c:64)
==652336== by 0x495D6C5: nb_cli_apply_changes_internal (northbound_cli.c:250)
==652336== Address 0x6f928e0 is 272 bytes inside a block of size 320 free'd
==652336== at 0x48399AB: free (vg_replace_malloc.c:538)
==652336== by 0x494BA30: qfree (memory.c:141)
==652336== by 0x493D99D: ls_ted_del (link_state.c:997)
==652336== by 0x493DC20: ls_ted_del_all (link_state.c:1018)
==652336== by 0x47E77B: isis_instance_mpls_te_destroy (isis_nb_config.c:1871)
==652336== by 0x495BE20: nb_callback_destroy (northbound.c:1131)
==652336== by 0x495B5AC: nb_callback_configuration (northbound.c:1356)
==652336== by 0x4958127: nb_transaction_process (northbound.c:1473)
==652336== by 0x4958275: nb_candidate_commit_apply (northbound.c:906)
==652336== by 0x49585B8: nb_candidate_commit (northbound.c:938)
==652336== by 0x495CE4A: nb_cli_classic_commit (northbound_cli.c:64)
==652336== by 0x495D6C5: nb_cli_apply_changes_internal (northbound_cli.c:250)
==652336== Block was alloc'd at
==652336== at 0x483AB65: calloc (vg_replace_malloc.c:760)
==652336== by 0x494B6F8: qcalloc (memory.c:116)
==652336== by 0x493D7D2: ls_ted_new (link_state.c:967)
==652336== by 0x47E4DD: isis_instance_mpls_te_create (isis_nb_config.c:1832)
==652336== by 0x495BB29: nb_callback_create (northbound.c:1034)
==652336== by 0x495B547: nb_callback_configuration (northbound.c:1348)
==652336== by 0x4958127: nb_transaction_process (northbound.c:1473)
==652336== by 0x4958275: nb_candidate_commit_apply (northbound.c:906)
==652336== by 0x49585B8: nb_candidate_commit (northbound.c:938)
==652336== by 0x495CE4A: nb_cli_classic_commit (northbound_cli.c:64)
==652336== by 0x495D6C5: nb_cli_apply_changes_internal (northbound_cli.c:250)
==652336== by 0x495D23E: nb_cli_apply_changes (northbound_cli.c:268)
Let's null out the pointer. After this change. Valgrind no longer reports issues
and isisd no longer crashes.
Fixes: #10939
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
These 3 values:
ONE_DAY_SECOND
ONE_WEEK_SECOND
ONE_YEAR_SECOND
Are defined based upon the number of seconds. Unfortunately doing math
on these values say something like:
days = t->tv_sec / ONE_DAY_SECOND;
Once you go over about a day causes the order of operations to cause the multiplication
to get messed up:
204 if (!t)
(gdb) n
207 w = d = h = m = ms = 0;
(gdb) set t->tv_sec = ONE_DAY_SECOND + 30
(gdb) n
208 memset(buf, 0, size);
(gdb)
210 us = t->tv_usec;
(gdb)
211 if (us >= 1000) {
(gdb)
212 ms = us / 1000;
(gdb)
213 us %= 1000;
(gdb)
217 if (ms >= 1000) {
(gdb)
222 if (t->tv_sec > ONE_WEEK_SECOND) {
(gdb)
227 if (t->tv_sec > ONE_DAY_SECOND) {
(gdb)
228 d = t->tv_sec / ONE_DAY_SECOND;
(gdb) n
229 t->tv_sec -= d * ONE_DAY_SECOND;
(gdb) n
232 if (t->tv_sec >= HOUR_IN_SECONDS) {
(gdb) p d
$6 = 2073600
(gdb) p t->tv_sec
$7 = -179158953570
(gdb)
Converting to adding paranthesis around around the ONE_DAY_SECOND causes
the order of operations to work as expected.
Fixes: #10880
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When using zlog_backtrace I am seeing this:
==66286== Syscall param write(buf) points to uninitialised byte(s)
==66286== at 0x4CDF48A: syscall (in /lib/libc.so.7)
==66286== by 0x4A0D409: ??? (in /usr/local/lib/libunwind.so.8.0.1)
==66286== by 0x4A0D694: ??? (in /usr/local/lib/libunwind.so.8.0.1)
==66286== by 0x4A0E2F4: _ULx86_64_step (in /usr/local/lib/libunwind.so.8.0.1)
==66286== by 0x49662DB: zlog_backtrace (log.c:250)
==66286== by 0x2AFFA6: if_get_mtu (ioctl.c:163)
==66286== by 0x2B2D9D: ifan_read (kernel_socket.c:457)
==66286== by 0x2B2D9D: kernel_read (kernel_socket.c:1406)
==66286== by 0x499F46E: thread_call (thread.c:2002)
==66286== by 0x495D2B7: frr_run (libfrr.c:1196)
==66286== by 0x2B4098: main (main.c:471)
==66286== Address 0x7fc000000 is on thread 1's stack
==66286== in frame #4, created by zlog_backtrace (log.c:239)
==66286==
Let's initialize some data
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When `terminal monitor` is issued I am seeing this for valgrind on freebsd:
2022/03/24 18:07:45 ZEBRA: [RHJDG-5FNSK][EC 100663304] can't open configuration file [/usr/local/etc/frr/zebra.conf]
==52993== Syscall param sendmsg(sendmsg.msg_control) points to uninitialised byte(s)
==52993== at 0x4CE268A: _sendmsg (in /lib/libc.so.7)
==52993== by 0x4B96245: ??? (in /lib/libthr.so.3)
==52993== by 0x4CDF329: sendmsg (in /lib/libc.so.7)
==52993== by 0x49A9994: vtysh_do_pass_fd (vty.c:2041)
==52993== by 0x49A9994: vtysh_flush (vty.c:2070)
==52993== by 0x499F4CE: thread_call (thread.c:2002)
==52993== by 0x495D317: frr_run (libfrr.c:1196)
==52993== by 0x2B4068: main (main.c:471)
==52993== Address 0x7fc000864 is on thread 1's stack
==52993== in frame #3, created by vtysh_flush (vty.c:2065)
Fix by initializing the memory to `0`
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The EAD-per-ES route carries ECs for all the ES-EVI RTs. As the number of VNIs
increase all RTs do not fit into a standard BGP UPDATE (4K) so the route needs
to be fragmented.
Each fragment is associated with a separate RD and frag-id -
1. Local ES-per-EAD -
ES route table - {ES-frag-ID, ESI, ET=0xffffffff, VTEP-IP}
global route table - {RD-=ES-frag-RD, ESI, ET=0xffffffff}
2. Remote ES-per-EAD -
VNI route table - {ESI, ET=0xffffffff, VTEP-IP}
global route table - {RD-=ES-frag-RD, ESI, ET=0xffffffff}
Note: The fragment ID is abandoned in the per-VNI routing table. At this
point that is acceptable as we dont expect more than one-ES-per-EAD fragment
to be imported into the per-VNI routing table. But that may need to be
re-worked at a later point.
CLI changes (sample with 4 VNIs per-fragment for experimental pruposes) -
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
root@torm-11:mgmt:~# vtysh -c "show bgp l2vpn evpn es 03:44:38:39:ff:ff:01:00:00:01"
ESI: 03:44:38:39:ff:ff:01:00:00:01
Type: LR
RD: 27.0.0.21:3
Originator-IP: 27.0.0.21
Local ES DF preference: 50000
VNI Count: 10
Remote VNI Count: 10
VRF Count: 3
MACIP EVI Path Count: 33
MACIP Global Path Count: 198
Inconsistent VNI VTEP Count: 0
Inconsistencies: -
Fragments: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
27.0.0.21:3 EVIs: 4
27.0.0.21:13 EVIs: 4
27.0.0.21:22 EVIs: 2
VTEPs:
27.0.0.22 flags: EA df_alg: preference df_pref: 32767
27.0.0.23 flags: EA df_alg: preference df_pref: 32767
root@torm-11:mgmt:~# vtysh -c "show bgp l2vpn evpn es-evi vni 1002 detail"
VNI: 1002 ESI: 03:44:38:39:ff:ff:01:00:00:01
Type: LR
ES fragment RD: 27.0.0.21:13 >>>>>>>>>>>>>>>>>>>>>>>>>
Inconsistencies: -
VTEPs: 27.0.0.22(EV),27.0.0.23(EV)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
PS: The number of EVIs per-fragment has been set to 128 and may need further
tuning.
Ticket: #2632967
Signed-off-by: Anuradha Karuppiah <anuradhak@nvidia.com>
The wheel data structure is a array of list pointers
but the alloc for it is using the sizeof (struct listnode *)
as the amount to allocate. Even though the (struct listnode *)
and (struct list *) sizes are the same, let's list the correct
values.
Signed-off-by: ron <lyq140hf2006@163.com>
