Re: improve pkg_add bandwidth usage with some mirrors

2020-06-19 Thread Solene Rapenne
On Fri, 19 Jun 2020 14:42:54 +0200
Theo Buehler :

> On Fri, Jun 19, 2020 at 11:42:44AM -, Christian Weisgerber wrote:
>  [...]  
>  [...]  
>  [...]  
> 
> Yes, jsing wanted to take a closer look.  I will commit the diff
> tonight UTC unless I hear an objection (I have an ok beck).
> 

I confirm that with this patch pkg_add doesn't download extra bytes.



Re: improve pkg_add bandwidth usage with some mirrors

2020-06-19 Thread Theo Buehler
On Fri, Jun 19, 2020 at 11:42:44AM -, Christian Weisgerber wrote:
> On 2020-06-18, Marc Espie  wrote:
> 
> > What pkg_add does internally is a pipeline:
> >
> > ftp | signify|internal gunzip
> >
> > closing the end file handle should kill the whole chain.
> > So I need to figure out where it goes wrong, what's the
> > part that doesn't die "instantly".
> 
> That's ftp(1).  Our SSL people are sitting on a patch to libtls^H^H^Hssl.

Yes, jsing wanted to take a closer look.  I will commit the diff tonight
UTC unless I hear an objection (I have an ok beck).

Index: tls13_legacy.c
===
RCS file: /var/cvs/src/lib/libssl/tls13_legacy.c,v
retrieving revision 1.8
diff -u -p -r1.8 tls13_legacy.c
--- tls13_legacy.c  29 May 2020 17:47:30 -  1.8
+++ tls13_legacy.c  11 Jun 2020 12:19:30 -
@@ -477,6 +477,7 @@ tls13_legacy_shutdown(SSL *ssl)
struct tls13_ctx *ctx = ssl->internal->tls13;
uint8_t buf[512]; /* XXX */
ssize_t ret;
+   int want_close_notify = 1;
 
/*
 * We need to return 0 when we have sent a close-notify but have not
@@ -492,6 +493,11 @@ tls13_legacy_shutdown(SSL *ssl)
/* Send close notify. */
if (!(ssl->internal->shutdown & SSL_SENT_SHUTDOWN)) {
ssl->internal->shutdown |= SSL_SENT_SHUTDOWN;
+   /*
+* Do not try to read application data to support unilateral
+* shutdown semantics for SSL_shutdown(3).
+*/
+   want_close_notify = 0;
if ((ret = tls13_send_alert(ctx->rl, TLS13_ALERT_CLOSE_NOTIFY)) 
< 0)
return tls13_legacy_return_code(ssl, ret);
}
@@ -501,7 +507,7 @@ tls13_legacy_shutdown(SSL *ssl)
return tls13_legacy_return_code(ssl, ret);
 
/* Receive close notify. */
-   if (!ctx->close_notify_recv) {
+   if (want_close_notify && !ctx->close_notify_recv) {
/*
 * If there is still application data pending then we have no
 * option but to discard it here. The application should have



Re: improve pkg_add bandwidth usage with some mirrors

2020-06-19 Thread Christian Weisgerber
On 2020-06-18, Marc Espie  wrote:

> What pkg_add does internally is a pipeline:
>
> ftp | signify|internal gunzip
>
> closing the end file handle should kill the whole chain.
> So I need to figure out where it goes wrong, what's the
> part that doesn't die "instantly".

That's ftp(1).  Our SSL people are sitting on a patch to libtls^H^H^Hssl.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: improve pkg_add bandwidth usage with some mirrors

2020-06-18 Thread Marc Espie
On Wed, Jun 17, 2020 at 11:34:20AM +0200, Solene Rapenne wrote:
> I propose a small diff for pkg_add when using http/https mirrors.
> Don't wait 30 seconds for the ftp process to stop when closing
> file handler, send SIGHUP immediately after closing the file handler.
> 
> Running pkg_add -u neovim (already installed and up to date) I got
> those results of bandwidth usage
> 
> using mirror https://mirror.one.com/pub/OpenBSD/
> 62513 kB without diff
> 9050  kB with diff
> 
> using mirror https://ftp.fr.openbsd.org/pub/OpenBSD
> 6530 kB without diff
> 6373 kB with diff
> 
> The 2500 kB difference between the two mirrors with the diff is
> explained by the html directory listing which is different
> between servers. mirror.one.com list is 3800 kB while
> ftp.fr.openbsd.org only 1387 kB.
> 
> I can't explain why but when using mirror.one.com the ftp command will
> continuously fetch packages until completion or stop if ftp is killed
> after 30 seconds. This extra downloaded data is useless.
> 
> I came with the following diff, but maybe the real issue is ftp(1)
> not stopping immediately if output is closed?

Like I said privately to solene, I would very much prefer
that we figure out why things don't die "instantly".

What pkg_add does internally is a pipeline:

ftp | signify|internal gunzip

closing the end file handle should kill the whole chain.
So I need to figure out where it goes wrong, what's the
part that doesn't die "instantly".

The thing to do would be to send explicit SIGPIPE to
either signify or ftp and see whether things work better.



improve pkg_add bandwidth usage with some mirrors

2020-06-17 Thread Solene Rapenne
I propose a small diff for pkg_add when using http/https mirrors.
Don't wait 30 seconds for the ftp process to stop when closing
file handler, send SIGHUP immediately after closing the file handler.

Running pkg_add -u neovim (already installed and up to date) I got
those results of bandwidth usage

using mirror https://mirror.one.com/pub/OpenBSD/
62513 kB without diff
9050  kB with diff

using mirror https://ftp.fr.openbsd.org/pub/OpenBSD
6530 kB without diff
6373 kB with diff

The 2500 kB difference between the two mirrors with the diff is
explained by the html directory listing which is different
between servers. mirror.one.com list is 3800 kB while
ftp.fr.openbsd.org only 1387 kB.

I can't explain why but when using mirror.one.com the ftp command will
continuously fetch packages until completion or stop if ftp is killed
after 30 seconds. This extra downloaded data is useless.

I came with the following diff, but maybe the real issue is ftp(1)
not stopping immediately if output is closed?

Index: OpenBSD/PackageRepository.pm
===
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm,v
retrieving revision 1.171
diff -u -p -r1.171 PackageRepository.pm
--- OpenBSD/PackageRepository.pm8 Nov 2019 14:50:58 -   1.171
+++ OpenBSD/PackageRepository.pm17 Jun 2020 09:21:41 -
@@ -206,12 +206,8 @@ sub close
my ($self, $object, $hint) = @_;
close($object->{fh}) if defined $object->{fh};
if (defined $object->{pid2}) {
-   local $SIG{ALRM} = sub {
-   kill HUP => $object->{pid2};
-   };
-   alarm(30);
+   kill HUP => $object->{pid2};
waitpid($object->{pid2}, 0);
-   alarm(0);
}
$self->parse_problems($object->{errors}, $hint, $object)
if defined $object->{errors};