Re: hostctl: Change from fixed length to variable length
ok yasuoka
On Fri, 06 Jan 2023 15:14:05 +0900 (JST)
Masato Asou wrote:
> I have updated my patch.
>
> From: YASUOKA Masahiko
> Date: Tue, 27 Dec 2022 11:58:34 +0900 (JST)
>
>> After diff, it doesn't use PAGE_SIZE any more. And VMware software
>> limit seems 1MB and changable by its configuration(*1). So we can't
>> say PVBUS_KVOP_MAXSIZE is enough.
>>
>> + * - Known pv backends other than vmware has a hard limit smaller than
>> + * PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
>> + * limit at 1MB, but current open-vm-tools has a limit at 64KB
>> + * (=PVBUS_KVOP_MAXSIZE).
>
> Use this comment.
>
> From: YASUOKA Masahiko
> Date: Tue, 27 Dec 2022 12:23:39 +0900 (JST)
>
>> Also I don't think replacing strlcat() by an own calculation is necessary.
>>
>> diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
>> index 494eb40bfb0..01ecebdf4af 100644
>> --- a/sys/dev/pv/xenstore.c
>> +++ b/sys/dev/pv/xenstore.c
>> @@ -1116,11 +1116,16 @@ xs_kvop(void *xsc, int op, char *key, char *value,
>> size_t valuelen)
>> /* FALLTHROUGH */
>> case XS_LIST:
>> for (i = 0; i < iov_cnt; i++) {
>> -if (i && strlcat(value, "\n", valuelen) >= valuelen)
>> +if (i > 0 && strlcat(value, "\n", valuelen) >=
>> +valuelen) {
>> +error = ERANGE;
>> break;
>> +}
>> if (strlcat(value, iovp[i].iov_base,
>> -valuelen) >= valuelen)
>> +valuelen) >= valuelen) {
>> +error = ERANGE;
>> break;
>> +}
>> }
>> xs_resfree(, iovp, iov_cnt);
>> break;
>> @@ -1128,5 +1133,5 @@ xs_kvop(void *xsc, int op, char *key, char *value,
>> size_t valuelen)
>> break;
>> }
>>
>> -return (0);
>> +return (error);
>> }
>>
>
> And use above diff.
>
> comment, ok?
> --
> ASOU Masato
>
> Index: share/man/man4/pvbus.4
> ===
> RCS file: /cvs/src/share/man/man4/pvbus.4,v
> retrieving revision 1.14
> diff -u -p -r1.14 pvbus.4
> --- share/man/man4/pvbus.414 Jun 2017 12:42:09 - 1.14
> +++ share/man/man4/pvbus.45 Jan 2023 23:20:39 -
> @@ -125,6 +125,13 @@ Read the value from
> .Fa pvr_key
> and return it in
> .Fa pvr_value .
> +If
> +.Fa pvr_valuelen
> +is not enough for the value,
> +the command will fail and
> +.Xr errno 2
> +is set to
> +.Er ERANGE .
> .It Dv PVBUSIOC_KVTYPE
> Return the type of the attached hypervisor interface as a string in
> .Fa pvr_key ;
> Index: sys/dev/pv/hypervic.c
> ===
> RCS file: /cvs/src/sys/dev/pv/hypervic.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 hypervic.c
> --- sys/dev/pv/hypervic.c 8 Sep 2022 10:22:06 - 1.17
> +++ sys/dev/pv/hypervic.c 5 Jan 2023 23:20:40 -
> @@ -1151,11 +1151,12 @@ hv_kvop(void *arg, int op, char *key, ch
> kvpl = >kvp_pool[pool];
> if (strlen(key) == 0) {
> for (next = 0; next < MAXPOOLENTS; next++) {
> - if ((val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2) ||
> - kvp_pool_keys(kvpl, next, vp, ))
> + if (val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2)
> + return (ERANGE);
> + if (kvp_pool_keys(kvpl, next, vp, ))
> goto out;
> if (strlcat(val, "\n", vallen) >= vallen)
> - goto out;
> + return (ERANGE);
> vp += keylen;
> }
> out:
> Index: sys/dev/pv/pvbus.c
> ===
> RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 pvbus.c
> --- sys/dev/pv/pvbus.c8 Dec 2022 05:45:36 - 1.26
> +++ sys/dev/pv/pvbus.c5 Jan 2023 23:20:40 -
> @@ -399,13 +399,14 @@ pvbusgetstr(size_t srclen, const char *s
>
> /*
>* Reject size that is too short or obviously too long:
> - * - at least one byte for the nul terminator.
> - * - PAGE_SIZE is an arbitrary value, but known pv backends seem
> - * to have a hard (PAGE_SIZE - x) limit in their messaging.
> + * - Known pv backends other than vmware have a hard limit smaller than
> + * PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
> + * limit at 1MB, but current open-vm-tools has a limit at 64KB
> + * (=PVBUS_KVOP_MAXSIZE).
>*/
> if (srclen < 1)
> return (EINVAL);
> - else if (srclen > PAGE_SIZE)
> + else if (srclen > PVBUS_KVOP_MAXSIZE)
> return (ENAMETOOLONG);
>
Re: hostctl: Change from fixed length to variable length
I have updated my patch.
From: YASUOKA Masahiko
Date: Tue, 27 Dec 2022 11:58:34 +0900 (JST)
> After diff, it doesn't use PAGE_SIZE any more. And VMware software
> limit seems 1MB and changable by its configuration(*1). So we can't
> say PVBUS_KVOP_MAXSIZE is enough.
>
> + * - Known pv backends other than vmware has a hard limit smaller than
> + * PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
> + * limit at 1MB, but current open-vm-tools has a limit at 64KB
> + * (=PVBUS_KVOP_MAXSIZE).
Use this comment.
From: YASUOKA Masahiko
Date: Tue, 27 Dec 2022 12:23:39 +0900 (JST)
> Also I don't think replacing strlcat() by an own calculation is necessary.
>
> diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
> index 494eb40bfb0..01ecebdf4af 100644
> --- a/sys/dev/pv/xenstore.c
> +++ b/sys/dev/pv/xenstore.c
> @@ -1116,11 +1116,16 @@ xs_kvop(void *xsc, int op, char *key, char *value,
> size_t valuelen)
> /* FALLTHROUGH */
> case XS_LIST:
> for (i = 0; i < iov_cnt; i++) {
> - if (i && strlcat(value, "\n", valuelen) >= valuelen)
> + if (i > 0 && strlcat(value, "\n", valuelen) >=
> + valuelen) {
> + error = ERANGE;
> break;
> + }
> if (strlcat(value, iovp[i].iov_base,
> - valuelen) >= valuelen)
> + valuelen) >= valuelen) {
> + error = ERANGE;
> break;
> + }
> }
> xs_resfree(, iovp, iov_cnt);
> break;
> @@ -1128,5 +1133,5 @@ xs_kvop(void *xsc, int op, char *key, char *value,
> size_t valuelen)
> break;
> }
>
> - return (0);
> + return (error);
> }
>
And use above diff.
comment, ok?
--
ASOU Masato
Index: share/man/man4/pvbus.4
===
RCS file: /cvs/src/share/man/man4/pvbus.4,v
retrieving revision 1.14
diff -u -p -r1.14 pvbus.4
--- share/man/man4/pvbus.4 14 Jun 2017 12:42:09 - 1.14
+++ share/man/man4/pvbus.4 5 Jan 2023 23:20:39 -
@@ -125,6 +125,13 @@ Read the value from
.Fa pvr_key
and return it in
.Fa pvr_value .
+If
+.Fa pvr_valuelen
+is not enough for the value,
+the command will fail and
+.Xr errno 2
+is set to
+.Er ERANGE .
.It Dv PVBUSIOC_KVTYPE
Return the type of the attached hypervisor interface as a string in
.Fa pvr_key ;
Index: sys/dev/pv/hypervic.c
===
RCS file: /cvs/src/sys/dev/pv/hypervic.c,v
retrieving revision 1.17
diff -u -p -r1.17 hypervic.c
--- sys/dev/pv/hypervic.c 8 Sep 2022 10:22:06 - 1.17
+++ sys/dev/pv/hypervic.c 5 Jan 2023 23:20:40 -
@@ -1151,11 +1151,12 @@ hv_kvop(void *arg, int op, char *key, ch
kvpl = >kvp_pool[pool];
if (strlen(key) == 0) {
for (next = 0; next < MAXPOOLENTS; next++) {
- if ((val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2) ||
- kvp_pool_keys(kvpl, next, vp, ))
+ if (val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2)
+ return (ERANGE);
+ if (kvp_pool_keys(kvpl, next, vp, ))
goto out;
if (strlcat(val, "\n", vallen) >= vallen)
- goto out;
+ return (ERANGE);
vp += keylen;
}
out:
Index: sys/dev/pv/pvbus.c
===
RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
retrieving revision 1.26
diff -u -p -r1.26 pvbus.c
--- sys/dev/pv/pvbus.c 8 Dec 2022 05:45:36 - 1.26
+++ sys/dev/pv/pvbus.c 5 Jan 2023 23:20:40 -
@@ -399,13 +399,14 @@ pvbusgetstr(size_t srclen, const char *s
/*
* Reject size that is too short or obviously too long:
-* - at least one byte for the nul terminator.
-* - PAGE_SIZE is an arbitrary value, but known pv backends seem
-* to have a hard (PAGE_SIZE - x) limit in their messaging.
+* - Known pv backends other than vmware have a hard limit smaller than
+* PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
+* limit at 1MB, but current open-vm-tools has a limit at 64KB
+* (=PVBUS_KVOP_MAXSIZE).
*/
if (srclen < 1)
return (EINVAL);
- else if (srclen > PAGE_SIZE)
+ else if (srclen > PVBUS_KVOP_MAXSIZE)
return (ENAMETOOLONG);
*dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
Index: sys/dev/pv/pvvar.h
===
RCS file: /cvs/src/sys/dev/pv/pvvar.h,v
Re: ifconfig description for wireguard peers
On Mon, Nov 29, 2021 at 01:25:20PM +0100, Stefan Sperling wrote: > This looks useful to me. > Did you get any feedback for this patch yet, Noah? Not tested the patch, but this functionality will be huge quality of life improvement for me. So from the user perspective, very, very nice to have. Best regards, Chris Narkiewicz
sparc64: normalize prom mappings
tl;dr: if you have a sparc64 machine, please try this diff and report if
your system no longer works with it.
On sparc64, the kernel keeps the existing OpenFirmware memory mappings
into the kernel pmap, so as to be able to use ofw routines and walk the
device tree.
However, these translation table entries have a few, software-defined,
bits, which matter to the OpenBSD kernel and may or may not matter to
the PROM.
Recent experiments have shown that, on at least one sparc64 system,
one of these software bits is set in the translations inherited from
OpenFirmware, and this could be misleading.
A quick examination of property dumps (eeprom -p, "translations"
property) collected from various sparc64 systems shows that apparently
most, if not all, systems with UltraSPARC I/II/IIi/IIe processors have
TTE values with unwanted bits set.
The following diff normalizes these values by clearing all
software-defined bits, when adding them to the kernel pmap. It needs to
be tested on as many systems as possible - if they still boot multiuser,
and can reboot without problems, then everything's fine. I'd like to
hear about systems no longer working correctly with this diff, should
there be any.
Miod
Index: sys/arch/sparc64/sparc64/pmap.c
===
RCS file: /OpenBSD/src/sys/arch/sparc64/sparc64/pmap.c,v
retrieving revision 1.106
diff -u -p -r1.106 pmap.c
--- sys/arch/sparc64/sparc64/pmap.c 10 Sep 2022 20:35:29 - 1.106
+++ sys/arch/sparc64/sparc64/pmap.c 5 Jan 2023 19:48:09 -
@@ -1074,6 +1074,7 @@ remap_data:
if (prom_map[i].vstart && ((prom_map[i].vstart>>32) == 0)) {
for (j = 0; j < prom_map[i].vsize; j += NBPG) {
int k;
+ uint64_t tte;
for (k = 0; page_size_map[k].mask; k++) {
if (((prom_map[i].vstart |
@@ -1084,9 +1085,14 @@ remap_data:
break;
}
/* Enter PROM map into pmap_kernel() */
+ tte = prom_map[i].tte;
+ if (CPU_ISSUN4V)
+ tte &= ~SUN4V_TLB_SOFT_MASK;
+ else
+ tte &= ~(SUN4U_TLB_SOFT2_MASK |
+ SUN4U_TLB_SOFT_MASK);
pmap_enter_kpage(prom_map[i].vstart + j,
- (prom_map[i].tte + j)|data|
- page_size_map[k].code);
+ (tte + j) | data | page_size_map[k].code);
}
}
}
Re: ifconfig description for wireguard peers
On 2.1.2023. 22:01, Mikolaj Kucharski wrote:
> This seems to work fine for me.
>
> Patch also available at:
>
> https://marc.info/?l=openbsd-tech=167185582521873=mbox
>
I've had some problems with 20+ wgpeers few days ago and at that time it
would have been good if I had wgdesc in ifconfig wg output ...
>
> On Sat, Dec 24, 2022 at 03:29:35AM +, Mikolaj Kucharski wrote:
>> On Sat, Nov 19, 2022 at 12:03:59PM +, Mikolaj Kucharski wrote:
>>> Kind reminder.
>>>
>>> Below diff also at:
>>>
>>> https://marc.info/?l=openbsd-tech=166806412910623=2
>>>
>>> This is diff by Noah Meier with small changes by me.
>>>
>>>
>>> On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
>> Hi,
>>
>> While wireguard interfaces can have a description set by ifconfig,
>> wireguard peers currently cannot. I now have a lot of peers and
>> descriptions of them in ifconfig would be helpful.
>>
>> This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
>> corresponding '-wgdesc' option). Man page also updated.
>>
>> NM
>
> Now that my `ifconfig, wireguard output less verbose, unless -A or `
> diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> bump of an old thread.
>
> Below is rebased on -current and tiny modified by me, Noah's diff.
>
> You need both kernel and ifconfig with below code, otherwise you may see
> issues bringing up wg(4) interface. If you may loose access to machine
> behind wg(4) VPN, make sure you update on that machine both kernel and
> ifconfig(8) at the same time.
>
>>
>> Rebased again, just a moment ago. Will test runtime again over the weekend,
>> are there no surprises.
>>
>> - ifconfig compiles
>> - GENERIC.MP/amd64 kernel compiles too
>>
>>
>> Index: sbin/ifconfig/ifconfig.c
>> ===
>> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
>> retrieving revision 1.460
>> diff -u -p -u -r1.460 ifconfig.c
>> --- sbin/ifconfig/ifconfig.c 18 Dec 2022 18:56:38 - 1.460
>> +++ sbin/ifconfig/ifconfig.c 24 Dec 2022 00:49:05 -
>> @@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
>> voidsetwgpeeraip(const char *, int);
>> voidsetwgpeerpsk(const char *, int);
>> voidsetwgpeerpka(const char *, int);
>> +voidsetwgpeerdesc(const char *, int);
>> voidsetwgport(const char *, int);
>> voidsetwgkey(const char *, int);
>> voidsetwgrtable(const char *, int);
>>
>> voidunsetwgpeer(const char *, int);
>> voidunsetwgpeerpsk(const char *, int);
>> +voidunsetwgpeerdesc(const char *, int);
>> voidunsetwgpeerall(const char *, int);
>>
>> voidwg_status(int);
>> @@ -623,11 +625,13 @@ const struct cmd {
>> { "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
>> { "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
>> { "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
>> +{ "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
>> { "wgport", NEXTARG,A_WIREGUARD,setwgport},
>> { "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
>> { "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
>> { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
>> { "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
>> +{ "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
>> { "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
>>
>> #else /* SMALL */
>> @@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
>> }
>>
>> void
>> +setwgpeerdesc(const char *wgdesc, int param)
>> +{
>> +if (wg_peer == NULL)
>> +errx(1, "wgdesc: wgpeer not set");
>> +if (strlen(wgdesc))
>> +strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
>> +wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
>> +}
>> +
>> +void
>> setwgport(const char *port, int param)
>> {
>> const char *errmsg = NULL;
>> @@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
>> }
>>
>> void
>> +unsetwgpeerdesc(const char *value, int param)
>> +{
>> +if (wg_peer == NULL)
>> +errx(1, "wgdesc: wgpeer not set");
>> +strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
>> +wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
>> +}
>> +
>> +void
>> unsetwgpeerall(const char *value, int param)
>> {
>> ensurewginterface();
>> @@ -5961,6 +5984,9 @@ wg_status(int ifaliases)
>> b64_ntop(wg_peer->p_public, WG_KEY_LEN,
>> key, sizeof(key));
>> printf("\twgpeer %s\n", key);
>> +
>> +if (strlen(wg_peer->p_description))
>> +
Re: Fix level-triggered ACPI GPIO interrupts on amd64
Hi Peter, * Peter Hessler wrote: > This was committed on Oct 20, and was shipped in OpenBSD 7.2. Indeed, you're quite right, thanks for the info! Since my touchpad hangs haven't improved since then it is unrelated to the change and I must be mistaken while testing. Cheers Matthias
Re: bgpd vs gcc4
On Thu, Jan 05, 2023 at 02:18:47PM +0100, David Demelier wrote:
> On Thu, 2023-01-05 at 13:59 +0100, Claudio Jeker wrote:
> > On Thu, Jan 05, 2023 at 11:09:57AM +0100, Theo Buehler wrote:
> > > On Thu, Jan 05, 2023 at 11:03:04AM +0100, Claudio Jeker wrote:
> > > > gcc4 does not really support C99 initalizers. It works most of
> > > > the time
> > > > but fails for more complex structs. Just fall back to memset()
> > > > here.
> > >
> > > deraadt used { {0} } in kr_send_dependon(). Apparently that works.
> > > I really don't understand why we can't use a 24 years old standard.
> >
> > It is very annoying. Would be nice if our gcc would allow it since
> > c99
> > initalizers are a good thing. Not sure I like the { { 0 } } fix.
> > Another
>
> Also note that = {0} is available since C89, it's only C99 that
> introduced designators (e.g. { .foo = 0 }) so there is no reason why
> GCC/clang should not accept that unless there is a bug.
>
> https://godbolt.org/z/6eMWdEEaP
>
The problem here is that in C89 you can't use = { 0 } for a struct that is
defined like:
struct foo {
struct bar {
int x;
} a;
int b;
};
{ 0 } is somewhat special in C99 since it works independent of object.
--
:wq Claudio
Re: bgpd vs gcc4
> Also I thought that gcc4 defaults to -std=c99 or gnu99. Reading the code, it appears to default to c89.
Re: bgpd vs gcc4
On Thu, Jan 05, 2023 at 06:01:37AM -0700, Theo de Raadt wrote:
> Florian Obser wrote:
>
> > On 2023-01-05 11:09 +01, Theo Buehler wrote:
> > > On Thu, Jan 05, 2023 at 11:03:04AM +0100, Claudio Jeker wrote:
> > >> gcc4 does not really support C99 initalizers. It works most of the time
> > >> but fails for more complex structs. Just fall back to memset() here.
> > >
> > > deraadt used { {0} } in kr_send_dependon(). Apparently that works.
> > > I really don't understand why we can't use a 24 years old standard.
> >
> > We ran into an c99 issue in nsd and upstream was quite adamant that they
> > want to keep using c99 stuff. Turned out gcc4 does not support all of
> > c99 by default so they did some autoconf magic that sprinkled -std=c99
> > into the compiler invocation when needed. This also worked on gcc3
> > archs. Which apparently also fully supports c99 if you ask it nicely.
>
> Maybe our old gcc should be flag-tweaked internally to always have that
> on. Are there any downsides?
The warning is not silenced by -std=c99:
cc -O2 -pipe -std=c99 -Wall -I/usr/src/usr.sbin/bgpd -Wstrict-prototypes
-Wmissing-prototypes -Wmissing-declarations -Wshadow -Wpointer-arith
-Wcast-qual -Wsign-compare -Werror-implicit-function-declaration -MD -MP
-c /usr/src/usr.sbin/bgpd/kroute.c
/usr/src/usr.sbin/bgpd/kroute.c: In function 'knexthop_true_nexthop':
/usr/src/usr.sbin/bgpd/kroute.c:2162: warning: missing braces around initializer
/usr/src/usr.sbin/bgpd/kroute.c:2162: warning: (near initialization for
'gateway.ba')
Also I thought that gcc4 defaults to -std=c99 or gnu99.
--
:wq Claudio
Re: more consistently use "st" as the name for struct pf_state variables
On Thu, Jan 05, 2023 at 08:32:44PM +1000, David Gwynne wrote:
>
>
> > On 5 Jan 2023, at 18:56, Alexandr Nedvedicky wrote:
> >
> > Hello,
> >
> > On Wed, Jan 04, 2023 at 09:36:38PM +1000, David Gwynne wrote:
> >> and "stp" for pf_state ** variables.
> >>
> >I agree with established naming conventions.
> >
> >I'm also fine with keeping some exceptions such as `a` and `b`
> >in pf_state_compare_id(), local variables `tail`, `head`
> >in pf_states_{clr, get}() and pf_purge_expired_states().
> >I'm also fine with leaving static variable `cur` unchanged.
> >
> > is there any reason we still keep `pf_state **sm` argument
> > in pf_test_rule()? the same in pf_create_state(). Is it intended?
>
> there were a bunch of other arguments that ended with m. happy to change it
> to **stp though. we can always do another sweep for other types.
>
I would change those occurrences to stp. my point is that we don't
expect pf_test_rule() to return matching state.
with those tweaks diff is OK
thanks and
regards
sashan
Re: [PATCH] Azalia bug in codec Realtek ALC892
El 5/1/23 a las 8:07, Alexandre Ratchov escribió: Your device seems to stop because the azalia pci host stops (probably interrupts stop and a reboot is needed), while this diff is about the Yes,exactly. A quick review using vmstat -i show that, azalia interrupts is frozen, only hard reboot resolve this temporaly. azalia codec (i.e. about handling the "analog" parts). So, I'm very surprised this fixes the random audio hangs (though any strange interaction between the host and the codec can't be excluded). For the patch I investigate various things: 1.- Using sndiod -mplay option the audio stuck randomly. This workaround is very use in other cases, but in mine don´t work. 2.- Disabled MSI signaling. In the Hudson audio problem, disable MSI signaling solve problem. AMD Summit Ridge/Raven Ridge HD Audio need this fix for work correctly, but in my case the audio don´t work. 3.- I tried using PCIE SNOOP aproximation (used in AMD Summit Ridge/Raven Ridge HD Audio) but again, the audio don't work. 4.- Investigating ALC892 datasheet, I can see that ALC888 and ALC892 are "mostly" same codecs in specs. ALC888 use same code: * this->qrks |= AZ_QRK_WID_CDIN_1C | AZ_QRK_WID_BEEP_1D; * In this case I use a conditional to apply this code and it works, there is no audio problem with this patch. The only problem I see is that it is very specific to my hardware case (subid), but since the hardware and its configuration can change, enabling the option globally to the codec may end up affecting the audio of other configurations. Are there other changes that may have fixed audio? Any BIOS tweaks? Suspend-resume cycles? Any devices disabled meanwhile? No, BIOS config is same, not changes or updates. Suspend-resume is a feature that I'm not use. Same device, nothing disabled. Could you test with and without the diff applied in exactly the same conditions? I test the patch using this approx: 1.- Using mpd for listen audio only. Without patch audio stuck in a short and randomly time (few minutes, generally less a hour) 2.- Using various apps streaming audio (Chrome, Firefox and MPD, precisely) in same time, the audio stuck very quickly without patch. 3.- Recording, audio stuck in few minutes (using Audacity). In all case, the patch resolve problem for me, various hours playing/recording and no audio stuck. These hangs are a longstanding problem (and hard to debug because of the randomness of the hangs), so I'm very intested in understanding what's causing it. Thanks Yeah, I can see this, in my investigation I see reports from 2018, and in various case the issue is very strange. *** Note: Sorry for private, I'm remember how use the mailing list *** -- Dios en su cielo, todo bien en la Tierra
Re: [PATCH] Azalia bug in codec Realtek ALC892
El 5/1/23 a las 1:41, Greg Steuck escribió: Thanks for the contribution Jose. The diff is OK gnezdo. Aaron, in case you still have that hardware, maybe you would like to test/commit? Thanks Greg Thanks! The patch is very specific in hardware but maybe work in other cases, need testing. -- Dios en su cielo, todo bien en la Tierra
Re: Fix level-triggered ACPI GPIO interrupts on amd64
This was committed on Oct 20, and was shipped in OpenBSD 7.2.
On 2023 Jan 05 (Thu) at 14:15:26 +0100 (+0100), Matthias Schmidt wrote:
:Hi,
:
:did anyone else on the list had the chance to test this patch? It
:really improved the touchpad hangs here.
:
:Cheers
:
: Matthias
:
:* Mark Kettenis wrote:
:> > Date: Thu, 13 Oct 2022 00:17:37 +0200
:> > From: Mark Kettenis
:> >
:> > > Date: Mon, 10 Oct 2022 17:02:41 +0200
:> > > From: Matthias Schmidt
:> > >
:> > > * Matthias Schmidt wrote:
:> > > > Hi Mark,
:> > > >
:> > > > Addendum after 24h of testing. Your patch fixes the frequent touchpad
:> > > > freezes I see on this model and which I reported back then in
:> > > > https://marc.info/?l=openbsd-bugs=165328803822857=2
:> > >
:> > > Any chance that this patch gets committed or wider testing in snaps?
:> > > It really improved the touchpad situation here.
:> >
:> > Right, I should probably just commit the diff instead of waiting for
:> > more tests.
:> >
:> > ok?
:>
:> For reference, here is the diff again.
:>
:>
:> Index: dev/acpi/amdgpio.c
:> ===
:> RCS file: /cvs/src/sys/dev/acpi/amdgpio.c,v
:> retrieving revision 1.9
:> diff -u -p -r1.9 amdgpio.c
:> --- dev/acpi/amdgpio.c 27 Jun 2022 08:00:31 - 1.9
:> +++ dev/acpi/amdgpio.c 3 Oct 2022 19:10:03 -
:> @@ -92,6 +92,8 @@ const char *amdgpio_hids[] = {
:> int amdgpio_read_pin(void *, int);
:> voidamdgpio_write_pin(void *, int, int);
:> voidamdgpio_intr_establish(void *, int, int, int (*)(void *), void
*);
:> +voidamdgpio_intr_enable(void *, int);
:> +voidamdgpio_intr_disable(void *, int);
:> int amdgpio_pin_intr(struct amdgpio_softc *, int);
:> int amdgpio_intr(void *);
:> voidamdgpio_save_pin(struct amdgpio_softc *, int pin);
:> @@ -163,6 +165,8 @@ amdgpio_attach(struct device *parent, st
:> sc->sc_gpio.read_pin = amdgpio_read_pin;
:> sc->sc_gpio.write_pin = amdgpio_write_pin;
:> sc->sc_gpio.intr_establish = amdgpio_intr_establish;
:> +sc->sc_gpio.intr_enable = amdgpio_intr_enable;
:> +sc->sc_gpio.intr_disable = amdgpio_intr_disable;
:> sc->sc_node->gpio = >sc_gpio;
:>
:> printf(", %d pins\n", sc->sc_npins);
:> @@ -275,6 +279,32 @@ amdgpio_intr_establish(void *cookie, int
:> if ((flags & LR_GPIO_POLARITY) == LR_GPIO_ACTBOTH)
:> reg |= AMDGPIO_CONF_ACTBOTH;
:> reg |= (AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
:> +bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
:> +}
:> +
:> +void
:> +amdgpio_intr_enable(void *cookie, int pin)
:> +{
:> +struct amdgpio_softc *sc = cookie;
:> +uint32_t reg;
:> +
:> +KASSERT(pin >= 0 && pin != 63 && pin < sc->sc_npins);
:> +
:> +reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
:> +reg |= (AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
:> +bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
:> +}
:> +
:> +void
:> +amdgpio_intr_disable(void *cookie, int pin)
:> +{
:> +struct amdgpio_softc *sc = cookie;
:> +uint32_t reg;
:> +
:> +KASSERT(pin >= 0 && pin != 63 && pin < sc->sc_npins);
:> +
:> +reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
:> +reg &= ~(AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
:> bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
:> }
:>
:> Index: dev/acpi/aplgpio.c
:> ===
:> RCS file: /cvs/src/sys/dev/acpi/aplgpio.c,v
:> retrieving revision 1.5
:> diff -u -p -r1.5 aplgpio.c
:> --- dev/acpi/aplgpio.c 6 Apr 2022 18:59:27 - 1.5
:> +++ dev/acpi/aplgpio.c 3 Oct 2022 19:10:03 -
:> @@ -76,6 +76,8 @@ const char *aplgpio_hids[] = {
:> int aplgpio_read_pin(void *, int);
:> voidaplgpio_write_pin(void *, int, int);
:> voidaplgpio_intr_establish(void *, int, int, int (*)(void *), void
*);
:> +voidaplgpio_intr_enable(void *, int);
:> +voidaplgpio_intr_disable(void *, int);
:> int aplgpio_intr(void *);
:>
:> int
:> @@ -150,6 +152,8 @@ aplgpio_attach(struct device *parent, st
:> sc->sc_gpio.read_pin = aplgpio_read_pin;
:> sc->sc_gpio.write_pin = aplgpio_write_pin;
:> sc->sc_gpio.intr_establish = aplgpio_intr_establish;
:> +sc->sc_gpio.intr_enable = aplgpio_intr_enable;
:> +sc->sc_gpio.intr_disable = aplgpio_intr_disable;
:> sc->sc_node->gpio = >sc_gpio;
:>
:> /* Mask and clear all interrupts. */
:> @@ -227,6 +231,36 @@ aplgpio_intr_establish(void *cookie, int
:> reg = bus_space_read_4(sc->sc_memt, sc->sc_memh,
:> APLGPIO_IRQ_EN + (pin / 32) * 4);
:> reg |= (1 << (pin % 32));
:> +bus_space_write_4(sc->sc_memt, sc->sc_memh,
:> +APLGPIO_IRQ_EN + (pin / 32) * 4, reg);
:> +}
:> +
:> +void
:> +aplgpio_intr_enable(void *cookie, int pin)
:> +{
:> +struct aplgpio_softc *sc = cookie;
:> +uint32_t reg;
:> +
:> +KASSERT(pin
Re: bgpd vs gcc4
On Thu, 2023-01-05 at 13:59 +0100, Claudio Jeker wrote:
> On Thu, Jan 05, 2023 at 11:09:57AM +0100, Theo Buehler wrote:
> > On Thu, Jan 05, 2023 at 11:03:04AM +0100, Claudio Jeker wrote:
> > > gcc4 does not really support C99 initalizers. It works most of
> > > the time
> > > but fails for more complex structs. Just fall back to memset()
> > > here.
> >
> > deraadt used { {0} } in kr_send_dependon(). Apparently that works.
> > I really don't understand why we can't use a 24 years old standard.
>
> It is very annoying. Would be nice if our gcc would allow it since
> c99
> initalizers are a good thing. Not sure I like the { { 0 } } fix.
> Another
Also note that = {0} is available since C89, it's only C99 that
introduced designators (e.g. { .foo = 0 }) so there is no reason why
GCC/clang should not accept that unless there is a bug.
https://godbolt.org/z/6eMWdEEaP
--
David
Re: Fix level-triggered ACPI GPIO interrupts on amd64
Hi,
did anyone else on the list had the chance to test this patch? It
really improved the touchpad hangs here.
Cheers
Matthias
* Mark Kettenis wrote:
> > Date: Thu, 13 Oct 2022 00:17:37 +0200
> > From: Mark Kettenis
> >
> > > Date: Mon, 10 Oct 2022 17:02:41 +0200
> > > From: Matthias Schmidt
> > >
> > > * Matthias Schmidt wrote:
> > > > Hi Mark,
> > > >
> > > > Addendum after 24h of testing. Your patch fixes the frequent touchpad
> > > > freezes I see on this model and which I reported back then in
> > > > https://marc.info/?l=openbsd-bugs=165328803822857=2
> > >
> > > Any chance that this patch gets committed or wider testing in snaps?
> > > It really improved the touchpad situation here.
> >
> > Right, I should probably just commit the diff instead of waiting for
> > more tests.
> >
> > ok?
>
> For reference, here is the diff again.
>
>
> Index: dev/acpi/amdgpio.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/amdgpio.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 amdgpio.c
> --- dev/acpi/amdgpio.c27 Jun 2022 08:00:31 - 1.9
> +++ dev/acpi/amdgpio.c3 Oct 2022 19:10:03 -
> @@ -92,6 +92,8 @@ const char *amdgpio_hids[] = {
> int amdgpio_read_pin(void *, int);
> void amdgpio_write_pin(void *, int, int);
> void amdgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void amdgpio_intr_enable(void *, int);
> +void amdgpio_intr_disable(void *, int);
> int amdgpio_pin_intr(struct amdgpio_softc *, int);
> int amdgpio_intr(void *);
> void amdgpio_save_pin(struct amdgpio_softc *, int pin);
> @@ -163,6 +165,8 @@ amdgpio_attach(struct device *parent, st
> sc->sc_gpio.read_pin = amdgpio_read_pin;
> sc->sc_gpio.write_pin = amdgpio_write_pin;
> sc->sc_gpio.intr_establish = amdgpio_intr_establish;
> + sc->sc_gpio.intr_enable = amdgpio_intr_enable;
> + sc->sc_gpio.intr_disable = amdgpio_intr_disable;
> sc->sc_node->gpio = >sc_gpio;
>
> printf(", %d pins\n", sc->sc_npins);
> @@ -275,6 +279,32 @@ amdgpio_intr_establish(void *cookie, int
> if ((flags & LR_GPIO_POLARITY) == LR_GPIO_ACTBOTH)
> reg |= AMDGPIO_CONF_ACTBOTH;
> reg |= (AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
> + bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
> +}
> +
> +void
> +amdgpio_intr_enable(void *cookie, int pin)
> +{
> + struct amdgpio_softc *sc = cookie;
> + uint32_t reg;
> +
> + KASSERT(pin >= 0 && pin != 63 && pin < sc->sc_npins);
> +
> + reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
> + reg |= (AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
> + bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
> +}
> +
> +void
> +amdgpio_intr_disable(void *cookie, int pin)
> +{
> + struct amdgpio_softc *sc = cookie;
> + uint32_t reg;
> +
> + KASSERT(pin >= 0 && pin != 63 && pin < sc->sc_npins);
> +
> + reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
> + reg &= ~(AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
> bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
> }
>
> Index: dev/acpi/aplgpio.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/aplgpio.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 aplgpio.c
> --- dev/acpi/aplgpio.c6 Apr 2022 18:59:27 - 1.5
> +++ dev/acpi/aplgpio.c3 Oct 2022 19:10:03 -
> @@ -76,6 +76,8 @@ const char *aplgpio_hids[] = {
> int aplgpio_read_pin(void *, int);
> void aplgpio_write_pin(void *, int, int);
> void aplgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void aplgpio_intr_enable(void *, int);
> +void aplgpio_intr_disable(void *, int);
> int aplgpio_intr(void *);
>
> int
> @@ -150,6 +152,8 @@ aplgpio_attach(struct device *parent, st
> sc->sc_gpio.read_pin = aplgpio_read_pin;
> sc->sc_gpio.write_pin = aplgpio_write_pin;
> sc->sc_gpio.intr_establish = aplgpio_intr_establish;
> + sc->sc_gpio.intr_enable = aplgpio_intr_enable;
> + sc->sc_gpio.intr_disable = aplgpio_intr_disable;
> sc->sc_node->gpio = >sc_gpio;
>
> /* Mask and clear all interrupts. */
> @@ -227,6 +231,36 @@ aplgpio_intr_establish(void *cookie, int
> reg = bus_space_read_4(sc->sc_memt, sc->sc_memh,
> APLGPIO_IRQ_EN + (pin / 32) * 4);
> reg |= (1 << (pin % 32));
> + bus_space_write_4(sc->sc_memt, sc->sc_memh,
> + APLGPIO_IRQ_EN + (pin / 32) * 4, reg);
> +}
> +
> +void
> +aplgpio_intr_enable(void *cookie, int pin)
> +{
> + struct aplgpio_softc *sc = cookie;
> + uint32_t reg;
> +
> + KASSERT(pin >= 0 && pin < sc->sc_npins);
> +
> + reg = bus_space_read_4(sc->sc_memt, sc->sc_memh,
> + APLGPIO_IRQ_EN + (pin / 32) * 4);
> + reg |= (1 << (pin % 32));
> + bus_space_write_4(sc->sc_memt, sc->sc_memh,
> + APLGPIO_IRQ_EN + (pin / 32) * 4, reg);
> +}
> +
>
Re: bgpd vs gcc4
Florian Obser wrote:
> On 2023-01-05 11:09 +01, Theo Buehler wrote:
> > On Thu, Jan 05, 2023 at 11:03:04AM +0100, Claudio Jeker wrote:
> >> gcc4 does not really support C99 initalizers. It works most of the time
> >> but fails for more complex structs. Just fall back to memset() here.
> >
> > deraadt used { {0} } in kr_send_dependon(). Apparently that works.
> > I really don't understand why we can't use a 24 years old standard.
>
> We ran into an c99 issue in nsd and upstream was quite adamant that they
> want to keep using c99 stuff. Turned out gcc4 does not support all of
> c99 by default so they did some autoconf magic that sprinkled -std=c99
> into the compiler invocation when needed. This also worked on gcc3
> archs. Which apparently also fully supports c99 if you ask it nicely.
Maybe our old gcc should be flag-tweaked internally to always have that
on. Are there any downsides?
Re: bgpd vs gcc4
On Thu, Jan 05, 2023 at 11:09:57AM +0100, Theo Buehler wrote:
> On Thu, Jan 05, 2023 at 11:03:04AM +0100, Claudio Jeker wrote:
> > gcc4 does not really support C99 initalizers. It works most of the time
> > but fails for more complex structs. Just fall back to memset() here.
>
> deraadt used { {0} } in kr_send_dependon(). Apparently that works.
> I really don't understand why we can't use a 24 years old standard.
It is very annoying. Would be nice if our gcc would allow it since c99
initalizers are a good thing. Not sure I like the { { 0 } } fix. Another
option is to init a specific value e.g.
struct bgpd_addr gateway = { .aid = 0 };
but again this is just an ugly workaround.
We could silence the warning in gcc but I'm not a big fan of that since
struct bgpd_addr gateway = { 1 };
should get us a warning.
gcc fixed this in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64709
but the code was already quite different to what we have.
As a third option one could use the C23
struct bgpd_addr gateway = { };
This seems to work with our gcc and llvm.
> > --
> > :wq Claudio
> >
> > Index: kroute.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> > retrieving revision 1.303
> > diff -u -p -r1.303 kroute.c
> > --- kroute.c28 Dec 2022 21:30:16 - 1.303
> > +++ kroute.c5 Jan 2023 09:55:39 -
> > @@ -2159,10 +2159,12 @@ kroute6_validate(struct kroute6 *kr)
> > int
> > knexthop_true_nexthop(struct ktable *kt, struct kroute_full *kf)
> > {
> > - struct bgpd_addr gateway = { 0 };
> > + struct bgpd_addr gateway;
> > struct knexthop *kn;
> > struct kroute *kr;
> > struct kroute6 *kr6;
> > +
> > + memset(, 0, sizeof(gateway));
> >
> > /*
> > * Ignore the nexthop for VPN routes. The gateway is forced
> >
>
--
:wq Claudio
Re: bgpd vs gcc4
On 2023-01-05 11:09 +01, Theo Buehler wrote:
> On Thu, Jan 05, 2023 at 11:03:04AM +0100, Claudio Jeker wrote:
>> gcc4 does not really support C99 initalizers. It works most of the time
>> but fails for more complex structs. Just fall back to memset() here.
>
> deraadt used { {0} } in kr_send_dependon(). Apparently that works.
> I really don't understand why we can't use a 24 years old standard.
We ran into an c99 issue in nsd and upstream was quite adamant that they
want to keep using c99 stuff. Turned out gcc4 does not support all of
c99 by default so they did some autoconf magic that sprinkled -std=c99
into the compiler invocation when needed. This also worked on gcc3
archs. Which apparently also fully supports c99 if you ask it nicely.
Re: [PATCH] Azalia bug in codec Realtek ALC892
On Tue, Jan 03, 2023 at 09:51:46PM -0400, Jose Maldonado wrote:
> Hi, everyone!
>
> Right now I'm using OBSD 7.2 (my first time in OBSD) and the only issue is
> my onboard audio. The audio randomly stuck (listen music or not, with high
> CPU use or not) and only a hard reboot give me back audio to next stuck.
>
> In dmesg don´t show nothing, but sndiod - give me this output:
>
> * snd/0 watchdog timeout *
>
> The error appear in debug info and my audio is gone. The issue is not new
> more info in this links:
>
> https://www.reddit.com/r/openbsd/comments/mybklx/keep_losing_audio_with_azalia_on_current/
>
> https://www.mail-archive.com/tech@openbsd.org/msg46701.html
>
> https://deftly.net/posts/2018-10-15-openbsd-on-lenovo-a485.html
>
> With this info I realized this patch (directly on OBSD-stable source, sorry)
> and...work! Not audio problems in my PC using this patch (one day testing,
> multiple apps using audio, not stuck/not latency problems).
>
> If anybody can test, verify and give OK to this simple patch, I would
> appreciate it (and I'm sure others would too, specially living in -release
> versions).
>
> PATCH:
>
> Index: azalia_codec.c
> ===
> RCS file: /cvs/src/sys/dev/pci/azalia_codec.c,v
> retrieving revision 1.189
> diff -u -p -u -r1.189 azalia_codec.c
> --- azalia_codec.c8 Sep 2022 01:35:39 - 1.189
> +++ azalia_codec.c2 Jan 2023 19:42:28 -
> @@ -312,6 +312,9 @@ azalia_codec_init_vtbl(codec_t *this)
> break;
> case 0x10ec0892:
> this->name = "Realtek ALC892";
> + if (this->subid == 0x1462ec95) {
> + this->qrks |= AZ_QRK_WID_CDIN_1C | AZ_QRK_WID_BEEP_1D;
> + }
> break;
> case 0x10ec0897:
> this->name = "Realtek ALC897";
>
>
Your device seems to stop because the azalia pci host stops (probably
interrupts stop and a reboot is needed), while this diff is about the
azalia codec (i.e. about handling the "analog" parts). So, I'm very
surprised this fixes the random audio hangs (though any strange
interaction between the host and the codec can't be excluded).
Are there other changes that may have fixed audio? Any BIOS tweaks?
Suspend-resume cycles? Any devices disabled meanwhile?
Could you test with and without the diff applied in exactly the same
conditions?
These hangs are a longstanding problem (and hard to debug because of
the randomness of the hangs), so I'm very intested in understanding
what's causing it. Thanks
Re: more consistently use "st" as the name for struct pf_state variables
> On 5 Jan 2023, at 18:56, Alexandr Nedvedicky wrote:
>
> Hello,
>
> On Wed, Jan 04, 2023 at 09:36:38PM +1000, David Gwynne wrote:
>> and "stp" for pf_state ** variables.
>>
>I agree with established naming conventions.
>
>I'm also fine with keeping some exceptions such as `a` and `b`
>in pf_state_compare_id(), local variables `tail`, `head`
>in pf_states_{clr, get}() and pf_purge_expired_states().
>I'm also fine with leaving static variable `cur` unchanged.
>
> is there any reason we still keep `pf_state **sm` argument
> in pf_test_rule()? the same in pf_create_state(). Is it intended?
there were a bunch of other arguments that ended with m. happy to change it to
**stp though. we can always do another sweep for other types.
>
> otherwise diff reads OK to me.
>
> thanks and
> regards
> sashan
Re: bgpd vs gcc4
On Thu, Jan 05, 2023 at 11:03:04AM +0100, Claudio Jeker wrote:
> gcc4 does not really support C99 initalizers. It works most of the time
> but fails for more complex structs. Just fall back to memset() here.
deraadt used { {0} } in kr_send_dependon(). Apparently that works.
I really don't understand why we can't use a 24 years old standard.
>
> --
> :wq Claudio
>
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.303
> diff -u -p -r1.303 kroute.c
> --- kroute.c 28 Dec 2022 21:30:16 - 1.303
> +++ kroute.c 5 Jan 2023 09:55:39 -
> @@ -2159,10 +2159,12 @@ kroute6_validate(struct kroute6 *kr)
> int
> knexthop_true_nexthop(struct ktable *kt, struct kroute_full *kf)
> {
> - struct bgpd_addr gateway = { 0 };
> + struct bgpd_addr gateway;
> struct knexthop *kn;
> struct kroute *kr;
> struct kroute6 *kr6;
> +
> + memset(, 0, sizeof(gateway));
>
> /*
>* Ignore the nexthop for VPN routes. The gateway is forced
>
bgpd vs gcc4
gcc4 does not really support C99 initalizers. It works most of the time
but fails for more complex structs. Just fall back to memset() here.
--
:wq Claudio
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.303
diff -u -p -r1.303 kroute.c
--- kroute.c28 Dec 2022 21:30:16 - 1.303
+++ kroute.c5 Jan 2023 09:55:39 -
@@ -2159,10 +2159,12 @@ kroute6_validate(struct kroute6 *kr)
int
knexthop_true_nexthop(struct ktable *kt, struct kroute_full *kf)
{
- struct bgpd_addr gateway = { 0 };
+ struct bgpd_addr gateway;
struct knexthop *kn;
struct kroute *kr;
struct kroute6 *kr6;
+
+ memset(, 0, sizeof(gateway));
/*
* Ignore the nexthop for VPN routes. The gateway is forced
Re: more consistently use "st" as the name for struct pf_state variables
Hello,
On Wed, Jan 04, 2023 at 09:36:38PM +1000, David Gwynne wrote:
> and "stp" for pf_state ** variables.
>
I agree with established naming conventions.
I'm also fine with keeping some exceptions such as `a` and `b`
in pf_state_compare_id(), local variables `tail`, `head`
in pf_states_{clr, get}() and pf_purge_expired_states().
I'm also fine with leaving static variable `cur` unchanged.
is there any reason we still keep `pf_state **sm` argument
in pf_test_rule()? the same in pf_create_state(). Is it intended?
otherwise diff reads OK to me.
thanks and
regards
sashan
