Re: smtpd filters: prettify proc-exec
On Fri, Aug 23, 2019 at 07:33:29PM +0200, Martijn van Duren wrote:
> On 8/23/19 7:06 PM, Gilles Chehade wrote:
> > On Fri, Aug 23, 2019 at 09:03:51AM +0200, Martijn van Duren wrote:
> >> Hello,
> >>
> >
> > Hello,
> >
> >
> >> When running processes with proc-exec any logging send over stderr is
> >> not very intelligible since the process name is automatically generated
> >> and results in , which make them hard to read.
> >>
> >
> > Agreed
> >
> >
> >> To clean this up I reckon we can do three things:
> >> 1) Add an extra optional proc parameter that sets the name. Diff below
> >>does this.
> >> 2) Change the process name to match the filter name.
> >> 3) Do both
> >>
> >> I reckon we're still early enough in the development process for us to
> >> change the default processor name in proc-exec.
> >>
> >> Thoughts?
> >>
> >
> > I'm unsure about 1- because it really reads cumbersome in my opinion:
> >
> >filter "rspamd" proc-exec proc "rspamd" "/usr/local/bin/filter-rspamd"
> >
> > Option 2- has my preference because it is really what you'd expect out
> > of the box, it was on my todo but if you're looking into it I'd rather
> > let you do it :-)
> >
> >
> I agree.
>
> That would be this diff.
>
> OK?
>
last_dynproc_id is no longer needed ?
nice
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.257
> diff -u -p -r1.257 parse.y
> --- parse.y 11 Aug 2019 17:23:12 - 1.257
> +++ parse.y 23 Aug 2019 17:33:17 -
> @@ -108,7 +108,6 @@ struct rule *rule;
> struct processor *processor;
> struct filter_config *filter_config;
> static uint32_t last_dynchain_id = 1;
> -static uint32_t last_dynproc_id = 1;
>
> enum listen_options {
> LO_FAMILY = 0x01,
> @@ -1598,12 +1597,6 @@ FILTER STRING PROC STRING {
> }
> |
> FILTER STRING PROC_EXEC STRING {
> - charbuffer[128];
> -
> - do {
> - (void)snprintf(buffer, sizeof buffer, "",
> last_dynproc_id++);
> - } while (dict_check(conf->sc_processors_dict, buffer));
> -
> if (dict_get(conf->sc_filters_dict, $2)) {
> yyerror("filter already exists with that name: %s", $2);
> free($2);
> @@ -1617,7 +1610,7 @@ FILTER STRING PROC_EXEC STRING {
> filter_config = xcalloc(1, sizeof *filter_config);
> filter_config->filter_type = FILTER_TYPE_PROC;
> filter_config->name = $2;
> - filter_config->proc = xstrdup(buffer);
> + filter_config->proc = xstrdup($2);
> dict_set(conf->sc_filters_dict, $2, filter_config);
> } proc_params {
> dict_set(conf->sc_processors_dict, filter_config->proc, processor);
>
--
Gilles Chehade @poolpOrg
https://www.poolp.orgpatreon: https://www.patreon.com/gilles
Re: smtpd filters: prettify proc-exec
On 8/23/19 7:06 PM, Gilles Chehade wrote:
> On Fri, Aug 23, 2019 at 09:03:51AM +0200, Martijn van Duren wrote:
>> Hello,
>>
>
> Hello,
>
>
>> When running processes with proc-exec any logging send over stderr is
>> not very intelligible since the process name is automatically generated
>> and results in , which make them hard to read.
>>
>
> Agreed
>
>
>> To clean this up I reckon we can do three things:
>> 1) Add an extra optional proc parameter that sets the name. Diff below
>>does this.
>> 2) Change the process name to match the filter name.
>> 3) Do both
>>
>> I reckon we're still early enough in the development process for us to
>> change the default processor name in proc-exec.
>>
>> Thoughts?
>>
>
> I'm unsure about 1- because it really reads cumbersome in my opinion:
>
>filter "rspamd" proc-exec proc "rspamd" "/usr/local/bin/filter-rspamd"
>
> Option 2- has my preference because it is really what you'd expect out
> of the box, it was on my todo but if you're looking into it I'd rather
> let you do it :-)
>
>
I agree.
That would be this diff.
OK?
martijn@
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.257
diff -u -p -r1.257 parse.y
--- parse.y 11 Aug 2019 17:23:12 - 1.257
+++ parse.y 23 Aug 2019 17:33:17 -
@@ -108,7 +108,6 @@ struct rule *rule;
struct processor *processor;
struct filter_config *filter_config;
static uint32_t last_dynchain_id = 1;
-static uint32_t last_dynproc_id = 1;
enum listen_options {
LO_FAMILY = 0x01,
@@ -1598,12 +1597,6 @@ FILTER STRING PROC STRING {
}
|
FILTER STRING PROC_EXEC STRING {
- charbuffer[128];
-
- do {
- (void)snprintf(buffer, sizeof buffer, "",
last_dynproc_id++);
- } while (dict_check(conf->sc_processors_dict, buffer));
-
if (dict_get(conf->sc_filters_dict, $2)) {
yyerror("filter already exists with that name: %s", $2);
free($2);
@@ -1617,7 +1610,7 @@ FILTER STRING PROC_EXEC STRING {
filter_config = xcalloc(1, sizeof *filter_config);
filter_config->filter_type = FILTER_TYPE_PROC;
filter_config->name = $2;
- filter_config->proc = xstrdup(buffer);
+ filter_config->proc = xstrdup($2);
dict_set(conf->sc_filters_dict, $2, filter_config);
} proc_params {
dict_set(conf->sc_processors_dict, filter_config->proc, processor);
Re: smtpd filters: prettify proc-exec
On Fri, Aug 23, 2019 at 09:03:51AM +0200, Martijn van Duren wrote:
> Hello,
>
Hello,
> When running processes with proc-exec any logging send over stderr is
> not very intelligible since the process name is automatically generated
> and results in , which make them hard to read.
>
Agreed
> To clean this up I reckon we can do three things:
> 1) Add an extra optional proc parameter that sets the name. Diff below
>does this.
> 2) Change the process name to match the filter name.
> 3) Do both
>
> I reckon we're still early enough in the development process for us to
> change the default processor name in proc-exec.
>
> Thoughts?
>
I'm unsure about 1- because it really reads cumbersome in my opinion:
filter "rspamd" proc-exec proc "rspamd" "/usr/local/bin/filter-rspamd"
Option 2- has my preference because it is really what you'd expect out
of the box, it was on my todo but if you're looking into it I'd rather
let you do it :-)
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.257
> diff -u -p -r1.257 parse.y
> --- parse.y 11 Aug 2019 17:23:12 - 1.257
> +++ parse.y 23 Aug 2019 07:00:08 -
> @@ -1625,6 +1625,37 @@ FILTER STRING PROC_EXEC STRING {
> filter_config = NULL;
> }
> |
> +FILTER STRING PROC_EXEC PROC STRING STRING {
> + if (dict_get(conf->sc_processors_dict, $5)) {
> + yyerror("processor already exists with that name: %s", $5);
> + free($2);
> + free($5);
> + free($6);
> + YYERROR;
> + }
> +
> + if (dict_get(conf->sc_filters_dict, $2)) {
> + yyerror("filter already exists with that name: %s", $2);
> + free($2);
> + free($5);
> + free($6);
> + YYERROR;
> + }
> +
> + processor = xcalloc(1, sizeof *processor);
> + processor->command = $6;
> +
> + filter_config = xcalloc(1, sizeof *filter_config);
> + filter_config->filter_type = FILTER_TYPE_PROC;
> + filter_config->name = $2;
> + filter_config->proc = $5;
> + dict_set(conf->sc_filters_dict, $2, filter_config);
> +} proc_params {
> + dict_set(conf->sc_processors_dict, filter_config->proc, processor);
> + processor = NULL;
> + filter_config = NULL;
> +}
> +|
> FILTER STRING PHASE {
> if (dict_get(conf->sc_filters_dict, $2)) {
> yyerror("filter already exists with that name: %s", $2);
>
--
Gilles Chehade @poolpOrg
https://www.poolp.orgpatreon: https://www.patreon.com/gilles
acpi GPE storm on X1 Carbon 6th
Hi,
My X1 still endures an ACPI storm if I use an external displayport
display.
Previously I posted a workaround that just made acpi_gpe() a no-op for
GPE 111. That solved it for me, but is not the right way of course.
I was looking at acpi_ev_gpe_dispatch() in Linux and noticed it does a
conditonal re-enable.
This version is a bit more subtle: do evaluate, just do not re-enable
for GPE 111.
Maybe I'm on to something? The printed type value is always 0
(AML_OBJTYPE_UNINITIALIZED), though, so I cannot base any decicison on
the value.
-Otto
Index: acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.371
diff -u -p -r1.371 acpi.c
--- acpi.c 2 Jul 2019 21:17:24 - 1.371
+++ acpi.c 23 Aug 2019 13:01:50 -
@@ -2294,10 +2294,18 @@ acpi_gpe(struct acpi_softc *sc, int gpe,
{
struct aml_node *node = arg;
uint8_t mask, en;
+ int ret;
+ struct aml_value res;
dnprintf(10, "handling GPE %.2x\n", gpe);
- aml_evalnode(sc, node, 0, NULL, NULL);
+ ret = aml_evalnode(sc, node, 0, NULL, );
+ printf("GPE %d ret %x %d\n", gpe, ret, res.type);
+ if (res.type == AML_OBJTYPE_INTEGER)
+ printf("Value is %llx\n", (long long)res.v_integer);
+ aml_freevalue();
+ if (gpe == 111)
+ return 0;
mask = (1L << (gpe & 7));
if (sc->gpe_table[gpe].flags & GPE_LEVEL)
acpi_write_pmreg(sc, ACPIREG_GPE_STS, gpe>>3, mask);
Re: typo in man page of sysupgrade option -s
On Fri, Aug 23, 2019 at 02:57:04PM +0200, Ingo Schwarze wrote: > Hi Florian, > > Florian Obser wrote on Fri, Aug 23, 2019 at 02:43:22PM +0200: > > > This is not a typo as such, it describes the default. It is a bit > > awkward since it uses the same text for -r and -s. I think we can just > > remove it from -s: > > The following seems more straightforward. yes, I like that. OK florian > > I don't feel strongly about it, though; you patch is OK with me, too. > > Yours, > Ingo > > > Index: sysupgrade.8 > === > RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v > retrieving revision 1.8 > diff -u -r1.8 sysupgrade.8 > --- sysupgrade.8 9 May 2019 21:09:37 - 1.8 > +++ sysupgrade.8 23 Aug 2019 12:54:38 - > @@ -62,16 +62,10 @@ > but do not reboot. > .It Fl r > Upgrade to the next release. > -The default is to find out if the system is running a release or a snapshot. > -In case of release > -.Nm > -downloads the next release. > +This is the default if the system is currently running a release. > .It Fl s > Upgrade to a snapshot. > -The default is to find out if the system is running a release or a snapshot. > -In case of release > -.Nm > -downloads the next release. > +This is the default if the system is currently running a snapshot. > .El > .Sh FILES > .Bl -tag -width "/home/_sysupgrade" -compact > -- I'm not entirely sure you are real.
Re: typo in man page of sysupgrade option -s
Hi Florian, Florian Obser wrote on Fri, Aug 23, 2019 at 02:43:22PM +0200: > This is not a typo as such, it describes the default. It is a bit > awkward since it uses the same text for -r and -s. I think we can just > remove it from -s: The following seems more straightforward. I don't feel strongly about it, though; you patch is OK with me, too. Yours, Ingo Index: sysupgrade.8 === RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v retrieving revision 1.8 diff -u -r1.8 sysupgrade.8 --- sysupgrade.89 May 2019 21:09:37 - 1.8 +++ sysupgrade.823 Aug 2019 12:54:38 - @@ -62,16 +62,10 @@ but do not reboot. .It Fl r Upgrade to the next release. -The default is to find out if the system is running a release or a snapshot. -In case of release -.Nm -downloads the next release. +This is the default if the system is currently running a release. .It Fl s Upgrade to a snapshot. -The default is to find out if the system is running a release or a snapshot. -In case of release -.Nm -downloads the next release. +This is the default if the system is currently running a snapshot. .El .Sh FILES .Bl -tag -width "/home/_sysupgrade" -compact
Re: typo in man page of sysupgrade option -s
On Fri, Aug 23, 2019 at 02:43:22PM +0200, Florian Obser wrote: > On Thu, Aug 22, 2019 at 10:27:38AM +, Rashad Kanavath wrote: > > update description of -s option in sysupgrade man page. > > > > Index: usr.sbin/sysupgrade/sysupgrade.8 > > === > > RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v > > retrieving revision 1.8 > > diff -u -p -u -p -r1.8 sysupgrade.8 > > --- usr.sbin/sysupgrade/sysupgrade.8 9 May 2019 21:09:37 - 1.8 > > +++ usr.sbin/sysupgrade/sysupgrade.8 22 Aug 2019 08:02:25 - > > @@ -67,11 +67,11 @@ In case of release > > .Nm > > downloads the next release. > > .It Fl s > > -Upgrade to a snapshot. > > +Upgrade to latest snapshot. > > I'd like to leave it vague. Otherwise one might think sysupgrade does > some magic to find out what the latest snapshot is. While it does not. > It contacts the mirror that's set in /etc/installurl and downloads > whatever it finds there. If that mirror is behind one does not get the > latest snap. > > > The default is to find out if the system is running a release or a > > snapshot. > > -In case of release > > +In case of snapshot > > .Nm > > -downloads the next release. > > +downloads the latest snapshot. > > > This is not a typo as such, it describes the default. It is a bit > awkward since it uses the same text for -r and -s. I think we can just > remove it from -s: > > jmc? > yes, i think it's clearer without that text. another tact would be to remove that text from both options and try to describe at page top what happens. jmc > diff --git sysupgrade.8 sysupgrade.8 > index ba34190d979..b067d5fe691 100644 > --- sysupgrade.8 > +++ sysupgrade.8 > @@ -68,10 +68,6 @@ In case of release > downloads the next release. > .It Fl s > Upgrade to a snapshot. > -The default is to find out if the system is running a release or a snapshot. > -In case of release > -.Nm > -downloads the next release. > .El > .Sh FILES > .Bl -tag -width "/home/_sysupgrade" -compact > > > > > .El > > .Sh FILES > > .Bl -tag -width "/home/_sysupgrade" -compact > > > > -- > > Regards, > >Rashad > > -- > I'm not entirely sure you are real. >
Re: typo in man page of sysupgrade option -s
On Thu, Aug 22, 2019 at 10:27:38AM +, Rashad Kanavath wrote: > update description of -s option in sysupgrade man page. > > Index: usr.sbin/sysupgrade/sysupgrade.8 > === > RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v > retrieving revision 1.8 > diff -u -p -u -p -r1.8 sysupgrade.8 > --- usr.sbin/sysupgrade/sysupgrade.8 9 May 2019 21:09:37 - 1.8 > +++ usr.sbin/sysupgrade/sysupgrade.8 22 Aug 2019 08:02:25 - > @@ -67,11 +67,11 @@ In case of release > .Nm > downloads the next release. > .It Fl s > -Upgrade to a snapshot. > +Upgrade to latest snapshot. I'd like to leave it vague. Otherwise one might think sysupgrade does some magic to find out what the latest snapshot is. While it does not. It contacts the mirror that's set in /etc/installurl and downloads whatever it finds there. If that mirror is behind one does not get the latest snap. > The default is to find out if the system is running a release or a > snapshot. > -In case of release > +In case of snapshot > .Nm > -downloads the next release. > +downloads the latest snapshot. This is not a typo as such, it describes the default. It is a bit awkward since it uses the same text for -r and -s. I think we can just remove it from -s: jmc? diff --git sysupgrade.8 sysupgrade.8 index ba34190d979..b067d5fe691 100644 --- sysupgrade.8 +++ sysupgrade.8 @@ -68,10 +68,6 @@ In case of release downloads the next release. .It Fl s Upgrade to a snapshot. -The default is to find out if the system is running a release or a snapshot. -In case of release -.Nm -downloads the next release. .El .Sh FILES .Bl -tag -width "/home/_sysupgrade" -compact > .El > .Sh FILES > .Bl -tag -width "/home/_sysupgrade" -compact > > -- > Regards, >Rashad -- I'm not entirely sure you are real.
Re: fix rpki client regression tests
On Thu, Aug 22, 2019 at 09:45:36PM +0200, Moritz Buhl wrote: > Hi, > > some experts forgot to run the rpki-client regression tests after some > changes. The tests are relinking parts of the source and this is not great > but after all I still prefer some running tests. Patch attached. OK claudio@ > thanks, > mbuhl > > Index: regress/usr.sbin/rpki-client/test-cert.c > === > RCS file: /mount/openbsd/cvs/src/regress/usr.sbin/rpki-client/test-cert.c,v > retrieving revision 1.2 > diff -u -p -r1.2 test-cert.c > --- regress/usr.sbin/rpki-client/test-cert.c 12 Aug 2019 18:03:17 - > 1.2 > +++ regress/usr.sbin/rpki-client/test-cert.c 22 Aug 2019 19:38:14 - > @@ -31,6 +31,7 @@ > #include > > #include "extern.h" > +int verbose = 0; > > static void > cert_print(const struct cert *p) > Index: regress/usr.sbin/rpki-client/test-ip.c > === > RCS file: /mount/openbsd/cvs/src/regress/usr.sbin/rpki-client/test-ip.c,v > retrieving revision 1.3 > diff -u -p -r1.3 test-ip.c > --- regress/usr.sbin/rpki-client/test-ip.c12 Aug 2019 18:03:17 - > 1.3 > +++ regress/usr.sbin/rpki-client/test-ip.c22 Aug 2019 19:38:19 - > @@ -30,6 +30,7 @@ > #include > > #include "extern.h" > +int verbose = 0; > > static void > test(const char *res, uint16_t afiv, size_t sz, size_t unused, ...) > Index: regress/usr.sbin/rpki-client/test-mft.c > === > RCS file: /mount/openbsd/cvs/src/regress/usr.sbin/rpki-client/test-mft.c,v > retrieving revision 1.2 > diff -u -p -r1.2 test-mft.c > --- regress/usr.sbin/rpki-client/test-mft.c 12 Aug 2019 18:03:17 - > 1.2 > +++ regress/usr.sbin/rpki-client/test-mft.c 22 Aug 2019 19:38:22 - > @@ -28,6 +28,7 @@ > #include > > #include "extern.h" > +int verbose = 0; > > static void > mft_print(const struct mft *p) > Index: regress/usr.sbin/rpki-client/test-roa.c > === > RCS file: /mount/openbsd/cvs/src/regress/usr.sbin/rpki-client/test-roa.c,v > retrieving revision 1.2 > diff -u -p -r1.2 test-roa.c > --- regress/usr.sbin/rpki-client/test-roa.c 12 Aug 2019 18:03:17 - > 1.2 > +++ regress/usr.sbin/rpki-client/test-roa.c 22 Aug 2019 19:38:25 - > @@ -28,6 +28,7 @@ > #include > > #include "extern.h" > +int verbose = 0; > > static void > roa_print(const struct roa *p) > Index: regress/usr.sbin/rpki-client/test-tal.c > === > RCS file: /mount/openbsd/cvs/src/regress/usr.sbin/rpki-client/test-tal.c,v > retrieving revision 1.2 > diff -u -p -r1.2 test-tal.c > --- regress/usr.sbin/rpki-client/test-tal.c 12 Aug 2019 18:03:17 - > 1.2 > +++ regress/usr.sbin/rpki-client/test-tal.c 22 Aug 2019 19:38:28 - > @@ -28,6 +28,7 @@ > #include > > #include "extern.h" > +int verbose = 0; > > static void > tal_print(const struct tal *p) > -- :wq Claudio
smtpd filters: prettify proc-exec
Hello,
When running processes with proc-exec any logging send over stderr is
not very intelligible since the process name is automatically generated
and results in , which make them hard to read.
To clean this up I reckon we can do three things:
1) Add an extra optional proc parameter that sets the name. Diff below
does this.
2) Change the process name to match the filter name.
3) Do both
I reckon we're still early enough in the development process for us to
change the default processor name in proc-exec.
Thoughts?
martijn@
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.257
diff -u -p -r1.257 parse.y
--- parse.y 11 Aug 2019 17:23:12 - 1.257
+++ parse.y 23 Aug 2019 07:00:08 -
@@ -1625,6 +1625,37 @@ FILTER STRING PROC_EXEC STRING {
filter_config = NULL;
}
|
+FILTER STRING PROC_EXEC PROC STRING STRING {
+ if (dict_get(conf->sc_processors_dict, $5)) {
+ yyerror("processor already exists with that name: %s", $5);
+ free($2);
+ free($5);
+ free($6);
+ YYERROR;
+ }
+
+ if (dict_get(conf->sc_filters_dict, $2)) {
+ yyerror("filter already exists with that name: %s", $2);
+ free($2);
+ free($5);
+ free($6);
+ YYERROR;
+ }
+
+ processor = xcalloc(1, sizeof *processor);
+ processor->command = $6;
+
+ filter_config = xcalloc(1, sizeof *filter_config);
+ filter_config->filter_type = FILTER_TYPE_PROC;
+ filter_config->name = $2;
+ filter_config->proc = $5;
+ dict_set(conf->sc_filters_dict, $2, filter_config);
+} proc_params {
+ dict_set(conf->sc_processors_dict, filter_config->proc, processor);
+ processor = NULL;
+ filter_config = NULL;
+}
+|
FILTER STRING PHASE {
if (dict_get(conf->sc_filters_dict, $2)) {
yyerror("filter already exists with that name: %s", $2);
