Claudio Jeker([email protected]) 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 -0000 1.100
> +++ control.c 21 Oct 2020 16:57:53 -0000
> @@ -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 -0000 1.402
> +++ session.c 21 Oct 2020 16:49:10 -0000
> @@ -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 += control_fill_pfds(pfd + i, pfd_elms -i);
> +
> + if (i > pfd_elms)
> + fatalx("poll pfd overflow");
>
> if (pauseaccept && timeout > 1)
> timeout = 1;
> @@ -511,7 +506,7 @@ session_main(int debug, int verbose)
> mrt_write(mrt_l[j - idx_peers]);
>
> for (; j < i; j++)
> - control_dispatch_msg(&pfd[j], &ctl_cnt, &conf->peers);
> + ctl_cnt -= control_dispatch_msg(&pfd[j], &conf->peers);
> }
>
> RB_FOREACH_SAFE(p, peer_head, &conf->peers, next) {
> Index: session.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
> retrieving revision 1.146
> diff -u -p -r1.146 session.h
> --- session.h 10 May 2020 13:38:46 -0000 1.146
> +++ session.h 21 Oct 2020 16:49:43 -0000
> @@ -146,8 +146,6 @@ struct ctl_conn {
> int terminate;
> };
>
> -TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns;
> -
> struct peer_stats {
> unsigned long long msg_rcvd_open;
> unsigned long long msg_rcvd_update;
> @@ -260,8 +258,9 @@ int prepare_listeners(struct bgpd_confi
> int control_check(char *);
> int control_init(int, char *);
> int control_listen(int);
> +size_t control_fill_pfds(struct pollfd *, size_t);
> void control_shutdown(int);
> -int control_dispatch_msg(struct pollfd *, u_int *, struct peer_head *);
> +int control_dispatch_msg(struct pollfd *, struct peer_head *);
> unsigned int control_accept(int, int);
>
> /* log.c */
>
