diff options
83 files changed, 604 insertions, 455 deletions
diff --git a/.clang-format b/.clang-format index 4bd962747f..654577d936 100644 --- a/.clang-format +++ b/.clang-format @@ -28,6 +28,8 @@ ForEachMacros: - frr_each - frr_each_safe - frr_each_from + - frr_with_mutex + - frr_with_privs - LIST_FOREACH - LIST_FOREACH_SAFE - SLIST_FOREACH diff --git a/bfdd/bfd.c b/bfdd/bfd.c index 5d143d4e5f..1f1568f511 100644 --- a/bfdd/bfd.c +++ b/bfdd/bfd.c @@ -1706,6 +1706,8 @@ static int bfd_vrf_disable(struct vrf *vrf) socket_close(&bvrf->bg_mhop); socket_close(&bvrf->bg_shop6); socket_close(&bvrf->bg_mhop6); + socket_close(&bvrf->bg_echo); + socket_close(&bvrf->bg_echov6); /* free context */ XFREE(MTYPE_BFDD_VRF, bvrf); diff --git a/bfdd/bfd_packet.c b/bfdd/bfd_packet.c index d68a1ad5fd..7fbe6db163 100644 --- a/bfdd/bfd_packet.c +++ b/bfdd/bfd_packet.c @@ -894,7 +894,7 @@ int bp_udp_shop(vrf_id_t vrf_id) { int sd; - frr_elevate_privs(&bglobal.bfdd_privs) { + frr_with_privs(&bglobal.bfdd_privs) { sd = vrf_socket(AF_INET, SOCK_DGRAM, PF_UNSPEC, vrf_id, NULL); } if (sd == -1) @@ -909,7 +909,7 @@ int bp_udp_mhop(vrf_id_t vrf_id) { int sd; - frr_elevate_privs(&bglobal.bfdd_privs) { + frr_with_privs(&bglobal.bfdd_privs) { sd = vrf_socket(AF_INET, SOCK_DGRAM, PF_UNSPEC, vrf_id, NULL); } if (sd == -1) @@ -934,7 +934,7 @@ int bp_peer_socket(const struct bfd_session *bs) && bs->key.vrfname[0]) device_to_bind = (const char *)bs->key.vrfname; - frr_elevate_privs(&bglobal.bfdd_privs) { + frr_with_privs(&bglobal.bfdd_privs) { sd = vrf_socket(AF_INET, SOCK_DGRAM, PF_UNSPEC, bs->vrf->vrf_id, device_to_bind); } @@ -1001,7 +1001,7 @@ int bp_peer_socketv6(const struct bfd_session *bs) && bs->key.vrfname[0]) device_to_bind = (const char *)bs->key.vrfname; - frr_elevate_privs(&bglobal.bfdd_privs) { + frr_with_privs(&bglobal.bfdd_privs) { sd = vrf_socket(AF_INET6, SOCK_DGRAM, PF_UNSPEC, bs->vrf->vrf_id, device_to_bind); } @@ -1121,7 +1121,7 @@ int bp_udp6_shop(vrf_id_t vrf_id) { int sd; - frr_elevate_privs(&bglobal.bfdd_privs) { + frr_with_privs(&bglobal.bfdd_privs) { sd = vrf_socket(AF_INET6, SOCK_DGRAM, PF_UNSPEC, vrf_id, NULL); } if (sd == -1) @@ -1137,7 +1137,7 @@ int bp_udp6_mhop(vrf_id_t vrf_id) { int sd; - frr_elevate_privs(&bglobal.bfdd_privs) { + frr_with_privs(&bglobal.bfdd_privs) { sd = vrf_socket(AF_INET6, SOCK_DGRAM, PF_UNSPEC, vrf_id, NULL); } if (sd == -1) @@ -1153,7 +1153,7 @@ int bp_echo_socket(vrf_id_t vrf_id) { int s; - frr_elevate_privs(&bglobal.bfdd_privs) { + frr_with_privs(&bglobal.bfdd_privs) { s = vrf_socket(AF_INET, SOCK_DGRAM, 0, vrf_id, NULL); } if (s == -1) @@ -1169,7 +1169,7 @@ int bp_echov6_socket(vrf_id_t vrf_id) { int s; - frr_elevate_privs(&bglobal.bfdd_privs) { + frr_with_privs(&bglobal.bfdd_privs) { s = vrf_socket(AF_INET6, SOCK_DGRAM, 0, vrf_id, NULL); } if (s == -1) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 4d395e3749..cc7f81d9ff 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -179,9 +179,7 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) * on various buffers. Those need to be transferred or dropped, * otherwise we'll get spurious failures during session establishment. */ - pthread_mutex_lock(&peer->io_mtx); - pthread_mutex_lock(&from_peer->io_mtx); - { + frr_with_mutex(&peer->io_mtx, &from_peer->io_mtx) { fd = peer->fd; peer->fd = from_peer->fd; from_peer->fd = fd; @@ -222,8 +220,6 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) ringbuf_copy(peer->ibuf_work, from_peer->ibuf_work, ringbuf_remain(from_peer->ibuf_work)); } - pthread_mutex_unlock(&from_peer->io_mtx); - pthread_mutex_unlock(&peer->io_mtx); peer->as = from_peer->as; peer->v_holdtime = from_peer->v_holdtime; @@ -552,10 +548,11 @@ const char *peer_down_str[] = {"", "Intf peering v6only config change", "BFD down received", "Interface down", - "Neighbor address lost" + "Neighbor address lost", "Waiting for NHT", - "Waiting for Peer IPv6 Addr", - "Waiting for VRF to be initialized"}; + "Waiting for Peer IPv6 LLA", + "Waiting for VRF to be initialized", + "No AFI/SAFI activated for peer"}; static int bgp_graceful_restart_timer_expire(struct thread *thread) { @@ -1166,8 +1163,7 @@ int bgp_stop(struct peer *peer) BGP_TIMER_OFF(peer->t_routeadv); /* Clear input and output buffer. */ - pthread_mutex_lock(&peer->io_mtx); - { + frr_with_mutex(&peer->io_mtx) { if (peer->ibuf) stream_fifo_clean(peer->ibuf); if (peer->obuf) @@ -1183,7 +1179,6 @@ int bgp_stop(struct peer *peer) peer->curr = NULL; } } - pthread_mutex_unlock(&peer->io_mtx); /* Close of file descriptor. */ if (peer->fd >= 0) { diff --git a/bgpd/bgp_io.c b/bgpd/bgp_io.c index f111822e53..c2d06a4b5a 100644 --- a/bgpd/bgp_io.c +++ b/bgpd/bgp_io.c @@ -132,12 +132,10 @@ static int bgp_process_writes(struct thread *thread) struct frr_pthread *fpt = bgp_pth_io; - pthread_mutex_lock(&peer->io_mtx); - { + frr_with_mutex(&peer->io_mtx) { status = bgp_write(peer); reschedule = (stream_fifo_head(peer->obuf) != NULL); } - pthread_mutex_unlock(&peer->io_mtx); /* no problem */ if (CHECK_FLAG(status, BGP_IO_TRANS_ERR)) { @@ -184,11 +182,9 @@ static int bgp_process_reads(struct thread *thread) struct frr_pthread *fpt = bgp_pth_io; - pthread_mutex_lock(&peer->io_mtx); - { + frr_with_mutex(&peer->io_mtx) { status = bgp_read(peer); } - pthread_mutex_unlock(&peer->io_mtx); /* error checking phase */ if (CHECK_FLAG(status, BGP_IO_TRANS_ERR)) { @@ -237,11 +233,9 @@ static int bgp_process_reads(struct thread *thread) assert(ringbuf_get(ibw, pktbuf, pktsize) == pktsize); stream_put(pkt, pktbuf, pktsize); - pthread_mutex_lock(&peer->io_mtx); - { + frr_with_mutex(&peer->io_mtx) { stream_fifo_push(peer->ibuf, pkt); } - pthread_mutex_unlock(&peer->io_mtx); added_pkt = true; } else diff --git a/bgpd/bgp_keepalives.c b/bgpd/bgp_keepalives.c index bec3bdcb8d..6de1c216a6 100644 --- a/bgpd/bgp_keepalives.c +++ b/bgpd/bgp_keepalives.c @@ -245,8 +245,7 @@ void bgp_keepalives_on(struct peer *peer) */ assert(peerhash_mtx); - pthread_mutex_lock(peerhash_mtx); - { + frr_with_mutex(peerhash_mtx) { holder.peer = peer; if (!hash_lookup(peerhash, &holder)) { struct pkat *pkat = pkat_new(peer); @@ -255,7 +254,6 @@ void bgp_keepalives_on(struct peer *peer) } SET_FLAG(peer->thread_flags, PEER_THREAD_KEEPALIVES_ON); } - pthread_mutex_unlock(peerhash_mtx); bgp_keepalives_wake(); } @@ -275,8 +273,7 @@ void bgp_keepalives_off(struct peer *peer) */ assert(peerhash_mtx); - pthread_mutex_lock(peerhash_mtx); - { + frr_with_mutex(peerhash_mtx) { holder.peer = peer; struct pkat *res = hash_release(peerhash, &holder); if (res) { @@ -285,16 +282,13 @@ void bgp_keepalives_off(struct peer *peer) } UNSET_FLAG(peer->thread_flags, PEER_THREAD_KEEPALIVES_ON); } - pthread_mutex_unlock(peerhash_mtx); } void bgp_keepalives_wake(void) { - pthread_mutex_lock(peerhash_mtx); - { + frr_with_mutex(peerhash_mtx) { pthread_cond_signal(peerhash_cond); } - pthread_mutex_unlock(peerhash_mtx); } int bgp_keepalives_stop(struct frr_pthread *fpt, void **result) diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 1dadf00e8f..887caee95e 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -122,7 +122,7 @@ static int bgp_md5_set_connect(int socket, union sockunion *su, int ret = -1; #if HAVE_DECL_TCP_MD5SIG - frr_elevate_privs(&bgpd_privs) { + frr_with_privs(&bgpd_privs) { ret = bgp_md5_set_socket(socket, su, prefixlen, password); } #endif /* HAVE_TCP_MD5SIG */ @@ -140,8 +140,7 @@ static int bgp_md5_set_password(struct peer *peer, const char *password) * Set or unset the password on the listen socket(s). Outbound * connections are taken care of in bgp_connect() below. */ - frr_elevate_privs(&bgpd_privs) - { + frr_with_privs(&bgpd_privs) { for (ALL_LIST_ELEMENTS_RO(bm->listen_sockets, node, listener)) if (listener->su.sa.sa_family == peer->su.sa.sa_family) { @@ -167,8 +166,7 @@ int bgp_md5_set_prefix(struct prefix *p, const char *password) struct bgp_listener *listener; /* Set or unset the password on the listen socket(s). */ - frr_elevate_privs(&bgpd_privs) - { + frr_with_privs(&bgpd_privs) { for (ALL_LIST_ELEMENTS_RO(bm->listen_sockets, node, listener)) if (listener->su.sa.sa_family == p->family) { prefix2sockunion(p, &su); @@ -610,7 +608,7 @@ int bgp_connect(struct peer *peer) zlog_debug("Peer address not learnt: Returning from connect"); return 0; } - frr_elevate_privs(&bgpd_privs) { + frr_with_privs(&bgpd_privs) { /* Make socket for the peer. */ peer->fd = vrf_sockunion_socket(&peer->su, peer->bgp->vrf_id, bgp_get_bound_name(peer)); @@ -630,7 +628,7 @@ int bgp_connect(struct peer *peer) sockopt_reuseport(peer->fd); #ifdef IPTOS_PREC_INTERNETCONTROL - frr_elevate_privs(&bgpd_privs) { + frr_with_privs(&bgpd_privs) { if (sockunion_family(&peer->su) == AF_INET) setsockopt_ipv4_tos(peer->fd, IPTOS_PREC_INTERNETCONTROL); @@ -708,7 +706,7 @@ static int bgp_listener(int sock, struct sockaddr *sa, socklen_t salen, sockopt_reuseaddr(sock); sockopt_reuseport(sock); - frr_elevate_privs(&bgpd_privs) { + frr_with_privs(&bgpd_privs) { #ifdef IPTOS_PREC_INTERNETCONTROL if (sa->sa_family == AF_INET) @@ -767,7 +765,7 @@ int bgp_socket(struct bgp *bgp, unsigned short port, const char *address) snprintf(port_str, sizeof(port_str), "%d", port); port_str[sizeof(port_str) - 1] = '\0'; - frr_elevate_privs(&bgpd_privs) { + frr_with_privs(&bgpd_privs) { ret = vrf_getaddrinfo(address, port_str, &req, &ainfo_save, bgp->vrf_id); } @@ -788,7 +786,7 @@ int bgp_socket(struct bgp *bgp, unsigned short port, const char *address) if (ainfo->ai_family != AF_INET && ainfo->ai_family != AF_INET6) continue; - frr_elevate_privs(&bgpd_privs) { + frr_with_privs(&bgpd_privs) { sock = vrf_socket(ainfo->ai_family, ainfo->ai_socktype, ainfo->ai_protocol, bgp->vrf_id, diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index cd94f421ef..c7c1780c21 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -124,9 +124,9 @@ int bgp_packet_set_size(struct stream *s) */ static void bgp_packet_add(struct peer *peer, struct stream *s) { - pthread_mutex_lock(&peer->io_mtx); - stream_fifo_push(peer->obuf, s); - pthread_mutex_unlock(&peer->io_mtx); + frr_with_mutex(&peer->io_mtx) { + stream_fifo_push(peer->obuf, s); + } } static struct stream *bgp_update_packet_eor(struct peer *peer, afi_t afi, @@ -665,7 +665,7 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code, struct stream *s; /* Lock I/O mutex to prevent other threads from pushing packets */ - pthread_mutex_lock(&peer->io_mtx); + frr_mutex_lock_autounlock(&peer->io_mtx); /* ============================================== */ /* Allocate new stream. */ @@ -756,9 +756,6 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code, stream_fifo_push(peer->obuf, s); bgp_write_notify(peer); - - /* ============================================== */ - pthread_mutex_unlock(&peer->io_mtx); } /* @@ -2237,11 +2234,9 @@ int bgp_process_packet(struct thread *thread) bgp_size_t size; char notify_data_length[2]; - pthread_mutex_lock(&peer->io_mtx); - { + frr_with_mutex(&peer->io_mtx) { peer->curr = stream_fifo_pop(peer->ibuf); } - pthread_mutex_unlock(&peer->io_mtx); if (peer->curr == NULL) // no packets to process, hmm... return 0; @@ -2360,15 +2355,13 @@ int bgp_process_packet(struct thread *thread) if (fsm_update_result != FSM_PEER_TRANSFERRED && fsm_update_result != FSM_PEER_STOPPED) { - pthread_mutex_lock(&peer->io_mtx); - { + frr_with_mutex(&peer->io_mtx) { // more work to do, come back later if (peer->ibuf->count > 0) thread_add_timer_msec( bm->master, bgp_process_packet, peer, 0, &peer->t_process_packet); } - pthread_mutex_unlock(&peer->io_mtx); } return 0; diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 7e7c0ea2d7..17bc83ed2e 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -7921,6 +7921,8 @@ static void bgp_show_peer_reset(struct vty * vty, struct peer *peer, } json_object_string_add(json_peer, "lastResetDueTo", peer_down_str[(int)peer->last_reset]); + json_object_int_add(json_peer, "lastResetCode", + peer->last_reset); } else { if (peer->last_reset == PEER_DOWN_NOTIFY_SEND || peer->last_reset == PEER_DOWN_NOTIFY_RECEIVED) { diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index fed8cd4d86..5b31fbb3a8 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1582,6 +1582,12 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, } active = peer_active(peer); + if (!active) { + if (peer->su.sa.sa_family == AF_UNSPEC) + peer->last_reset = PEER_DOWN_NBR_ADDR; + else + peer->last_reset = PEER_DOWN_NOAFI_ACTIVATED; + } /* Last read and reset time set */ peer->readtime = peer->resettime = bgp_clock(); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 880fceb8f1..477c036009 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1197,6 +1197,7 @@ struct peer { #define PEER_DOWN_WAITING_NHT 27 /* Waiting for NHT to resolve */ #define PEER_DOWN_NBR_ADDR 28 /* Waiting for peer IPv6 IP Addr */ #define PEER_DOWN_VRF_UNINIT 29 /* Associated VRF is not init yet */ +#define PEER_DOWN_NOAFI_ACTIVATED 30 /* No AFI/SAFI activated for peer */ size_t last_reset_cause_size; uint8_t last_reset_cause[BGP_MAX_PACKET_SIZE]; diff --git a/bgpd/rfapi/rfapi.c b/bgpd/rfapi/rfapi.c index e7905e5622..9b8f64ee67 100644 --- a/bgpd/rfapi/rfapi.c +++ b/bgpd/rfapi/rfapi.c @@ -1282,8 +1282,7 @@ static int rfapi_open_inner(struct rfapi_descriptor *rfd, struct bgp *bgp, * since this peer is not on the I/O thread, this lock is not strictly * necessary, but serves as a reminder to those who may meddle... */ - pthread_mutex_lock(&rfd->peer->io_mtx); - { + frr_with_mutex(&rfd->peer->io_mtx) { // we don't need any I/O related facilities if (rfd->peer->ibuf) stream_fifo_free(rfd->peer->ibuf); @@ -1300,7 +1299,6 @@ static int rfapi_open_inner(struct rfapi_descriptor *rfd, struct bgp *bgp, rfd->peer->obuf_work = NULL; rfd->peer->ibuf_work = NULL; } - pthread_mutex_unlock(&rfd->peer->io_mtx); { /* base code assumes have valid host pointer */ char buf[BUFSIZ]; diff --git a/bgpd/rfapi/vnc_zebra.c b/bgpd/rfapi/vnc_zebra.c index 481500dfb4..80a590f56a 100644 --- a/bgpd/rfapi/vnc_zebra.c +++ b/bgpd/rfapi/vnc_zebra.c @@ -191,8 +191,7 @@ static void vnc_redistribute_add(struct prefix *p, uint32_t metric, * is not strictly necessary, but serves as a reminder * to those who may meddle... */ - pthread_mutex_lock(&vncHD1VR.peer->io_mtx); - { + frr_with_mutex(&vncHD1VR.peer->io_mtx) { // we don't need any I/O related facilities if (vncHD1VR.peer->ibuf) stream_fifo_free(vncHD1VR.peer->ibuf); @@ -209,7 +208,6 @@ static void vnc_redistribute_add(struct prefix *p, uint32_t metric, vncHD1VR.peer->obuf_work = NULL; vncHD1VR.peer->ibuf_work = NULL; } - pthread_mutex_unlock(&vncHD1VR.peer->io_mtx); /* base code assumes have valid host pointer */ vncHD1VR.peer->host = diff --git a/configure.ac b/configure.ac index a940739ae4..6c1b35b5f2 100755 --- a/configure.ac +++ b/configure.ac @@ -2038,6 +2038,20 @@ if test "${enable_capabilities}" != "no"; then -o x"${frr_ac_lcaps}" = x"yes"; then AC_DEFINE([HAVE_CAPABILITIES], [1], [capabilities]) fi + + case "$host_os" in + linux*) + if test "$frr_ac_lcaps" != "yes"; then + AC_MSG_ERROR([libcap and/or its headers were not found. Running FRR without libcap support built in causes a huge performance penalty.]) + fi + ;; + esac +else + case "$host_os" in + linux*) + AC_MSG_WARN([Running FRR without libcap support built in causes a huge performance penalty.]) + ;; + esac fi AC_SUBST([LIBCAP]) diff --git a/doc/developer/library.rst b/doc/developer/library.rst index 7cd493ccc4..a904a4e778 100644 --- a/doc/developer/library.rst +++ b/doc/developer/library.rst @@ -11,6 +11,7 @@ Library Facilities (libfrr) rcu lists logging + locking hooks cli modules diff --git a/doc/developer/locking.rst b/doc/developer/locking.rst new file mode 100644 index 0000000000..aee05aae06 --- /dev/null +++ b/doc/developer/locking.rst @@ -0,0 +1,73 @@ +Locking +======= + +FRR ships two small wrappers around ``pthread_mutex_lock()`` / +``pthread_mutex_unlock``. Use ``#include "frr_pthread.h"`` to get these +macros. + +.. c:function:: frr_with_mutex(pthread_mutex_t *mutex) + + Begin a C statement block that is executed with the mutex locked. Any + exit from the block (``break``, ``return``, ``goto``, end of block) will + cause the mutex to be unlocked:: + + int somefunction(int option) + { + frr_with_mutex(&my_mutex) { + /* mutex will be locked */ + + if (!option) + /* mutex will be unlocked before return */ + return -1; + + if (something(option)) + /* mutex will be unlocked before goto */ + goto out_err; + + somethingelse(); + + /* mutex will be unlocked at end of block */ + } + + return 0; + + out_err: + somecleanup(); + return -1; + } + + This is a macro that internally uses a ``for`` loop. It is explicitly + acceptable to use ``break`` to get out of the block. Even though a single + statement works correctly, FRR coding style requires that this macro always + be used with a ``{ ... }`` block. + +.. c:function:: frr_mutex_lock_autounlock(pthread_mutex_t *mutex) + + Lock mutex and unlock at the end of the current C statement block:: + + int somefunction(int option) + { + frr_mutex_lock_autounlock(&my_mutex); + /* mutex will be locked */ + + ... + if (error) + /* mutex will be unlocked before return */ + return -1; + ... + + /* mutex will be unlocked before return */ + return 0; + } + + This is a macro that internally creates a variable with a destructor. + When the variable goes out of scope (i.e. the block ends), the mutex is + released. + + .. warning:: + + This macro should only used when :c:func:`frr_with_mutex` would + result in excessively/weirdly nested code. This generally is an + indicator that the code might be trying to do too many things with + the lock held. Try any possible venues to reduce the amount of + code covered by the lock and move to :c:func:`frr_with_mutex`. diff --git a/doc/developer/subdir.am b/doc/developer/subdir.am index 1fc593e566..557a41c51f 100644 --- a/doc/developer/subdir.am +++ b/doc/developer/subdir.am @@ -31,6 +31,7 @@ dev_RSTFILES = \ doc/developer/index.rst \ doc/developer/library.rst \ doc/developer/lists.rst \ + doc/developer/locking.rst \ doc/developer/logging.rst \ doc/developer/maintainer-release-build.rst \ doc/developer/memtypes.rst \ diff --git a/doc/developer/workflow.rst b/doc/developer/workflow.rst index 07c43ac2de..3c6887fbac 100644 --- a/doc/developer/workflow.rst +++ b/doc/developer/workflow.rst @@ -767,6 +767,28 @@ ways that can be unexpected for the original implementor. As such debugs ability to turn on/off debugs from the CLI and it is expected that the developer will use this convention to allow control of their debugs. +Custom syntax-like block macros +------------------------------- + +FRR uses some macros that behave like the ``for`` or ``if`` C keywords. These +macros follow these patterns: + +- loop-style macros are named ``frr_each_*`` (and ``frr_each``) +- single run macros are named ``frr_with_*`` +- to avoid confusion, ``frr_with_*`` macros must always use a ``{ ... }`` + block even if the block only contains one statement. The ``frr_each`` + constructs are assumed to be well-known enough to use normal ``for`` rules. +- ``break``, ``return`` and ``goto`` all work correctly. For loop-style + macros, ``continue`` works correctly too. + +Both the ``each`` and ``with`` keywords are inspired by other (more +higher-level) programming languages that provide these constructs. + +There are also some older iteration macros, e.g. ``ALL_LIST_ELEMENTS`` and +``FOREACH_AFI_SAFI``. These macros in some cases do **not** fulfill the above +pattern (e.g. ``break`` does not work in ``FOREACH_AFI_SAFI`` because it +expands to 2 nested loops.) + Static Analysis and Sanitizers ------------------------------ Clang/LLVM and GCC come with a variety of tools that can be used to help find diff --git a/doc/user/pbr.rst b/doc/user/pbr.rst index 6230bf777a..fab4343f50 100644 --- a/doc/user/pbr.rst +++ b/doc/user/pbr.rst @@ -91,6 +91,12 @@ end destination. both v4 and v6 prefixes. This command is used in conjunction of the :clicmd:`match src-ip PREFIX` command for matching. +.. clicmd:: match mark (1-4294967295) + + Select the mark to match. This is a linux only command and if attempted + on another platform it will be denied. This mark translates to the + underlying `ip rule .... fwmark XXXX` command. + .. clicmd:: set nexthop-group NAME Use the nexthop-group NAME as the place to forward packets when the match diff --git a/eigrpd/eigrp_network.c b/eigrpd/eigrp_network.c index 0d3c4279ec..c7ffbf9f0e 100644 --- a/eigrpd/eigrp_network.c +++ b/eigrpd/eigrp_network.c @@ -61,7 +61,7 @@ int eigrp_sock_init(struct vrf *vrf) int hincl = 1; #endif - frr_elevate_privs(&eigrpd_privs) { + frr_with_privs(&eigrpd_privs) { eigrp_sock = vrf_socket( AF_INET, SOCK_RAW, IPPROTO_EIGRPIGP, vrf->vrf_id, vrf->vrf_id != VRF_DEFAULT ? vrf->name : NULL); @@ -285,7 +285,7 @@ void eigrp_if_update(struct interface *ifp) { struct listnode *node, *nnode; struct route_node *rn; - struct eigrp *eigrp = eigrp_lookup(ifp->vrf_id); + struct eigrp *eigrp; /* * In the event there are multiple eigrp autonymnous systems running, diff --git a/isisd/isis_bpf.c b/isisd/isis_bpf.c index 4e9aef47ad..d6b85b2fa3 100644 --- a/isisd/isis_bpf.c +++ b/isisd/isis_bpf.c @@ -187,7 +187,7 @@ int isis_sock_init(struct isis_circuit *circuit) { int retval = ISIS_OK; - frr_elevate_privs(&isisd_privs) { + frr_with_privs(&isisd_privs) { retval = open_bpf_dev(circuit); diff --git a/isisd/isis_dlpi.c b/isisd/isis_dlpi.c index a96dd93804..7d3dfcb01e 100644 --- a/isisd/isis_dlpi.c +++ b/isisd/isis_dlpi.c @@ -467,7 +467,7 @@ int isis_sock_init(struct isis_circuit *circuit) { int retval = ISIS_OK; - frr_elevate_privs(&isisd_privs) { + frr_with_privs(&isisd_privs) { retval = open_dlpi_dev(circuit); diff --git a/isisd/isis_pfpacket.c b/isisd/isis_pfpacket.c index ea66e6950e..69ac3fc555 100644 --- a/isisd/isis_pfpacket.c +++ b/isisd/isis_pfpacket.c @@ -183,7 +183,7 @@ int isis_sock_init(struct isis_circuit *circuit) { int retval = ISIS_OK; - frr_elevate_privs(&isisd_privs) { + frr_with_privs(&isisd_privs) { retval = open_packet_socket(circuit); diff --git a/ldpd/socket.c b/ldpd/socket.c index b31db2c7bc..8706d03c6f 100644 --- a/ldpd/socket.c +++ b/ldpd/socket.c @@ -79,7 +79,7 @@ ldp_create_socket(int af, enum socket_type type) sock_set_bindany(fd, 1); break; } - frr_elevate_privs(&ldpd_privs) { + frr_with_privs(&ldpd_privs) { if (sock_set_reuse(fd, 1) == -1) { close(fd); return (-1); @@ -254,7 +254,7 @@ int sock_set_bindany(int fd, int enable) { #ifdef HAVE_SO_BINDANY - frr_elevate_privs(&ldpd_privs) { + frr_with_privs(&ldpd_privs) { if (setsockopt(fd, SOL_SOCKET, SO_BINDANY, &enable, sizeof(int)) < 0) { log_warn("%s: error setting SO_BINDANY", __func__); @@ -269,7 +269,7 @@ sock_set_bindany(int fd, int enable) } return (0); #elif defined(IP_BINDANY) - frr_elevate_privs(&ldpd_privs) { + frr_with_privs(&ldpd_privs) { if (setsockopt(fd, IPPROTO_IP, IP_BINDANY, &enable, sizeof(int)) < 0) { log_warn("%s: error setting IP_BINDANY", __func__); @@ -304,7 +304,7 @@ sock_set_md5sig(int fd, int af, union ldpd_addr *addr, const char *password) #if HAVE_DECL_TCP_MD5SIG addr2sa(af, addr, 0, &su); - frr_elevate_privs(&ldpe_privs) { + frr_with_privs(&ldpe_privs) { ret = sockopt_tcp_signature(fd, &su, password); save_errno = errno; } diff --git a/lib/compiler.h b/lib/compiler.h index 6700ca9e8b..e430925e69 100644 --- a/lib/compiler.h +++ b/lib/compiler.h @@ -121,6 +121,49 @@ extern "C" { #define macro_inline static inline __attribute__((unused)) #define macro_pure static inline __attribute__((unused, pure)) + +/* variadic macros, use like: + * #define V_0() ... + * #define V_1(x) ... + * #define V(...) MACRO_VARIANT(V, ##__VA_ARGS__)(__VA_ARGS__) + */ +#define _MACRO_VARIANT(A0,A1,A2,A3,A4,A5,A6,A7,A8,A9,A10, N, ...) N + +#define _CONCAT2(a, b) a ## b +#define _CONCAT(a, b) _CONCAT2(a,b) + +#define MACRO_VARIANT(NAME, ...) \ + _CONCAT(NAME, _MACRO_VARIANT(0, ##__VA_ARGS__, \ + _10, _9, _8, _7, _6, _5, _4, _3, _2, _1, _0)) + +#define NAMECTR(name) _CONCAT(name, __COUNTER__) + +/* per-arg repeat macros, use like: + * #define PERARG(n) ...n... + * #define FOO(...) MACRO_REPEAT(PERARG, ##__VA_ARGS__) + */ + +#define _MACRO_REPEAT_0(NAME) +#define _MACRO_REPEAT_1(NAME, A1) \ + NAME(A1) +#define _MACRO_REPEAT_2(NAME, A1, A2) \ + NAME(A1) NAME(A2) +#define _MACRO_REPEAT_3(NAME, A1, A2, A3) \ + NAME(A1) NAME(A2) NAME(A3) +#define _MACRO_REPEAT_4(NAME, A1, A2, A3, A4) \ + NAME(A1) NAME(A2) NAME(A3) NAME(A4) +#define _MACRO_REPEAT_5(NAME, A1, A2, A3, A4, A5) \ + NAME(A1) NAME(A2) NAME(A3) NAME(A4) NAME(A5) +#define _MACRO_REPEAT_6(NAME, A1, A2, A3, A4, A5, A6) \ + NAME(A1) NAME(A2) NAME(A3) NAME(A4) NAME(A5) NAME(A6) +#define _MACRO_REPEAT_7(NAME, A1, A2, A3, A4, A5, A6, A7) \ + NAME(A1) NAME(A2) NAME(A3) NAME(A4) NAME(A5) NAME(A6) NAME(A7) +#define _MACRO_REPEAT_8(NAME, A1, A2, A3, A4, A5, A6, A7, A8) \ + NAME(A1) NAME(A2) NAME(A3) NAME(A4) NAME(A5) NAME(A6) NAME(A7) NAME(A8) + +#define MACRO_REPEAT(NAME, ...) \ + MACRO_VARIANT(_MACRO_REPEAT, ##__VA_ARGS__)(NAME, ##__VA_ARGS__) + /* * for warnings on macros, put in the macro content like this: * #define MACRO BLA CPP_WARN("MACRO has been deprecated") diff --git a/lib/ferr.c b/lib/ferr.c index fd5fb50172..ccf63dea17 100644 --- a/lib/ferr.c +++ b/lib/ferr.c @@ -33,6 +33,7 @@ #include "command.h" #include "json.h" #include "linklist.h" +#include "frr_pthread.h" DEFINE_MTYPE_STATIC(LIB, ERRINFO, "error information") @@ -83,14 +84,12 @@ void log_ref_add(struct log_ref *ref) { uint32_t i = 0; - pthread_mutex_lock(&refs_mtx); - { + frr_with_mutex(&refs_mtx) { while (ref[i].code != END_FERR) { hash_get(refs, &ref[i], hash_alloc_intern); i++; } } - pthread_mutex_unlock(&refs_mtx); } struct log_ref *log_ref_get(uint32_t code) @@ -99,11 +98,9 @@ struct log_ref *log_ref_get(uint32_t code) struct log_ref *ref; holder.code = code; - pthread_mutex_lock(&refs_mtx); - { + frr_with_mutex(&refs_mtx) { ref = hash_lookup(refs, &holder); } - pthread_mutex_unlock(&refs_mtx); return ref; } @@ -118,11 +115,9 @@ void log_ref_display(struct vty *vty, uint32_t code, bool json) if (json) top = json_object_new_object(); - pthread_mutex_lock(&refs_mtx); - { + frr_with_mutex(&refs_mtx) { errlist = code ? list_new() : hash_to_list(refs); } - pthread_mutex_unlock(&refs_mtx); if (code) { ref = log_ref_get(code); @@ -189,23 +184,19 @@ DEFUN_NOSH(show_error_code, void log_ref_init(void) { - pthread_mutex_lock(&refs_mtx); - { + frr_with_mutex(&refs_mtx) { refs = hash_create(ferr_hash_key, ferr_hash_cmp, "Error Reference Texts"); } - pthread_mutex_unlock(&refs_mtx); } void log_ref_fini(void) { - pthread_mutex_lock(&refs_mtx); - { + frr_with_mutex(&refs_mtx) { hash_clean(refs, NULL); hash_free(refs); refs = NULL; } - pthread_mutex_unlock(&refs_mtx); } void log_ref_vty_init(void) diff --git a/lib/frr_pthread.c b/lib/frr_pthread.c index bdb6c2a397..21dfc9256f 100644 --- a/lib/frr_pthread.c +++ b/lib/frr_pthread.c @@ -49,21 +49,17 @@ static struct list *frr_pthread_list; void frr_pthread_init(void) { - pthread_mutex_lock(&frr_pthread_list_mtx); - { + frr_with_mutex(&frr_pthread_list_mtx) { frr_pthread_list = list_new(); frr_pthread_list->del = (void (*)(void *))&frr_pthread_destroy; } - pthread_mutex_unlock(&frr_pthread_list_mtx); } void frr_pthread_finish(void) { - pthread_mutex_lock(&frr_pthread_list_mtx); - { + frr_with_mutex(&frr_pthread_list_mtx) { list_delete(&frr_pthread_list); } - pthread_mutex_unlock(&frr_pthread_list_mtx); } struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr, @@ -94,11 +90,9 @@ struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr, pthread_mutex_init(fpt->running_cond_mtx, NULL); pthread_cond_init(fpt->running_cond, NULL); - pthread_mutex_lock(&frr_pthread_list_mtx); - { + frr_with_mutex(&frr_pthread_list_mtx) { listnode_add(frr_pthread_list, fpt); } - pthread_mutex_unlock(&frr_pthread_list_mtx); return fpt; } @@ -162,23 +156,19 @@ int frr_pthread_run(struct frr_pthread *fpt, const pthread_attr_t *attr) void frr_pthread_wait_running(struct frr_pthread *fpt) { - pthread_mutex_lock(fpt->running_cond_mtx); - { + frr_with_mutex(fpt->running_cond_mtx) { while (!fpt->running) pthread_cond_wait(fpt->running_cond, fpt->running_cond_mtx); } - pthread_mutex_unlock(fpt->running_cond_mtx); } void frr_pthread_notify_running(struct frr_pthread *fpt) { - pthread_mutex_lock(fpt->running_cond_mtx); - { + frr_with_mutex(fpt->running_cond_mtx) { fpt->running = true; pthread_cond_signal(fpt->running_cond); } - pthread_mutex_unlock(fpt->running_cond_mtx); } int frr_pthread_stop(struct frr_pthread *fpt, void **result) @@ -190,14 +180,12 @@ int frr_pthread_stop(struct frr_pthread *fpt, void **result) void frr_pthread_stop_all(void) { - pthread_mutex_lock(&frr_pthread_list_mtx); - { + frr_with_mutex(&frr_pthread_list_mtx) { struct listnode *n; struct frr_pthread *fpt; for (ALL_LIST_ELEMENTS_RO(frr_pthread_list, n, fpt)) frr_pthread_stop(fpt, NULL); } - pthread_mutex_unlock(&frr_pthread_list_mtx); } /* diff --git a/lib/frr_pthread.h b/lib/frr_pthread.h index 6096a50370..f70c8a0db4 100644 --- a/lib/frr_pthread.h +++ b/lib/frr_pthread.h @@ -215,6 +215,54 @@ void frr_pthread_stop_all(void); #define pthread_condattr_setclock(A, B) #endif +/* mutex auto-lock/unlock */ + +/* variant 1: + * (for short blocks, multiple mutexes supported) + * break & return can be used for aborting the block + * + * frr_with_mutex(&mtx, &mtx2) { + * if (error) + * break; + * ... + * } + */ +#define _frr_with_mutex(mutex) \ + *NAMECTR(_mtx_) __attribute__(( \ + unused, cleanup(_frr_mtx_unlock))) = _frr_mtx_lock(mutex), \ + /* end */ + +#define frr_with_mutex(...) \ + for (pthread_mutex_t MACRO_REPEAT(_frr_with_mutex, ##__VA_ARGS__) \ + *_once = NULL; _once == NULL; _once = (void *)1) \ + /* end */ + +/* variant 2: + * (more suitable for long blocks, no extra indentation) + * + * frr_mutex_lock_autounlock(&mtx); + * ... + */ +#define frr_mutex_lock_autounlock(mutex) \ + pthread_mutex_t *NAMECTR(_mtx_) \ + __attribute__((unused, cleanup(_frr_mtx_unlock))) = \ + _frr_mtx_lock(mutex) \ + /* end */ + +static inline pthread_mutex_t *_frr_mtx_lock(pthread_mutex_t *mutex) +{ + pthread_mutex_lock(mutex); + return mutex; +} + +static inline void _frr_mtx_unlock(pthread_mutex_t **mutex) +{ + if (!*mutex) + return; + pthread_mutex_unlock(*mutex); + *mutex = NULL; +} + #ifdef __cplusplus } #endif diff --git a/lib/hash.c b/lib/hash.c index 9d9d39702e..7f8a237047 100644 --- a/lib/hash.c +++ b/lib/hash.c @@ -28,6 +28,7 @@ #include "vty.h" #include "command.h" #include "libfrr.h" +#include "frr_pthread.h" DEFINE_MTYPE_STATIC(LIB, HASH, "Hash") DEFINE_MTYPE_STATIC(LIB, HASH_BACKET, "Hash Bucket") @@ -54,14 +55,12 @@ struct hash *hash_create_size(unsigned int size, hash->name = name ? XSTRDUP(MTYPE_HASH, name) : NULL; hash->stats.empty = hash->size; - pthread_mutex_lock(&_hashes_mtx); - { + frr_with_mutex(&_hashes_mtx) { if (!_hashes) _hashes = list_new(); listnode_add(_hashes, hash); } - pthread_mutex_unlock(&_hashes_mtx); return hash; } @@ -311,8 +310,7 @@ struct list *hash_to_list(struct hash *hash) void hash_free(struct hash *hash) { - pthread_mutex_lock(&_hashes_mtx); - { + frr_with_mutex(&_hashes_mtx) { if (_hashes) { listnode_delete(_hashes, hash); if (_hashes->count == 0) { @@ -320,7 +318,6 @@ void hash_free(struct hash *hash) } } } - pthread_mutex_unlock(&_hashes_mtx); XFREE(MTYPE_HASH, hash->name); @@ -31,6 +31,7 @@ #include "lib_errors.h" #include "lib/hook.h" #include "printfrr.h" +#include "frr_pthread.h" #ifndef SUNOS_5 #include <sys/un.h> @@ -83,89 +84,70 @@ static int zlog_filter_lookup(const char *lookup) void zlog_filter_clear(void) { - pthread_mutex_lock(&loglock); - zlog_filter_count = 0; - pthread_mutex_unlock(&loglock); + frr_with_mutex(&loglock) { + zlog_filter_count = 0; + } } int zlog_filter_add(const char *filter) { - pthread_mutex_lock(&loglock); + frr_with_mutex(&loglock) { + if (zlog_filter_count >= ZLOG_FILTERS_MAX) + return 1; - int ret = 0; + if (zlog_filter_lookup(filter) != -1) + /* Filter already present */ + return -1; - if (zlog_filter_count >= ZLOG_FILTERS_MAX) { - ret = 1; - goto done; - } + strlcpy(zlog_filters[zlog_filter_count], filter, + sizeof(zlog_filters[0])); - if (zlog_filter_lookup(filter) != -1) { - /* Filter already present */ - ret = -1; - goto done; - } + if (zlog_filters[zlog_filter_count][0] == '\0') + /* Filter was either empty or didn't get copied + * correctly + */ + return -1; - strlcpy(zlog_filters[zlog_filter_count], filter, - sizeof(zlog_filters[0])); - - if (zlog_filters[zlog_filter_count][0] == '\0') { - /* Filter was either empty or didn't get copied correctly */ - ret = -1; - goto done; + zlog_filter_count++; } - - zlog_filter_count++; - -done: - pthread_mutex_unlock(&loglock); - return ret; + return 0; } int zlog_filter_del(const char *filter) { - pthread_mutex_lock(&loglock); - - int found_idx = zlog_filter_lookup(filter); - int last_idx = zlog_filter_count - 1; - int ret = 0; + frr_with_mutex(&loglock) { + int found_idx = zlog_filter_lookup(filter); + int last_idx = zlog_filter_count - 1; - if (found_idx == -1) { - /* Didn't find the filter to delete */ - ret = -1; - goto done; - } - - /* Adjust the filter array */ - memmove(zlog_filters[found_idx], zlog_filters[found_idx + 1], - (last_idx - found_idx) * sizeof(zlog_filters[0])); + if (found_idx == -1) + /* Didn't find the filter to delete */ + return -1; - zlog_filter_count--; + /* Adjust the filter array */ + memmove(zlog_filters[found_idx], zlog_filters[found_idx + 1], + (last_idx - found_idx) * sizeof(zlog_filters[0])); -done: - pthread_mutex_unlock(&loglock); - return ret; + zlog_filter_count--; + } + return 0; } /* Dump all filters to buffer, delimited by new line */ int zlog_filter_dump(char *buf, size_t max_size) { - pthread_mutex_lock(&loglock); - - int ret = 0; int len = 0; - for (int i = 0; i < zlog_filter_count; i++) { - ret = snprintf(buf + len, max_size - len, " %s\n", - zlog_filters[i]); - len += ret; - if ((ret < 0) || ((size_t)len >= max_size)) { - len = -1; - goto done; + frr_with_mutex(&loglock) { + for (int i = 0; i < zlog_filter_count; i++) { + int ret; + ret = snprintf(buf + len, max_size - len, " %s\n", + zlog_filters[i]); + len += ret; + if ((ret < 0) || ((size_t)len >= max_size)) + return -1; } } -done: - pthread_mutex_unlock(&loglock); return len; } @@ -363,7 +345,7 @@ search: /* va_list version of zlog. */ void vzlog(int priority, const char *format, va_list args) { - pthread_mutex_lock(&loglock); + frr_mutex_lock_autounlock(&loglock); char proto_str[32] = ""; int original_errno = errno; @@ -430,36 +412,31 @@ out: if (msg != buf) XFREE(MTYPE_TMP, msg); errno = original_errno; - pthread_mutex_unlock(&loglock); } int vzlog_test(int priority) { - pthread_mutex_lock(&loglock); - - int ret = 0; + frr_mutex_lock_autounlock(&loglock); struct zlog *zl = zlog_default; /* When zlog_default is also NULL, use stderr for logging. */ if (zl == NULL) - ret = 1; + return 1; /* Syslog output */ else if (priority <= zl->maxlvl[ZLOG_DEST_SYSLOG]) - ret = 1; + return 1; /* File output. */ else if ((priority <= zl->maxlvl[ZLOG_DEST_FILE]) && zl->fp) - ret = 1; + return 1; /* stdout output. */ else if (priority <= zl->maxlvl[ZLOG_DEST_STDOUT]) - ret = 1; + return 1; /* Terminal monitor. */ else if (priority <= zl->maxlvl[ZLOG_DEST_MONITOR]) - ret = 1; - - pthread_mutex_unlock(&loglock); + return 1; - return ret; + return 0; } /* @@ -870,9 +847,9 @@ void openzlog(const char *progname, const char *protoname, openlog(progname, syslog_flags, zl->facility); - pthread_mutex_lock(&loglock); - zlog_default = zl; - pthread_mutex_unlock(&loglock); + frr_with_mutex(&loglock) { + zlog_default = zl; + } #ifdef HAVE_GLIBC_BACKTRACE /* work around backtrace() using lazily resolved dynamically linked @@ -889,7 +866,8 @@ void openzlog(const char *progname, const char *protoname, void closezlog(void) { - pthread_mutex_lock(&loglock); + frr_mutex_lock_autounlock(&loglock); + struct zlog *zl = zlog_default; closelog(); @@ -901,15 +879,14 @@ void closezlog(void) XFREE(MTYPE_ZLOG, zl); zlog_default = NULL; - pthread_mutex_unlock(&loglock); } /* Called from command.c. */ void zlog_set_level(zlog_dest_t dest, int log_level) { - pthread_mutex_lock(&loglock); - zlog_default->maxlvl[dest] = log_level; - pthread_mutex_unlock(&loglock); + frr_with_mutex(&loglock) { + zlog_default->maxlvl[dest] = log_level; + } } int zlog_set_file(const char *filename, int log_level) @@ -929,15 +906,15 @@ int zlog_set_file(const char *filename, int log_level) if (fp == NULL) { ret = 0; } else { - pthread_mutex_lock(&loglock); - zl = zlog_default; - - /* Set flags. */ - zl->filename = XSTRDUP(MTYPE_ZLOG, filename); - zl->maxlvl[ZLOG_DEST_FILE] = log_level; - zl->fp = fp; - logfile_fd = fileno(fp); - pthread_mutex_unlock(&loglock); + frr_with_mutex(&loglock) { + zl = zlog_default; + + /* Set flags. */ + zl->filename = XSTRDUP(MTYPE_ZLOG, filename); + zl->maxlvl[ZLOG_DEST_FILE] = log_level; + zl->fp = fp; + logfile_fd = fileno(fp); + } } return ret; @@ -946,7 +923,7 @@ int zlog_set_file(const char *filename, int log_level) /* Reset opend file. */ int zlog_reset_file(void) { - pthread_mutex_lock(&loglock); + frr_mutex_lock_autounlock(&loglock); struct zlog *zl = zlog_default; @@ -959,8 +936,6 @@ int zlog_reset_file(void) XFREE(MTYPE_ZLOG, zl->filename); zl->filename = NULL; - pthread_mutex_unlock(&loglock); - return 1; } diff --git a/lib/northbound.c b/lib/northbound.c index 48b450e969..a814f23e14 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -26,6 +26,7 @@ #include "command.h" #include "debug.h" #include "db.h" +#include "frr_pthread.h" #include "northbound.h" #include "northbound_cli.h" #include "northbound_db.h" @@ -723,8 +724,7 @@ int nb_running_lock(enum nb_client client, const void *user) { int ret = -1; - pthread_mutex_lock(&running_config_mgmt_lock.mtx); - { + frr_with_mutex(&running_config_mgmt_lock.mtx) { if (!running_config_mgmt_lock.locked) { running_config_mgmt_lock.locked = true; running_config_mgmt_lock.owner_client = client; @@ -732,7 +732,6 @@ int nb_running_lock(enum nb_client client, const void *user) ret = 0; } } - pthread_mutex_unlock(&running_config_mgmt_lock.mtx); return ret; } @@ -741,8 +740,7 @@ int nb_running_unlock(enum nb_client client, const void *user) { int ret = -1; - pthread_mutex_lock(&running_config_mgmt_lock.mtx); - { + frr_with_mutex(&running_config_mgmt_lock.mtx) { if (running_config_mgmt_lock.locked && running_config_mgmt_lock.owner_client == client && running_config_mgmt_lock.owner_user == user) { @@ -752,7 +750,6 @@ int nb_running_unlock(enum nb_client client, const void *user) ret = 0; } } - pthread_mutex_unlock(&running_config_mgmt_lock.mtx); return ret; } @@ -761,14 +758,12 @@ int nb_running_lock_check(enum nb_client client, const void *user) { int ret = -1; - pthread_mutex_lock(&running_config_mgmt_lock.mtx); - { + frr_with_mutex(&running_config_mgmt_lock.mtx) { if (!running_config_mgmt_lock.locked || (running_config_mgmt_lock.owner_client == client && running_config_mgmt_lock.owner_user == user)) ret = 0; } - pthread_mutex_unlock(&running_config_mgmt_lock.mtx); return ret; } diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c index 44a55137f8..77183282ba 100644 --- a/lib/northbound_sysrepo.c +++ b/lib/northbound_sysrepo.c @@ -425,7 +425,7 @@ static int frr_sr_state_cb(const char *xpath, sr_val_t **values, exit: list_delete(&elements); *values = NULL; - values_cnt = 0; + *values_cnt = 0; return SR_ERR_OK; } diff --git a/lib/privs.c b/lib/privs.c index a3314c6c3c..09efedf684 100644 --- a/lib/privs.c +++ b/lib/privs.c @@ -24,6 +24,7 @@ #include "log.h" #include "privs.h" #include "memory.h" +#include "frr_pthread.h" #include "lib_errors.h" #include "lib/queue.h" @@ -760,8 +761,7 @@ struct zebra_privs_t *_zprivs_raise(struct zebra_privs_t *privs, * Serialize 'raise' operations; particularly important for * OSes where privs are process-wide. */ - pthread_mutex_lock(&(privs->mutex)); - { + frr_with_mutex(&(privs->mutex)) { /* Locate ref-counting object to use */ refs = get_privs_refs(privs); @@ -775,7 +775,6 @@ struct zebra_privs_t *_zprivs_raise(struct zebra_privs_t *privs, refs->raised_in_funcname = funcname; } } - pthread_mutex_unlock(&(privs->mutex)); return privs; } @@ -791,8 +790,7 @@ void _zprivs_lower(struct zebra_privs_t **privs) /* Serialize 'lower privs' operation - particularly important * when OS privs are process-wide. */ - pthread_mutex_lock(&(*privs)->mutex); - { + frr_with_mutex(&(*privs)->mutex) { refs = get_privs_refs(*privs); if (--(refs->refcount) == 0) { @@ -806,7 +804,6 @@ void _zprivs_lower(struct zebra_privs_t **privs) refs->raised_in_funcname = NULL; } } - pthread_mutex_unlock(&(*privs)->mutex); *privs = NULL; } diff --git a/lib/privs.h b/lib/privs.h index 2b0b44b3f2..db5707d675 100644 --- a/lib/privs.h +++ b/lib/privs.h @@ -109,16 +109,16 @@ extern void zprivs_get_ids(struct zprivs_ids_t *); /* * Wrapper around zprivs, to be used as: - * frr_elevate_privs(&privs) { + * frr_with_privs(&privs) { * ... code ... * if (error) * break; -- break can be used to get out of the block * ... code ... * } * - * The argument to frr_elevate_privs() can be NULL to leave privileges as-is + * The argument to frr_with_privs() can be NULL to leave privileges as-is * (mostly useful for conditional privilege-raising, i.e.:) - * frr_elevate_privs(cond ? &privs : NULL) {} + * frr_with_privs(cond ? &privs : NULL) {} * * NB: The code block is always executed, regardless of whether privileges * could be raised or not, or whether NULL was given or not. This is fully @@ -138,7 +138,7 @@ extern struct zebra_privs_t *_zprivs_raise(struct zebra_privs_t *privs, const char *funcname); extern void _zprivs_lower(struct zebra_privs_t **privs); -#define frr_elevate_privs(privs) \ +#define frr_with_privs(privs) \ for (struct zebra_privs_t *_once = NULL, \ *_privs __attribute__( \ (unused, cleanup(_zprivs_lower))) = \ diff --git a/lib/stream.c b/lib/stream.c index dfd13ca186..2e1a0193a2 100644 --- a/lib/stream.c +++ b/lib/stream.c @@ -28,6 +28,7 @@ #include "network.h" #include "prefix.h" #include "log.h" +#include "frr_pthread.h" #include "lib_errors.h" DEFINE_MTYPE_STATIC(LIB, STREAM, "Stream") @@ -1136,11 +1137,9 @@ void stream_fifo_push(struct stream_fifo *fifo, struct stream *s) void stream_fifo_push_safe(struct stream_fifo *fifo, struct stream *s) { - pthread_mutex_lock(&fifo->mtx); - { + frr_with_mutex(&fifo->mtx) { stream_fifo_push(fifo, s); } - pthread_mutex_unlock(&fifo->mtx); } /* Delete first stream from fifo. */ @@ -1170,11 +1169,9 @@ struct stream *stream_fifo_pop_safe(struct stream_fifo *fifo) { struct stream *ret; - pthread_mutex_lock(&fifo->mtx); - { + frr_with_mutex(&fifo->mtx) { ret = stream_fifo_pop(fifo); } - pthread_mutex_unlock(&fifo->mtx); return ret; } @@ -1188,11 +1185,9 @@ struct stream *stream_fifo_head_safe(struct stream_fifo *fifo) { struct stream *ret; - pthread_mutex_lock(&fifo->mtx); - { + frr_with_mutex(&fifo->mtx) { ret = stream_fifo_head(fifo); } - pthread_mutex_unlock(&fifo->mtx); return ret; } @@ -1212,11 +1207,9 @@ void stream_fifo_clean(struct stream_fifo *fifo) void stream_fifo_clean_safe(struct stream_fifo *fifo) { - pthread_mutex_lock(&fifo->mtx); - { + frr_with_mutex(&fifo->mtx) { stream_fifo_clean(fifo); } - pthread_mutex_unlock(&fifo->mtx); } size_t stream_fifo_count_safe(struct stream_fifo *fifo) diff --git a/lib/thread.c b/lib/thread.c index 943b849ebf..90c794e88d 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -33,6 +33,7 @@ #include "network.h" #include "jhash.h" #include "frratomic.h" +#include "frr_pthread.h" #include "lib_errors.h" DEFINE_MTYPE_STATIC(LIB, THREAD, "Thread") @@ -173,8 +174,7 @@ static void cpu_record_print(struct vty *vty, uint8_t filter) tmp.funcname = "TOTAL"; tmp.types = filter; - pthread_mutex_lock(&masters_mtx); - { + frr_with_mutex(&masters_mtx) { for (ALL_LIST_ELEMENTS_RO(masters, ln, m)) { const char *name = m->name ? m->name : "main"; @@ -206,7 +206,6 @@ static void cpu_record_print(struct vty *vty, uint8_t filter) vty_out(vty, "\n"); } } - pthread_mutex_unlock(&masters_mtx); vty_out(vty, "\n"); vty_out(vty, "Total thread statistics\n"); @@ -240,11 +239,9 @@ static void cpu_record_clear(uint8_t filter) struct thread_master *m; struct listnode *ln; - pthread_mutex_lock(&masters_mtx); - { + frr_with_mutex(&masters_mtx) { for (ALL_LIST_ELEMENTS_RO(masters, ln, m)) { - pthread_mutex_lock(&m->mtx); - { + frr_with_mutex(&m->mtx) { void *args[2] = {tmp, m->cpu_record}; hash_iterate( m->cpu_record, @@ -252,10 +249,8 @@ static void cpu_record_clear(uint8_t filter) void *))cpu_record_hash_clear, args); } - pthread_mutex_unlock(&m->mtx); } } - pthread_mutex_unlock(&masters_mtx); } static uint8_t parse_filter(const char *filterstr) @@ -370,13 +365,11 @@ DEFUN (show_thread_poll, struct listnode *node; struct thread_master *m; - pthread_mutex_lock(&masters_mtx); - { + frr_with_mutex(&masters_mtx) { for (ALL_LIST_ELEMENTS_RO(masters, node, m)) { show_thread_poll_helper(vty, m); } } - pthread_mutex_unlock(&masters_mtx); return CMD_SUCCESS; } @@ -487,26 +480,22 @@ struct thread_master *thread_master_create(const char *name) sizeof(struct pollfd) * rv->handler.pfdsize); /* add to list of threadmasters */ - pthread_mutex_lock(&masters_mtx); - { + frr_with_mutex(&masters_mtx) { if (!masters) masters = list_new(); listnode_add(masters, rv); } - pthread_mutex_unlock(&masters_mtx); return rv; } void thread_master_set_name(struct thread_master *master, const char *name) { - pthread_mutex_lock(&master->mtx); - { + frr_with_mutex(&master->mtx) { XFREE(MTYPE_THREAD_MASTER, master->name); master->name = XSTRDUP(MTYPE_THREAD_MASTER, name); } - pthread_mutex_unlock(&master->mtx); } #define THREAD_UNUSED_DEPTH 10 @@ -569,13 +558,11 @@ static void thread_array_free(struct thread_master *m, */ void thread_master_free_unused(struct thread_master *m) { - pthread_mutex_lock(&m->mtx); - { + frr_with_mutex(&m->mtx) { struct thread *t; while ((t = thread_list_pop(&m->unuse))) thread_free(m, t); } - pthread_mutex_unlock(&m->mtx); } /* Stop thread scheduler. */ @@ -583,14 +570,12 @@ void thread_master_free(struct thread_master *m) { struct thread *t; - pthread_mutex_lock(&masters_mtx); - { + frr_with_mutex(&masters_mtx) { listnode_delete(masters, m); if (masters->count == 0) { list_delete(&masters); } } - pthread_mutex_unlock(&masters_mtx); thread_array_free(m, m->read); thread_array_free(m, m->write); @@ -621,11 +606,9 @@ unsigned long thread_timer_remain_msec(struct thread *thread) { int64_t remain; - pthread_mutex_lock(&thread->mtx); - { + frr_with_mutex(&thread->mtx) { remain = monotime_until(&thread->u.sands, NULL) / 1000LL; } - pthread_mutex_unlock(&thread->mtx); return remain < 0 ? 0 : remain; } @@ -642,11 +625,9 @@ unsigned long thread_timer_remain_second(struct thread *thread) struct timeval thread_timer_remain(struct thread *thread) { struct timeval remain; - pthread_mutex_lock(&thread->mtx); - { + frr_with_mutex(&thread->mtx) { monotime_until(&thread->u.sands, &remain); } - pthread_mutex_unlock(&thread->mtx); return remain; } @@ -770,14 +751,10 @@ struct thread *funcname_thread_add_read_write(int dir, struct thread_master *m, struct thread **thread_array; assert(fd >= 0 && fd < m->fd_limit); - pthread_mutex_lock(&m->mtx); - { - if (t_ptr - && *t_ptr) // thread is already scheduled; don't reschedule - { - pthread_mutex_unlock(&m->mtx); - return NULL; - } + frr_with_mutex(&m->mtx) { + if (t_ptr && *t_ptr) + // thread is already scheduled; don't reschedule + break; /* default to a new pollfd */ nfds_t queuepos = m->handler.pfdcount; @@ -817,12 +794,10 @@ struct thread *funcname_thread_add_read_write(int dir, struct thread_master *m, m->handler.pfdcount++; if (thread) { - pthread_mutex_lock(&thread->mtx); - { + frr_with_mutex(&thread->mtx) { thread->u.fd = fd; thread_array[thread->u.fd] = thread; } - pthread_mutex_unlock(&thread->mtx); if (t_ptr) { *t_ptr = thread; @@ -832,7 +807,6 @@ struct thread *funcname_thread_add_read_write(int dir, struct thread_master *m, AWAKEN(m); } - pthread_mutex_unlock(&m->mtx); return thread; } @@ -850,19 +824,14 @@ funcname_thread_add_timer_timeval(struct thread_master *m, assert(type == THREAD_TIMER); assert(time_relative); - pthread_mutex_lock(&m->mtx); - { - if (t_ptr - && *t_ptr) // thread is already scheduled; don't reschedule - { - pthread_mutex_unlock(&m->mtx); + frr_with_mutex(&m->mtx) { + if (t_ptr && *t_ptr) + // thread is already scheduled; don't reschedule return NULL; - } thread = thread_get(m, type, func, arg, debugargpass); - pthread_mutex_lock(&thread->mtx); - { + frr_with_mutex(&thread->mtx) { monotime(&thread->u.sands); timeradd(&thread->u.sands, time_relative, &thread->u.sands); @@ -872,11 +841,9 @@ funcname_thread_add_timer_timeval(struct thread_master *m, thread->ref = t_ptr; } } - pthread_mutex_unlock(&thread->mtx); AWAKEN(m); } - pthread_mutex_unlock(&m->mtx); return thread; } @@ -933,26 +900,20 @@ struct thread *funcname_thread_add_event(struct thread_master *m, void *arg, int val, struct thread **t_ptr, debugargdef) { - struct thread *thread; + struct thread *thread = NULL; assert(m != NULL); - pthread_mutex_lock(&m->mtx); - { - if (t_ptr - && *t_ptr) // thread is already scheduled; don't reschedule - { - pthread_mutex_unlock(&m->mtx); - return NULL; - } + frr_with_mutex(&m->mtx) { + if (t_ptr && *t_ptr) + // thread is already scheduled; don't reschedule + break; thread = thread_get(m, THREAD_EVENT, func, arg, debugargpass); - pthread_mutex_lock(&thread->mtx); - { + frr_with_mutex(&thread->mtx) { thread->u.val = val; thread_list_add_tail(&m->event, thread); } - pthread_mutex_unlock(&thread->mtx); if (t_ptr) { *t_ptr = thread; @@ -961,7 +922,6 @@ struct thread *funcname_thread_add_event(struct thread_master *m, AWAKEN(m); } - pthread_mutex_unlock(&m->mtx); return thread; } @@ -1143,15 +1103,13 @@ void thread_cancel_event(struct thread_master *master, void *arg) { assert(master->owner == pthread_self()); - pthread_mutex_lock(&master->mtx); - { + frr_with_mutex(&master->mtx) { struct cancel_req *cr = XCALLOC(MTYPE_TMP, sizeof(struct cancel_req)); cr->eventobj = arg; listnode_add(master->cancel_req, cr); do_thread_cancel(master); } - pthread_mutex_unlock(&master->mtx); } /** @@ -1167,15 +1125,13 @@ void thread_cancel(struct thread *thread) assert(master->owner == pthread_self()); - pthread_mutex_lock(&master->mtx); - { + frr_with_mutex(&master->mtx) { struct cancel_req *cr = XCALLOC(MTYPE_TMP, sizeof(struct cancel_req)); cr->thread = thread; listnode_add(master->cancel_req, cr); do_thread_cancel(master); } - pthread_mutex_unlock(&master->mtx); } /** @@ -1208,8 +1164,7 @@ void thread_cancel_async(struct thread_master *master, struct thread **thread, assert(!(thread && eventobj) && (thread || eventobj)); assert(master->owner != pthread_self()); - pthread_mutex_lock(&master->mtx); - { + frr_with_mutex(&master->mtx) { master->canceled = false; if (thread) { @@ -1228,7 +1183,6 @@ void thread_cancel_async(struct thread_master *master, struct thread **thread, while (!master->canceled) pthread_cond_wait(&master->cancel_cond, &master->mtx); } - pthread_mutex_unlock(&master->mtx); } /* ------------------------------------------------------------------------- */ @@ -1527,22 +1481,18 @@ unsigned long thread_consumed_time(RUSAGE_T *now, RUSAGE_T *start, int thread_should_yield(struct thread *thread) { int result; - pthread_mutex_lock(&thread->mtx); - { + frr_with_mutex(&thread->mtx) { result = monotime_since(&thread->real, NULL) > (int64_t)thread->yield; } - pthread_mutex_unlock(&thread->mtx); return result; } void thread_set_yield_time(struct thread *thread, unsigned long yield_time) { - pthread_mutex_lock(&thread->mtx); - { + frr_with_mutex(&thread->mtx) { thread->yield = yield_time; } - pthread_mutex_unlock(&thread->mtx); } void thread_getrusage(RUSAGE_T *r) @@ -1637,20 +1587,16 @@ void funcname_thread_execute(struct thread_master *m, struct thread *thread; /* Get or allocate new thread to execute. */ - pthread_mutex_lock(&m->mtx); - { + frr_with_mutex(&m->mtx) { thread = thread_get(m, THREAD_EVENT, func, arg, debugargpass); /* Set its event value. */ - pthread_mutex_lock(&thread->mtx); - { + frr_with_mutex(&thread->mtx) { thread->add_type = THREAD_EXECUTE; thread->u.val = val; thread->ref = &thread; } - pthread_mutex_unlock(&thread->mtx); } - pthread_mutex_unlock(&m->mtx); /* Execute thread doing all accounting. */ thread_call(thread); @@ -755,7 +755,7 @@ DEFUN_NOSH (vrf_netns, if (!pathname) return CMD_WARNING_CONFIG_FAILED; - frr_elevate_privs(vrf_daemon_privs) { + frr_with_privs(vrf_daemon_privs) { ret = vrf_netns_handler_create(vty, vrf, pathname, NS_UNKNOWN, NS_UNKNOWN); } diff --git a/ospf6d/ospf6_network.c b/ospf6d/ospf6_network.c index 625ad884f2..9a18680b8b 100644 --- a/ospf6d/ospf6_network.c +++ b/ospf6d/ospf6_network.c @@ -85,7 +85,7 @@ void ospf6_serv_close(void) /* Make ospf6d's server socket. */ int ospf6_serv_sock(void) { - frr_elevate_privs(&ospf6d_privs) { + frr_with_privs(&ospf6d_privs) { ospf6_sock = socket(AF_INET6, SOCK_RAW, IPPROTO_OSPFIGP); if (ospf6_sock < 0) { diff --git a/ospfd/ospf_network.c b/ospfd/ospf_network.c index 1415a6e8b7..b8e2dac70e 100644 --- a/ospfd/ospf_network.c +++ b/ospfd/ospf_network.c @@ -190,7 +190,7 @@ int ospf_sock_init(struct ospf *ospf) /* silently return since VRF is not ready */ return -1; } - frr_elevate_privs(&ospfd_privs) { + frr_with_privs(&ospfd_privs) { ospf_sock = vrf_socket(AF_INET, SOCK_RAW, IPPROTO_OSPFIGP, ospf->vrf_id, ospf->name); if (ospf_sock < 0) { diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c index b91a55f635..e48a5b4d36 100644 --- a/ospfd/ospfd.c +++ b/ospfd/ospfd.c @@ -2097,7 +2097,7 @@ static int ospf_vrf_enable(struct vrf *vrf) old_vrf_id); if (old_vrf_id != ospf->vrf_id) { - frr_elevate_privs(&ospfd_privs) { + frr_with_privs(&ospfd_privs) { /* stop zebra redist to us for old vrf */ zclient_send_dereg_requests(zclient, old_vrf_id); diff --git a/pbrd/pbr_map.c b/pbrd/pbr_map.c index 5e67990d5e..1a8461c6c1 100644 --- a/pbrd/pbr_map.c +++ b/pbrd/pbr_map.c @@ -316,7 +316,7 @@ struct pbr_map_sequence *pbrms_get(const char *name, uint32_t seqno) pbrms->ruleno = pbr_nht_get_next_rule(seqno); pbrms->parent = pbrm; pbrms->reason = - PBR_MAP_INVALID_SRCDST | + PBR_MAP_INVALID_EMPTY | PBR_MAP_INVALID_NO_NEXTHOPS; QOBJ_REG(pbrms, pbr_map_sequence); @@ -350,10 +350,10 @@ pbr_map_sequence_check_nexthops_valid(struct pbr_map_sequence *pbrms) } } -static void pbr_map_sequence_check_src_dst_valid(struct pbr_map_sequence *pbrms) +static void pbr_map_sequence_check_not_empty(struct pbr_map_sequence *pbrms) { - if (!pbrms->src && !pbrms->dst) - pbrms->reason |= PBR_MAP_INVALID_SRCDST; + if (!pbrms->src && !pbrms->dst && !pbrms->mark) + pbrms->reason |= PBR_MAP_INVALID_EMPTY; } /* @@ -364,7 +364,7 @@ static void pbr_map_sequence_check_valid(struct pbr_map_sequence *pbrms) { pbr_map_sequence_check_nexthops_valid(pbrms); - pbr_map_sequence_check_src_dst_valid(pbrms); + pbr_map_sequence_check_not_empty(pbrms); } static bool pbr_map_check_valid_internal(struct pbr_map *pbrm) diff --git a/pbrd/pbr_map.h b/pbrd/pbr_map.h index 945f76bb2b..112acfe44e 100644 --- a/pbrd/pbr_map.h +++ b/pbrd/pbr_map.h @@ -87,6 +87,7 @@ struct pbr_map_sequence { */ struct prefix *src; struct prefix *dst; + uint32_t mark; /* * Family of the src/dst. Needed when deleting since we clear them @@ -126,7 +127,7 @@ struct pbr_map_sequence { #define PBR_MAP_INVALID_NEXTHOP (1 << 1) #define PBR_MAP_INVALID_NO_NEXTHOPS (1 << 2) #define PBR_MAP_INVALID_BOTH_NHANDGRP (1 << 3) -#define PBR_MAP_INVALID_SRCDST (1 << 4) +#define PBR_MAP_INVALID_EMPTY (1 << 4) uint64_t reason; QOBJ_FIELDS diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c index 95f38563b1..5e7addc9d2 100644 --- a/pbrd/pbr_vty.c +++ b/pbrd/pbr_vty.c @@ -172,6 +172,33 @@ DEFPY(pbr_map_match_dst, pbr_map_match_dst_cmd, return CMD_SUCCESS; } +DEFPY(pbr_map_match_mark, pbr_map_match_mark_cmd, + "[no] match mark (1-4294967295)$mark", + NO_STR + "Match the rest of the command\n" + "Choose the mark value to use\n" + "mark\n") +{ + struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence); + +#ifndef GNU_LINUX + vty_out(vty, "pbr marks are not supported on this platform"); + return CMD_WARNING_CONFIG_FAILED; +#endif + + if (!no) { + if (pbrms->mark == (uint32_t) mark) + return CMD_SUCCESS; + pbrms->mark = (uint32_t) mark; + } else { + pbrms->mark = 0; + } + + pbr_map_check(pbrms); + + return CMD_SUCCESS; + } + DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd, "[no] set nexthop-group NHGNAME$name", NO_STR @@ -453,6 +480,8 @@ DEFPY (show_pbr_map, vty_out(vty, "\tDST Match: %s\n", prefix2str(pbrms->dst, buf, sizeof(buf))); + if (pbrms->mark) + vty_out(vty, "\tMARK Match: %u\n", pbrms->mark); if (pbrms->nhgrp_name) { vty_out(vty, @@ -632,6 +661,9 @@ static int pbr_vty_map_config_write_sequence(struct vty *vty, vty_out(vty, " match dst-ip %s\n", prefix2str(pbrms->dst, buff, sizeof(buff))); + if (pbrms->mark) + vty_out(vty, " match mark %u\n", pbrms->mark); + if (pbrms->nhgrp_name) vty_out(vty, " set nexthop-group %s\n", pbrms->nhgrp_name); @@ -704,6 +736,7 @@ void pbr_vty_init(void) install_element(INTERFACE_NODE, &pbr_policy_cmd); install_element(PBRMAP_NODE, &pbr_map_match_src_cmd); install_element(PBRMAP_NODE, &pbr_map_match_dst_cmd); + install_element(PBRMAP_NODE, &pbr_map_match_mark_cmd); install_element(PBRMAP_NODE, &pbr_map_nexthop_group_cmd); install_element(PBRMAP_NODE, &pbr_map_nexthop_cmd); install_element(VIEW_NODE, &show_pbr_cmd); diff --git a/pbrd/pbr_zebra.c b/pbrd/pbr_zebra.c index 466a9a13ae..d74d0fcd23 100644 --- a/pbrd/pbr_zebra.c +++ b/pbrd/pbr_zebra.c @@ -526,7 +526,7 @@ static void pbr_encode_pbr_map_sequence(struct stream *s, stream_putw(s, 0); /* src port */ pbr_encode_pbr_map_sequence_prefix(s, pbrms->dst, family); stream_putw(s, 0); /* dst port */ - stream_putl(s, 0); /* fwmark */ + stream_putl(s, pbrms->mark); if (pbrms->nhgrp_name) stream_putl(s, pbr_nht_get_table(pbrms->nhgrp_name)); else if (pbrms->nhg) diff --git a/pimd/pim_mroute.c b/pimd/pim_mroute.c index 1c66007fbb..f7f4b54aea 100644 --- a/pimd/pim_mroute.c +++ b/pimd/pim_mroute.c @@ -57,7 +57,7 @@ static int pim_mroute_set(struct pim_instance *pim, int enable) * We need to create the VRF table for the pim mroute_socket */ if (pim->vrf_id != VRF_DEFAULT) { - frr_elevate_privs(&pimd_privs) { + frr_with_privs(&pimd_privs) { data = pim->vrf->data.l.table_id; err = setsockopt(pim->mroute_socket, IPPROTO_IP, @@ -75,7 +75,7 @@ static int pim_mroute_set(struct pim_instance *pim, int enable) } } - frr_elevate_privs(&pimd_privs) { + frr_with_privs(&pimd_privs) { opt = enable ? MRT_INIT : MRT_DONE; /* * *BSD *cares* about what value we pass down @@ -735,7 +735,7 @@ int pim_mroute_socket_enable(struct pim_instance *pim) { int fd; - frr_elevate_privs(&pimd_privs) { + frr_with_privs(&pimd_privs) { fd = socket(AF_INET, SOCK_RAW, IPPROTO_IGMP); diff --git a/pimd/pim_msdp_socket.c b/pimd/pim_msdp_socket.c index b1f7cfd2c6..22eb8bc7b4 100644 --- a/pimd/pim_msdp_socket.c +++ b/pimd/pim_msdp_socket.c @@ -175,7 +175,7 @@ int pim_msdp_sock_listen(struct pim_instance *pim) } } - frr_elevate_privs(&pimd_privs) { + frr_with_privs(&pimd_privs) { /* bind to well known TCP port */ rc = bind(sock, (struct sockaddr *)&sin, socklen); } diff --git a/pimd/pim_sock.c b/pimd/pim_sock.c index c4538a4ac5..82255cd3b0 100644 --- a/pimd/pim_sock.c +++ b/pimd/pim_sock.c @@ -46,7 +46,7 @@ int pim_socket_raw(int protocol) { int fd; - frr_elevate_privs(&pimd_privs) { + frr_with_privs(&pimd_privs) { fd = socket(AF_INET, SOCK_RAW, protocol); @@ -65,7 +65,7 @@ void pim_socket_ip_hdr(int fd) { const int on = 1; - frr_elevate_privs(&pimd_privs) { + frr_with_privs(&pimd_privs) { if (setsockopt(fd, IPPROTO_IP, IP_HDRINCL, &on, sizeof(on))) zlog_err("%s: Could not turn on IP_HDRINCL option: %s", @@ -83,7 +83,7 @@ int pim_socket_bind(int fd, struct interface *ifp) int ret = 0; #ifdef SO_BINDTODEVICE - frr_elevate_privs(&pimd_privs) { + frr_with_privs(&pimd_privs) { ret = setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, ifp->name, strlen(ifp->name)); diff --git a/ripd/ripd.c b/ripd/ripd.c index 561fbcb52d..ad373aebdf 100644 --- a/ripd/ripd.c +++ b/ripd/ripd.c @@ -1395,7 +1395,7 @@ int rip_create_socket(struct vrf *vrf) /* Make datagram socket. */ if (vrf->vrf_id != VRF_DEFAULT) vrf_dev = vrf->name; - frr_elevate_privs(&ripd_privs) { + frr_with_privs(&ripd_privs) { sock = vrf_socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP, vrf->vrf_id, vrf_dev); if (sock < 0) { @@ -1415,7 +1415,7 @@ int rip_create_socket(struct vrf *vrf) #endif setsockopt_so_recvbuf(sock, RIP_UDP_RCV_BUF); - frr_elevate_privs(&ripd_privs) { + frr_with_privs(&ripd_privs) { if ((ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr))) < 0) { zlog_err("%s: Can't bind socket %d to %s port %d: %s", diff --git a/ripngd/ripng_interface.c b/ripngd/ripng_interface.c index 49ed13a2c2..9ed9dc28fe 100644 --- a/ripngd/ripng_interface.c +++ b/ripngd/ripng_interface.c @@ -75,7 +75,7 @@ static int ripng_multicast_join(struct interface *ifp, int sock) * While this is bogus, privs are available and easy to use * for this call as a workaround. */ - frr_elevate_privs(&ripngd_privs) { + frr_with_privs(&ripngd_privs) { ret = setsockopt(sock, IPPROTO_IPV6, IPV6_JOIN_GROUP, (char *)&mreq, sizeof(mreq)); diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c index 3314892e74..49f7dda646 100644 --- a/ripngd/ripngd.c +++ b/ripngd/ripngd.c @@ -120,8 +120,7 @@ int ripng_make_socket(struct vrf *vrf) /* Make datagram socket. */ if (vrf->vrf_id != VRF_DEFAULT) vrf_dev = vrf->name; - frr_elevate_privs(&ripngd_privs) - { + frr_with_privs(&ripngd_privs) { sock = vrf_socket(AF_INET6, SOCK_DGRAM, IPPROTO_UDP, vrf->vrf_id, vrf_dev); if (sock < 0) { @@ -160,7 +159,7 @@ int ripng_make_socket(struct vrf *vrf) #endif /* SIN6_LEN */ ripaddr.sin6_port = htons(RIPNG_PORT_DEFAULT); - frr_elevate_privs(&ripngd_privs) { + frr_with_privs(&ripngd_privs) { ret = bind(sock, (struct sockaddr *)&ripaddr, sizeof(ripaddr)); if (ret < 0) { zlog_err("Can't bind ripng socket: %s.", diff --git a/tests/lib/test_privs.c b/tests/lib/test_privs.c index fc3d908661..de638bc67a 100644 --- a/tests/lib/test_privs.c +++ b/tests/lib/test_privs.c @@ -113,7 +113,7 @@ int main(int argc, char **argv) ((test_privs.current_state() == ZPRIVS_RAISED) ? "Raised" : "Lowered") printf("%s\n", PRIV_STATE()); - frr_elevate_privs(&test_privs) { + frr_with_privs(&test_privs) { printf("%s\n", PRIV_STATE()); } @@ -125,7 +125,7 @@ int main(int argc, char **argv) /* but these should continue to work... */ printf("%s\n", PRIV_STATE()); - frr_elevate_privs(&test_privs) { + frr_with_privs(&test_privs) { printf("%s\n", PRIV_STATE()); } diff --git a/tools/checkpatch.pl b/tools/checkpatch.pl index 1b48d10a09..c0624d933e 100755 --- a/tools/checkpatch.pl +++ b/tools/checkpatch.pl @@ -3351,7 +3351,7 @@ sub process { # if/while/etc brace do not go on next line, unless defining a do while loop, # or if that brace on the next line is for something else - if ($line =~ /(.*)\b((?:if|while|for|switch|(?:[a-z_]+|)for_each[a-z_]+)\s*\(|do\b|else\b)/ && $line !~ /^.\s*\#/) { + if ($line =~ /(.*)\b((?:if|while|for|switch|(?:[a-z_]+|)frr_(each|with)[a-z_]+)\s*\(|do\b|else\b)/ && $line !~ /^.\s*\#/) { my $pre_ctx = "$1$2"; my ($level, @ctx) = ctx_statement_level($linenr, $realcnt, 0); @@ -3397,7 +3397,7 @@ sub process { } # Check relative indent for conditionals and blocks. - if ($line =~ /\b(?:(?:if|while|for|(?:[a-z_]+|)for_each[a-z_]+)\s*\(|(?:do|else)\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) { + if ($line =~ /\b(?:(?:if|while|for|(?:[a-z_]+|)frr_(each|with)[a-z_]+)\s*\(|(?:do|else)\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) { ($stat, $cond, $line_nr_next, $remain_next, $off_next) = ctx_statement_block($linenr, $realcnt, 0) if (!defined $stat); @@ -5177,6 +5177,31 @@ sub process { } } + if (!defined $suppress_ifbraces{$linenr - 1} && + $line =~ /\b(frr_with_)/) { + my ($level, $endln, @chunks) = + ctx_statement_full($linenr, $realcnt, $-[0]); + + # Check the condition. + my ($cond, $block) = @{$chunks[0]}; + #print "CHECKING<$linenr> cond<$cond> block<$block>\n"; + if (defined $cond) { + substr($block, 0, length($cond), ''); + } + + if ($level == 0 && $block !~ /^\s*\{/) { + my $herectx = $here . "\n"; + my $cnt = statement_rawlines($block); + + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + } + + WARN("BRACES", + "braces {} are mandatory for frr_with_* blocks\n" . $herectx); + } + } + # check for single line unbalanced braces if ($sline =~ /^.\s*\}\s*else\s*$/ || $sline =~ /^.\s*else\s*\{\s*$/) { diff --git a/tools/coccinelle/frr_with_mutex.cocci b/tools/coccinelle/frr_with_mutex.cocci new file mode 100644 index 0000000000..ec8b73917c --- /dev/null +++ b/tools/coccinelle/frr_with_mutex.cocci @@ -0,0 +1,23 @@ +@@ +expression E; +iterator name frr_with_mutex; +@@ + +- pthread_mutex_lock(E); ++ frr_with_mutex(E) { +- { + ... +- } +- pthread_mutex_unlock(E); ++ } + + +@@ +expression E; +@@ + +- pthread_mutex_lock(E); ++ frr_with_mutex(E) { + ... +- pthread_mutex_unlock(E); ++ } diff --git a/tools/coccinelle/zprivs.cocci b/tools/coccinelle/zprivs.cocci index 76d13c3f0d..11628a7eae 100644 --- a/tools/coccinelle/zprivs.cocci +++ b/tools/coccinelle/zprivs.cocci @@ -2,12 +2,12 @@ identifier change; identifier end; expression E, f, g; -iterator name frr_elevate_privs; +iterator name frr_with_privs; @@ - if (E.change(ZPRIVS_RAISE)) - f; -+ frr_elevate_privs(&E) { ++ frr_with_privs(&E) { <+... - goto end; + break; @@ -20,7 +20,7 @@ iterator name frr_elevate_privs; @@ identifier change, errno, safe_strerror, exit; expression E, f1, f2, f3, ret, fn; -iterator name frr_elevate_privs; +iterator name frr_with_privs; @@ if (E.change(ZPRIVS_RAISE)) @@ -44,7 +44,7 @@ iterator name frr_elevate_privs; @@ identifier change; expression E, f1, f2, f3, ret; -iterator name frr_elevate_privs; +iterator name frr_with_privs; @@ if (E.change(ZPRIVS_RAISE)) @@ -64,12 +64,12 @@ iterator name frr_elevate_privs; @@ identifier change; expression E, f, g; -iterator name frr_elevate_privs; +iterator name frr_with_privs; @@ - if (E.change(ZPRIVS_RAISE)) - f; -+ frr_elevate_privs(&E) { ++ frr_with_privs(&E) { ... - if (E.change(ZPRIVS_LOWER)) - g; diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c index 951ad3f58f..b4049b55eb 100644 --- a/vrrpd/vrrp.c +++ b/vrrpd/vrrp.c @@ -1065,8 +1065,7 @@ static int vrrp_socket(struct vrrp_router *r) int ret; bool failed = false; - frr_elevate_privs(&vrrp_privs) - { + frr_with_privs(&vrrp_privs) { r->sock_rx = socket(r->family, SOCK_RAW, IPPROTO_VRRP); r->sock_tx = socket(r->family, SOCK_RAW, IPPROTO_VRRP); } @@ -1102,8 +1101,7 @@ static int vrrp_socket(struct vrrp_router *r) setsockopt_ipv4_multicast_loop(r->sock_tx, 0); /* Bind Rx socket to exact interface */ - frr_elevate_privs(&vrrp_privs) - { + frr_with_privs(&vrrp_privs) { ret = setsockopt(r->sock_rx, SOL_SOCKET, SO_BINDTODEVICE, r->vr->ifp->name, strlen(r->vr->ifp->name)); @@ -1213,8 +1211,7 @@ static int vrrp_socket(struct vrrp_router *r) setsockopt_ipv6_multicast_loop(r->sock_tx, 0); /* Bind Rx socket to exact interface */ - frr_elevate_privs(&vrrp_privs) - { + frr_with_privs(&vrrp_privs) { ret = setsockopt(r->sock_rx, SOL_SOCKET, SO_BINDTODEVICE, r->vr->ifp->name, strlen(r->vr->ifp->name)); diff --git a/vrrpd/vrrp_arp.c b/vrrpd/vrrp_arp.c index 78e153a082..750050e8c3 100644 --- a/vrrpd/vrrp_arp.c +++ b/vrrpd/vrrp_arp.c @@ -188,7 +188,7 @@ void vrrp_garp_init(void) /* Create the socket descriptor */ /* FIXME: why ETH_P_RARP? */ errno = 0; - frr_elevate_privs(&vrrp_privs) { + frr_with_privs(&vrrp_privs) { garp_fd = socket(PF_PACKET, SOCK_RAW | SOCK_CLOEXEC, htons(ETH_P_RARP)); } diff --git a/vrrpd/vrrp_ndisc.c b/vrrpd/vrrp_ndisc.c index 348958509a..dc546b09a2 100644 --- a/vrrpd/vrrp_ndisc.c +++ b/vrrpd/vrrp_ndisc.c @@ -214,8 +214,7 @@ int vrrp_ndisc_una_send_all(struct vrrp_router *r) void vrrp_ndisc_init(void) { - frr_elevate_privs(&vrrp_privs) - { + frr_with_privs(&vrrp_privs) { ndisc_fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_IPV6)); } diff --git a/zebra/connected.c b/zebra/connected.c index ffc991861c..6b92945c63 100644 --- a/zebra/connected.c +++ b/zebra/connected.c @@ -235,7 +235,7 @@ void connected_up(struct interface *ifp, struct connected *ifc) return; break; case AFI_IP6: -#ifndef LINUX +#ifndef GNU_LINUX /* XXX: It is already done by rib_bogus_ipv6 within rib_add */ if (IN6_IS_ADDR_UNSPECIFIED(&p.u.prefix6)) return; diff --git a/zebra/if_ioctl_solaris.c b/zebra/if_ioctl_solaris.c index 8b539a9049..2a2504ebf8 100644 --- a/zebra/if_ioctl_solaris.c +++ b/zebra/if_ioctl_solaris.c @@ -60,7 +60,7 @@ static int interface_list_ioctl(int af) size_t needed, lastneeded = 0; char *buf = NULL; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = socket(af, SOCK_DGRAM, 0); } @@ -72,7 +72,7 @@ static int interface_list_ioctl(int af) } calculate_lifc_len: - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { lifn.lifn_family = af; lifn.lifn_flags = LIFC_NOXMIT; /* we want NOXMIT interfaces too */ @@ -107,7 +107,7 @@ calculate_lifc_len: lifconf.lifc_len = needed; lifconf.lifc_buf = buf; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { ret = ioctl(sock, SIOCGLIFCONF, &lifconf); } diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index e157c2d70a..c71b95f753 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -385,7 +385,7 @@ static int get_iflink_speed(struct interface *interface) ifdata.ifr_data = (caddr_t)&ecmd; /* use ioctl to get IP address of an interface */ - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sd = vrf_socket(PF_INET, SOCK_DGRAM, IPPROTO_IP, interface->vrf_id, NULL); diff --git a/zebra/ioctl.c b/zebra/ioctl.c index 8202e076af..b461a08881 100644 --- a/zebra/ioctl.c +++ b/zebra/ioctl.c @@ -57,7 +57,7 @@ int if_ioctl(unsigned long request, caddr_t buffer) int ret; int err = 0; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = socket(AF_INET, SOCK_DGRAM, 0); if (sock < 0) { zlog_err("Cannot create UDP socket: %s", @@ -83,7 +83,7 @@ int vrf_if_ioctl(unsigned long request, caddr_t buffer, vrf_id_t vrf_id) int ret; int err = 0; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = vrf_socket(AF_INET, SOCK_DGRAM, 0, vrf_id, NULL); if (sock < 0) { zlog_err("Cannot create UDP socket: %s", @@ -110,7 +110,7 @@ static int if_ioctl_ipv6(unsigned long request, caddr_t buffer) int ret; int err = 0; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = socket(AF_INET6, SOCK_DGRAM, 0); if (sock < 0) { zlog_err("Cannot create IPv6 datagram socket: %s", diff --git a/zebra/ioctl_solaris.c b/zebra/ioctl_solaris.c index 1f96fa23ea..2c71d949f7 100644 --- a/zebra/ioctl_solaris.c +++ b/zebra/ioctl_solaris.c @@ -66,7 +66,7 @@ int if_ioctl(unsigned long request, caddr_t buffer) int ret; int err; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = socket(AF_INET, SOCK_DGRAM, 0); if (sock < 0) { @@ -96,7 +96,7 @@ int if_ioctl_ipv6(unsigned long request, caddr_t buffer) int ret; int err; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = socket(AF_INET6, SOCK_DGRAM, 0); if (sock < 0) { diff --git a/zebra/ipforward_proc.c b/zebra/ipforward_proc.c index 8f44c377b3..709d2176aa 100644 --- a/zebra/ipforward_proc.c +++ b/zebra/ipforward_proc.c @@ -76,7 +76,7 @@ int ipforward_on(void) { FILE *fp; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { fp = fopen(proc_ipv4_forwarding, "w"); @@ -97,7 +97,7 @@ int ipforward_off(void) { FILE *fp; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { fp = fopen(proc_ipv4_forwarding, "w"); @@ -143,7 +143,7 @@ int ipforward_ipv6_on(void) { FILE *fp; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { fp = fopen(proc_ipv6_forwarding, "w"); @@ -165,7 +165,7 @@ int ipforward_ipv6_off(void) { FILE *fp; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { fp = fopen(proc_ipv6_forwarding, "w"); diff --git a/zebra/ipforward_solaris.c b/zebra/ipforward_solaris.c index 1bb743059c..1a45328248 100644 --- a/zebra/ipforward_solaris.c +++ b/zebra/ipforward_solaris.c @@ -83,7 +83,7 @@ static int solaris_nd(const int cmd, const char *parameter, const int value) strioctl.ic_len = ND_BUFFER_SIZE; strioctl.ic_dp = nd_buf; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { if ((fd = open(device, O_RDWR)) < 0) { flog_err_sys(EC_LIB_SYSTEM_CALL, "failed to open device %s - %s", device, diff --git a/zebra/ipforward_sysctl.c b/zebra/ipforward_sysctl.c index cc9421c275..ac8f537075 100644 --- a/zebra/ipforward_sysctl.c +++ b/zebra/ipforward_sysctl.c @@ -56,7 +56,7 @@ int ipforward_on(void) int ipforwarding = 1; len = sizeof ipforwarding; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { if (sysctl(mib, MIB_SIZ, NULL, NULL, &ipforwarding, len) < 0) { flog_err_sys(EC_LIB_SYSTEM_CALL, "Can't set ipforwarding on"); @@ -72,7 +72,7 @@ int ipforward_off(void) int ipforwarding = 0; len = sizeof ipforwarding; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { if (sysctl(mib, MIB_SIZ, NULL, NULL, &ipforwarding, len) < 0) { flog_err_sys(EC_LIB_SYSTEM_CALL, "Can't set ipforwarding on"); @@ -97,7 +97,7 @@ int ipforward_ipv6(void) int ip6forwarding = 0; len = sizeof ip6forwarding; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { if (sysctl(mib_ipv6, MIB_SIZ, &ip6forwarding, &len, 0, 0) < 0) { flog_err_sys(EC_LIB_SYSTEM_CALL, "can't get ip6forwarding value"); @@ -113,7 +113,7 @@ int ipforward_ipv6_on(void) int ip6forwarding = 1; len = sizeof ip6forwarding; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { if (sysctl(mib_ipv6, MIB_SIZ, NULL, NULL, &ip6forwarding, len) < 0) { flog_err_sys(EC_LIB_SYSTEM_CALL, @@ -130,7 +130,7 @@ int ipforward_ipv6_off(void) int ip6forwarding = 0; len = sizeof ip6forwarding; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { if (sysctl(mib_ipv6, MIB_SIZ, NULL, NULL, &ip6forwarding, len) < 0) { flog_err_sys(EC_LIB_SYSTEM_CALL, diff --git a/zebra/irdp_main.c b/zebra/irdp_main.c index 38d241eaa5..0de618625d 100644 --- a/zebra/irdp_main.c +++ b/zebra/irdp_main.c @@ -82,7 +82,7 @@ int irdp_sock_init(void) int save_errno; int sock; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = socket(AF_INET, SOCK_RAW, IPPROTO_ICMP); save_errno = errno; diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c index 2c306434a3..f52b4746ae 100644 --- a/zebra/kernel_netlink.c +++ b/zebra/kernel_netlink.c @@ -183,7 +183,7 @@ static int netlink_recvbuf(struct nlsock *nl, uint32_t newsize) } /* Try force option (linux >= 2.6.14) and fall back to normal set */ - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { ret = setsockopt(nl->sock, SOL_SOCKET, SO_RCVBUFFORCE, &nl_rcvbufsize, sizeof(nl_rcvbufsize)); @@ -220,7 +220,7 @@ static int netlink_socket(struct nlsock *nl, unsigned long groups, int sock; int namelen; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = ns_socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE, ns_id); if (sock < 0) { zlog_err("Can't open %s socket: %s", nl->name, @@ -352,7 +352,7 @@ static void netlink_write_incoming(const char *buf, const unsigned int size, FILE *f; snprintf(fname, MAXPATHLEN, "%s/%s_%u", frr_vtydir, "netlink", counter); - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { f = fopen(fname, "w"); } if (f) { @@ -373,7 +373,7 @@ static long netlink_read_file(char *buf, const char *fname) FILE *f; long file_bytes = -1; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { f = fopen(fname, "r"); } if (f) { @@ -989,7 +989,7 @@ int netlink_talk_info(int (*filter)(struct nlmsghdr *, ns_id_t, int startup), n->nlmsg_flags); /* Send message to netlink interface. */ - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { status = sendmsg(nl->sock, &msg, 0); save_errno = errno; } @@ -1056,7 +1056,7 @@ int netlink_request(struct nlsock *nl, struct nlmsghdr *n) snl.nl_family = AF_NETLINK; /* Raise capabilities and send message, then lower capabilities. */ - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { ret = sendto(nl->sock, (void *)n, n->nlmsg_len, 0, (struct sockaddr *)&snl, sizeof snl); } diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index 156ce50725..60fbbcc059 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -1426,7 +1426,7 @@ static int kernel_read(struct thread *thread) /* Make routing socket. */ static void routing_socket(struct zebra_ns *zns) { - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { routing_sock = ns_socket(AF_ROUTE, SOCK_RAW, 0, zns->ns_id); dplane_routing_sock = diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 7e9a42a617..ea3b2b6ad8 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -314,7 +314,7 @@ enum zebra_dplane_result kernel_route_update(struct zebra_dplane_ctx *ctx) type = dplane_ctx_get_type(ctx); old_type = dplane_ctx_get_old_type(ctx); - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_DELETE) { if (!RSYSTEM_ROUTE(type)) diff --git a/zebra/rtadv.c b/zebra/rtadv.c index 5841c44b03..b084fb99ca 100644 --- a/zebra/rtadv.c +++ b/zebra/rtadv.c @@ -760,7 +760,7 @@ static int rtadv_make_socket(ns_id_t ns_id) int ret = 0; struct icmp6_filter filter; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { sock = ns_socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6, ns_id); diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index b6a8ee950c..fa6a2f62ec 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -2507,7 +2507,7 @@ static void zserv_write_incoming(struct stream *orig, uint16_t command) snprintf(fname, MAXPATHLEN, "%s/%u", frr_vtydir, command); - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { fd = open(fname, O_CREAT | O_WRONLY | O_EXCL, 0644); } stream_flush(copy, fd); diff --git a/zebra/zebra_fpm.c b/zebra/zebra_fpm.c index eaf43095bc..4144c0afe0 100644 --- a/zebra/zebra_fpm.c +++ b/zebra/zebra_fpm.c @@ -1927,6 +1927,9 @@ static inline void zfpm_init_message_format(const char *format) "FPM protobuf message format is not available"); return; } + flog_warn(EC_ZEBRA_PROTOBUF_NOT_AVAILABLE, + "FPM protobuf message format is deprecated and scheduled to be removed. " + "Please convert to using netlink format or contact dev@lists.frrouting.org with your use case."); zfpm_g->message_format = ZFPM_MSG_FORMAT_PROTOBUF; return; } diff --git a/zebra/zebra_mpls_openbsd.c b/zebra/zebra_mpls_openbsd.c index 9f3ea70c77..fcd476dc2c 100644 --- a/zebra/zebra_mpls_openbsd.c +++ b/zebra/zebra_mpls_openbsd.c @@ -119,7 +119,7 @@ static int kernel_send_rtmsg_v4(int action, mpls_label_t in_label, hdr.rtm_mpls = MPLS_OP_SWAP; } - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { ret = writev(kr_state.fd, iov, iovcnt); } @@ -226,7 +226,7 @@ static int kernel_send_rtmsg_v6(int action, mpls_label_t in_label, hdr.rtm_mpls = MPLS_OP_SWAP; } - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { ret = writev(kr_state.fd, iov, iovcnt); } diff --git a/zebra/zebra_netns_notify.c b/zebra/zebra_netns_notify.c index 476638591b..d42cf3d60a 100644 --- a/zebra/zebra_netns_notify.c +++ b/zebra/zebra_netns_notify.c @@ -77,7 +77,7 @@ static void zebra_ns_notify_create_context_from_entry_name(const char *name) if (netnspath == NULL) return; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { ns_id = zebra_ns_id_get(netnspath); } if (ns_id == NS_UNKNOWN) @@ -97,7 +97,7 @@ static void zebra_ns_notify_create_context_from_entry_name(const char *name) ns_map_nsid_with_external(ns_id, false); return; } - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { ret = vrf_netns_handler_create(NULL, vrf, netnspath, ns_id_external, ns_id); } @@ -202,14 +202,14 @@ static int zebra_ns_ready_read(struct thread *t) netnspath = zns_info->netnspath; if (--zns_info->retries == 0) stop_retry = 1; - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { err = ns_switch_to_netns(netnspath); } if (err < 0) return zebra_ns_continue_read(zns_info, stop_retry); /* go back to default ns */ - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { err = ns_switchback_to_initial(); } if (err < 0) diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 895a545bfe..ee2956d3ea 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -138,6 +138,7 @@ static int nexthop_active(afi_t afi, struct route_entry *re, struct nexthop *newhop; struct interface *ifp; rib_dest_t *dest; + struct zebra_vrf *zvrf; if ((nexthop->type == NEXTHOP_TYPE_IPV4) || nexthop->type == NEXTHOP_TYPE_IPV6) @@ -212,7 +213,9 @@ static int nexthop_active(afi_t afi, struct route_entry *re, } /* Lookup table. */ table = zebra_vrf_table(afi, SAFI_UNICAST, nexthop->vrf_id); - if (!table) { + /* get zvrf */ + zvrf = zebra_vrf_lookup_by_id(nexthop->vrf_id); + if (!table || !zvrf) { if (IS_ZEBRA_DEBUG_RIB_DETAILED) zlog_debug("\t%s: Table not found", __PRETTY_FUNCTION__); @@ -242,7 +245,7 @@ static int nexthop_active(afi_t afi, struct route_entry *re, /* However, do not resolve over default route unless explicitly * allowed. */ if (is_default_prefix(&rn->p) - && !rnh_resolve_via_default(p.family)) { + && !rnh_resolve_via_default(zvrf, p.family)) { if (IS_ZEBRA_DEBUG_RIB_DETAILED) zlog_debug( "\t:%s: Resolved against default route", diff --git a/zebra/zebra_ns.c b/zebra/zebra_ns.c index 94918365a3..37f53bf911 100644 --- a/zebra/zebra_ns.c +++ b/zebra/zebra_ns.c @@ -180,7 +180,7 @@ int zebra_ns_init(const char *optional_default_name) dzns = zebra_ns_alloc(); - frr_elevate_privs(&zserv_privs) { + frr_with_privs(&zserv_privs) { ns_id = zebra_ns_id_get_default(); } ns_id_external = ns_map_nsid_with_external(ns_id, true); diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 335cc8294c..a7058e7928 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -37,6 +37,7 @@ #include "vrf.h" #include "workqueue.h" #include "nexthop_group_private.h" +#include "frr_pthread.h" #include "zebra/zebra_router.h" #include "zebra/connected.h" @@ -3204,12 +3205,10 @@ static int rib_process_dplane_results(struct thread *thread) TAILQ_INIT(&ctxlist); /* Take lock controlling queue of results */ - pthread_mutex_lock(&dplane_mutex); - { + frr_with_mutex(&dplane_mutex) { /* Dequeue list of context structs */ dplane_ctx_list_append(&ctxlist, &rib_dplane_q); } - pthread_mutex_unlock(&dplane_mutex); /* Dequeue context block */ ctx = dplane_ctx_dequeue(&ctxlist); @@ -3300,12 +3299,10 @@ static int rib_process_dplane_results(struct thread *thread) static int rib_dplane_results(struct dplane_ctx_q *ctxlist) { /* Take lock controlling queue of results */ - pthread_mutex_lock(&dplane_mutex); - { + frr_with_mutex(&dplane_mutex) { /* Enqueue context blocks */ dplane_ctx_list_append(&rib_dplane_q, ctxlist); } - pthread_mutex_unlock(&dplane_mutex); /* Ensure event is signalled to zebra main pthread */ thread_add_event(zrouter.master, rib_process_dplane_results, NULL, 0, diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index da2fe4a30c..666ebb70e8 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -62,9 +62,6 @@ static int send_client(struct rnh *rnh, struct zserv *client, rnh_type_t type, static void print_rnh(struct route_node *rn, struct vty *vty); static int zebra_client_cleanup_rnh(struct zserv *client); -int zebra_rnh_ip_default_route = 0; -int zebra_rnh_ipv6_default_route = 0; - void zebra_rnh_init(void) { hook_register(zserv_client_close, zebra_client_cleanup_rnh); @@ -656,7 +653,7 @@ zebra_rnh_resolve_nexthop_entry(struct zebra_vrf *zvrf, afi_t afi, * match route to be exact if so specified */ if (is_default_prefix(&rn->p) - && !rnh_resolve_via_default(rn->p.family)) { + && !rnh_resolve_via_default(zvrf, rn->p.family)) { if (IS_ZEBRA_DEBUG_NHT_DETAILED) zlog_debug( "\tNot allowed to resolve through default prefix"); @@ -1213,3 +1210,12 @@ static int zebra_client_cleanup_rnh(struct zserv *client) return 0; } + +int rnh_resolve_via_default(struct zebra_vrf *zvrf, int family) +{ + if (((family == AF_INET) && zvrf->zebra_rnh_ip_default_route) + || ((family == AF_INET6) && zvrf->zebra_rnh_ipv6_default_route)) + return 1; + else + return 0; +} diff --git a/zebra/zebra_rnh.h b/zebra/zebra_rnh.h index c7d2c0d298..6e2dab8d9f 100644 --- a/zebra/zebra_rnh.h +++ b/zebra/zebra_rnh.h @@ -29,20 +29,8 @@ extern "C" { #endif -extern int zebra_rnh_ip_default_route; -extern int zebra_rnh_ipv6_default_route; - extern void zebra_rnh_init(void); -static inline int rnh_resolve_via_default(int family) -{ - if (((family == AF_INET) && zebra_rnh_ip_default_route) - || ((family == AF_INET6) && zebra_rnh_ipv6_default_route)) - return 1; - else - return 0; -} - static inline const char *rnh_type2str(rnh_type_t type) { switch (type) { @@ -72,6 +60,8 @@ extern void zebra_print_rnh_table(vrf_id_t vrfid, afi_t afi, struct vty *vty, rnh_type_t type, struct prefix *p); extern char *rnh_str(struct rnh *rnh, char *buf, int size); +extern int rnh_resolve_via_default(struct zebra_vrf *zvrf, int family); + #ifdef __cplusplus } #endif diff --git a/zebra/zebra_vrf.c b/zebra/zebra_vrf.c index fcc94a7be9..72d0b6866d 100644 --- a/zebra/zebra_vrf.c +++ b/zebra/zebra_vrf.c @@ -488,6 +488,11 @@ static int vrf_config_write(struct vty *vty) if (zvrf_id(zvrf) == VRF_DEFAULT) { if (zvrf->l3vni) vty_out(vty, "vni %u\n", zvrf->l3vni); + if (zvrf->zebra_rnh_ip_default_route) + vty_out(vty, "ip nht resolve-via-default\n"); + + if (zvrf->zebra_rnh_ipv6_default_route) + vty_out(vty, "ipv6 nht resolve-via-default\n"); } else { vty_frame(vty, "vrf %s\n", zvrf_name(zvrf)); if (zvrf->l3vni) @@ -497,8 +502,14 @@ static int vrf_config_write(struct vty *vty) ? " prefix-routes-only" : ""); zebra_ns_config_write(vty, (struct ns *)vrf->ns_ctxt); + if (zvrf->zebra_rnh_ip_default_route) + vty_out(vty, " ip nht resolve-via-default\n"); + + if (zvrf->zebra_rnh_ipv6_default_route) + vty_out(vty, " ipv6 nht resolve-via-default\n"); } + zebra_routemap_config_write_protocol(vty, zvrf); if (zvrf_id(zvrf) != VRF_DEFAULT) diff --git a/zebra/zebra_vrf.h b/zebra/zebra_vrf.h index f92e1a010b..6c80f9bcb4 100644 --- a/zebra/zebra_vrf.h +++ b/zebra/zebra_vrf.h @@ -174,6 +174,9 @@ struct zebra_vrf { #if defined(HAVE_RTADV) struct rtadv rtadv; #endif /* HAVE_RTADV */ + + int zebra_rnh_ip_default_route; + int zebra_rnh_ipv6_default_route; }; #define PROTO_RM_NAME(zvrf, afi, rtype) zvrf->proto_rm[afi][rtype].name #define NHT_RM_NAME(zvrf, afi, rtype) zvrf->nht_rm[afi][rtype].name diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index e6487b8334..38de01e228 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -1090,10 +1090,10 @@ DEFUN (ip_nht_default_route, if (!zvrf) return CMD_WARNING; - if (zebra_rnh_ip_default_route) + if (zvrf->zebra_rnh_ip_default_route) return CMD_SUCCESS; - zebra_rnh_ip_default_route = 1; + zvrf->zebra_rnh_ip_default_route = 1; zebra_evaluate_rnh(zvrf, AFI_IP, 1, RNH_NEXTHOP_TYPE, NULL); return CMD_SUCCESS; @@ -1112,10 +1112,10 @@ DEFUN (no_ip_nht_default_route, if (!zvrf) return CMD_WARNING; - if (!zebra_rnh_ip_default_route) + if (!zvrf->zebra_rnh_ip_default_route) return CMD_SUCCESS; - zebra_rnh_ip_default_route = 0; + zvrf->zebra_rnh_ip_default_route = 0; zebra_evaluate_rnh(zvrf, AFI_IP, 1, RNH_NEXTHOP_TYPE, NULL); return CMD_SUCCESS; } @@ -1132,10 +1132,10 @@ DEFUN (ipv6_nht_default_route, if (!zvrf) return CMD_WARNING; - if (zebra_rnh_ipv6_default_route) + if (zvrf->zebra_rnh_ipv6_default_route) return CMD_SUCCESS; - zebra_rnh_ipv6_default_route = 1; + zvrf->zebra_rnh_ipv6_default_route = 1; zebra_evaluate_rnh(zvrf, AFI_IP6, 1, RNH_NEXTHOP_TYPE, NULL); return CMD_SUCCESS; } @@ -1154,10 +1154,10 @@ DEFUN (no_ipv6_nht_default_route, if (!zvrf) return CMD_WARNING; - if (!zebra_rnh_ipv6_default_route) + if (!zvrf->zebra_rnh_ipv6_default_route) return CMD_SUCCESS; - zebra_rnh_ipv6_default_route = 0; + zvrf->zebra_rnh_ipv6_default_route = 0; zebra_evaluate_rnh(zvrf, AFI_IP6, 1, RNH_NEXTHOP_TYPE, NULL); return CMD_SUCCESS; } @@ -2629,12 +2629,6 @@ static int config_write_protocol(struct vty *vty) if (allow_delete) vty_out(vty, "allow-external-route-update\n"); - if (zebra_rnh_ip_default_route) - vty_out(vty, "ip nht resolve-via-default\n"); - - if (zebra_rnh_ipv6_default_route) - vty_out(vty, "ipv6 nht resolve-via-default\n"); - if (zrouter.ribq->spec.hold != ZEBRA_RIB_PROCESS_HOLD_TIME) vty_out(vty, "zebra work-queue %u\n", zrouter.ribq->spec.hold); diff --git a/zebra/zserv.c b/zebra/zserv.c index 70b4594813..c008441d6a 100644 --- a/zebra/zserv.c +++ b/zebra/zserv.c @@ -231,13 +231,11 @@ static int zserv_write(struct thread *thread) cache = stream_fifo_new(); - pthread_mutex_lock(&client->obuf_mtx); - { + frr_with_mutex(&client->obuf_mtx) { while (stream_fifo_head(client->obuf_fifo)) stream_fifo_push(cache, stream_fifo_pop(client->obuf_fifo)); } - pthread_mutex_unlock(&client->obuf_mtx); if (cache->tail) { msg = cache->tail; @@ -427,13 +425,11 @@ static int zserv_read(struct thread *thread) memory_order_relaxed); /* publish read packets on client's input queue */ - pthread_mutex_lock(&client->ibuf_mtx); - { + frr_with_mutex(&client->ibuf_mtx) { while (cache->head) stream_fifo_push(client->ibuf_fifo, stream_fifo_pop(cache)); } - pthread_mutex_unlock(&client->ibuf_mtx); /* Schedule job to process those packets */ zserv_event(client, ZSERV_PROCESS_MESSAGES); @@ -499,8 +495,7 @@ static int zserv_process_messages(struct thread *thread) uint32_t p2p = zrouter.packets_to_process; bool need_resched = false; - pthread_mutex_lock(&client->ibuf_mtx); - { + frr_with_mutex(&client->ibuf_mtx) { uint32_t i; for (i = 0; i < p2p && stream_fifo_head(client->ibuf_fifo); ++i) { @@ -516,7 +511,6 @@ static int zserv_process_messages(struct thread *thread) if (stream_fifo_head(client->ibuf_fifo)) need_resched = true; } - pthread_mutex_unlock(&client->ibuf_mtx); while (stream_fifo_head(cache)) { msg = stream_fifo_pop(cache); @@ -535,11 +529,9 @@ static int zserv_process_messages(struct thread *thread) int zserv_send_message(struct zserv *client, struct stream *msg) { - pthread_mutex_lock(&client->obuf_mtx); - { + frr_with_mutex(&client->obuf_mtx) { stream_fifo_push(client->obuf_fifo, msg); } - pthread_mutex_unlock(&client->obuf_mtx); zserv_client_event(client, ZSERV_CLIENT_WRITE); @@ -790,7 +782,7 @@ void zserv_start(char *path) setsockopt_so_recvbuf(zsock, 1048576); setsockopt_so_sendbuf(zsock, 1048576); - frr_elevate_privs((sa.ss_family != AF_UNIX) ? &zserv_privs : NULL) { + frr_with_privs((sa.ss_family != AF_UNIX) ? &zserv_privs : NULL) { ret = bind(zsock, (struct sockaddr *)&sa, sa_len); } if (ret < 0) { |