From 61fa0b976aa323b6158ab45961000e4a45cffc42 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 24 May 2019 08:22:07 -0400 Subject: [PATCH 1/7] lib: 'show thread cpu' help strings referenced a weird option The 'show thread cpu' command referenced a 'b' option. Which is not parsed at all in the parse_filter function. As such I do not know what this was referencing as that it has been removed. Update the help strings to reflect this reality. Signed-off-by: Donald Sharp --- lib/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/thread.c b/lib/thread.c index 7a9a0ab608..04abb7e5f4 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -281,7 +281,7 @@ DEFUN (show_thread_cpu, SHOW_STR "Thread information\n" "Thread CPU usage\n" - "Display filter (rwtexb)\n") + "Display filter (rwtex)\n") { uint8_t filter = (uint8_t)-1U; int idx = 0; From 6c19478a3b62c243dc44a6d175db761486ab2921 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 24 May 2019 07:53:32 -0400 Subject: [PATCH 2/7] lib: Display to end user the MAX_FDS allowed Upon startup FRR reads in the MAX_FDS variable from it's control files via the getrlimit call. We then setup code to limit the poll data structure size to that value. The OS also limits our FD's to that value because that is what is set. Provide a methodology that a interested end user can figure this data out. Signed-off-by: Donald Sharp --- lib/thread.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/thread.c b/lib/thread.c index 04abb7e5f4..d3fb2cdf36 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -312,7 +312,8 @@ static void show_thread_poll_helper(struct vty *vty, struct thread_master *m) vty_out(vty, "\nShowing poll FD's for %s\n", name); vty_out(vty, "----------------------%s\n", underline); - vty_out(vty, "Count: %u\n", (uint32_t)m->handler.pfdcount); + vty_out(vty, "Count: %u/%d\n", (uint32_t)m->handler.pfdcount, + m->fd_limit); for (i = 0; i < m->handler.pfdcount; i++) vty_out(vty, "\t%6d fd:%6d events:%2d revents:%2d\n", i, m->handler.pfds[i].fd, From 610b53283ba6cb7994b84a4962cdec4d54f27216 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 24 May 2019 08:04:33 -0400 Subject: [PATCH 3/7] doc, tools: Update to code to show example MAX_FDS Place in the code the ability for end operators to know how to modify MAX_FDS so that they can run large scale operations. Signed-off-by: Donald Sharp --- doc/user/setup.rst | 35 ++++++++++++++++++++--------------- tools/etc/frr/daemons | 8 ++++++++ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/doc/user/setup.rst b/doc/user/setup.rst index ffefe37905..2c9ca41762 100644 --- a/doc/user/setup.rst +++ b/doc/user/setup.rst @@ -6,8 +6,8 @@ Basic Setup After installing FRR, some basic configuration must be completed before it is ready to use. -Daemons File ------------- +Daemons Configuration File +-------------------------- After a fresh install, starting FRR will do nothing. This is because daemons must be explicitly enabled by editing a file in your configuration directory. This file is usually located at :file:`/etc/frr/daemons` and determines which @@ -34,19 +34,6 @@ systemd. The file initially looks like this: bfdd=no fabricd=no -To enable a particular daemon, simply change the corresponding 'no' to 'yes'. -Subsequent service restarts should start the daemon. - -Daemons Configuration File --------------------------- -There is another file that controls the default options passed to daemons when -starting FRR as a service. This file is located in your configuration -directory, usually at :file:`/etc/frr/daemons`. - -This file has several parts. Here is an example: - -:: - # # If this option is set the /etc/init.d/frr script automatically loads # the config via "vtysh -b" when the servers are started. @@ -71,6 +58,7 @@ This file has several parts. Here is an example: bfdd_options=" --daemon -A 127.0.0.1" fabricd_options=" --daemon -A 127.0.0.1" + #MAX_FDS=1024 # The list of daemons to watch is automatically generated by the init script. #watchfrr_options="" @@ -83,6 +71,13 @@ This file has several parts. Here is an example: Breaking this file down: +:: + + bgpd=yes + +To enable a particular daemon, simply change the corresponding 'no' to 'yes'. +Subsequent service restarts should start the daemon. + :: vtysh_enable=yes @@ -91,6 +86,16 @@ As the comment says, this causes :ref:`VTYSH ` to apply configuration when starting the daemons. This is useful for a variety of reasons touched on in the VTYSH documentation and should generally be enabled. +:: + + MAX_FDS=1024 + +This allows the operator to control the number of open file descriptors +each daemon is allowed to start with. The current assumed value on +most operating systems is 1024. If the operator plans to run bgp with +several thousands of peers than this is where we would modify FRR to +allow this to happen. + :: zebra_options=" -s 90000000 --daemon -A 127.0.0.1" diff --git a/tools/etc/frr/daemons b/tools/etc/frr/daemons index b920621d70..79c52d30d1 100644 --- a/tools/etc/frr/daemons +++ b/tools/etc/frr/daemons @@ -56,6 +56,14 @@ bfdd_options=" -A 127.0.0.1" fabricd_options="-A 127.0.0.1" vrrpd_options=" -A 127.0.0.1" +# +# This is the maximum number of FD's that will be available. +# Upon startup this is read by the control files and ulimit +# is called. Uncomment and use a reasonable value for your +# setup if you are expecting a large number of peers in +# say BGP. +#MAX_FDS=1024 + # The list of daemons to watch is automatically generated by the init script. #watchfrr_options="" From 8b1148bf59f68350edc3ae84b053a715b03c629a Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 24 May 2019 08:15:40 -0400 Subject: [PATCH 4/7] doc: Add 'show thread cpu' and 'show thread poll' documentation Add some very basic 'show thread cpu' and 'show thread poll' documentation to our system. Signed-off-by: Donald Sharp --- doc/user/basic.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/doc/user/basic.rst b/doc/user/basic.rst index f55b1b9d73..3df60169f7 100644 --- a/doc/user/basic.rst +++ b/doc/user/basic.rst @@ -414,6 +414,22 @@ Terminal Mode Commands (view) show [ip] bgp l2vpn evpn all overlay ... +.. _common-show-commands: + +.. index:: show thread cpu +.. clicmd:: show thread cpu [r|w|t|e|x] + + This command displays system run statistics for all the different event + types. If no options is specified all different run types are displayed + together. Additionally you can ask to look at (r)ead, (w)rite, (t)imer, + (e)vent and e(x)ecute thread event types. + +.. index:: show thread poll +.. clicmd:: show thread poll + + This command displays FRR's poll data. It allows a glimpse into how + we are setting each individual fd for the poll command at that point + in time. .. _common-invocation-options: From cac9e917ebcf8551f2fe05295ddeeae69a352f5f Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 24 May 2019 08:27:19 -0400 Subject: [PATCH 5/7] bgpd: Display FD used for peer When issuing a `show bgp neighbor...` command display to the end user the FD used for communication. Signed-off-by: Donald Sharp --- bgpd/bgp_vty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 2a08619ec3..ae51f1d780 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -10909,11 +10909,11 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, if (p->password) vty_out(vty, "Peer Authentication Enabled\n"); - vty_out(vty, "Read thread: %s Write thread: %s\n", + vty_out(vty, "Read thread: %s Write thread: %s FD used: %d\n", p->t_read ? "on" : "off", CHECK_FLAG(p->thread_flags, PEER_THREAD_WRITES_ON) ? "on" - : "off"); + : "off", p->fd); } if (p->notify.code == BGP_NOTIFY_OPEN_ERR From 51da3896265c601f79f765122e73304cf0943f70 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 24 May 2019 08:30:53 -0400 Subject: [PATCH 6/7] bgpd, lib: Remove SO_MARK The SO_MARK socket option was being used pre vrf to allow for the separation of the front panel -vs- the management port. This was facilitated by a ip rule. Since this is undocumented anywhere in our system( other than old commits see ed40466af80c9d0b88436c637a1d54b28a669b1c ). We should remove this because this will cause interference with people using rules and are not aware of this offshoot of functionality. Signed-off-by: Donald Sharp --- bgpd/bgp_errors.c | 6 ------ bgpd/bgp_errors.h | 1 - bgpd/bgp_network.c | 6 ------ lib/sockunion.c | 15 --------------- lib/sockunion.h | 1 - 5 files changed, 29 deletions(-) diff --git a/bgpd/bgp_errors.c b/bgpd/bgp_errors.c index 753ee6baf1..6e181697d6 100644 --- a/bgpd/bgp_errors.c +++ b/bgpd/bgp_errors.c @@ -121,12 +121,6 @@ static struct log_ref ferr_bgp_warn[] = { .description = "BGP attempted to setup TCP MD5 configuration on the socket as per configuration but was unable to", .suggestion = "Please collect log files and open Issue", }, - { - .code = EC_BGP_NO_SOCKOPT_MARK, - .title = "Unable to set socket MARK option", - .description = "BGP attempted to set the SO_MARK option for a socket and was unable to do so", - .suggestion = "Please collect log files and open Issue", - }, { .code = EC_BGP_EVPN_PMSI_PRESENT, .title = "BGP Received a EVPN NLRI with PMSI included", diff --git a/bgpd/bgp_errors.h b/bgpd/bgp_errors.h index 13bd318e27..39d043ff13 100644 --- a/bgpd/bgp_errors.h +++ b/bgpd/bgp_errors.h @@ -88,7 +88,6 @@ enum bgp_log_refs { EC_BGP_UPDATE_PACKET_LONG, EC_BGP_UNRECOGNIZED_CAPABILITY, EC_BGP_NO_TCP_MD5, - EC_BGP_NO_SOCKOPT_MARK, EC_BGP_EVPN_PMSI_PRESENT, EC_BGP_EVPN_VPN_VNI, EC_BGP_EVPN_ESI, diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 6a5c2c4b38..8e18ed7529 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -588,8 +588,6 @@ static int bgp_update_source(struct peer *peer) return ret; } -#define DATAPLANE_MARK 254 /* main table ID */ - /* BGP try to connect to the peer. */ int bgp_connect(struct peer *peer) { @@ -619,10 +617,6 @@ int bgp_connect(struct peer *peer) sockopt_reuseaddr(peer->fd); sockopt_reuseport(peer->fd); - if (sockopt_mark_default(peer->fd, DATAPLANE_MARK, &bgpd_privs) < 0) - flog_warn(EC_BGP_NO_SOCKOPT_MARK, - "Unable to set mark on FD for peer %s, err=%s", - peer->host, safe_strerror(errno)); #ifdef IPTOS_PREC_INTERNETCONTROL frr_elevate_privs(&bgpd_privs) { diff --git a/lib/sockunion.c b/lib/sockunion.c index 5afd10eb48..8fa9a3fad9 100644 --- a/lib/sockunion.c +++ b/lib/sockunion.c @@ -366,21 +366,6 @@ int sockopt_cork(int sock, int onoff) return 0; } -int sockopt_mark_default(int sock, int mark, struct zebra_privs_t *cap) -{ -#ifdef SO_MARK - int ret; - - frr_elevate_privs(cap) { - ret = setsockopt(sock, SOL_SOCKET, SO_MARK, &mark, - sizeof(mark)); - } - return ret; -#else - return 0; -#endif -} - int sockopt_minttl(int family, int sock, int minttl) { #ifdef IP_MINTTL diff --git a/lib/sockunion.h b/lib/sockunion.h index b996735550..7091c1b5e7 100644 --- a/lib/sockunion.h +++ b/lib/sockunion.h @@ -93,7 +93,6 @@ extern int sockunion_bind(int sock, union sockunion *, unsigned short, extern int sockopt_ttl(int family, int sock, int ttl); extern int sockopt_minttl(int family, int sock, int minttl); extern int sockopt_cork(int sock, int onoff); -extern int sockopt_mark_default(int sock, int mark, struct zebra_privs_t *); extern int sockunion_socket(const union sockunion *su); extern const char *inet_sutop(const union sockunion *su, char *str); extern enum connect_result sockunion_connect(int fd, const union sockunion *su, From 4b031e20c5fa08379e3e397d80b05564a4ad3774 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 25 May 2019 11:37:06 +0200 Subject: [PATCH 7/7] doc: fix space character nit Signed-off-by: David Lamparter --- doc/user/setup.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/setup.rst b/doc/user/setup.rst index 2c9ca41762..2cdd3a7c7f 100644 --- a/doc/user/setup.rst +++ b/doc/user/setup.rst @@ -73,7 +73,7 @@ Breaking this file down: :: - bgpd=yes + bgpd=yes To enable a particular daemon, simply change the corresponding 'no' to 'yes'. Subsequent service restarts should start the daemon.