Re: Refactor bgpd control code
Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.11.03 09:09:47 +0100:
> On Wed, Oct 21, 2020 at 07:16:07PM +0200, Claudio Jeker wrote:
> > This refactors the control code a bit and removes the common var from the
> > session.h header. The session engine no longer walks the control
> > connection list. Additionally cleanup the control.c code around
> > control_dispatch_msg(). E.g. don't do double lookups of control sessions
> > by fd to close them.
> >
> > OK?
ok
>
> Ping
>
> --
> :wq Claudio
>
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 control.c
> --- control.c 10 May 2020 13:38:46 - 1.100
> +++ control.c 21 Oct 2020 16:57:53 -
> @@ -29,11 +29,13 @@
> #include "session.h"
> #include "log.h"
>
> +TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns =
> TAILQ_HEAD_INITIALIZER(ctl_conns);
> +
> #define CONTROL_BACKLOG 5
>
> struct ctl_conn *control_connbyfd(int);
> struct ctl_conn *control_connbypid(pid_t);
> -int control_close(int);
> +int control_close(struct ctl_conn *);
> void control_result(struct ctl_conn *, u_int);
> ssize_t imsg_read_nofd(struct imsgbuf *);
>
> @@ -136,6 +138,22 @@ control_shutdown(int fd)
> close(fd);
> }
>
> +size_t
> +control_fill_pfds(struct pollfd *pfd, size_t size)
> +{
> + struct ctl_conn *ctl_conn;
> + size_t i = 0;
> +
> + TAILQ_FOREACH(ctl_conn, _conns, entry) {
> + pfd[i].fd = ctl_conn->ibuf.fd;
> + pfd[i].events = POLLIN;
> + if (ctl_conn->ibuf.w.queued > 0)
> + pfd[i].events |= POLLOUT;
> + i++;
> + }
> + return i;
> +}
> +
> unsigned int
> control_accept(int listenfd, int restricted)
> {
> @@ -198,15 +216,8 @@ control_connbypid(pid_t pid)
> }
>
> int
> -control_close(int fd)
> +control_close(struct ctl_conn *c)
> {
> - struct ctl_conn *c;
> -
> - if ((c = control_connbyfd(fd)) == NULL) {
> - log_warn("control_close: fd %d: not found", fd);
> - return (0);
> - }
> -
> if (c->terminate && c->ibuf.pid)
> imsg_ctl_rde(IMSG_CTL_TERMINATE, c->ibuf.pid, NULL, 0);
>
> @@ -220,8 +231,7 @@ control_close(int fd)
> }
>
> int
> -control_dispatch_msg(struct pollfd *pfd, u_int *ctl_cnt,
> -struct peer_head *peers)
> +control_dispatch_msg(struct pollfd *pfd, struct peer_head *peers)
> {
> struct imsg imsg;
> struct ctl_conn *c;
> @@ -237,10 +247,8 @@ control_dispatch_msg(struct pollfd *pfd,
> }
>
> if (pfd->revents & POLLOUT) {
> - if (msgbuf_write(>ibuf.w) <= 0 && errno != EAGAIN) {
> - *ctl_cnt -= control_close(pfd->fd);
> - return (1);
> - }
> + if (msgbuf_write(>ibuf.w) <= 0 && errno != EAGAIN)
> + return control_close(c);
> if (c->throttled && c->ibuf.w.queued < CTL_MSG_LOW_MARK) {
> if (imsg_ctl_rde(IMSG_XON, c->ibuf.pid, NULL, 0) != -1)
> c->throttled = 0;
> @@ -251,16 +259,12 @@ control_dispatch_msg(struct pollfd *pfd,
> return (0);
>
> if (((n = imsg_read_nofd(>ibuf)) == -1 && errno != EAGAIN) ||
> - n == 0) {
> - *ctl_cnt -= control_close(pfd->fd);
> - return (1);
> - }
> + n == 0)
> + return control_close(c);
>
> for (;;) {
> - if ((n = imsg_get(>ibuf, )) == -1) {
> - *ctl_cnt -= control_close(pfd->fd);
> - return (1);
> - }
> + if ((n = imsg_get(>ibuf, )) == -1)
> + return control_close(c);
>
> if (n == 0)
> break;
> Index: session.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.402
> diff -u -p -r1.402 session.c
> --- session.c 27 Jun 2020 07:24:42 - 1.402
> +++ session.c 21 Oct 2020 16:49:10 -
> @@ -196,7 +196,6 @@ session_main(int debug, int verbose)
> struct peer *p, **peer_l = NULL, *next;
> struct mrt *m, *xm, **mrt_l = NULL;
> struct pollfd *pfd = NULL;
> - struct ctl_conn *ctl_conn;
> struct listen_addr *la;
> void*newp;
> time_t now;
> @@ -237,7 +236,6 @@ session_main(int debug, int verbose)
> fatal(NULL);
> imsg_init(ibuf_main, 3);
>
> - TAILQ_INIT(_conns);
> LIST_INIT();
> listener_cnt = 0;
> peer_cnt = 0;
> @@ -438,13 +436,10 @@ session_main(int debug, int verbose)
>
> idx_mrts = i;
>
> - TAILQ_FOREACH(ctl_conn, _conns, entry) {
> -
Re: Refactor bgpd control code
On Wed, Oct 21, 2020 at 07:16:07PM +0200, Claudio Jeker wrote:
> This refactors the control code a bit and removes the common var from the
> session.h header. The session engine no longer walks the control
> connection list. Additionally cleanup the control.c code around
> control_dispatch_msg(). E.g. don't do double lookups of control sessions
> by fd to close them.
>
> OK?
Ping
--
:wq Claudio
Index: control.c
===
RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
retrieving revision 1.100
diff -u -p -r1.100 control.c
--- control.c 10 May 2020 13:38:46 - 1.100
+++ control.c 21 Oct 2020 16:57:53 -
@@ -29,11 +29,13 @@
#include "session.h"
#include "log.h"
+TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns = TAILQ_HEAD_INITIALIZER(ctl_conns);
+
#defineCONTROL_BACKLOG 5
struct ctl_conn*control_connbyfd(int);
struct ctl_conn*control_connbypid(pid_t);
-int control_close(int);
+int control_close(struct ctl_conn *);
voidcontrol_result(struct ctl_conn *, u_int);
ssize_t imsg_read_nofd(struct imsgbuf *);
@@ -136,6 +138,22 @@ control_shutdown(int fd)
close(fd);
}
+size_t
+control_fill_pfds(struct pollfd *pfd, size_t size)
+{
+ struct ctl_conn *ctl_conn;
+ size_t i = 0;
+
+ TAILQ_FOREACH(ctl_conn, _conns, entry) {
+ pfd[i].fd = ctl_conn->ibuf.fd;
+ pfd[i].events = POLLIN;
+ if (ctl_conn->ibuf.w.queued > 0)
+ pfd[i].events |= POLLOUT;
+ i++;
+ }
+ return i;
+}
+
unsigned int
control_accept(int listenfd, int restricted)
{
@@ -198,15 +216,8 @@ control_connbypid(pid_t pid)
}
int
-control_close(int fd)
+control_close(struct ctl_conn *c)
{
- struct ctl_conn *c;
-
- if ((c = control_connbyfd(fd)) == NULL) {
- log_warn("control_close: fd %d: not found", fd);
- return (0);
- }
-
if (c->terminate && c->ibuf.pid)
imsg_ctl_rde(IMSG_CTL_TERMINATE, c->ibuf.pid, NULL, 0);
@@ -220,8 +231,7 @@ control_close(int fd)
}
int
-control_dispatch_msg(struct pollfd *pfd, u_int *ctl_cnt,
-struct peer_head *peers)
+control_dispatch_msg(struct pollfd *pfd, struct peer_head *peers)
{
struct imsg imsg;
struct ctl_conn *c;
@@ -237,10 +247,8 @@ control_dispatch_msg(struct pollfd *pfd,
}
if (pfd->revents & POLLOUT) {
- if (msgbuf_write(>ibuf.w) <= 0 && errno != EAGAIN) {
- *ctl_cnt -= control_close(pfd->fd);
- return (1);
- }
+ if (msgbuf_write(>ibuf.w) <= 0 && errno != EAGAIN)
+ return control_close(c);
if (c->throttled && c->ibuf.w.queued < CTL_MSG_LOW_MARK) {
if (imsg_ctl_rde(IMSG_XON, c->ibuf.pid, NULL, 0) != -1)
c->throttled = 0;
@@ -251,16 +259,12 @@ control_dispatch_msg(struct pollfd *pfd,
return (0);
if (((n = imsg_read_nofd(>ibuf)) == -1 && errno != EAGAIN) ||
- n == 0) {
- *ctl_cnt -= control_close(pfd->fd);
- return (1);
- }
+ n == 0)
+ return control_close(c);
for (;;) {
- if ((n = imsg_get(>ibuf, )) == -1) {
- *ctl_cnt -= control_close(pfd->fd);
- return (1);
- }
+ if ((n = imsg_get(>ibuf, )) == -1)
+ return control_close(c);
if (n == 0)
break;
Index: session.c
===
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.402
diff -u -p -r1.402 session.c
--- session.c 27 Jun 2020 07:24:42 - 1.402
+++ session.c 21 Oct 2020 16:49:10 -
@@ -196,7 +196,6 @@ session_main(int debug, int verbose)
struct peer *p, **peer_l = NULL, *next;
struct mrt *m, *xm, **mrt_l = NULL;
struct pollfd *pfd = NULL;
- struct ctl_conn *ctl_conn;
struct listen_addr *la;
void*newp;
time_t now;
@@ -237,7 +236,6 @@ session_main(int debug, int verbose)
fatal(NULL);
imsg_init(ibuf_main, 3);
- TAILQ_INIT(_conns);
LIST_INIT();
listener_cnt = 0;
peer_cnt = 0;
@@ -438,13 +436,10 @@ session_main(int debug, int verbose)
idx_mrts = i;
- TAILQ_FOREACH(ctl_conn, _conns, entry) {
- pfd[i].fd = ctl_conn->ibuf.fd;
- pfd[i].events = POLLIN;
- if (ctl_conn->ibuf.w.queued > 0)
- pfd[i].events |= POLLOUT;
- i++;
-
