https://github.com/FRRouting/frr/pull/11465 enabled account verification,
but the pam config declares rootok as sufficient in authentication only
and not in account verification, what causes warning in the log:
Donald Sharp [Tue, 8 Nov 2022 19:38:02 +0000 (14:38 -0500)]
bgpd: rpki was decrementing the node lock one time too many
The code was this:
1) match = bgp_table_subtree_lookup(rrp->bgp->rib[rrp->afi][rrp->safi],
&rrp->prefix);
2) node = match;
while (node) {
if (bgp_dest_has_bgp_path_info_data(node)) {
revalidate_bgp_node(node, rrp->afi, rrp->safi);
}
3) node = bgp_route_next_until(node, match);
}
if (match)
4) bgp_dest_unlock_node(match);
At 1) match was locked and became +1
At 2) match and node are now equal
At 3) On first iteration, match is decremented( as that node points
at it ) and the next item is locked, if it is found, and returned which becomes node
If 3 is run again because node is non-null then, current node is decremented
and the next node found is incremented and returned which becomes node again.
So if we get to 4) match is unlocked again which is now a double unlock
which, frankly, is not good. In all code paths that I can see the
test for `if (match) ...` is not needed so let's just remove it.
zebra/netconf_netlink.c: In function 'netlink_netconf_change':
zebra/netconf_netlink.c:109:32: error: 'AF_MPLS' undeclared (first use in this function)
109 | if (ncm->ncm_family == AF_MPLS)
| ^~~~~~~
Donald Sharp [Tue, 8 Nov 2022 12:36:56 +0000 (07:36 -0500)]
bgpd: Make rpki soft_reconfig calling events
An end operator is showing cases with multiple bgp feeds
and a rpki table that calling the revalidation functions
is extremely expensive and they are seeing lots of thread
WARNS about timers being late and eventually the whole
thing gets unresponsive. Let's break up soft reconfiguration
in to a series of events per peer so that all the work
for this is not done at the same exact time.
ospf6d: Show if the interface is passive for `show ipv6 ospf6 interface`
donatas-pc# sh ipv6 ospf6 interface enp3s0
enp3s0 is up, type BROADCAST
Interface ID: 2
Internet Address:
inet : 192.168.10.17/24
inet6: fe80::ca5d:fd0d:cd8:1bb7/64
Instance ID 0, Interface MTU 1500 (autodetect: 1500)
MTU mismatch detection: enabled
Area ID 0.0.0.0, Cost 1000
State Waiting, Transmit Delay 1 sec, Priority 1
Timer intervals configured:
Hello 10(8.149), Dead 40, Retransmit 5
DR: 0.0.0.0 BDR: 0.0.0.0
Number of I/F scoped LSAs is 1
0 Pending LSAs for LSUpdate in Time 00:00:00 [thread off]
0 Pending LSAs for LSAck in Time 00:00:00 [thread off]
Authentication Trailer is disabled
donatas-pc# con
donatas-pc(config)# int enp3s0
donatas-pc(config-if)# ipv6 ospf6 passive
donatas-pc(config-if)# do sh ipv6 ospf6 interface enp3s0
enp3s0 is up, type BROADCAST
Interface ID: 2
Internet Address:
inet : 192.168.10.17/24
inet6: fe80::ca5d:fd0d:cd8:1bb7/64
Instance ID 0, Interface MTU 1500 (autodetect: 1500)
MTU mismatch detection: enabled
Area ID 0.0.0.0, Cost 1000
State Waiting, Transmit Delay 1 sec, Priority 1
Timer intervals configured:
No Hellos (Passive interface)
DR: 0.0.0.0 BDR: 0.0.0.0
Number of I/F scoped LSAs is 1
0 Pending LSAs for LSUpdate in Time 00:00:00 [thread off]
0 Pending LSAs for LSAck in Time 00:00:00 [thread off]
Authentication Trailer is disabled
donatas-pc(config-if)#
zebra: Reuse netinet/if_ether.h to avoid redefinition of struct ethhdr
In file included from /usr/include/net/ethernet.h:10,
from ./lib/prefix.h:26,
from zebra/tc_netlink.c:32:
/usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
115 | struct ethhdr {
| ^~~~~~
In file included from zebra/tc_netlink.c:28:
/usr/include/linux/if_ether.h:169:8: note: originally defined here
169 | struct ethhdr {
| ^~~~~~
Donald Sharp [Mon, 24 Oct 2022 13:25:54 +0000 (09:25 -0400)]
*: Add ability for daemons to notice resilience changes
This patch just introduces the callback mechanism for the
resilient nexthop changes so that upper level daemons
can take advantage of the change. This does nothing
at this point but just call some code.
Donald Sharp [Mon, 24 Oct 2022 18:08:35 +0000 (14:08 -0400)]
lib: When adding to front of list ensure we handle tail to
When inserting to the front of a list with listnode_add_head
if the list is empty, the tail will not be properly set and
subsuquent calls to insert/remove will cause the function
to crash.
Donald Sharp [Tue, 1 Nov 2022 12:00:14 +0000 (08:00 -0400)]
lib, zebra: Allow for zebra to recognize that a route has gotten desynced
FRR does not use the NLM_F_APPEND semantics ( in fact I would argue that
the NLM_F_APPEND semantics just introduce pain for all parties involved )
I would also argue that most people who use the kernel netlink api
have recognized that NLM_F_APPEND for a route is a recipe for disaster
that is well documented and as such it is not used as anything other
than a curiousity by operators.
Are 2 great examples of how confusing it is for anyone in user
space to know what the correct thing to do is. Given that
new fields can be added with no semantics to allow us to know
what has resulted in a change or not.
In an attempt to recognize this, let's note that FRR
believes it has gotten out of sync with the kernel.
Future commits will react to the desynchronized route
and request from the kernel a reload of that specific
route if possible.
Donald Sharp [Fri, 4 Nov 2022 00:39:39 +0000 (20:39 -0400)]
bgpd: Limit snmp trap for backwards state movement from established
Currently the bgp mib specifies two traps:
a) Into established state
b) transition backwards from a state
b) really is an interesting case. It means transitioning
from say established to starting over. It can also
mean when bgp is trying to connect and that fails and
the state transitions backwards.
Now let's imagine 500 peers with tight timers (say a data center)
and there is network trauma you have just created an inordinately
large number of traps for each peer.
Let's limit FRR to changing from the old status as Established
to something else. This will greatly limit the trap but it
will also be something end operators are actually interested in.
I actually had several operators say they had to write special code
to ignore all the backward state transitions that they didn't care
about.
Donald Sharp [Wed, 2 Nov 2022 17:24:48 +0000 (13:24 -0400)]
bgpd: Ensure that bgp open message stream has enough data to read
If a operator receives an invalid packet that is of insufficient size
then it is possible for BGP to assert during reading of the packet
instead of gracefully resetting the connection with the peer.
anlan_cs [Mon, 31 Oct 2022 10:14:07 +0000 (06:14 -0400)]
include: remove one unused macro
In "rtnetlink.h", four items are a group, e.g. 116/117/118/119 should be
a group. But "RTM_SETHWFLAGS" is not in use and has nothing to do with
"NEXTHOPBUCKET".
After comparing with kernel header, better remove it imo.
`srv6_locator_chunk_free()` is a wrapper around the `XFREE()` macro.
Passing a NULL pointer to `XFREE()` is safe. Therefore, checking that
the pointer passed to the `srv6_locator_chunk_free()` is not null is
unnecessary.
`srv6_locator_chunk_free()` takes care of freeing the memory allocated
for a `struct srv6_locator_chunk` and setting the
`struct srv6_locator_chunk` pointer to NULL.
It is not necessary to explicitly set the pointer to NULL after invoking
`srv6_locator_chunk_free()`.
lib, bgpd: Enhance `srv6_locator_chunk_free()` API
A programmer can use the `srv6_locator_chunk_free()` function to free
the memory allocated for a `struct srv6_locator_chunk`.
The programmer invokes `srv6_locator_chunk_free()` by passing a single
pointer to the `struct srv6_locator_chunk` to be freed.
`srv6_locator_chunk_free()` uses `XFREE()` to free the memory.
It is the responsibility of the programmer to set the
`struct srv6_locator_chunk` pointer to NULL after freeing memory with
`srv6_locator_chunk_free()`.
This commit modifies the `srv6_locator_chunk_free()` function to take a
double pointer instead of a single pointer. In this way, setting the
`struct srv6_locator_chunk` pointer to NULL is no longer the
programmer's responsibility but is the responsibility of
`srv6_locator_chunk_free()`. This prevents programmers from making
mistakes such as forgetting to set the pointer to NULL after invoking
`srv6_locator_chunk_free()`.