Re: fix ldapd/ldapctl data directory handling
Committed, thanks.
On 3/4/20 9:11 PM, Robert Klein wrote:
> Hi,
>
> On Tue, 3 Mar 2020 20:45:17 +0100
> Martijn van Duren wrote:
>
>> I don't agree with this diff, even though you're right with it being
>> broken. Right now the regress test uses it/tries to use it and even
>> though someone trying to run regress on an actual production machine
>> somewhat deserves to get shot in the foot, I don't think we should
>> make that our goal.
>>
>> Also, I can see this being a helpful feature for people doing some
>> development work (e.g. testing with delegations etc without setting
>> up a vm-environment).
>>
>> Could you please look into fixing the actual issue?
>
> this patch fixes only the two issues. I de-const'd "char * datadir" in
> ldapd to get rid of the new warning.
>
> Best regards
> Robert
>
>
> diff 1813335e849f285a868ea3d474b4704812b1843e /usr/src
> blob - c8564c5543f518a720e049c559556f87edda6b8a
> file + usr.sbin/ldapctl/ldapctl.c
> --- usr.sbin/ldapctl/ldapctl.c
> +++ usr.sbin/ldapctl/ldapctl.c
> @@ -150,7 +150,7 @@ index_namespace(struct namespace *ns, const char *data
>
> log_info("indexing namespace %s", ns->suffix);
>
> - if (asprintf(, "%s/%s_data.db", DATADIR, ns->suffix) == -1)
> + if (asprintf(, "%s/%s_data.db", datadir, ns->suffix) == -1)
> return -1;
> data_db = btree_open(path, BT_NOSYNC | BT_REVERSEKEY, 0644);
> free(path);
> blob - 797a36f89f33233c2d9655a307ec5e727b9fbaa1
> file + usr.sbin/ldapd/ldapd.c
> --- usr.sbin/ldapd/ldapd.c
> +++ usr.sbin/ldapd/ldapd.c
> @@ -55,7 +55,7 @@ static pid_t start_child(enum ldapd_process, char
> *,
>
> struct ldapd_statsstats;
> pid_t ldape_pid;
> -const char *datadir = DATADIR;
> +char *datadir = DATADIR;
>
> void
> usage(void)
> @@ -415,7 +415,7 @@ static pid_t
> start_child(enum ldapd_process p, char *argv0, int fd, int debug,
> int verbose, char *csockpath, char *conffile)
> {
> - char*argv[9];
> + char*argv[11];
> int argc = 0;
> pid_tpid;
>
> @@ -459,7 +459,11 @@ start_child(enum ldapd_process p, char *argv0, int fd,
> argv[argc++] = "-f";
> argv[argc++] = conffile;
> }
> -
> + if (datadir) {
> + argv[argc++] = "-r";
> + argv[argc++] = datadir;
> + }
> +
> argv[argc++] = NULL;
>
> execvp(argv0, argv);
> blob - 2259f57bde3c2918a8b200747c194e5768ae40f1
> file + usr.sbin/ldapd/namespace.c
> --- usr.sbin/ldapd/namespace.c
> +++ usr.sbin/ldapd/namespace.c
> @@ -29,7 +29,7 @@
> #include "ldapd.h"
> #include "log.h"
>
> -extern const char*datadir;
> +extern char *datadir;
>
> /* Maximum number of requests to queue per namespace during compaction.
> * After this many requests, we return LDAP_BUSY.
>
>>
>> martijn@
>>
>> On 3/2/20 8:00 PM, Robert Klein wrote:
>>> Hi,
>>>
>>> in ldapd and ldapctl the "-r directory" command line argument does
>>> not work:
>>>
>>> ldapd fork/execs itself but the directory command line argument is
>>> not given to the execvp call which then uses the default data
>>> directory.
>>>
>>> ldapctl has a thinko/typo which causes the data_db to be always
>>> opened in the default directory for indexing.
>>>
>>> The patch below removes the command line argument and corresponding
>>> global variable and instead uses a configuration file directive
>>> "datadir". Incidentally removes the global 'datadir' variable used
>>> before.
>>>
>>> Best regards
>>> Robert
>>>
>>>
>>> ---
>>> commit 6a71c90c19b5dc5850305c15696af3f14d26c168 (master)
>>> from: Robert Klein
>>> date: Mon Mar 2 18:50:12 2020 UTC
>>>
>>> fix ldapd/ldapctl data directory handling
>>>
>>> -r directive didn't work as intended in both
>>> ldapd and ldapctl.
>>>
>>> this patch replaces command line argument '-r directory'
>>> with a configuration file directive 'datadir'.
>>>
>>> diff f62b80b397c780e8273b5cc67d544b951c4c9d1f
>>> 35bb29194b2067dab925ec4566dfb82f9bf34d65 blob -
>>> 48cff01b435bca0deebf28f55e6f71675c5752f2 blob +
>>> 231af2ddbbee2da446d2f03c9c48e679e216e993 ---
>>> usr.sbin/ldapctl/ldapctl.8 +++ usr.sbin/ldapctl/ldapctl.8
>>> @@ -24,7 +24,6 @@
>>> .Nm ldapctl
>>> .Op Fl v
>>> .Op Fl f Ar file
>>> -.Op Fl r Ar directory
>>> .Op Fl s Ar socket
>>> .Ar command
>>> .Op Ar argument ...
>>> @@ -42,11 +41,6 @@ Use
>>> .Ar file
>>> as the configuration file, instead of the default
>>> .Pa /etc/ldapd.conf .
>>> -.It Fl r Ar directory
>>> -Store and read database files in
>>> -.Ar directory ,
>>> -instead of the default
>>> -.Pa /var/db/ldap .
>>> .It Fl s Ar socket
>>> Use
>>> .Ar socket
>>> blob - c8564c5543f518a720e049c559556f87edda6b8a
>>> blob + 6830a603d153794390faafbc9aadc8ef0bd985c0
>>> --- usr.sbin/ldapctl/ldapctl.c
>>> +++ usr.sbin/ldapctl/ldapctl.c
>>> @@ -58,10 +58,10 @@ void
Re: fix ldapd/ldapctl data directory handling
This reads fine to me and fixes the regress test location.
Does anyone else want to chime in?
martijn@
On 3/4/20 9:11 PM, Robert Klein wrote:
> Hi,
>
> On Tue, 3 Mar 2020 20:45:17 +0100
> Martijn van Duren wrote:
>
>> I don't agree with this diff, even though you're right with it being
>> broken. Right now the regress test uses it/tries to use it and even
>> though someone trying to run regress on an actual production machine
>> somewhat deserves to get shot in the foot, I don't think we should
>> make that our goal.
>>
>> Also, I can see this being a helpful feature for people doing some
>> development work (e.g. testing with delegations etc without setting
>> up a vm-environment).
>>
>> Could you please look into fixing the actual issue?
>
> this patch fixes only the two issues. I de-const'd "char * datadir" in
> ldapd to get rid of the new warning.
>
> Best regards
> Robert
>
>
> diff 1813335e849f285a868ea3d474b4704812b1843e /usr/src
> blob - c8564c5543f518a720e049c559556f87edda6b8a
> file + usr.sbin/ldapctl/ldapctl.c
> --- usr.sbin/ldapctl/ldapctl.c
> +++ usr.sbin/ldapctl/ldapctl.c
> @@ -150,7 +150,7 @@ index_namespace(struct namespace *ns, const char *data
>
> log_info("indexing namespace %s", ns->suffix);
>
> - if (asprintf(, "%s/%s_data.db", DATADIR, ns->suffix) == -1)
> + if (asprintf(, "%s/%s_data.db", datadir, ns->suffix) == -1)
> return -1;
> data_db = btree_open(path, BT_NOSYNC | BT_REVERSEKEY, 0644);
> free(path);
> blob - 797a36f89f33233c2d9655a307ec5e727b9fbaa1
> file + usr.sbin/ldapd/ldapd.c
> --- usr.sbin/ldapd/ldapd.c
> +++ usr.sbin/ldapd/ldapd.c
> @@ -55,7 +55,7 @@ static pid_t start_child(enum ldapd_process, char
> *,
>
> struct ldapd_statsstats;
> pid_t ldape_pid;
> -const char *datadir = DATADIR;
> +char *datadir = DATADIR;
>
> void
> usage(void)
> @@ -415,7 +415,7 @@ static pid_t
> start_child(enum ldapd_process p, char *argv0, int fd, int debug,
> int verbose, char *csockpath, char *conffile)
> {
> - char*argv[9];
> + char*argv[11];
> int argc = 0;
> pid_tpid;
>
> @@ -459,7 +459,11 @@ start_child(enum ldapd_process p, char *argv0, int fd,
> argv[argc++] = "-f";
> argv[argc++] = conffile;
> }
> -
> + if (datadir) {
> + argv[argc++] = "-r";
> + argv[argc++] = datadir;
> + }
> +
> argv[argc++] = NULL;
>
> execvp(argv0, argv);
> blob - 2259f57bde3c2918a8b200747c194e5768ae40f1
> file + usr.sbin/ldapd/namespace.c
> --- usr.sbin/ldapd/namespace.c
> +++ usr.sbin/ldapd/namespace.c
> @@ -29,7 +29,7 @@
> #include "ldapd.h"
> #include "log.h"
>
> -extern const char*datadir;
> +extern char *datadir;
>
> /* Maximum number of requests to queue per namespace during compaction.
> * After this many requests, we return LDAP_BUSY.
>
>>
>> martijn@
>>
>> On 3/2/20 8:00 PM, Robert Klein wrote:
>>> Hi,
>>>
>>> in ldapd and ldapctl the "-r directory" command line argument does
>>> not work:
>>>
>>> ldapd fork/execs itself but the directory command line argument is
>>> not given to the execvp call which then uses the default data
>>> directory.
>>>
>>> ldapctl has a thinko/typo which causes the data_db to be always
>>> opened in the default directory for indexing.
>>>
>>> The patch below removes the command line argument and corresponding
>>> global variable and instead uses a configuration file directive
>>> "datadir". Incidentally removes the global 'datadir' variable used
>>> before.
>>>
>>> Best regards
>>> Robert
>>>
>>>
>>> ---
>>> commit 6a71c90c19b5dc5850305c15696af3f14d26c168 (master)
>>> from: Robert Klein
>>> date: Mon Mar 2 18:50:12 2020 UTC
>>>
>>> fix ldapd/ldapctl data directory handling
>>>
>>> -r directive didn't work as intended in both
>>> ldapd and ldapctl.
>>>
>>> this patch replaces command line argument '-r directory'
>>> with a configuration file directive 'datadir'.
>>>
>>> diff f62b80b397c780e8273b5cc67d544b951c4c9d1f
>>> 35bb29194b2067dab925ec4566dfb82f9bf34d65 blob -
>>> 48cff01b435bca0deebf28f55e6f71675c5752f2 blob +
>>> 231af2ddbbee2da446d2f03c9c48e679e216e993 ---
>>> usr.sbin/ldapctl/ldapctl.8 +++ usr.sbin/ldapctl/ldapctl.8
>>> @@ -24,7 +24,6 @@
>>> .Nm ldapctl
>>> .Op Fl v
>>> .Op Fl f Ar file
>>> -.Op Fl r Ar directory
>>> .Op Fl s Ar socket
>>> .Ar command
>>> .Op Ar argument ...
>>> @@ -42,11 +41,6 @@ Use
>>> .Ar file
>>> as the configuration file, instead of the default
>>> .Pa /etc/ldapd.conf .
>>> -.It Fl r Ar directory
>>> -Store and read database files in
>>> -.Ar directory ,
>>> -instead of the default
>>> -.Pa /var/db/ldap .
>>> .It Fl s Ar socket
>>> Use
>>> .Ar socket
>>> blob - c8564c5543f518a720e049c559556f87edda6b8a
>>> blob + 6830a603d153794390faafbc9aadc8ef0bd985c0
>>> ---
newlines and #comments in httpd parse.y
An accidentally unterminated string in httpd.conf results in
the famed yacc "syntax error" message.
For instance:
root "/
}
}
# "hi" <- line xx: syntax error
The loop starting at line 1515 in httpd parse.y
}
switch (c) {
case '\'':
case '"':
quotec = c;
will uncomplainingly eat -anything- up to the matching quote
character even in a comment line.
The resulting error message usually a very long way from
the actual mistake.
Would a change to terminate the string with an error at the
next newline be appropriate? Should quoted newlines be allowed?
Should # comments be allowed and elided, included verbatim,
or cause an error?
I can provide a patch for any of those cases.
geoff steckel
syslog regress and libressl
I noticed that some regress test fail since February 7:
- run-args-server-tls-reconnect.pl
- run-args-server-tls-tcp.pl
- run-args-tls-cipher-null.pl
(http://bluhm.genua.de/regress/results/regress-ot6.html)
It is related to changes in LibreSSL. Is this intended? Should the regress
tests be adapted?
Below diff makes two of the tests succeed.
Remi
Index: args-server-tls-tcp.pl
===
RCS file: /cvs/src/regress/usr.sbin/syslogd/args-server-tls-tcp.pl,v
retrieving revision 1.10
diff -u -p -r1.10 args-server-tls-tcp.pl
--- args-server-tls-tcp.pl 22 May 2018 15:01:16 - 1.10
+++ args-server-tls-tcp.pl 2 Mar 2020 21:30:01 -
@@ -41,7 +41,7 @@ our %args = (
loggrep => {
qr/syslogd\[\d+\]: loghost .* connection error: /.
qr/handshake failed: error:.*:SSL routines:/.
- qr/CONNECT_CR_SRVR_HELLO:wrong version number/ => 1,
+ qr/\(UNKNOWN\)SSL_internal:unknown failure occurred/ => 1,
},
},
);
Index: args-tls-cipher-null.pl
===
RCS file: /cvs/src/regress/usr.sbin/syslogd/args-tls-cipher-null.pl,v
retrieving revision 1.8
diff -u -p -r1.8 args-tls-cipher-null.pl
--- args-tls-cipher-null.pl 5 Apr 2017 22:32:14 - 1.8
+++ args-tls-cipher-null.pl 2 Mar 2020 22:22:32 -
@@ -16,7 +16,7 @@ our %args = (
qr/Logging to FORWTLS \@tls:\/\/localhost:\d+/ => '>=4',
qr/syslogd\[\d+\]: loghost .* connection error: /.
qr/handshake failed: error:.*:SSL routines:/.
- qr/CONNECT_CR_SRVR_HELLO:sslv3 alert handshake failure/ => 1,
+ qr/ST_CONNECT:sslv3 alert handshake failure/ => 1,
get_testgrep() => 1,
},
cacrt => "ca.crt",
Re: fix ldapd/ldapctl data directory handling
Hi,
On Tue, 3 Mar 2020 20:45:17 +0100
Martijn van Duren wrote:
> I don't agree with this diff, even though you're right with it being
> broken. Right now the regress test uses it/tries to use it and even
> though someone trying to run regress on an actual production machine
> somewhat deserves to get shot in the foot, I don't think we should
> make that our goal.
>
> Also, I can see this being a helpful feature for people doing some
> development work (e.g. testing with delegations etc without setting
> up a vm-environment).
>
> Could you please look into fixing the actual issue?
this patch fixes only the two issues. I de-const'd "char * datadir" in
ldapd to get rid of the new warning.
Best regards
Robert
diff 1813335e849f285a868ea3d474b4704812b1843e /usr/src
blob - c8564c5543f518a720e049c559556f87edda6b8a
file + usr.sbin/ldapctl/ldapctl.c
--- usr.sbin/ldapctl/ldapctl.c
+++ usr.sbin/ldapctl/ldapctl.c
@@ -150,7 +150,7 @@ index_namespace(struct namespace *ns, const char *data
log_info("indexing namespace %s", ns->suffix);
- if (asprintf(, "%s/%s_data.db", DATADIR, ns->suffix) == -1)
+ if (asprintf(, "%s/%s_data.db", datadir, ns->suffix) == -1)
return -1;
data_db = btree_open(path, BT_NOSYNC | BT_REVERSEKEY, 0644);
free(path);
blob - 797a36f89f33233c2d9655a307ec5e727b9fbaa1
file + usr.sbin/ldapd/ldapd.c
--- usr.sbin/ldapd/ldapd.c
+++ usr.sbin/ldapd/ldapd.c
@@ -55,7 +55,7 @@ static pid_t start_child(enum ldapd_process, char *,
struct ldapd_stats stats;
pid_t ldape_pid;
-const char *datadir = DATADIR;
+char *datadir = DATADIR;
void
usage(void)
@@ -415,7 +415,7 @@ static pid_t
start_child(enum ldapd_process p, char *argv0, int fd, int debug,
int verbose, char *csockpath, char *conffile)
{
- char*argv[9];
+ char*argv[11];
int argc = 0;
pid_tpid;
@@ -459,7 +459,11 @@ start_child(enum ldapd_process p, char *argv0, int fd,
argv[argc++] = "-f";
argv[argc++] = conffile;
}
-
+ if (datadir) {
+ argv[argc++] = "-r";
+ argv[argc++] = datadir;
+ }
+
argv[argc++] = NULL;
execvp(argv0, argv);
blob - 2259f57bde3c2918a8b200747c194e5768ae40f1
file + usr.sbin/ldapd/namespace.c
--- usr.sbin/ldapd/namespace.c
+++ usr.sbin/ldapd/namespace.c
@@ -29,7 +29,7 @@
#include "ldapd.h"
#include "log.h"
-extern const char *datadir;
+extern char*datadir;
/* Maximum number of requests to queue per namespace during compaction.
* After this many requests, we return LDAP_BUSY.
>
> martijn@
>
> On 3/2/20 8:00 PM, Robert Klein wrote:
> > Hi,
> >
> > in ldapd and ldapctl the "-r directory" command line argument does
> > not work:
> >
> > ldapd fork/execs itself but the directory command line argument is
> > not given to the execvp call which then uses the default data
> > directory.
> >
> > ldapctl has a thinko/typo which causes the data_db to be always
> > opened in the default directory for indexing.
> >
> > The patch below removes the command line argument and corresponding
> > global variable and instead uses a configuration file directive
> > "datadir". Incidentally removes the global 'datadir' variable used
> > before.
> >
> > Best regards
> > Robert
> >
> >
> > ---
> > commit 6a71c90c19b5dc5850305c15696af3f14d26c168 (master)
> > from: Robert Klein
> > date: Mon Mar 2 18:50:12 2020 UTC
> >
> > fix ldapd/ldapctl data directory handling
> >
> > -r directive didn't work as intended in both
> > ldapd and ldapctl.
> >
> > this patch replaces command line argument '-r directory'
> > with a configuration file directive 'datadir'.
> >
> > diff f62b80b397c780e8273b5cc67d544b951c4c9d1f
> > 35bb29194b2067dab925ec4566dfb82f9bf34d65 blob -
> > 48cff01b435bca0deebf28f55e6f71675c5752f2 blob +
> > 231af2ddbbee2da446d2f03c9c48e679e216e993 ---
> > usr.sbin/ldapctl/ldapctl.8 +++ usr.sbin/ldapctl/ldapctl.8
> > @@ -24,7 +24,6 @@
> > .Nm ldapctl
> > .Op Fl v
> > .Op Fl f Ar file
> > -.Op Fl r Ar directory
> > .Op Fl s Ar socket
> > .Ar command
> > .Op Ar argument ...
> > @@ -42,11 +41,6 @@ Use
> > .Ar file
> > as the configuration file, instead of the default
> > .Pa /etc/ldapd.conf .
> > -.It Fl r Ar directory
> > -Store and read database files in
> > -.Ar directory ,
> > -instead of the default
> > -.Pa /var/db/ldap .
> > .It Fl s Ar socket
> > Use
> > .Ar socket
> > blob - c8564c5543f518a720e049c559556f87edda6b8a
> > blob + 6830a603d153794390faafbc9aadc8ef0bd985c0
> > --- usr.sbin/ldapctl/ldapctl.c
> > +++ usr.sbin/ldapctl/ldapctl.c
> > @@ -58,10 +58,10 @@ void show_stats(struct imsg
> > *imsg); void show_dbstats(const char *prefix,
> > struct btree_stat *st); void show_nsstats(struct
> > imsg
pfkeyv2.c: Mem leak
Hi,
pfkeyv2.c:1091:pfkeyv2_send(struct socket *so, void *message, int len)
leaks memory in the SADB_REGISTER case (line 1579).
It reuses void *freeme multiple times to build up void *headers[].
headers[] are bcopy'ed to another buffer inside of pfkeyv2_sendmessage()
(line 2064) so as the name implies *freeme is safe to be freed at the end.
Found by llvm/scan-build.
Also don't check for NULL before free(), could do a pass over the entire file.
Only compile tested.
- Ben
Index: pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.198
diff -u -p -r1.198 pfkeyv2.c
--- pfkeyv2.c 17 Jul 2019 18:52:46 - 1.198
+++ pfkeyv2.c 3 Mar 2020 17:58:22 -
@@ -1098,7 +1098,8 @@ pfkeyv2_send(struct socket *so, void *me
struct radix_node_head *rnh;
struct radix_node *rn = NULL;
struct pkpcb *kp, *bkp;
- void *freeme = NULL, *bckptr = NULL;
+ void *freeme = NULL, *freeme2 = NULL, *freeme3 = NULL;
+ void *bckptr = NULL;
void *headers[SADB_EXT_MAX + 1];
union sockaddr_union *sunionp;
struct tdb *sa1 = NULL, *sa2 = NULL;
@@ -1605,7 +1606,7 @@ pfkeyv2_send(struct socket *so, void *me
i = sizeof(struct sadb_supported) + sizeof(aalgs);
- if (!(freeme = malloc(i, M_PFKEY, M_NOWAIT | M_ZERO))) {
+ if (!(freeme2 = malloc(i, M_PFKEY, M_NOWAIT | M_ZERO))) {
rval = ENOMEM;
goto ret;
}
@@ -1616,34 +1617,34 @@ pfkeyv2_send(struct socket *so, void *me
(1 << ((struct sadb_msg *)message)->sadb_msg_satype);
keyunlock(kp, s);
- ssup = (struct sadb_supported *) freeme;
+ ssup = (struct sadb_supported *) freeme2;
ssup->sadb_supported_len = i / sizeof(uint64_t);
{
- void *p = freeme + sizeof(struct sadb_supported);
+ void *p = freeme2 + sizeof(struct sadb_supported);
bcopy([0], p, sizeof(aalgs));
}
- headers[SADB_EXT_SUPPORTED_AUTH] = freeme;
+ headers[SADB_EXT_SUPPORTED_AUTH] = freeme2;
i = sizeof(struct sadb_supported) + sizeof(calgs);
- if (!(freeme = malloc(i, M_PFKEY, M_NOWAIT | M_ZERO))) {
+ if (!(freeme3 = malloc(i, M_PFKEY, M_NOWAIT | M_ZERO))) {
rval = ENOMEM;
goto ret;
}
- ssup = (struct sadb_supported *) freeme;
+ ssup = (struct sadb_supported *) freeme3;
ssup->sadb_supported_len = i / sizeof(uint64_t);
{
- void *p = freeme + sizeof(struct sadb_supported);
+ void *p = freeme3 + sizeof(struct sadb_supported);
bcopy([0], p, sizeof(calgs));
}
- headers[SADB_X_EXT_SUPPORTED_COMP] = freeme;
+ headers[SADB_X_EXT_SUPPORTED_COMP] = freeme3;
break;
@@ -2064,14 +2065,14 @@ ret:
realret:
- if (freeme)
- free(freeme, M_PFKEY, 0);
+ free(freeme, M_PFKEY, 0);
+ free(freeme2, M_PFKEY, 0);
+ free(freeme3, M_PFKEY, 0);
explicit_bzero(message, len);
free(message, M_PFKEY, 0);
- if (sa1)
- free(sa1, M_PFKEY, 0);
+ free(sa1, M_PFKEY, 0);
return (rval);
}
[PATCH] fixes relayd Websocket "Connection: close" header when Upgrade is requested
After migrating my home setup from nginx reverse proxying to relayd, i noticed my iOS devices having issues connecting through Websockets. After debugging, i noticed that relayd adds the "Connection: close" regardless of upgrade being requested. This issue is also reported on a blog-post using relayd as a Websocket Client. https://deftly.net/posts/2019-10-23-websockets-with-relayd.html This resulted in the response having two Connection Headers, one "Connection: upgrade" and one "Connection: close", which iOS strict implementation does not allow. I have fixed the if condition that leads to this issue. The fix has been tested with Apple iOS 13 and the problem can be observed using Firefox and just connecting to something Websocket over relayd. Both "Connection:" headers will appear. best regards Franz diff --git usr.sbin/relayd/relay_http.c usr.sbin/relayd/relay_http.c index 960d4c54a..3a6678790 100644 --- usr.sbin/relayd/relay_http.c +++ usr.sbin/relayd/relay_http.c @@ -524,9 +524,11 @@ relay_read_http(struct bufferevent *bev, void *arg) /* * Ask the server to close the connection after this request -* since we don't read any further request headers. +* since we don't read any further request headers, unless +* an Upgrade is requested, in which case we do NOT want to add +* this header. */ - if (cre->toread == TOREAD_UNLIMITED) + if (cre->toread == TOREAD_UNLIMITED && upgrade == NULL) if (kv_add(>http_headers, "Connection", "close", 0) == NULL) goto fail;
Re: em(4) hw setup vs queues
On 03/03/20(Tue) 11:37, Martin Pieuchot wrote:
> Currently em_hw_init() uses some hardcorded values to configure TX
> rings. Diff below convert it to use the value of the first queue.
> This is currently a no-op. It makes the code consistent with the
> rest of the driver and reduce the size of upcoming diffs.
>
> Note that even if a single queue is currently used two of them are
> setup. Document this has an historical behavior and keep it like it
> is, there's not much need to introduce regression here :)
Updated diff that includes a chunk I missed in previous spotted by
jmatthew@.
ok?
Index: dev/pci/if_em.c
===
RCS file: /cvs/src/sys/dev/pci/if_em.c,v
retrieving revision 1.346
diff -u -p -r1.346 if_em.c
--- dev/pci/if_em.c 3 Mar 2020 10:43:29 - 1.346
+++ dev/pci/if_em.c 4 Mar 2020 17:08:18 -
@@ -1883,7 +1883,7 @@ em_hardware_init(struct em_softc *sc)
em_disable_aspm(sc);
- if ((ret_val = em_init_hw(>hw)) != 0) {
+ if ((ret_val = em_init_hw(sc)) != 0) {
if (ret_val == E1000_DEFER_INIT) {
INIT_DEBUGOUT("\nHardware Initialization Deferred ");
return (EAGAIN);
Index: dev/pci/if_em_hw.c
===
RCS file: /cvs/src/sys/dev/pci/if_em_hw.c,v
retrieving revision 1.106
diff -u -p -r1.106 if_em_hw.c
--- dev/pci/if_em_hw.c 4 Feb 2020 10:59:23 - 1.106
+++ dev/pci/if_em_hw.c 4 Mar 2020 17:09:31 -
@@ -56,6 +56,7 @@
#include
#include
+#include
#include
#include
@@ -92,7 +93,7 @@ static int32_tem_init_lcd_from_nvm_conf
static int32_t em_init_lcd_from_nvm(struct em_hw *);
static int32_t em_phy_no_cable_workaround(struct em_hw *);
static voidem_init_rx_addrs(struct em_hw *);
-static voidem_initialize_hardware_bits(struct em_hw *);
+static voidem_initialize_hardware_bits(struct em_softc *);
static voidem_toggle_lanphypc_pch_lpt(struct em_hw *);
static int em_disable_ulp_lpt_lp(struct em_hw *hw, bool force);
static boolean_t em_is_onboard_nvm_eeprom(struct em_hw *);
@@ -712,6 +713,7 @@ em_set_mac_type(struct em_hw *hw)
return E1000_SUCCESS;
}
+
/*
* Set media type and TBI compatibility.
*
@@ -1115,8 +1117,11 @@ em_reset_hw(struct em_hw *hw)
*
*/
STATIC void
-em_initialize_hardware_bits(struct em_hw *hw)
+em_initialize_hardware_bits(struct em_softc *sc)
{
+ struct em_hw *hw = >hw;
+ struct em_queue *que = sc->queues; /* Use only first queue. */
+
DEBUGFUNC("em_initialize_hardware_bits");
if ((hw->mac_type >= em_82571) && (!hw->initialize_hw_bits_disable)) {
@@ -1124,18 +1129,24 @@ em_initialize_hardware_bits(struct em_hw
uint32_t reg_ctrl, reg_ctrl_ext;
uint32_t reg_tarc0, reg_tarc1;
uint32_t reg_tctl;
- uint32_t reg_txdctl, reg_txdctl1;
+ uint32_t reg_txdctl;
reg_tarc0 = E1000_READ_REG(hw, TARC0);
reg_tarc0 &= ~0x7800; /* Clear bits 30, 29, 28, and
* 27 */
- reg_txdctl = E1000_READ_REG(hw, TXDCTL(0));
+ reg_txdctl = E1000_READ_REG(hw, TXDCTL(que->me));
reg_txdctl |= E1000_TXDCTL_COUNT_DESC; /* Set bit 22 */
- E1000_WRITE_REG(hw, TXDCTL(0), reg_txdctl);
+ E1000_WRITE_REG(hw, TXDCTL(que->me), reg_txdctl);
- reg_txdctl1 = E1000_READ_REG(hw, TXDCTL(1));
- reg_txdctl1 |= E1000_TXDCTL_COUNT_DESC; /* Set bit 22 */
- E1000_WRITE_REG(hw, TXDCTL(1), reg_txdctl1);
+ /*
+* Old code always initialized queue 1,
+* even when unused, keep behaviour
+*/
+ if (sc->num_queues == 1) {
+ reg_txdctl = E1000_READ_REG(hw, TXDCTL(1));
+ reg_txdctl |= E1000_TXDCTL_COUNT_DESC;
+ E1000_WRITE_REG(hw, TXDCTL(1), reg_txdctl);
+ }
switch (hw->mac_type) {
case em_82571:
@@ -1430,8 +1441,10 @@ out:
* the transmit and receive units disabled and uninitialized.
*/
int32_t
-em_init_hw(struct em_hw *hw)
+em_init_hw(struct em_softc *sc)
{
+ struct em_hw *hw = >hw;
+ struct em_queue *que = sc->queues; /* Use only first queue. */
uint32_t ctrl;
uint32_t i;
int32_t ret_val;
@@ -1513,7 +1526,7 @@ em_init_hw(struct em_hw *hw)
/* Magic delay that improves problems with i219LM on HP Elitebook */
msec_delay(1);
/* Must be called after em_set_media_type because
Re: BIRD 1.x/2.x support at rpki-client
On Wed, Mar 04, 2020 at 07:48:28AM -0700, Theo de Raadt wrote:
> Job Snijders wrote:
>
> > I think we still need to support BIRD 1 for the foreseeable future, NIC.CZ
> > hasn’t communicated plans to deprecate BIRD1 and still supports it; and
> > BIRD1 still is widely deployed.
> >
> > I’m somewhat preferential to just generate all 3 BIRD flavors if -B is
> > given as command line option.
>
> output.c does the mkstemp dance above the output function, and it is
> table driven to generate one file with a function, per option.
>
> But imagine if the table was changed to do this:
>
> } outputs[] = {
> { FORMAT_OPENBGPD, "openbgpd", output_bgpd },
> { FORMAT_BIRD, "bird1v4", output_bird1v4 },
> { FORMAT_BIRD, "bird1v6", output_bird1v6 }
> { FORMAT_BIRD, "bird", output_bird2 }
> { FORMAT_CSV, "csv", output_csv },
> { FORMAT_JSON, "json", output_json },
> ...
>
> I'd like if someone wrote output_bird1v4, output_bird1v6, and output_bird2
> functions (or whatever the case may be), is that the way to do it?
I think that is a good way to solve this issue.
--
:wq Claudio
Re: BIRD 1.x/2.x support at rpki-client
Job Snijders wrote:
> I think we still need to support BIRD 1 for the foreseeable future, NIC.CZ
> hasn’t communicated plans to deprecate BIRD1 and still supports it; and BIRD1
> still is widely deployed.
>
> I’m somewhat preferential to just generate all 3 BIRD flavors if -B is given
> as command line option.
output.c does the mkstemp dance above the output function, and it is
table driven to generate one file with a function, per option.
But imagine if the table was changed to do this:
} outputs[] = {
{ FORMAT_OPENBGPD, "openbgpd", output_bgpd },
{ FORMAT_BIRD, "bird1v4", output_bird1v4 },
{ FORMAT_BIRD, "bird1v6", output_bird1v6 }
{ FORMAT_BIRD, "bird", output_bird2 }
{ FORMAT_CSV, "csv", output_csv },
{ FORMAT_JSON, "json", output_json },
...
I'd like if someone wrote output_bird1v4, output_bird1v6, and output_bird2
functions (or whatever the case may be), is that the way to do it?
Re: BIRD 1.x/2.x support at rpki-client
We are still at the early stages of RPKI deployment, so if we make it easier to plug things into BIRD1 is beneficial given the wide deployment scale. Only /very/ recently was rpki-client packaged for some of the Linux distros, so if we add support for all formats now - it’ll improve the applicability of the rpki-client software. When NIC.CZ announces the EOL for BIRD1 we can plan to remove that output format. It’s too soon to not support BIRD1 imho. Kind regards, Job
Re: BIRD 1.x/2.x support at rpki-client
I think we still need to support BIRD 1 for the foreseeable future, NIC.CZ hasn’t communicated plans to deprecate BIRD1 and still supports it; and BIRD1 still is widely deployed. I’m somewhat preferential to just generate all 3 BIRD flavors if -B is given as command line option. Kind regards, Job
Re: BIRD 1.x/2.x support at rpki-client
On Wed, Mar 4, 2020, at 00:55, Robert Scheck wrote: > > The idea is you can specify many outputs. That will make the commandline > > very long, especially for the way we run it in cron. > > Oh! I'm sorry, I didn't see the idea of specifying many outputs. Yeah, its nice to do things in one batch for all things that might exist in an organisation: job@vurt ~$ doas rpki-client -Bjoc job@vurt ~$ ls -alhtr /var/db/rpki-client/ total 49128 drwxr-xr-x 14 root wheel 1.0K Mar 1 14:53 .. -rw-r--r-- 1 _rpki-client wheel 4.3M Mar 4 12:16 openbgpd -rw-r--r-- 1 _rpki-client wheel 4.8M Mar 4 12:16 bird -rw-r--r-- 1 _rpki-client wheel 4.1M Mar 4 12:16 csv -rw-r--r-- 1 _rpki-client wheel 10.7M Mar 4 12:16 json drwxr-xr-x 2 _rpki-client wheel 512B Mar 4 12:16 . Saves everyone valuable time! ;-) Kind regards, Job
