Merge pull request #11177 from opensourcerouting/fix/memset_memcpy

*: memcpy/memset zeroing

Merge pull request #11188 from opensourcerouting/fix/argv_find

bgpd: Cleanup

Merge pull request #9454 from anlancs/fix-ospf6-gr-length

ospf6d: suppress coverity warnings of insecure data handling

Merge pull request #11190 from donaldsharp/bgp_data

Fix #11178 and various other cleanup

bgpd: Align bgp_dump.h to our standards

bgp_dump.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_debug.h to our standards

bgp_debug.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_damp.h to our standards

bgp_damp.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_community.h to our standards

bgp_community.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_clist.h to our standards

bgp_clist.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_attr.h to our standards

bgp_attr.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_aspath.h to our standards

bgp_aspath.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_advertise.h to our standards

bgp_advertise.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_zebra.h to our standards

bgp_zebra.h has function declarations that are
not properly aligned with our standard on how
to do so.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Align bgp_mpath.h to our standards

The bgp_mpath.h file was missing some variable names.  Let's
add them in to align with our standard for header files.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Change single value bitfield to a bool

The maxpaths same_clusterlen value was a uint16_t
with a single bit being used.  No other values are
being stored.  Let's remove the bitfield and simplify
to a bool.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Prevent crash when issuing various forms of `bgp no-rib`

The `bgp no-rib` command cycles through all the bgp rib tables
and removes them from zebra.  Modify the code so that FRR notices
that it is attempting to cycle through the safi's that are two level
tables.  In addition these safi's cannot just blindly remove the routes
from the rib as that there are none explicitly.

This code just prevents the crash in bgpd.  It does not properly cycle
through and remove the zebra changes made that are explicit to these afi's.
This should be handled as appropriate by the developers on these safi's when
it becomes important to them.

Fixes: #11178
Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Merge pull request #11176 from anlancs/fix/bgpd-remove-for-type2-prefix

bgpd: remove unncessary check for evpn

Merge pull request #11186 from anlancs/fix/bgpd-comment-should-es

bgpd,zebra: correct one debug log for evpn-mh

bgpd: Check and validate return value for str2ipaddr()

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Check argv_find() value instead of the index

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

tools: Catch more argv_find() when not checked properly

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Reuse index from argv_find only if found

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd,zebra: correct one debug log for evpn-mh

Correct one debug log in evpn-mh.
BTW, correct one misspelled word in comment.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

*: Properly use memset() when zeroing

    Wrong: memset(&a, 0, sizeof(struct ...));
    Good:  memset(&a, 0, sizeof(a));

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

ripd: Use correct usage of memcpy() when zeroing struct

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

tools: Add coccinelle script to catch memset/memcpy wrong usage

Wrong: memset(&a, 0, sizeof(struct ...));
Good:  memset(&a, 0, sizeof(a));

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Use sizeof() for memset instead of numeric

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #11151 from opensourcerouting/pim-various-20220506

pimd, pim6d: fix various smaller issues

bgpd: remove unncessary check for evpn

In current code, `build_evpn_type2_prefix()` doesn't distinguish ARP
according to the `ip` parameter. The `ip` parameter from caller is
always non-NULL.

Be consistent and not confused, just remove the unnecessary check.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

Merge pull request #11155 from LabNConsulting/ziemba/link-delay-min-max

zebra bugfix interface link-param: allow delay min <= avg <= max (was: min<avg<max)

Merge pull request #11160 from opensourcerouting/feature/BGP_NOTIFY_CEASE_ADMIN_RESET_hard_reset

bgpd: Send Hard Reset Notification for BGP_NOTIFY_CEASE_ADMIN_RESET

ospf6d: add header size check for LSA

Add header size check for safe. If the check fails, just jump out of current
function.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

bgpd: Reset bgp_notify.hard_reset on receive to avoid stale data

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Send Hard Reset Notification for BGP_NOTIFY_CEASE_ADMIN_RESET

`clear bgp neighbor` should send Hard Reset and graceful restart should be
activated. Let's make this adjustable.

https://datatracker.ietf.org/doc/html/rfc8538#section-5.1

   +-------+------------------------------------+----------------------+
   | Value |                Name                |  Suggested Behavior  |
   +-------+------------------------------------+----------------------+
   |   1   | Maximum Number of Prefixes Reached |      Hard Reset      |
   |   2   |      Administrative Shutdown       |      Hard Reset      |
   |   3   |         Peer De-configured         |      Hard Reset      |
   |   4   |        Administrative Reset        | Provide user control |
   |   5   |        Connection Rejected         |    Graceful Cease    |
   |   6   |     Other Configuration Change     |    Graceful Cease    |
   |   7   |  Connection Collision Resolution   |    Graceful Cease    |
   |   8   |          Out of Resources          |    Graceful Cease    |
   |   9   |             Hard Reset             |      Hard Reset      |
   +-------+------------------------------------+----------------------+

Enabled by default.

Co-authored-by: Biswajit Sadhu <biswajit.sadhu@gmail.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #11138 from opensourcerouting/fix/rpki

bgpd: RPKI reverts, nits...

Merge pull request #11147 from opensourcerouting/feature/gr_n-bit_cli_knob

bgpd: Add CLI knob to enable graceful restart for BGP notifications

Merge pull request #11148 from anlancs/fix/bfdd-wrong-discriminator-mismatch

bfdd: fix override between sessions

Merge pull request #11163 from opensourcerouting/fix/same_type_casting

*: Avoid casting to the same type as on the left

Merge pull request #11154 from opensourcerouting/fix/increase_bgp_flags

bgpd: Use uint64_t for bgp->flags

Merge pull request #11161 from opensourcerouting/fix/no_need_to_check_for_xcalloc

zebra, ospf6d: Do not check if NULL for XCALLOC()

Merge pull request #11162 from anlancs/fix/bgpd-cleanup-5

bgpd: remove unnecessary check for evpn

Merge pull request #11170 from anlancs/fix/bgpd-cleanup-8

bgpd: remove one unnecessary parameter for evpn-mh

Merge pull request #11096 from anlancs/fix/bgpd-unlock

bgpd: clear misleading mismatched check

bgpd: remove one unnecessary parameter for evpn-mh

The "add" parameter of `bgp_evpn_mh_route_update()` makes no sense.
Just remove it to clarify this function, and remove the relevant check
with "add" as well.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

bgpd: remove unnecessary check for evpn

When `bgp_evpn_new()` is called, the `bgp` parameter MUST be non-NULL,
remove this unnecessary check and remove the NULL check for returned
`struct bgpevpn *`, which should be non-NULL.

And modify `import_rt_new()` in the same way.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

*: Avoid casting to the same type as on the left

Just not necessary.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

zebra, ospf6d: Do not check if NULL for XCALLOC()

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: clear misleading mismatched check

Two changes for `delete_global_type2_routes()`:

1) Remove check of `bgp_dest_has_bgp_path_info_data(rddest)`.
It is unnecessary(`dest->info` should not be NULL) and misleading.
`if (rddest && bgp_dest_has_bgp_path_info_data(rddest))`
Use (locked) node with this check, but unlock with `if (rddest)`,
The mismatched condition is misleading, there seems to be a
mistake to extra unlock.
Just make it clear, immediately exit with `(!rddest)`.

2) Remove checking returned value for it, and use `void` as return type.
It is unnecessary and wrong. Even the check failed, it should continue
to delete other types of routes.
Just remove the check and go through.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

Merge pull request #11156 from donaldsharp/spelling_three

valgrind issues and general cleanup

isisd: Prevent direct copy of different size prefix'es

Memory is allocated for a `struct prefix_ipv6` but
it was directly copied into a `struct prefix` via
direct pointer copy, which leads to a read past
end of memory.  Fix by using prefix_copy

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

isisd: Remove unneeded pre-declarations

I don't know what was going on.  Removing the weird c structures.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

isisd: Fix read past end of string

using a memcpy for a strdup'ed string.  bad mojo

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

*: Fix doesnt spelling mistakes

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

zebra/interface.c: allow link-param delay min <= avg <= max

    RFC 7471 Section 4.2.7:
It is possible for min delay and max delay to be the same value.

    Prior to this change, the code required min < avg < max. This
    change allows min == avg and avg == max.

    test case:

    interface eth-rt1
      link-params
        delay 8000 min 8000 max 8000

Signed-off-by: G. Paul Ziemba <paulz@labn.net>

bgpd: Use uint64_t for bgp->flags

We will hit this soon because uint32_t will be not enough.

Two more flags gonna be added for rfc8538.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

pimd: un-dependency-hell `pim_instance.h`

This is causing build issues on BSD by including (transitively)
`linux/mroute6.h` - try to address by disentangling the headers a bunch.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

pimd: pass down length for register messages

The IPv6 `mrt6msg` kernel pseudo-header does not have a length field;
accessing what would be the IPv6 payload length reads zeroes.

Pass down the proper length and use that instead.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

pim6d: fix Linux kernel header macro

__KERNEL_DIV_ROUND_UP isn't in older versions of linux/const.h.  Rather
than pull in more headers, just inline the definition.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

bfdd: fix override between sessions

After two single-hop sessions (*no local address are configured*) on two
interfaces are UP, remove one address of one interface, both of them
(actually, quite independent sessions) come to be DOWN, not just one.

Consider two boxes: A with `a1` and `a2` adddress on two interfaces,
and B with `b1` and `b2`.
Two sessions are set up and ok: `s1` with <a1,b1> and `s2` with <a2,b2>.
After `a1` of A is removed, there is an unhappy coincidence:
1) On A: `s1` changes local address, and sends <a2,b1> packets with help
of route.
2) On B: wrongly regarded <a2,b1> packets with non-zero remote descriminator
as part of `s2`, and are dropped for mismatched remote remote descriminator.
3) On A: `s1` sends <a2,b1> packets with zero remote descriminator to
initialize this session.
4) On B: wrongly regarded <a2,b1> packets with zero remote descriminator as
part of `s2`. Then `s2` will vibrate.

So the good sessions are overridden.

In this case, the <a2,b1> packets with zero remote descriminator won't take
effect until the current good sessions become bad.

Since single-hop sessions are allowed to be set without bound inteface in
current code, this commit adds one check in `bfd_recv_cb()` to avoid wrong
override.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

Merge pull request #11145 from donaldsharp/bgp_capability

Bgp capability

bgpd: Add CLI knob to enable graceful restart for BGP notifications

N-bit flag should be exchanged in BGP OPEN messages, not only when the
bgpd is restarted/started.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #11059 from anlancs/fix/bgpd-evnp-wrong-check-hashget

bgpd: fix memory leak for evpn

Merge pull request #11144 from ton31337/fix/bgp_notification_gr

bgpd: Fix memory leak for bgp_notify_receive()

bgpd: peer groups should not toggle interface based nht tracking

Peer groups when various forms of `bgp capability extended-nexthop`
is entered on them are toggling the nexthop tracking status of peers
in their peer group.  This is ok when the peer is not interface based.
But it is not ok when the peer is interface based as that it will turn
off the ability of FRR to properly work with that peer type.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Refuse to let interfaced peering to change capability extended-nexthop

Do not interface based peers change anything about when a
[no] neighbor <interface> capability extended-nexthop
is entered.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: enhanced capability is always turned on for interface based peers

FRR is displaying that the peer enhanced capability command is not
turned on when the interface is part of a peer group.  Saving the
config and then reloading actually turns it off.

Fix the code so that FRR does not display the enhanced capability
for interface based peers.

Fixes: #11108
Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Merge pull request #11137 from opensourcerouting/if-name-len-fixes

*: standardize interface name maximum length

bgpd: Fix memory leak for bgp_notify_receive()

Initialize outer/inner bgp_notify structs to zero.

Free bgp_notify.raw_data after use.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #11142 from patrasar/pim_register_show_fix

pimd: fixing wrong diplay registerTx

bgpd: Show which RPKI server we are connected to

Before we just showed always the first server which is wrong.

Now we have:

```
spine1-debian-11# show rpki cache-connection
Connected to group 1
rpki tcp cache 192.168.10.17 8283 pref 1
rpki tcp cache 192.168.10.17 8282 pref 2 (connected)
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

pimd: fixing wrong diplay registerTx

Signed-off-by: sarita patra <saritap@vmware.com>

bgpd: Add `rpki reset` to ENABLE node

`rpki stop` and `rpki start` were already, let's add `rpki reset` as well.

Instead of going into configure mode.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Convert some variables from int to bool for RPKI

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

doc: Add missing commands for RPKI

rpki expire_interval
rpki retry_interval

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Make sure we print stats if we are sync at least with a single group

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Revert "bgpd: Handle TCP connection errors with connection callbacks for RPKI"

This reverts commit db3aca462b7d721da85dde12bcc5808f9c32c981.

Connection handling is already fixed in librtr 0.8.0.
https://github.com/rtrlib/rtrlib/releases/tag/v0.8.0

https://github.com/rtrlib/rtrlib/commit/179e7efb59529008eed77b3cf783667435dfba9f

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Revert "bgpd: Fix while(read()) for RPKI sync callback"

This reverts commit b4fc876a327a89dc212efa48a04b76e53263722f.

Merge pull request #11110 from anlancs/fix/bfdd-wrong-local

bfdd: fix wrong local address of display command

Merge pull request #11136 from donaldsharp/stupidity

tests: Use XCALLOC instead of XMALLOC for threads

Merge pull request #11128 from opensourcerouting/feature/rfc8538

bgpd: Implement GR N-Bit and CEASE/Hard Reset notification

Merge pull request #11113 from opensourcerouting/pim6-icmp6-replace-bpf

pim6d: use ICMP6_FILTER instead of BPF on mroute

Merge pull request #11132 from anlancs/fix/bgpd-prefix

bgpd: delay "stream_new" in str2prefix_rd()

tests: Use XCALLOC instead of XMALLOC for threads

Someone decided to do malloc testing over make check.

Fixes: #11120
Signed-off-by: Donald Sharp <sharpd@nvidia.com>

*: remove the checking returned value for hash_get()

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>

bgpd: fix memory leak for evpn

If `hash_get()` returns NULL, the list created with
`list_new()` is not be freed.

Since `hash_get()` should not fail, we don't need
`list_delete()` and other boring `XFREE()`s for its
failure case.

Just ignore returning value of `hash_get()` in these
cases.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

lib: change FRR interface name length definition

Use the OS provided maximum name length instead of defining an arbitrary
one.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

*: use FRR interface name definition everywhere

Don't rely on the OS interface name length definition and use the FRR
definition instead.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

bfdd: remove "local_address" of bfd session

The "local_address" of bfd is only used in `show bfd peers brief`
for single hop sessions which are configured without "local address".
Since it is set by destination address of received packet, not
completely correct, so remove it.

Signed-off-by: ewlumpkin <ewlumpkin@gmail.com>
Signed-off-by: anlan_cs <vic.lan@pica8.com>

pbrd: fix interface compare usage

Don't use `strncmp` when we expect to match the whole string.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

bgpd: fix interface name compare

We always want to compare the whole string, lets not use `strncmp` for
this case.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

bgpd: delay "stream_new" in str2prefix_rd()

`stream_new` in `str2prefix_rd()` can be called after some
checkings are passed, so the last `if (s)` in this fuction will
make sense.

Additionally some changes for `str2prefix_rd()`:
1) Use `RD_BYTES` instead of hard number for `stream_new()`
2) Remove unnecessary `lret` variable

Signed-off-by: anlan_cs <vic.lan@pica8.com>

Merge pull request #10742 from plsaranya/pim_register

pim6d: Register message send handling

bgpd: Implement CEASE/Hard Reset notification

Also, add N-Bit (Notification) flag for Graceful Restart.

This is a preparation for RFC8538.

More information: https://datatracker.ietf.org/doc/html/rfc8538

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #11119 from kuldeepkash/pim_v6

tests: [PIMv6] Using generic names for PIM verification APIs

tests: [PIMv6] Using generic names for pim verification APIs

1. Modified pim APIs name to generic one, same APIs would be used for PIMv4 and PIMv6
verifications
2. Modified all affacted scripts and ran multiple times locally to avoid CI failures

Signed-off-by: Kuldeep Kashyap <kashyapk@vmware.com>

Merge pull request #11122 from ak503/bgp_esi_str

bgpd: fix size of esi_buf

Merge pull request #11118 from opensourcerouting/time-fmt-nulls

lib: format NULL timevals correctly

bgpd: fix size of esi_buf

Fixes: #11111
Signed-off-by: Dmitrii Turlupov <dturlupov@factor-ts.ru>

Merge pull request #10988 from AbhishekNR/ipv6_mroute_cli

Merge pull request #11114 from opensourcerouting/vrf-declvar-macros

lib, zebra, pimd: clean up/fix VRF DECLVAR macros