Re: iscsid issues with Synology NAS
On 31.03., David Alten wrote:
> Hello,
>
> I???m having issues getting iscsid to work with my Synology NAS.
>
> The first issue was that the NAS was returning an error code. Turns out
> it didn???t like not?? missing the default SessionType=Normal.
>
> The second issue was that the login sequence never comleted. It seems we
> keep hardcoding MaxConnections and MaxRecvDataSegmentLength in
> connection.c:conn_gen_kvp() instead of negotiating to the lowest value.
> So I just bypassed that function entirely and hardcoded all the default
> settings in initiator.c.
>
> This makes the login step complete, but then nothing else happens after
> that. I would expect a new scsi device to be attached so I could use it.
>
> I did notice that MaxConnection shows as 0, but the NAS does show the
> connection as established.
>
> b1# iscsictl show?? ??
> Initiator: ISID base 80d3cf6f qalifier 6e7d
>
> Session 'disk2':
> ?? ?? SessionType: normal MaxConnections: 0
> ?? ?? TargetName: iqn.2000-01.com.synology:Target.02
> ?? ?? TargetAddr: 192.168.0.4:iscsi
> ?? ?? InitiatorName: iqn.1995-11.org.openbsd.iscsid:b1
> ?? ?? InitiatorAddr: 192.168.0.9
> command successful
>
> b1$ cat /etc/iscsi.conf?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??
> ?? ?? ?? ?? ?? ?? ??
> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??
> target disk2 {
> ?? ?? ?? ?? initiatoraddr 192.168.0.9
> ?? ?? ?? ?? targetaddr 192.168.0.4
> ?? ?? ?? ?? targetname "iqn.2000-01.com.synology:Target.02"
> }
> Any suggestions on where to go from here?
>
> Thanks,
>
> -David
>
Hi David,
I had the same problem some years ago when I tried to attach an iSCSI
LUN from my Synology to my APU box. I wrote the patch below which fixed
the problem for me. Maybe you can give it a try?
Cheers,
Bruno
Index: usr.sbin/iscsid/connection.c
===
RCS file: /cvs/src/usr.sbin/iscsid/connection.c,v
retrieving revision 1.21
diff -u -p -r1.21 connection.c
--- usr.sbin/iscsid/connection.c5 Dec 2015 06:38:18 - 1.21
+++ usr.sbin/iscsid/connection.c1 Apr 2021 05:18:29 -
@@ -316,44 +316,6 @@ log_debug("conn_parse_kvp: %s = %s", k->
#undef SET_NUM
#undef SET_BOOL
-int
-conn_gen_kvp(struct connection *c, struct kvp *kvp, size_t *nkvp)
-{
- struct session *s = c->session;
- size_t i = 0;
-
- if (s->mine.MaxConnections != iscsi_sess_defaults.MaxConnections) {
- if (kvp && i < *nkvp) {
- kvp[i].key = strdup("MaxConnections");
- if (kvp[i].key == NULL)
- return -1;
- if (asprintf([i].value, "%hu",
- s->mine.MaxConnections) == -1) {
- kvp[i].value = NULL;
- return -1;
- }
- }
- i++;
- }
- if (c->mine.MaxRecvDataSegmentLength !=
- iscsi_conn_defaults.MaxRecvDataSegmentLength) {
- if (kvp && i < *nkvp) {
- kvp[i].key = strdup("MaxRecvDataSegmentLength");
- if (kvp[i].key == NULL)
- return -1;
- if (asprintf([i].value, "%u",
- c->mine.MaxRecvDataSegmentLength) == -1) {
- kvp[i].value = NULL;
- return -1;
- }
- }
- i++;
- }
-
- *nkvp = i;
- return 0;
-}
-
void
conn_pdu_write(struct connection *c, struct pdu *p)
{
Index: usr.sbin/iscsid/initiator.c
===
RCS file: /cvs/src/usr.sbin/iscsid/initiator.c,v
retrieving revision 1.15
diff -u -p -r1.15 initiator.c
--- usr.sbin/iscsid/initiator.c 16 Jan 2015 15:57:06 - 1.15
+++ usr.sbin/iscsid/initiator.c 1 Apr 2021 05:18:29 -
@@ -250,39 +250,113 @@ initiator_nop_in_imm(struct connection *
conn_task_issue(c, t);
}
+#define WRITE_BOOL(k, v) \
+do { \
+ if (v) \
+ k = "Yes"; \
+ else\
+ k = "No"; \
+} while (0)
+
+#define WRITE_NUM(k, v)\
+do { \
+ if (asprintf(, "%hu", v) == -1) \
+ errors++; \
+} while (0)
+
+#define WRITE_INT(k, v)\
+do { \
+ if (asprintf(, "%u", v) == -1)\
+ errors++; \
+} while (0)
+
+#define WRITE_DIGEST(k, v) \
+do { \
+ if (v) \
+ k = "CRC32";\
+ else\
+ k = "None"; \
+} while (0)\
+
struct kvp *
Re: Mention -N to shutdown(2) network socket after EOF in man nc(1)
On Tue, Mar 30, 2021 at 02:48:17AM +0200, Robert Scheck wrote: > Jakub Jelen reported at Fedora that "nc -l 8080" (terminal #1) and "echo > XXX | nc localhost 8080" (terminal #2) keeps hanging both client and server > after reading the EOF, even the last sentence of "CLIENT/SERVER MODEL" in > the man page of nc(1) says "The connection may be terminated using an EOF > (???^D???)." > > Based on Freenode #openbsd IRC the observed behaviour is correct but the > man page should be updated, so my proposal is posted below. As I'm not sure > if -N should be added to 'server' and 'client' side, I've cloned the "DATA > TRANSFER" section, which covers the 'client' side only. Feel free to adjust > my proposal as necessary. > > > Thanks, > Robert > hi. thanks for your diff. it has been committed, with a small wording change. jmc > --- nc.1 2020-02-12 14:46:36.831500390 + 1.95 > +++ nc.1 2021-03-30 02:22:34.545148296 +0200 > @@ -414,7 +414,7 @@ > .Pq or a second machine , > connect to the machine and port being listened on: > .Pp > -.Dl $ nc 127.0.0.1 1234 > +.Dl $ nc -N 127.0.0.1 1234 > .Pp > There should now be a connection between the ports. > Anything typed at the second console will be concatenated to the first, > @@ -427,7 +427,10 @@ > .Sq client . > The connection may be terminated using an > .Dv EOF > -.Pq Sq ^D . > +.Pq Sq ^D > +if the > +.Fl N > +flag was given. > .Sh DATA TRANSFER > The example in the previous section can be expanded to build a > basic data transfer model. >
Re: smtpd: trace and vfprintf %s NULL
Any objection or ok?
On Sat, Mar 27, 2021 at 12:52:11PM +0100, Eric Faurot wrote:
> Hello.
>
> I get reports from people seeing "vfprintf %s NULL" in their logs
> recently. The problem is in a function that should be fixed,
> but that function is only expected to but used for debug traces.
>
> This diff turns log_trace() into a macro, so the parameters
> are not needlessly evaluated when tracing is not set.
>
> Eric.
>
> Index: smtpd.h
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.662
> diff -u -p -r1.662 smtpd.h
> --- smtpd.h 5 Mar 2021 12:37:32 - 1.662
> +++ smtpd.h 23 Mar 2021 07:16:39 -
> @@ -1747,8 +1747,9 @@ int base64_encode_rfc3548(unsigned char
> char *, size_t);
>
> void log_trace_verbose(int);
> -void log_trace(int, const char *, ...)
> -__attribute__((format (printf, 2, 3)));
> +void log_trace0(const char *, ...)
> +__attribute__((format (printf, 1, 2)));
> +#define log_trace(m, ...) do { if (tracing & (m)) log_trace0(__VA_ARGS__);
> } while (0)
>
> /* waitq.c */
> int waitq_wait(void *, void (*)(void *, void *, void *), void *);
> Index: util.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/util.c,v
> retrieving revision 1.152
> diff -u -p -r1.152 util.c
> --- util.c29 Nov 2020 20:07:38 - 1.152
> +++ util.c23 Mar 2021 07:17:05 -
> @@ -823,15 +823,13 @@ base64_encode_rfc3548(unsigned char cons
> }
>
> void
> -log_trace(int mask, const char *emsg, ...)
> +log_trace0(const char *emsg, ...)
> {
> va_list ap;
>
> - if (tracing & mask) {
> - va_start(ap, emsg);
> - vlog(LOG_DEBUG, emsg, ap);
> - va_end(ap);
> - }
> + va_start(ap, emsg);
> + vlog(LOG_DEBUG, emsg, ap);
> + va_end(ap);
> }
>
> void
>
>
Re: rpki-client move encoding functions into own file
On Wed, Mar 31, 2021 at 01:13:08PM +0200, Claudio Jeker wrote:
> As mentioned before move the base64 and hex encoding / decoding functions
> into one file. This is just minor cleanup.
ok tb
>
> --
> :wq Claudio
>
> PS: I know this will break regress and I will fix that once this goes in.
>
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
> retrieving revision 1.19
> diff -u -p -r1.19 Makefile
> --- Makefile 4 Mar 2021 13:01:41 - 1.19
> +++ Makefile 31 Mar 2021 10:56:42 -
> @@ -1,9 +1,10 @@
> #$OpenBSD: Makefile,v 1.19 2021/03/04 13:01:41 claudio Exp $
>
> PROG=rpki-client
> -SRCS=as.c cert.c cms.c crl.c gbr.c http.c io.c ip.c log.c main.c
> mft.c \
> - mkdir.c output.c output-bgpd.c output-bird.c output-csv.c \
> - output-json.c parser.c roa.c rsync.c tal.c validate.c x509.c
> +SRCS=as.c cert.c cms.c crl.c encoding.c gbr.c http.c io.c ip.c log.c
> \
> + main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
> + output-csv.c output-json.c parser.c roa.c rsync.c tal.c validate.c \
> + x509.c
> MAN= rpki-client.8
>
> LDADD+= -ltls -lssl -lcrypto -lutil
> Index: encoding.c
> ===
> RCS file: encoding.c
> diff -N encoding.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ encoding.c31 Mar 2021 11:00:49 -
> @@ -0,0 +1,88 @@
> +/* $OpenBSD$ */
> +/*
> + * Copyright (c) 2020 Claudio Jeker
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include "extern.h"
> +
> +/*
> + * Decode base64 encoded string into binary buffer returned in out.
> + * The out buffer size is stored in outlen.
> + * Returns 0 on success or -1 for any errors.
> + */
> +int
> +base64_decode(const unsigned char *in, unsigned char **out, size_t *outlen)
> +{
> + static EVP_ENCODE_CTX *ctx;
> + unsigned char *to;
> + size_t inlen;
> + int tolen;
> +
> + if (ctx == NULL && (ctx = EVP_ENCODE_CTX_new()) == NULL)
> + err(1, "EVP_ENCODE_CTX_new");
> +
> + *out = NULL;
> + *outlen = 0;
> +
> + inlen = strlen(in);
> + if (inlen >= INT_MAX - 3)
> + return -1;
> + tolen = ((inlen + 3) / 4) * 3 + 1;
> + if ((to = malloc(tolen)) == NULL)
> + return -1;
> +
> + EVP_DecodeInit(ctx);
> + if (EVP_DecodeUpdate(ctx, to, , in, inlen) == -1)
> + goto fail;
> + *outlen = tolen;
> + if (EVP_DecodeFinal(ctx, to + tolen, ) == -1)
> + goto fail;
> + *outlen += tolen;
> + *out = to;
> + return 0;
> +
> +fail:
> + free(to);
> + return -1;
> +}
> +
> +/*
> + * Convert binary buffer of size dsz into an upper-case hex-string.
> + * Returns pointer to the newly allocated string. Function can't fail.
> + */
> +char *
> +hex_encode(const unsigned char *in, size_t insz)
> +{
> + const char hex[] = "0123456789ABCDEF";
> + size_t i;
> + char *out;
> +
> + if ((out = calloc(2, insz + 1)) == NULL)
> + err(1, NULL);
> +
> + for (i = 0; i < insz; i++) {
> + out[i * 2] = hex[in[i] >> 4];
> + out[i * 2 + 1] = hex[in[i] & 0xf];
> + }
> + out[i * 2] = '\0';
> +
> + return out;
> +}
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.59
> diff -u -p -r1.59 extern.h
> --- extern.h 29 Mar 2021 12:41:34 - 1.59
> +++ extern.h 31 Mar 2021 10:55:49 -
> @@ -419,6 +419,13 @@ void cryptoerrx(const char *, ...)
> __attribute__((format(printf, 1, 2)))
> __attribute__((noreturn));
>
> +/* Encoding functions for hex and base64. */
> +
> +int base64_decode(const unsigned char *, unsigned char **,
> + size_t *);
> +char *hex_encode(const unsigned char *, size_t);
> +
> +
> /* Functions for moving data between processes. */
>
> void io_socket_blocking(int);
> Index: tal.c
>
iscsid issues with Synology NAS
Hello,
Iâm having issues getting iscsid to work with my Synology NAS.
The first issue was that the NAS was returning an error code. Turns out
it didnât like not missing the default SessionType=Normal.
The second issue was that the login sequence never comleted. It seems we
keep hardcoding MaxConnections and MaxRecvDataSegmentLength in
connection.c:conn_gen_kvp() instead of negotiating to the lowest value.
So I just bypassed that function entirely and hardcoded all the default
settings in initiator.c.
This makes the login step complete, but then nothing else happens after
that. I would expect a new scsi device to be attached so I could use it.
I did notice that MaxConnection shows as 0, but the NAS does show the
connection as established.
b1# iscsictl show Â
Initiator: ISID base 80d3cf6f qalifier 6e7d
Session 'disk2':
  SessionType: normal MaxConnections: 0
  TargetName: iqn.2000-01.com.synology:Target.02
  TargetAddr: 192.168.0.4:iscsi
  InitiatorName: iqn.1995-11.org.openbsd.iscsid:b1
  InitiatorAddr: 192.168.0.9
command successful
b1$ cat /etc/iscsi.conf                  Â
     Â
                     Â
target disk2 {
    initiatoraddr 192.168.0.9
    targetaddr 192.168.0.4
    targetname "iqn.2000-01.com.synology:Target.02"
}
Any suggestions on where to go from here?
Thanks,
-David
b1$ doas /usr/sbin/iscsid -dvÂ
startup
session_fsm[disk2]: INIT ev start timeout 0
sess_fsm[disk2]: INIT ev start
new connection to 192.168.0.4:3260
conn_fsm[disk2]: FREE ev connect
conn_fsm[disk2]: new state XPT_WAIT
sess_fsm[disk2]: new state FREE
sess_fsm: done
conn_fsm[disk2]: XPT_WAIT ev connected
conn_fsm[disk2]: new state IN_LOGIN
conn_parse_kvp: AuthMethod = None
conn_parse_kvp: TargetAlias = Synology Target
conn_parse_kvp: TargetPortalGroupTag = 1
SET_NUM: TargetPortalGroupTag = 1
conn_parse_kvp: HeaderDigest = None
conn_parse_kvp: DataDigest = None
conn_parse_kvp: MaxConnections = 1
SET_NUM: MaxConnections = 1
conn_parse_kvp: ImmediateData = Yes
SET_BOOL: ImmediateData = 1
conn_parse_kvp: MaxRecvDataSegmentLength = 262144
SET_NUM: MaxRecvDataSegmentLength = 262144
conn_parse_kvp: MaxBurstLength = 262144
SET_NUM: MaxBurstLength = 262144
conn_parse_kvp: FirstBurstLength = 65536
SET_NUM: FirstBurstLength = 65536
conn_parse_kvp: DefaultTime2Wait = 2
SET_NUM: DefaultTime2Wait = 2
conn_parse_kvp: DefaultTime2Retain = 20
SET_NUM: DefaultTime2Retain = 20
conn_parse_kvp: MaxOutstandingR2T = 1
SET_NUM: MaxOutstandingR2T = 1
conn_parse_kvp: DataPDUInOrder = Yes
SET_BOOL: DataPDUInOrder = 1
conn_parse_kvp: DataSequenceInOrder = Yes
SET_BOOL: DataSequenceInOrder = 1
conn_parse_kvp: ErrorRecoveryLevel = 0
SET_NUM: ErrorRecoveryLevel = 0
conn_fsm[disk2]: IN_LOGIN ev logged in
session_fsm[disk2]: FREE ev connection logged in timeout 0
conn_fsm[disk2]: new state LOGGED_IN
sess_fsm[disk2]: FREE ev connection logged in
sess_fsm[disk2]: new state LOGGED_IN
sess_fsm: done
Index: initiator.c
===
RCS file: /cvs/src/usr.sbin/iscsid/initiator.c,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 initiator.c
--- initiator.c 16 Jan 2015 15:57:06 -Â Â Â 1.15
+++ initiator.c 31 Mar 2021 03:24:44 -
@@ -254,11 +254,10 @@ struct kvp *
 initiator_login_kvp(struct connection *c, u_int8_t stage)
 {
    struct kvp *kvp;
-Â Â Â Â size_t nkvp;
Â
    switch (stage) {
    case ISCSI_LOGIN_STG_SECNEG:
-Â Â Â Â Â Â Â Â if (!(kvp = calloc(4, sizeof(*kvp
+Â Â Â Â Â Â Â Â if (!(kvp = calloc(5, sizeof(*kvp
            return NULL;
        kvp[0].key = "AuthMethod";
        kvp[0].value = "None";
@@ -269,20 +268,39 @@ initiator_login_kvp(struct connection *c
            kvp[2].key = "SessionType";
            kvp[2].value = "Discovery";
        } else {
-Â Â Â Â Â Â Â Â Â Â Â Â kvp[2].key = "TargetName";
-Â Â Â Â Â Â Â Â Â Â Â Â kvp[2].value =
c->session->config.TargetName;
+Â Â Â Â Â Â Â Â Â Â Â Â kvp[2].key = "SessionType";
+Â Â Â Â Â Â Â Â Â Â Â Â kvp[2].value = "Normal";
+Â Â Â Â Â Â Â Â Â Â Â Â kvp[3].key = "TargetName";
+Â Â Â Â Â Â Â Â Â Â Â Â kvp[3].value =
c->session->config.TargetName;
        }
        break;
    case ISCSI_LOGIN_STG_OPNEG:
-Â Â Â Â Â Â Â Â if (conn_gen_kvp(c, NULL, ) == -1)
-Â Â Â Â Â Â Â Â Â Â Â Â return NULL;
-Â Â Â Â Â Â Â Â nkvp += 1; /* add slot for terminator */
-Â Â Â Â Â Â Â Â if (!(kvp = calloc(nkvp, sizeof(*kvp
-Â Â Â Â Â Â Â Â Â Â Â Â return NULL;
-Â Â Â Â Â Â Â Â if (conn_gen_kvp(c, kvp, ) == -1) {
-Â Â Â Â Â Â Â Â Â Â Â Â free(kvp);
+Â Â Â Â Â Â Â Â
Re: iwm(4) A-MSDU support
On 29 Mar 19:27, Stefan Sperling wrote: > This patch attempts to add support for receiving A-MSDUs to iwm(4). > If you are using iwm(4) then please run with this patch and let me > know if it causes regressions. Thanks! > > ACHTUNG: This patch breaks iwx(4)! Don't use it there! For this reason, > the patch can neither be committed nor be put into snapshots!!! > > Our net80211 stack de-aggregates A-MSDUs in software. This works fine with > iwm 7k and 8k devices. However, iwm 9k devices de-aggregate A-MSDUs in > hardware and net80211 is currently unable to handle this. > > Our current iwm 9k code only works because long ago I disabled reception > of A-MSDUs for all devices. Unfortunately, the only way to get A-MSDUs > working on 9k hardware is to add a bunch of driver-specific code which > handles de-aggregation. Otherwise, net80211 will drop frames which appear > to arrive out of order, or appear as duplicates even though they were > simply part of the same A-MSDU and thus share a sequence number. > The driver can re-order frames correctly based on information provided > by firmware. I'd rather implement this than letting these devices miss > out on A-MSDUs because the Rx speed advantage can be significant, around > 80/90 Mbps (but this will very much depend on the AP). > > If these A-* acronyms don't make sense and you'd like to know more, here > is a fairly good explanation: https://mrncciew.com/2013/04/11/a-mpdu-a-msdu/ > Note that we care about the nested case, which is also mentioned in this > article but not explained in detail. It's simply an A-MSDU stashed inside > an A-MPDU. If an AP can do 11AC, then it should support this. > > iwx(4) hardware has the same problem. > If this patch works fine on iwm(4) then I can port the changes to iwx(4), > do another round of testing, and eventually commit to both drivers at > the same time. > > Some of the changes below are based on code from the Linux iwlwifi driver. > I am not entirely happy with some of its style. But the code should be in > good enough shape to be tested. > Hi Stefan, I tested the patch with my: iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x78, msi iwm0: hw rev 0x230, fw ver 34.0.1, address xx:xx:xx:xx:xx:xx against my Technicolor MediaAccess TG789vac. 5GHz: Down: Conn: 1 Mbps: 105.479 Peak Mbps: 107.955 Avg Mbps: 105.479 25076 13282504 106.366 100.00% Up: Conn: 1 Mbps: 28.056 Peak Mbps: 28.096 Avg Mbps: 28.056 17004 3483888 27.899 100.00% 2GHz: Down: Conn: 1 Mbps: 87.640 Peak Mbps: 87.965 Avg Mbps: 87.640 14041 11096024 88.680 100.00% UP: Conn: 1 Mbps: 22.786 Peak Mbps: 22.844 Avg Mbps: 22.786 9003 3176912 25.441 100.00% Thanks for your hard work! -- wq: ~uw
Re: rpki-client use setproctitle(3)
Absolutely -- this allows me to skip a few steps when tracing.
Claudio Jeker wrote:
> It is time to use setproctitle() in rpki-client.
> Right now there are three processes, soon it will be four.
> This should help identify the different processes.
> I did not change the proc title of the main process to keep the arguments.
>
> --
> :wq Claudio
>
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.126
> diff -u -p -r1.126 main.c
> --- main.c29 Mar 2021 03:45:35 - 1.126
> +++ main.c31 Mar 2021 15:53:03 -
> @@ -979,6 +979,7 @@ main(int argc, char *argv[])
> if (procpid == 0) {
> close(fd[1]);
>
> + setproctitle("parser");
> /* change working directory to the cache directory */
> if (fchdir(cachefd) == -1)
> err(1, "fchdir");
> @@ -1012,6 +1013,7 @@ main(int argc, char *argv[])
> close(proc);
> close(fd[1]);
>
> + setproctitle("rsync");
> /* change working directory to the cache directory */
> if (fchdir(cachefd) == -1)
> err(1, "fchdir");
> @@ -1047,6 +1049,7 @@ main(int argc, char *argv[])
> close(rsync);
> close(fd[1]);
>
> + setproctitle("http");
> /* change working directory to the cache directory */
> if (fchdir(cachefd) == -1)
> err(1, "fchdir");
>
rpki-client use setproctitle(3)
It is time to use setproctitle() in rpki-client.
Right now there are three processes, soon it will be four.
This should help identify the different processes.
I did not change the proc title of the main process to keep the arguments.
--
:wq Claudio
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.126
diff -u -p -r1.126 main.c
--- main.c 29 Mar 2021 03:45:35 - 1.126
+++ main.c 31 Mar 2021 15:53:03 -
@@ -979,6 +979,7 @@ main(int argc, char *argv[])
if (procpid == 0) {
close(fd[1]);
+ setproctitle("parser");
/* change working directory to the cache directory */
if (fchdir(cachefd) == -1)
err(1, "fchdir");
@@ -1012,6 +1013,7 @@ main(int argc, char *argv[])
close(proc);
close(fd[1]);
+ setproctitle("rsync");
/* change working directory to the cache directory */
if (fchdir(cachefd) == -1)
err(1, "fchdir");
@@ -1047,6 +1049,7 @@ main(int argc, char *argv[])
close(rsync);
close(fd[1]);
+ setproctitle("http");
/* change working directory to the cache directory */
if (fchdir(cachefd) == -1)
err(1, "fchdir");
Re: athn(4): switch Tx rate control to RA
On 23 Mar 18:01, Stefan Sperling wrote: > This switches athn(4) to the new RA Tx rate adaptation module. > Tests on athn(4) PCI devices are welcome. > USB devices don't need to be tested in this case Tx rate adaptation > is taken care of by firmware. > > I could only test on AR9285 so far, but the result looks promising. > Hi Stefan, I tested between my laptop with iwm: iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x78, msi iwm0: hw rev 0x230, fw ver 34.0.1, address xx:xx:xx:xx:xx:xx:xx and my APU1 with: athn0 at pci4 dev 0 function 0 "Atheros AR5418" rev 0x01: apic 2 int 19 athn0: MAC AR5418 rev 2, RF AR5133 (2T3R), ROM rev 8, address xx:xx:xx:xx:xx:xx:xx With patch laptop -> apu: Conn: 1 Mbps: 19.021 Peak Mbps: 19.021 Avg Mbps: 19.021 30042287840 18.284 100.00% With patch apu -> laptop: Conn: 1 Mbps: 27.987 Peak Mbps: 30.027 Avg Mbps: 27.987 50033528776 28.202 100.00% Without patch laptop -> apu: Conn: 1 Mbps:7.900 Peak Mbps: 13.750 Avg Mbps:7.900 7015 9281687.388 100.00% Without patch apu -> laptop: Conn: 1 Mbps:4.247 Peak Mbps:7.231 Avg Mbps:4.247 6055 4898043.868 100.00% So notable performance improvements! Thank you very much for your hard work! -- wq: ~uw
Re: rpki-client: better cleanup code
On Wed, Mar 31, 2021 at 01:41:36PM +0200, Claudio Jeker wrote:
> On Wed, Mar 31, 2021 at 12:40:57PM +0200, Claudio Jeker wrote:
> > The current code to cleanup the repository after validation did not
> > cleanup directories and also skipped any repo directory that is no
> > referenced.
> >
> > Adjust the cleanup code to fix these two issues. This uses the fact that
> > the cache directory now only contains 2 directories rsync & ta.
> > Thanks to the internal order of the RB tree, the code can use RB_NFIND()
> > to figure out if a path is used or not.
> >
> > With this the cleanup works but is also more aggressiv.
> > For example if you run
> > rpki-client -t /etc/rpki/afrinic.tal
> > then this will clean out all other repositories from arin, apnic, lacnic
> > and ripe. For regular use this is no isssue since you normally run
> > rpki-client over and over again with the same flags.
> >
>
> This version also skips the cleanup run if rpki-client was started with -n
There is one empty directory that remained undetected. The problem was
that for the code rsync/foo.bar.baz/repo/foo is a subpath for
rsync/foo.bar.baz/repo/foofoo/abc
The diff below fixes this issue.
--
:wq Claudio
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.59
diff -u -p -r1.59 extern.h
--- extern.h29 Mar 2021 12:41:34 - 1.59
+++ extern.h31 Mar 2021 08:38:33 -
@@ -306,6 +306,7 @@ struct stats {
size_t vrps; /* total number of vrps */
size_t uniqs; /* number of unique vrps */
size_t del_files; /* number of files removed in cleanup */
+ size_t del_dirs ;/* number of directories removed in cleanup */
char*talnames;
struct timeval elapsed_time;
struct timeval user_time;
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.126
diff -u -p -r1.126 main.c
--- main.c 29 Mar 2021 03:45:35 - 1.126
+++ main.c 31 Mar 2021 12:58:24 -
@@ -154,6 +154,26 @@ filepath_exists(char *file)
return RB_FIND(filepath_tree, , ) != NULL;
}
+/*
+ * Return true if a filepath entry exists that starts with path.
+ */
+static int
+filepath_dir_exists(char *path)
+{
+ struct filepath needle;
+ struct filepath *res;
+
+ needle.file = path;
+ res = RB_NFIND(filepath_tree, , );
+ while (res != NULL && strstr(res->file, path) == res->file) {
+ /* make sure that filepath acctually is in that path */
+ if (res->file[strlen(path)] == '/')
+ return 1;
+ res = RB_NEXT(filepath_tree, , res);
+ }
+ return 0;
+}
+
RB_GENERATE(filepath_tree, filepath, entry, filepathcmp);
void
@@ -767,56 +787,56 @@ add_to_del(char **del, size_t *dsz, char
return del;
}
-static size_t
+static void
repo_cleanup(void)
{
- size_t i, delsz = 0;
- char *argv[2], **del = NULL;
- struct repo *rp;
+ size_t i, delsz = 0, dirsz = 0;
+ char *argv[3], **del = NULL, **dir = NULL;
FTS *fts;
FTSENT *e;
- SLIST_FOREACH(rp, , entry) {
- argv[0] = rp->local;
- argv[1] = NULL;
- if ((fts = fts_open(argv, FTS_PHYSICAL | FTS_NOSTAT,
- NULL)) == NULL)
- err(1, "fts_open");
- errno = 0;
- while ((e = fts_read(fts)) != NULL) {
- switch (e->fts_info) {
- case FTS_NSOK:
- if (!filepath_exists(e->fts_path))
- del = add_to_del(del, ,
- e->fts_path);
- break;
- case FTS_D:
- case FTS_DP:
- /* TODO empty directory pruning */
- break;
- case FTS_SL:
- case FTS_SLNONE:
- warnx("symlink %s", e->fts_path);
- del = add_to_del(del, , e->fts_path);
- break;
- case FTS_NS:
- case FTS_ERR:
- warnx("fts_read %s: %s", e->fts_path,
- strerror(e->fts_errno));
- break;
- default:
- warnx("unhandled[%x] %s", e->fts_info,
+ argv[0] = "ta";
+ argv[1] = "rsync";
+ argv[2] = NULL;
+ if ((fts = fts_open(argv, FTS_PHYSICAL | FTS_NOSTAT, NULL)) == NULL)
+ err(1, "fts_open");
+ errno = 0;
+ while ((e = fts_read(fts)) != NULL) {
+ switch (e->fts_info) {
+
Re: cwfg: Use meaningful alert level, track apm's battery state better
On Wed, Mar 31, 2021 at 01:35:12PM +0200, Patrick Wildt wrote:
> If there's no alert-level property, then maybe we should just remove it.
> Then you could hard code a value for "below will be critical", like you
> now do with the 50%?
This makes sense, I did not remove it when writing the diff due to my
uncertainty about the design, intention and usage of properties (which
are not part of linux upstream's bindings) -- more reading and help from
patrick have changed that.
No longer look for the nonexistent "alert-level" property and hardcode
thresholds the same way acpipat(4) does.
Also don't bother with this threshold in the CONFIG register: our driver
and overall sensor framework is designed for polling values every N
seconds, so there is no need for the hardware to tell us whether the
threshold has been reached -- cwfg closely watches SOC readings and does
its own threshold metrics (APM_BATT_HIGH/LOW/CRITICAL).
NetBSD (from which this driver was ported) also handles "alert-level",
but they also do the same sensors polling we so, so I don't see why that
is done in the first place.
Linux does not look for "alert-level" property but still updates the
register -- although I fail to see any code that updates their driver's
`alert_level' value with anything, the seem to simply update the CONFIG
register with the zero initialised value. Their capacity calculations
are then also based on SOC readings like ours and do not use the
hardware threshold support at all.
Feedback? Objections? OK?
Index: cwfg.c
===
RCS file: /cvs/src/sys/dev/fdt/cwfg.c,v
retrieving revision 1.4
diff -u -p -r1.4 cwfg.c
--- cwfg.c 26 Mar 2021 22:54:41 - 1.4
+++ cwfg.c 31 Mar 2021 12:09:59 -
@@ -58,8 +58,6 @@
#define RTT_LO_MASK0xff
#define RTT_LO_SHIFT 0
#defineCONFIG_REG 0x08
-#define CONFIG_ATHD_MASK 0x1f
-#define CONFIG_ATHD_SHIFT 3
#define CONFIG_UFG (1 << 1)
#defineMODE_REG0x0a
#define MODE_SLEEP_MASK(0x3 << 6)
@@ -91,7 +89,6 @@ struct cwfg_softc {
uint8_t sc_batinfo[BATINFO_SIZE];
- uint32_tsc_alert_level;
uint32_tsc_monitor_interval;
uint32_tsc_design_capacity;
@@ -101,7 +98,6 @@ struct cwfg_softc {
#defineCWFG_MONITOR_INTERVAL_DEFAULT 5000
#defineCWFG_DESIGN_CAPACITY_DEFAULT2000
-#defineCWFG_ALERT_LEVEL_DEFAULT0
int cwfg_match(struct device *, void *, void *);
void cwfg_attach(struct device *, struct device *, void *);
@@ -189,8 +185,6 @@ cwfg_attach(struct device *parent, struc
"cellwise,monitor-interval-ms", CWFG_MONITOR_INTERVAL_DEFAULT);
sc->sc_design_capacity = OF_getpropint(sc->sc_node,
"cellwise,design-capacity", CWFG_DESIGN_CAPACITY_DEFAULT);
- sc->sc_alert_level = OF_getpropint(sc->sc_node,
- "cellwise,alert-level", CWFG_ALERT_LEVEL_DEFAULT);
if (cwfg_init(sc) != 0) {
printf(": failed to initialize device\n");
@@ -271,7 +265,6 @@ done:
int
cwfg_set_config(struct cwfg_softc *sc)
{
- uint32_t alert_level;
uint8_t config, mode, val;
int need_update;
int error, n;
@@ -280,19 +273,6 @@ cwfg_set_config(struct cwfg_softc *sc)
if ((error = cwfg_read(sc, CONFIG_REG, )) != 0)
return error;
- /* Update alert level, if necessary */
- alert_level = (config >> CONFIG_ATHD_SHIFT) & CONFIG_ATHD_MASK;
- if (alert_level != sc->sc_alert_level) {
- config &= ~(CONFIG_ATHD_MASK << CONFIG_ATHD_SHIFT);
- config |= (sc->sc_alert_level << CONFIG_ATHD_SHIFT);
- if ((error = cwfg_write(sc, CONFIG_REG, config)) != 0)
- return error;
- }
-
- /* Re-read current config */
- if ((error = cwfg_read(sc, CONFIG_REG, )) != 0)
- return error;
-
/*
* We need to upload a battery profile if either the UFG flag
* is unset, or the current battery profile differs from the
@@ -387,9 +367,13 @@ cwfg_update_sensors(void *arg)
sc->sc_sensor[CWFG_SENSOR_SOC].value = val * 1000;
sc->sc_sensor[CWFG_SENSOR_SOC].flags &= ~SENSOR_FINVALID;
#if NAPM > 0
- cwfg_power.battery_state = val > sc->sc_alert_level ?
- APM_BATT_HIGH : APM_BATT_LOW;
cwfg_power.battery_life = val;
+ if (val > 50)
+ cwfg_power.battery_state = APM_BATT_HIGH;
+ else if (val > 25)
+ cwfg_power.battery_state = APM_BATT_LOW;
+ else
+ cwfg_power.battery_state = APM_BATT_CRITICAL;
#endif
}
Re: rpki-client: better cleanup code
On Wed, Mar 31, 2021 at 12:40:57PM +0200, Claudio Jeker wrote:
> The current code to cleanup the repository after validation did not
> cleanup directories and also skipped any repo directory that is no
> referenced.
>
> Adjust the cleanup code to fix these two issues. This uses the fact that
> the cache directory now only contains 2 directories rsync & ta.
> Thanks to the internal order of the RB tree, the code can use RB_NFIND()
> to figure out if a path is used or not.
>
> With this the cleanup works but is also more aggressiv.
> For example if you run
> rpki-client -t /etc/rpki/afrinic.tal
> then this will clean out all other repositories from arin, apnic, lacnic
> and ripe. For regular use this is no isssue since you normally run
> rpki-client over and over again with the same flags.
>
This version also skips the cleanup run if rpki-client was started with -n
--
:wq Claudio
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.59
diff -u -p -r1.59 extern.h
--- extern.h29 Mar 2021 12:41:34 - 1.59
+++ extern.h31 Mar 2021 08:38:33 -
@@ -306,6 +306,7 @@ struct stats {
size_t vrps; /* total number of vrps */
size_t uniqs; /* number of unique vrps */
size_t del_files; /* number of files removed in cleanup */
+ size_t del_dirs ;/* number of directories removed in cleanup */
char*talnames;
struct timeval elapsed_time;
struct timeval user_time;
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.126
diff -u -p -r1.126 main.c
--- main.c 29 Mar 2021 03:45:35 - 1.126
+++ main.c 31 Mar 2021 11:39:56 -
@@ -154,6 +154,22 @@ filepath_exists(char *file)
return RB_FIND(filepath_tree, , ) != NULL;
}
+/*
+ * Return true if a filepath entry exists that starts with path.
+ */
+static int
+filepath_dir_exists(char *path)
+{
+ struct filepath needle;
+ struct filepath *res;
+
+ needle.file = path;
+ res = RB_NFIND(filepath_tree, , );
+ if (res != NULL && strstr(res->file, path) == res->file)
+ return 1;
+ return 0;
+}
+
RB_GENERATE(filepath_tree, filepath, entry, filepathcmp);
void
@@ -767,56 +783,56 @@ add_to_del(char **del, size_t *dsz, char
return del;
}
-static size_t
+static void
repo_cleanup(void)
{
- size_t i, delsz = 0;
- char *argv[2], **del = NULL;
- struct repo *rp;
+ size_t i, delsz = 0, dirsz = 0;
+ char *argv[3], **del = NULL, **dir = NULL;
FTS *fts;
FTSENT *e;
- SLIST_FOREACH(rp, , entry) {
- argv[0] = rp->local;
- argv[1] = NULL;
- if ((fts = fts_open(argv, FTS_PHYSICAL | FTS_NOSTAT,
- NULL)) == NULL)
- err(1, "fts_open");
- errno = 0;
- while ((e = fts_read(fts)) != NULL) {
- switch (e->fts_info) {
- case FTS_NSOK:
- if (!filepath_exists(e->fts_path))
- del = add_to_del(del, ,
- e->fts_path);
- break;
- case FTS_D:
- case FTS_DP:
- /* TODO empty directory pruning */
- break;
- case FTS_SL:
- case FTS_SLNONE:
- warnx("symlink %s", e->fts_path);
- del = add_to_del(del, , e->fts_path);
- break;
- case FTS_NS:
- case FTS_ERR:
- warnx("fts_read %s: %s", e->fts_path,
- strerror(e->fts_errno));
- break;
- default:
- warnx("unhandled[%x] %s", e->fts_info,
+ argv[0] = "ta";
+ argv[1] = "rsync";
+ argv[2] = NULL;
+ if ((fts = fts_open(argv, FTS_PHYSICAL | FTS_NOSTAT, NULL)) == NULL)
+ err(1, "fts_open");
+ errno = 0;
+ while ((e = fts_read(fts)) != NULL) {
+ switch (e->fts_info) {
+ case FTS_NSOK:
+ if (!filepath_exists(e->fts_path))
+ del = add_to_del(del, ,
e->fts_path);
- break;
- }
-
- errno = 0;
+ break;
+ case FTS_D:
+ break;
+ case FTS_DP:
+ if (!filepath_dir_exists(e->fts_path))
+ dir =
Re: cwfg: Use meaningful alert level, track apm's battery state better
Am Mon, Mar 29, 2021 at 07:16:18AM +0200 schrieb Klemens Nanni:
> The datasheet says the hardware's default State-Of-Charge threshold is
> three percent, i.e. the gauge pulls down the pin to logic low at 3%
> remaining battery life.
>
> My Pinebook Pro's fuel gauge actually shows an alert level of zero
> percent however and the latest device tree (both from our dtb package
> and other sources) no longer provide the "cellwise,alert-level"
> property.
If there's no alert-level property, then maybe we should just remove it.
Then you could hard code a value for "below will be critical", like you
now do with the 50%?
> The current code still looks for that property but falls back to the
> define; crank it such that apm(8) does not always report "high" battery
> state.
>
> While here, use all three available states in the same way acpibat(4)
> sys/dev/acpi/acpi.c does.
>
> Feedback? OK?
>
> Index: cwfg.c
> ===
> RCS file: /cvs/src/sys/dev/fdt/cwfg.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 cwfg.c
> --- cwfg.c26 Mar 2021 22:54:41 - 1.4
> +++ cwfg.c29 Mar 2021 05:03:58 -
> @@ -101,7 +101,7 @@ struct cwfg_softc {
>
> #define CWFG_MONITOR_INTERVAL_DEFAULT 5000
> #define CWFG_DESIGN_CAPACITY_DEFAULT2000
> -#define CWFG_ALERT_LEVEL_DEFAULT0
> +#define CWFG_ALERT_LEVEL_DEFAULT25
>
> int cwfg_match(struct device *, void *, void *);
> void cwfg_attach(struct device *, struct device *, void *);
> @@ -387,9 +387,13 @@ cwfg_update_sensors(void *arg)
> sc->sc_sensor[CWFG_SENSOR_SOC].value = val * 1000;
> sc->sc_sensor[CWFG_SENSOR_SOC].flags &= ~SENSOR_FINVALID;
> #if NAPM > 0
> - cwfg_power.battery_state = val > sc->sc_alert_level ?
> - APM_BATT_HIGH : APM_BATT_LOW;
> cwfg_power.battery_life = val;
> + if (val > 50)
> + cwfg_power.battery_state = APM_BATT_HIGH;
> + else if (val > sc->sc_alert_level)
> + cwfg_power.battery_state = APM_BATT_LOW;
> + else
> + cwfg_power.battery_state = APM_BATT_CRITICAL;
> #endif
> }
>
>
Re: athn(4): switch Tx rate control to RA
On Wed, Mar 31, 2021 at 11:24:19AM +, Mikolaj Kucharski wrote: > On Tue, Mar 23, 2021 at 07:47:08PM +, Mikolaj Kucharski wrote: > > On Tue, Mar 23, 2021 at 07:06:33PM +, Mikolaj Kucharski wrote: > > > On Tue, Mar 23, 2021 at 06:01:27PM +0100, Stefan Sperling wrote: > > > > This switches athn(4) to the new RA Tx rate adaptation module. > > > > Tests on athn(4) PCI devices are welcome. > > > > USB devices don't need to be tested in this case Tx rate adaptation > > > > is taken care of by firmware. > > > > > > > > I could only test on AR9285 so far, but the result looks promising. > > > > > > > > diff c6a6a64b49f2287751632205d64f594eb6c1ee42 > > > > 636e9964c6e5313bb1c75823249d483597a0e93a > > > ... > > > > > > > > > Tested on athn(4) in `mediaopt hostap` mode. Uptime 1hr, so not a lot of > > > testing yet. So far no issues. I have two systems like that (in hostap) > > > and in `dmesg | grep athn` only difference is mac address between them. > > > I can send full dmesg from second system as well if you want. > > > > > > > I also have third system, with the same athn(4) card (only mac address > > is different in `dmesg | grep athn` output), but it acts as a Wi-Fi > > client and is connected to OpenBSD athn(4)-based access point from > > my previous email (full dmesg output in an earlier email). > > > > After week of running this I have similar observations like Scott > Bennett. > > I will focus on one location, as I have 2 access points running on > athn(4) with the diff from this thread, but I'm only present at one of > those locations. All athn(4) machines have the diff applied. > > OpenBSD, athn(4), hostap > OpenBSD, athn(4), wi-fi client to above access point > OpenBSD, iwm(4), wi-fi client to above access point > > I do see some packets dropped with RA patch: > > # mtr -rwb -c 1000 192.168.220.76 > Start: 2021-03-31T08:17:57+ > HOST: pce-0041.home.lan Loss% Snt Last Avg Best Wrst StDev > 1.|-- 192.168.220.76 9.2% 10002.2 2.6 1.2 82.9 3.4 > > Above is from hostap machine to athn(4) client. Below is the other way > around. Client to hostap. Measurments with mtr on both ends were not > executed at the same time, but one after the other, with couple of > mintues gap. > > # mtr -rwb -c 1000 192.168.220.1 > Start: 2021-03-31T10:38:07+ > HOST: pce-0067.home.local Loss% Snt Last Avg Best Wrst StDev > 1.|-- 192.168.220.1 10.5% 10003.1 2.5 1.5 43.7 2.2 > > The loss is big, but I didn't notice this too much over short > interactive ssh sessions. However I do notice problem havily when I'm on > a laptop with iwm(4) ssh'ing to athn(4) client. Then ssh stalls are > sigificant that I need to wait until TCP recovers and I can type again. > I'm often using scp(1) between athn(4) client and iwm(4) laptop and that > amplifies the problem during simultaneous interactive ssh session. > SSH stalls are more prominient, longer. > > I need to say it's still better than I think without this RA diff, as > communitcating with athn(4) client from iwm(4) laptop before was worse > as that very often triggered those famous athn device timeouts and > recovering from them took way longer than from the packet loss and RA. > Packet loss revovery with RA is in tens of seconds, recovery from athn > device timeout was in minutes, because usually timeouts occured one > after another, like 3 or 4 in a row. With RA I don't see this happening > any more. > > So, all in all I prefer RA, but I do see packet losses. > I did also from athn client to athn hostap: # ping -g -c1000 192.168.220.1 PING 192.168.220.1 (192.168.220.1): 56 data bytes .!.!.!.!!.!!.!!!.!.!! ..!.. ..!!! .!.!.!.!!.!.! !!.!.!....!!! ..!.! .!.!.!!..!.!! !.!.! !!!.!.!!!.!!! .!... !!!.!.!.!.!!! !.!.!!!.!.!.!!!.!.!!! !!!.!.!.!..!.!.!! . !!.!!!.!. --- 192.168.220.1 ping statistics --- 1000 packets transmitted, 925 packets received, 7.5% packet loss round-trip min/avg/max/std-dev = 1.026/2.818/34.711/1.993 ms Maybe that would help to visualise dropped packets. Both machines are on: # dmesg | grep athn athn0 at pci4 dev 0 function 0 "Atheros AR9281" rev 0x01: apic 5
Re: athn(4): switch Tx rate control to RA
On Tue, Mar 23, 2021 at 07:47:08PM +, Mikolaj Kucharski wrote: > On Tue, Mar 23, 2021 at 07:06:33PM +, Mikolaj Kucharski wrote: > > On Tue, Mar 23, 2021 at 06:01:27PM +0100, Stefan Sperling wrote: > > > This switches athn(4) to the new RA Tx rate adaptation module. > > > Tests on athn(4) PCI devices are welcome. > > > USB devices don't need to be tested in this case Tx rate adaptation > > > is taken care of by firmware. > > > > > > I could only test on AR9285 so far, but the result looks promising. > > > > > > diff c6a6a64b49f2287751632205d64f594eb6c1ee42 > > > 636e9964c6e5313bb1c75823249d483597a0e93a > > ... > > > > > > Tested on athn(4) in `mediaopt hostap` mode. Uptime 1hr, so not a lot of > > testing yet. So far no issues. I have two systems like that (in hostap) > > and in `dmesg | grep athn` only difference is mac address between them. > > I can send full dmesg from second system as well if you want. > > > > I also have third system, with the same athn(4) card (only mac address > is different in `dmesg | grep athn` output), but it acts as a Wi-Fi > client and is connected to OpenBSD athn(4)-based access point from > my previous email (full dmesg output in an earlier email). > After week of running this I have similar observations like Scott Bennett. I will focus on one location, as I have 2 access points running on athn(4) with the diff from this thread, but I'm only present at one of those locations. All athn(4) machines have the diff applied. OpenBSD, athn(4), hostap OpenBSD, athn(4), wi-fi client to above access point OpenBSD, iwm(4), wi-fi client to above access point I do see some packets dropped with RA patch: # mtr -rwb -c 1000 192.168.220.76 Start: 2021-03-31T08:17:57+ HOST: pce-0041.home.lan Loss% Snt Last Avg Best Wrst StDev 1.|-- 192.168.220.76 9.2% 10002.2 2.6 1.2 82.9 3.4 Above is from hostap machine to athn(4) client. Below is the other way around. Client to hostap. Measurments with mtr on both ends were not executed at the same time, but one after the other, with couple of mintues gap. # mtr -rwb -c 1000 192.168.220.1 Start: 2021-03-31T10:38:07+ HOST: pce-0067.home.local Loss% Snt Last Avg Best Wrst StDev 1.|-- 192.168.220.1 10.5% 10003.1 2.5 1.5 43.7 2.2 The loss is big, but I didn't notice this too much over short interactive ssh sessions. However I do notice problem havily when I'm on a laptop with iwm(4) ssh'ing to athn(4) client. Then ssh stalls are sigificant that I need to wait until TCP recovers and I can type again. I'm often using scp(1) between athn(4) client and iwm(4) laptop and that amplifies the problem during simultaneous interactive ssh session. SSH stalls are more prominient, longer. I need to say it's still better than I think without this RA diff, as communitcating with athn(4) client from iwm(4) laptop before was worse as that very often triggered those famous athn device timeouts and recovering from them took way longer than from the packet loss and RA. Packet loss revovery with RA is in tens of seconds, recovery from athn device timeout was in minutes, because usually timeouts occured one after another, like 3 or 4 in a row. With RA I don't see this happening any more. So, all in all I prefer RA, but I do see packet losses. -- Regards, Mikolaj
rpki-client move encoding functions into own file
As mentioned before move the base64 and hex encoding / decoding functions
into one file. This is just minor cleanup.
--
:wq Claudio
PS: I know this will break regress and I will fix that once this goes in.
Index: Makefile
===
RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
retrieving revision 1.19
diff -u -p -r1.19 Makefile
--- Makefile4 Mar 2021 13:01:41 - 1.19
+++ Makefile31 Mar 2021 10:56:42 -
@@ -1,9 +1,10 @@
# $OpenBSD: Makefile,v 1.19 2021/03/04 13:01:41 claudio Exp $
PROG= rpki-client
-SRCS= as.c cert.c cms.c crl.c gbr.c http.c io.c ip.c log.c main.c mft.c \
- mkdir.c output.c output-bgpd.c output-bird.c output-csv.c \
- output-json.c parser.c roa.c rsync.c tal.c validate.c x509.c
+SRCS= as.c cert.c cms.c crl.c encoding.c gbr.c http.c io.c ip.c log.c \
+ main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
+ output-csv.c output-json.c parser.c roa.c rsync.c tal.c validate.c \
+ x509.c
MAN= rpki-client.8
LDADD+= -ltls -lssl -lcrypto -lutil
Index: encoding.c
===
RCS file: encoding.c
diff -N encoding.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ encoding.c 31 Mar 2021 11:00:49 -
@@ -0,0 +1,88 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2020 Claudio Jeker
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+#include
+#include
+#include
+#include
+
+#include
+
+#include "extern.h"
+
+/*
+ * Decode base64 encoded string into binary buffer returned in out.
+ * The out buffer size is stored in outlen.
+ * Returns 0 on success or -1 for any errors.
+ */
+int
+base64_decode(const unsigned char *in, unsigned char **out, size_t *outlen)
+{
+ static EVP_ENCODE_CTX *ctx;
+ unsigned char *to;
+ size_t inlen;
+ int tolen;
+
+ if (ctx == NULL && (ctx = EVP_ENCODE_CTX_new()) == NULL)
+ err(1, "EVP_ENCODE_CTX_new");
+
+ *out = NULL;
+ *outlen = 0;
+
+ inlen = strlen(in);
+ if (inlen >= INT_MAX - 3)
+ return -1;
+ tolen = ((inlen + 3) / 4) * 3 + 1;
+ if ((to = malloc(tolen)) == NULL)
+ return -1;
+
+ EVP_DecodeInit(ctx);
+ if (EVP_DecodeUpdate(ctx, to, , in, inlen) == -1)
+ goto fail;
+ *outlen = tolen;
+ if (EVP_DecodeFinal(ctx, to + tolen, ) == -1)
+ goto fail;
+ *outlen += tolen;
+ *out = to;
+ return 0;
+
+fail:
+ free(to);
+ return -1;
+}
+
+/*
+ * Convert binary buffer of size dsz into an upper-case hex-string.
+ * Returns pointer to the newly allocated string. Function can't fail.
+ */
+char *
+hex_encode(const unsigned char *in, size_t insz)
+{
+ const char hex[] = "0123456789ABCDEF";
+ size_t i;
+ char *out;
+
+ if ((out = calloc(2, insz + 1)) == NULL)
+ err(1, NULL);
+
+ for (i = 0; i < insz; i++) {
+ out[i * 2] = hex[in[i] >> 4];
+ out[i * 2 + 1] = hex[in[i] & 0xf];
+ }
+ out[i * 2] = '\0';
+
+ return out;
+}
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.59
diff -u -p -r1.59 extern.h
--- extern.h29 Mar 2021 12:41:34 - 1.59
+++ extern.h31 Mar 2021 10:55:49 -
@@ -419,6 +419,13 @@ voidcryptoerrx(const char *, ...)
__attribute__((format(printf, 1, 2)))
__attribute__((noreturn));
+/* Encoding functions for hex and base64. */
+
+int base64_decode(const unsigned char *, unsigned char **,
+ size_t *);
+char *hex_encode(const unsigned char *, size_t);
+
+
/* Functions for moving data between processes. */
voidio_socket_blocking(int);
Index: tal.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/tal.c,v
retrieving revision 1.29
diff -u -p -r1.29 tal.c
--- tal.c 25 Mar 2021 09:27:38 - 1.29
+++ tal.c 31 Mar 2021 11:10:27 -
@@ -19,49 +19,12 @@
#include
#include
#include
-#include
#include
#include
rpki-client: better cleanup code
The current code to cleanup the repository after validation did not
cleanup directories and also skipped any repo directory that is no
referenced.
Adjust the cleanup code to fix these two issues. This uses the fact that
the cache directory now only contains 2 directories rsync & ta.
Thanks to the internal order of the RB tree, the code can use RB_NFIND()
to figure out if a path is used or not.
With this the cleanup works but is also more aggressiv.
For example if you run
rpki-client -t /etc/rpki/afrinic.tal
then this will clean out all other repositories from arin, apnic, lacnic
and ripe. For regular use this is no isssue since you normally run
rpki-client over and over again with the same flags.
--
:wq Claudio
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.59
diff -u -p -r1.59 extern.h
--- extern.h29 Mar 2021 12:41:34 - 1.59
+++ extern.h31 Mar 2021 08:38:33 -
@@ -306,6 +306,7 @@ struct stats {
size_t vrps; /* total number of vrps */
size_t uniqs; /* number of unique vrps */
size_t del_files; /* number of files removed in cleanup */
+ size_t del_dirs ;/* number of directories removed in cleanup */
char*talnames;
struct timeval elapsed_time;
struct timeval user_time;
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.126
diff -u -p -r1.126 main.c
--- main.c 29 Mar 2021 03:45:35 - 1.126
+++ main.c 31 Mar 2021 10:12:40 -
@@ -154,6 +154,22 @@ filepath_exists(char *file)
return RB_FIND(filepath_tree, , ) != NULL;
}
+/*
+ * Return true if a filepath entry exists that starts with path.
+ */
+static int
+filepath_dir_exists(char *path)
+{
+ struct filepath needle;
+ struct filepath *res;
+
+ needle.file = path;
+ res = RB_NFIND(filepath_tree, , );
+ if (res != NULL && strstr(res->file, path) == res->file)
+ return 1;
+ return 0;
+}
+
RB_GENERATE(filepath_tree, filepath, entry, filepathcmp);
void
@@ -767,56 +783,56 @@ add_to_del(char **del, size_t *dsz, char
return del;
}
-static size_t
+static void
repo_cleanup(void)
{
- size_t i, delsz = 0;
- char *argv[2], **del = NULL;
- struct repo *rp;
+ size_t i, delsz = 0, dirsz = 0;
+ char *argv[3], **del = NULL, **dir = NULL;
FTS *fts;
FTSENT *e;
- SLIST_FOREACH(rp, , entry) {
- argv[0] = rp->local;
- argv[1] = NULL;
- if ((fts = fts_open(argv, FTS_PHYSICAL | FTS_NOSTAT,
- NULL)) == NULL)
- err(1, "fts_open");
- errno = 0;
- while ((e = fts_read(fts)) != NULL) {
- switch (e->fts_info) {
- case FTS_NSOK:
- if (!filepath_exists(e->fts_path))
- del = add_to_del(del, ,
- e->fts_path);
- break;
- case FTS_D:
- case FTS_DP:
- /* TODO empty directory pruning */
- break;
- case FTS_SL:
- case FTS_SLNONE:
- warnx("symlink %s", e->fts_path);
- del = add_to_del(del, , e->fts_path);
- break;
- case FTS_NS:
- case FTS_ERR:
- warnx("fts_read %s: %s", e->fts_path,
- strerror(e->fts_errno));
- break;
- default:
- warnx("unhandled[%x] %s", e->fts_info,
+ argv[0] = "ta";
+ argv[1] = "rsync";
+ argv[2] = NULL;
+ if ((fts = fts_open(argv, FTS_PHYSICAL | FTS_NOSTAT, NULL)) == NULL)
+ err(1, "fts_open");
+ errno = 0;
+ while ((e = fts_read(fts)) != NULL) {
+ switch (e->fts_info) {
+ case FTS_NSOK:
+ if (!filepath_exists(e->fts_path))
+ del = add_to_del(del, ,
e->fts_path);
- break;
- }
-
- errno = 0;
+ break;
+ case FTS_D:
+ break;
+ case FTS_DP:
+ if (!filepath_dir_exists(e->fts_path))
+ dir = add_to_del(dir, ,
+ e->fts_path);
+ break;
+ case FTS_SL:
+ case FTS_SLNONE:
+
Re: athn(4): switch Tx rate control to RA
On Tue, Mar 30, 2021 at 11:06:43PM -0400, Scott Bennett wrote: > However, my laptop with AR9287 was noticeably worse with this diff (dropped > pings, stuttering keystrokes in interactive ssh session, estimated 20 > minutes to scp(1) a 20M file...). The combination of apu2 with diff and my > laptop sans diff is giving me good results though :) > > athn0 at pci2 dev 0 function 0 "Atheros AR9287" rev 0x01: apic 2 int 17 > athn0: AR9287 rev 2 (2T2R), ROM rev 4, address 74:de:2b:xx:xx:xx This device (AR9287) has known issues in 11n mode already: https://marc.info/?t=15899228758=1=2 Granted, that is a sample size of one, and unlike the person who filed that bug report you didn't have problems before the RA patch. But perhaps these issues are somehow related? I don't have this hardware, and I cannot tell what's wrong with it. My best guess is that this device doesn't report retries in the same way as the 9280 does, so RA ends up picking a rate that's too high. But that's just a wild guess. Can you please try the following: Set a fixed Tx rate and figure out the loss/throughput characteristics of each, while the laptop stays put at the same place relative to the AP. The supported Tx rates for this device are grouped into two groups: Group1 : MCS 0 (low) to MCS 7 (high) Group2 : MCS 8 (low) to MCS 15 (high) Start with a fixed Tx rate of MCS 0: ifconfig athn0 media HT-MCS0 mode 11n Now do you see dropped pings, stuttering keystrokes, slow transfers? Ideally you'd show ping packet loss at the default size and at ping -s 1500, plus the transfer rate when sending a big file from the laptop towards the AP. Note that scp isn't a good tool for measuring Tx performance; rsync or a benchmarking tool such as tcpbench or iperf are suited better for this. Then go up and repeat the test at each rate: ifconfig athn0 media HT-MCS1 mode 11n ... ifconfig athn0 media HT-MCS7 mode 11n Once you've reached MCS 7, move on the next group: ifconfig athn0 media HT-MCS8 mode 11n ... ifconfig athn0 media HT-MCS15 mode 11n It's a bit tedious, but if you could give me a test result for all the 16 supported MCS we might be able to find a fix. Thanks!
