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, &ctl_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(&c->ibuf.w) <= 0 && errno != EAGAIN) { > - *ctl_cnt -= control_close(pfd->fd); > - return (1); > - } > + if (msgbuf_write(&c->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(&c->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(&c->ibuf, &imsg)) == -1) { > - *ctl_cnt -= control_close(pfd->fd); > - return (1); > - } > + if ((n = imsg_get(&c->ibuf, &imsg)) == -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(&ctl_conns); > LIST_INIT(&mrthead); > listener_cnt = 0; > peer_cnt = 0; > @@ -438,13 +436,10 @@ session_main(int debug, int verbose) > > idx_mrts = i; > > - TAILQ_FOREACH(ctl_conn,
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, &ctl_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(&c->ibuf.w) <= 0 && errno != EAGAIN) { - *ctl_cnt -= control_close(pfd->fd); - return (1); - } + if (msgbuf_write(&c->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(&c->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(&c->ibuf, &imsg)) == -1) { - *ctl_cnt -= control_close(pfd->fd); - return (1); - } + if ((n = imsg_get(&c->ibuf, &imsg)) == -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(&ctl_conns); LIST_INIT(&mrthead); listener_cnt = 0; peer_cnt = 0; @@ -438,13 +436,10 @@ session_main(int debug, int verbose) idx_mrts = i; - TAILQ_FOREACH(ctl_conn, &ctl_conns, entry) { - pfd[i].fd = ctl_conn->ibuf.fd; - pfd[i].events = POLLIN; - if (ctl_conn->ibuf.w.queued > 0) - pfd[i].events
Refactor bgpd control code
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? -- :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, &ctl_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(&c->ibuf.w) <= 0 && errno != EAGAIN) { - *ctl_cnt -= control_close(pfd->fd); - return (1); - } + if (msgbuf_write(&c->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(&c->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(&c->ibuf, &imsg)) == -1) { - *ctl_cnt -= control_close(pfd->fd); - return (1); - } + if ((n = imsg_get(&c->ibuf, &imsg)) == -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(&ctl_conns); LIST_INIT(&mrthead); listener_cnt = 0; peer_cnt = 0; @@ -438,13 +436,10 @@ session_main(int debug, int verbose) idx_mrts = i; - TAILQ_FOREACH(ctl_conn, &ctl_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++; - } + i += con