From: Philipp <philipp+openbsd@bureaucracy.de>
Subject: Re: smtpd: change the table protocol
To: Omar Polo <op@omarpolo.com>
Cc: tech@openbsd.org
Date: Tue, 30 Apr 2024 10:53:57 +0200

Hi

[2024-04-29 13:37] Omar Polo <op@omarpolo.com>
> This changes the transport layer for the smtpd <-> proc tables
> communications.
>
> Using imsg here has proven to be quite painful because smtpd internals
> (struct definition etc.) needs to be kept in sync with the table
> implementations, and it's also hard to actually write external tables.
>
> Instead, a filter-like protocol for tables completely decouples the
> implementations and allows to write tables more easily.
>
> All the OpenSMTPD-extras tables were converted to the new stdio protocol
> and I intend to commit this together with a diff for ports to avoid a
> fallout.
>
> The table themselves will continue to work as they do now, only the
> transport layer will change.
>
> The configuration also won't need any change.
>
> Regarding the diff: there's a bit of churn due to the addition of a
> field to fork_proc_backend().  When forking the tables we need to
> redirect stdout too, and i'm not sure if it's safe to do so for the
> queues and schedulers too.  The new flag ensures that we dup() stdout to
> the socketpair only for the tables.
>
> The protocol is documented in the added smtpd-tables.7 manual page.
>
> ok? :)

General it looks good, only one problem.

>  table_proc_open(struct table *table)
>  {
> [...]
> +	int			 fd, fdd;
>  
>  	priv = xcalloc(1, sizeof(*priv));
>  
> -	fd = fork_proc_backend("table", table->t_config, table->t_name);
> +	fd = fork_proc_backend("table", table->t_config, table->t_name, 1);
>  	if (fd == -1)
>  		fatalx("table-proc: exiting");
> +	if ((fdd = dup(fd)) == -1) {

fdd is only created but not used.

> +		log_warnx("warn: table-proc: dup");
> +		fatalx("table-proc: exiting");
> +	}
> +	if ((priv->in = fdopen(fd, "r")) == NULL)
> +		fatalx("table-proc: fdopen");
> +	if ((priv->out = fdopen(fd, "w")) == NULL)
> +		fatalx("table-proc: fdopen");

I guess you wanted to use fdd in one of the fdopen().

Philipp