On Fri, Jun 16, 2017 at 07:12:43PM +0300, Henri Kemppainen wrote:
> > > Nice catch, the diff reads fine to me, I'll commit later today when I
> > > have another ok from eric@
>
> > Yes, this looks correct. But, I would rather move the resume test before
> > the EOM test, to avoid touching the session after the transfer has been
> > finalized by smtp_data_io_done().
>
> It oughtn't make a difference as long as the duplex control is correct,
> but I'm fine with that change.
>
> > > side note, smtpd should not have been able to leak more than 5 fd, it
> > > should not have been able to exhaust, is this what you observed ?
>
> For the record, we discussed this with Gilles on irc and while we saw
> more than a dozen leaked fds, it's okay as smtpd will allow as many
> incoming sessions as the dtable can accommodate (with an fd reserve).
> The lower limits are on outgoing connections.
>
> New diff with reordered code. I'll see if I can get Adam to run one more
> round of testing..
>
Tested the diff on -current OpenSMTPD running on a 6.1 server.
Sent 11 emails with ~2MB of base64 encoded garbage (like with our previous
tests)
all emails were delivered with no FD leaks.
I am also willing to test an errata diff on 6.1 if this qualifies
(reliability/performance).
>
> Index: usr.sbin/smtpd/smtp_session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.303
> diff -u -p -r1.303 smtp_session.c
> --- usr.sbin/smtpd/smtp_session.c 17 May 2017 14:00:06 -0000 1.303
> +++ usr.sbin/smtpd/smtp_session.c 16 Jun 2017 14:56:11 -0000
> @@ -1474,12 +1474,12 @@ smtp_data_io(struct io *io, int evt, voi
> break;
>
> case IO_LOWAT:
> - if (s->tx->dataeom && io_queued(s->tx->oev) == 0) {
> - smtp_data_io_done(s);
> - } else if (io_paused(s->io, IO_IN)) {
> + if (io_paused(s->io, IO_IN)) {
> log_debug("debug: smtp: %p: filter congestion over:
> resuming session", s);
> io_resume(s->io, IO_IN);
> }
> + if (s->tx->dataeom && io_queued(s->tx->oev) == 0)
> + smtp_data_io_done(s);
> break;
>
> default:
>
