Found by coverity. Let's just lock the writeable
amount to see if it is possible. It's ok because
we want to know if we have room *now*. If another
pthread runs it will only remove data from fnc->obuf
and make more room. So this is ok.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Containers inside a choice's case must be treated as presence containers
as they can be explicitly created and deleted. They must have `create`
and `destroy` callbacks, otherwise the internal data they represent may
never be deleted.
The issue can be reproduced with the following steps:
- create an access-list with destination-network params
```
# access-list test seq 1 permit ip any 10.10.10.0 0.0.0.255
```
- delete the `destination-network` container
```
# mgmt delete-config /frr-filter:lib/access-list[name='test'][type='ipv4']/entry[sequence='1']/destination-network
# mgmt commit apply
MGMTD: No changes found to be committed!
```
As the `destination-network` container is non-presence, and all its
leafs are mandatory, mgmtd doesn't see any changes to be commited and
simply updates its YANG data tree without passing any updates to backend
daemons.
This commit fixes the issue by requiring `create` and `destroy`
callbacks for containers inside choice's cases.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
A macvlan interface can have its underlying link-interface in another
namespace (aka. netns). However, by default, zebra does not know the
interface from the other namespaces. It results in a crash the pointer
to the link interface is NULL.
> 6 0x0000559d77a329d3 in zebra_vxlan_macvlan_up (ifp=0x559d798b8e00) at /root/frr/zebra/zebra_vxlan.c:4676
> 4676 link_zif = link_ifp->info;
> (gdb) list
> 4671 struct interface *link_ifp, *link_if;
> 4672
> 4673 zif = ifp->info;
> 4674 assert(zif);
> 4675 link_ifp = zif->link;
> 4676 link_zif = link_ifp->info;
> 4677 assert(link_zif);
> 4678
> (gdb) p zif->link
> $2 = (struct interface *) 0x0
> (gdb) p zif->link_ifindex
> $3 = 15
Fix the crash by returning when the macvlan link-interface is in another
namespace. No need to go further because any vxlan under the macvlan
interface would not be accessible by zebra.
Link: https://github.com/FRRouting/frr/issues/15370
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
The current code is unsetting the fact that the
NHG is installed. It is installed but we are
reinstalling it. Let's note this in the code
appropriately as REINSTALL and not remove the
INSTALLED FLAG.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The function call in to zebra_interface_nhg_reinstall
is an action that takes place on interface up events
*not* when the connected addresses are added to
a system. this will prevent this function being
called when new connected interfaces come alive
that is an independent operation of the interface
coming up.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
zebra_if_nhg_dependents_XXX were just simple wrapper
functions that inited/deleted data structures.
These were already function calls themselves. Let's
remove the abstraction and make the code simpler.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
These functions provided a level of abstraction that forced
us to call multiple functions when a simple data structure
change was all that is needed. Let's consolidate down
and make things a bit simpler.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The nexthop group is marked as valid/invalid and then
installed. Not installed and then marked valid.
This is just a bit of code removed that might be covering
up other problems that need to be sorted.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Convert the dplane results function for nhg's over to
using a switch for the result enum. Let's specifically
call out the unexpected state and also set the nexthop
group as not installed when installation fails.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When installing a NHG via dplane_nexthop_add, it can only return
REQUEST_QUEUED or REQUEST_FAILURE. There is no way SUCCESS can
be returned with the way the dplane works at this point in time.
Remove the code that attempts to set the NHE state appropriately
as it is impossible.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Create a single registry of default port values that daemons
are using. Most of these are vty ports, but there are some
others for features like ospfapi and zebra FPM.
Signed-off-by: Mark Stapp <mjs@labn.net>
Allow bandwidth up to 1000000 Mb/s (ie. 1 Tb/s) and document it.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
get_iflink_speed() returns UINT32_MAX when the speeds is unknown.
Routing daemons (at least ospfd) interprets it as the high value.
Return errors in get_iflink_speed() to avoid the confusion.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
- use `apply_finish` callback when possible to avoid multiple applies per commit
- move table range working to the CLI handler
- remove unnecessary conditional compilation
- remove unnecessary boolean conversion
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
These commands don't really provide any functionality. VRF is associated
with netns automatically based on its name, and it's not possible to
associate VRF and netns with different names with these commands:
- When trying to assosiate a VRF with an already existing netns with a
different name:
`NS /run/netns/test is already configured with VRF 1(test)`
- When trying to assiciate a VRF with a non-existing netns, so they
become linked once the netns is created:
`Invalid pathname for /run/netns/test: No such file or directory`
- When doing "no netns" to unlink the netns and link it back to the same
VRF:
`VRF 1 is already configured with VRF test`
- When doing "no netns" to unlink the netns and link it to another VRF:
`Can not associate NS 4294967295 with NETNS /run/netns/test`
As shown above, not a single usecase is working. We can't remove them
completely to preserve backwards-compatibility, so just make them empty.
The main reason for this change is not to spend a lot of time trying to
figure out how to convert them to northbound.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
- unnecessary command duplication
- usage of oper data during validation
- unnecessary checks for things that can't happen
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Make link-params a presence container and activate it when entering the
node. The "enable" command is not necessary anymore but kept hidden for
backwards compatibility.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Replace "shutdown" leaf with "enabled" leaf in frr-zebra YANG module
to make it in line with standard YANG models.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Introduce new "[no] multicast <enable|disable>" command to be able to
remove the configuration. Current "[no] multicast" command cannot be
removed. Current command is hidden but still works for backwards
compatibility.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
clang-format doesn't understand FRR_DAEMON_INFO is a long macro where
laying out items semantically makes sense.
(Also use only one `FRR_DAEMON_INFO(` in isisd so editors don't get
confused with the mismatching `( ( )`.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This is not complicated code and if zebra is allocating
a new one. Zebra does not need to inform the operator
about the process during debugs.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When debugging NHG detail there is a whole bunch
of lines surrounding the nexthop group. Let's
clean these up since they are extremely chatty and
spawn several lines.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
dest was shadowing dest inside of an if statement additionally
both legs needed dest to be assigned. Let's clean this up a
slight bit and use it appropriately
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Fix this:
***********************************************************************************
Address Sanitizer Error detected in zebra_opaque.test_zebra_opaque/r3.asan.zebra.11099
=================================================================
==11099==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 66 byte(s) in 1 object(s) allocated from:
#0 0x7f527fc06b40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f527f5e852b in qmalloc lib/memory.c:100
#2 0x56418d20832d in zread_route_add zebra/zapi_msg.c:2125
#3 0x56418d215d08 in zserv_handle_commands zebra/zapi_msg.c:4011
#4 0x56418d32ab5b in zserv_process_messages zebra/zserv.c:520
#5 0x7f527f6938d3 in event_call lib/event.c:2003
#6 0x7f527f5cb692 in frr_run lib/libfrr.c:1218
#7 0x56418d1c3336 in main zebra/main.c:508
#8 0x7f527e656c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 66 byte(s) leaked in 1 allocation(s).
***********************************************************************************
Code inspection leads to some code paths where the opaque data was not
freed up.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Currently, when editing a leaf-list, `nb_candidate_edit` expects to
receive it's xpath without a predicate and the value in a separate
argument, and then creates the full xpath. This hack is complicated,
because it depends on the operation and on the caller being a backend or
not. Instead, let's require to always include the predicate in a
leaf-list xpath. Update all the usages in the code accordingly.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
There is a goto statement that would be better served
with a break statement. Let's try to minimize this
in the code.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
- initialize the necessary bit when creating if_link_params
- fix CLI description to mark extended as the default mode
- correctly set mode to extended when using the "no" form of the command
- handle the "show_defaults" parameter correctly in cli_show callback
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>