This fixes a theoretical bug that could occur when trying to change an
ifindex on an interface to that of an existing interface. We would
remove the interface from the ifindex tree, and change the ifindex, but
when we tried to reinsert the interface, the insert would fail. It was
impossible to know if this failed due to the insertion / deletion macros
capturing the result value of the underlying BSD tree macros. So we
would effectively delete the interface.
Instead of failing on insert, we just check if the prospective ifindex
already exists and return -1 if it does.
Macros have been changed to statement expressions so the result can be
checked, and bubbled up.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
* Don't crash if we get a request to create an existing VRF
* Ensure interface & vrf names are null terminated...again
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
If Zebra sends us an interface add notification with a garbage VRF we
crash on an assert(vrf_get(vrf_id, NULL)); let's not
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
In some places we log the interface but not the vfr the
interface is in. In others we only output the vrf id, which
can be difficult for human to read. This commit makes zebra
debugs more vrf aware.
Signed-off-by: Jakub Urbańczyk <xthaid@gmail.com>
Use more limited matching logic so that nexthops within a
nexthop-group are unique based only on vrf, type, and gateway.
Treat configuration of a nexthop that matches an existing
nexthop as a replace operation.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Old gcc versions (< 5.x) have a bug that prevents C99 flexible
arrays from working properly on shared libraries.
We already have a hack in place to work around this problem, but it
needs to be replicated in every declaration of a frr_yang_module_info
variable within libfrr. This clearly isn't a good solution if we
consider that many more libfrr YANG modules are about to come in
the future.
This commit introduces a different workaround that operates within
the northbound layer itself, such that implementers of libfrr YANG
modules won't need to worry about this problem anymore.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Our two northbound tools don't have embedded YANG modules like the
other FRR binaries. As such, ly_ctx_set_module_imp_clb() shouldn't be
called when the YANG subsystem it being initialized by a northbound
tool. To make that possible, add a new "embedded_modules" parameter
to the yang_init() function to control whether libyang should look
for embedded modules or not.
With this fix, "gen_northbound_callbacks" and "gen_yang_deviations"
won't emit "YANG model X not embedded, trying external file"
warnings anymore.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
hook_register() invocations generally are in some initialization
function and not looped over or similar. We can use a static struct
hookent variable for the hook list entry in 99.999% of cases, so let's
do that and not malloc() memory.
Signed-off-by: David Lamparter <equinox@diac24.net>
This is most of the old code bolted on top of the new "backend"
infrastructure. It just wraps around zlog_fd() with the string search.
Originally-by: Stephen Worley <sworley@cumulusnetworks.com>
Signed-off-by: David Lamparter <equinox@diac24.net>
In some cases we really don't want to clean up things even when exiting
(i.e. to keep the logging subsystem going.) This adds a flag on MGROUPs
to indicate that.
[v2: add "(active at exit)" marker text to debug memstats-at-exit]
Signed-off-by: David Lamparter <equinox@diac24.net>
This is a full rewrite of the "back end" logging code. It now uses a
lock-free list to iterate over logging targets, and the targets
themselves are as lock-free as possible. (syslog() may have a hidden
internal mutex in the C library; the file/fd targets use a single
write() call which should ensure atomicity kernel-side.)
Note that some functionality is lost in this patch:
- Solaris printstack() backtraces are ditched (unlikely to come back)
- the `log-filter` machinery is gone (re-added in followup commit)
- `terminal monitor` is temporarily stubbed out. The old code had a
race condition with VTYs going away. It'll likely come back rewritten
and with vtysh support.
- The `zebra_ext_log` hook is gone. Instead, it's now much easier to
add a "proper" logging target.
v2: TLS buffer to get some actual performance
Signed-off-by: David Lamparter <equinox@diac24.net>
When using the GRPC northbound plugin, initialization occurs at the
frr_late_init hook. This is called before fork() when daemonizing (using
-d). Because the GRPC library internally creates threads, this means our
threads go away in the child process, so GRPC doesn't work when used
with -d. Rectify this situation by deferring plugin init to after fork
by scheduling a task on the threadmaster, since those are executed by
the child.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Just a small hack to use printfrr() in tests, since otherwise the
redefined PRId64 trips some warnings.
Signed-off-by: David Lamparter <equinox@diac24.net>
Use const with some args to ipaddr, zebra vxlan, mpls
lsp, and nexthop apis; add some extra checks to some
nexthop-related apis.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
RCA:
when client is killed, show running-config command crashes vtysh.
vtysh_client_config function temporarily makes vty->of which is standard output file
pointer to null inorder to suppress output to user.
This call further tries to communicate with each client and when the client
is terminated, socket call fails and hits the exception path to print the
connection has failed using vty_out.
vty_out crashes because vtysh_client_config has temporarily made vty->of
pointer to NULL to supress o/p to user.
Fix:
vty_out function should check for the sanity of vty->of pointer.
If it doesn't exist, this must have hit exception path, so use the
vty->saved_of if exists.
Signed-off-by: Saravanan K <saravanank@vmware.com>
The old version was creating a multi-line log message, which we can't
properly handle right now.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Adapt the zebra route map code to use the transaction CLI output (e.g.
the CLI show callbacks) instead of the fallback compatibility.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Modify code to use lookup function agg_node_get_prefix()
as the abstraction layer. When we rework bgp_node to
bgp_dest this will allow us to greatly limit the amount
of work needed to do that.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Zebra is currently sending messages on interface add/delete/update,
VRF add/delete, and interface address change - regardless of whether
its clients had requested them. This is problematic for lde and isis,
which only listens to label chunk messages, and only when it is
waiting for one (synchronous client). The effect is the that messages
accumulate on the lde synchronous message queue.
With this change:
- Zebra does not send unsolicited messages to synchronous clients.
- Synchronous clients send a ZEBRA_HELLO to zebra.
The ZEBRA_HELLO contains a new boolean field: sychronous.
- LDP and PIM have been updated to send a ZEBRA_HELLO for their
synchronous clients.
Signed-off-by: Karen Schoener <karen@voltanet.io>
More second order effects of cleaning up rn usage
in bgp. Sprinkle the fairy const's all over the place.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Tell the compiler that the prefix is being used for lookups
and it will never change.
Setup for future work.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
GCC 10 thinks we memcpy into a 0-sized array (which we're not).
Use a C99 flexible array member instead.
Fixes:
CC lib/stream.lo
lib/stream.c: In function ‘stream_put_in_addr’:
lib/stream.c:824:2: warning: writing 4 bytes into a region of size 0 [-Wstringop-overflow=]
824 | memcpy(s->data + s->endp, addr, sizeof(uint32_t));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
isisd/isis_tlvs.c: In function ‘auth_validator_hmac_md5’:
isisd/isis_tlvs.c:4279:2: warning: writing 16 bytes into a region of size 0 [-Wstringop-overflow=]
4279 | memcpy(STREAM_DATA(stream) + auth->offset, auth->value, 16);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘update_auth_hmac_md5’,
inlined from ‘update_auth’ at isisd/isis_tlvs.c:3734:4,
inlined from ‘isis_pack_tlvs’ at isisd/isis_tlvs.c:3897:2:
isisd/isis_tlvs.c:3722:2: warning: writing 16 bytes into a region of size 0 [-Wstringop-overflow=]
3722 | memcpy(STREAM_DATA(s) + auth->offset, digest, 16);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
isisd/isis_tlvs.c:3722:2: warning: writing 16 bytes into a region of size 0 [-Wstringop-overflow=]
Signed-off-by: Ruben Kerkhof <ruben@rubenkerkhof.com>
Add a common api that formats a time interval into a string
with different output for short and longer intervals. We do
this in several places, for cli/ui output.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
This patch does two things:
1) Ensure the decoding of stream data between pim <-> zebra is properly
decoded and we don't read beyond the end of the stream.
2) In zebra when we are freeing memory alloced ensure that we
actually have memory to delete before we do so.
Ticket: CM-27055
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
It's been a year search and destroy.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
vtysh should handle going back up one level to try the command, there is
no need to be able to recurse inside route-map.
This also fixes a problem with northbound hitting the XPath queue limit
of 8 nodes.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Use better TAILQ free idiom to avoid coverity scan warnings. This fixes
the coverity scan issue 1491240 .
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
* This commit implements the code style suggestions from Polychaeta.
* This commit also introduces a CLI to toggle the optimization and, a hidden
CLI to display the contents of the constructed prefix tree.
Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
This commit introduces the logic that computes the best-match route-map index
for a given prefix.
Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
* This commit introduces the building blocks.
A per-route-map prefix tree is introduced.
This tree will consist of the prefixes defined within the prefix-lists
that are added to the match clause of that route-map.
Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
The `--enable-pcreposix` configure option was not actually compiling
properly. Follow pre-existing pattern for inclusion of regex.h
or the pcreposix.h header.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Use the zapi_nexthop struct with the mpls_labels
zapi messages instead of the special-purpose (and
more limited) nexthop struct that was being used.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
This string is used in some logging for e.g. in zclient_read -
>>>>>>>>>>>>>>>>>>>>>>>>>>
if (zclient_debug)
zlog_debug("zclient 0x%p command %s VRF %u",
(void *)zclient, zserv_command_string(command),
vrf_id);
>>>>>>>>>>>>>>>>>>>>>>>>>>
Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
Add some additional output/debug to code to allow
us to see the vrf name instead of just the vrf id.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Add a quick macro to allow for safe dereference of the vrf
since it may or may not exist in all cases.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
A route where ESI, GW IP, MAC and Label are all zero at the same time SHOULD
be treat-as-withdraw.
Invalid MAC addresses are broadcast or multicast MAC addresses. The route
MUST be treat-as-withdraw in case of an invalid MAC address.
As FRR support Ethernet NVO Tunnels only.
Route will be withdrawn when ESI, GW IP and MAC are zero or Invalid MAC
Test cases:
1) ET-5 route with valid RMAC extended community
2) ET-5 route no RMAC extended community
3) ET-5 route with Multicast MAC in RMAC extended community
4) ET-5 route with Broadcast MAC in RMAC extended community
Signed-off-by: Kishore Aramalla <karamalla@vmware.com>
Copy the fix made in 'lib/if.c' to 'lib/routemap_northbound.c' so we can
have a working YANG model when compiled with GCC version less than 5.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Keep a list of hook contexts used by northbound so we don't lose the
pointer when free()ing the route map index entry data.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Allow old CLI users to still print their configuration without migrating
to northbound.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Two fixes here:
* Don't attempt to use `vty` pointer in vty;
* When `vty` is unavailable output to log;
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
This fixes a warning on daemons that use route map about filter yang
model not being included in the binary.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Based on the route map old CLI, implement the route map handling using
the exported functions.
Use a curry-like programming pattern avoid code repetition when
destroying match/set entries. This is needed by other daemons that
implement custom route map functions and need to pass to lib their
specific destroy functions.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
The agentx.c code was calling fcntl but not testing return
code and handling it, thus making SA unhappy.
Fix.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
For Graceful restart clients have to send GR capabilities
library functions are added to encode capabilities and
also for zebra to decode client capabilities.
Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: Soman K S <somanks@vmware.com>
Signed-off-by: Santosh P K <sapk@vmware.com>
These changes are for Zebra lib in order to supportGraceful Restart
feature. These changes are addedtemporarily, until Zebra Graceful
Restart lib Pr is merged.
Signed-off-by: Biswajit Sadhu <sadhub@vmware.com>
Signed-off-by: Soman K S <somanks@vmware.com>
* Added FSM for peer and global configuration for graceful restart
* Added debug option BGP_GRACEFUL_RESTART for logs specific to
graceful restart processing
Signed-off-by: Biswajit Sadhu <sadhub@vmware.com>
Commit
68a02e06e5 broke nexthop encoding
for nexthop tracking.
This code combined the different types of nexthop encoding
being done in the zapi protocol. What was missed that
resolved nexthops of type NEXTHOP_TYPE_IPV4|6 have an ifindex
value that was not being reported. This commit ensures
that we always send this data( even if it is 0).
The following test commit will ensure that this stays working
as is expected by an upper level protocol.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This script was written back when `git describe` would abbreviate to
7-char commit IDs; they're longer now and we're grabbing the tail
end...
Signed-off-by: David Lamparter <equinox@diac24.net>
If someone tries to add a nexthop with a list of nexthops
already attached to it, let's just assert. This standardizes
the API to say we assume this is an individual nexthop
you are appending to a group.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Make the nexthop_copy/nexthop_dup APIs more consistent by
adding a secondary, non-recursive, version of them. Before,
it was inconsistent whether the APIs were expected to copy
recursive info or not. Make it clear now that the default is
recursive info is copied unless the _no_recurse() version is
called. These APIs are not heavily used so it is fine to
change them for now.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
bgpd already supports BGP Prefix-SID path attribute and
there are some sub-types of Prefix-SID path attribute.
This commits makes bgpd to support additional sub-types.
sub-Type-4 and sub-Type-5 for construct the VPNv4 SRv6 backend
with vpnv4-unicast address family.
This path attributes is already supported by Ciscos IOS-XR and NX-OS.
Prefix-SID sub-Type-4 and sub-Type-5 is defined on following
IETF-drafts.
Supports(A-part-of):
- https://tools.ietf.org/html/draft-dawra-idr-srv6-vpn-04
- https://tools.ietf.org/html/draft-dawra-idr-srv6-vpn-05
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
Add error handling for top level failures (not able to
execute command, unable to find vrf for command, etc.)
With this error handling we add a new zapi message type
of ZEBRA_ERROR used when we are unable to properly handle
a zapi command and pass it down into the lower level code.
In the event of this, we reply with a message of type
enum zebra_error_types containing the error type.
The sent packet will look like so:
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Length | Marker | Version |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| VRF ID |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Command |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| ERROR TYPE |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Also add appropriate hooks for clients to subscribe to for
handling these types of errors.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
If someone provides us more nexthops than our configured multipath
setting, drop the rest of them
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
