Re: sys: use C99 struct init for struct audio_hw_if
On Tue, Oct 18, 2022 at 08:02:34PM +, Klemens Nanni wrote: > > Here's the actual complete diff that was tested as per above, > the previous did not include a bunch of files. > > I can also send/commit this in smaller per driver/arch/whatever chunks > if that's preferred. Your call ;-) ok ratchov
Re: sys: use C99 struct init for struct audio_hw_if
On Tue, Oct 18, 2022 at 02:17:54PM +, Klemens Nanni wrote:
> audio(9) cleanup demands removing a member from this struct, which is
> cumbersome in our current tree as drivers initialise it inconsistently,
> i.e. a simple removal of ".member_name = driver_func," is not always
> possible.
>
> Most drivers call their driver_*() functions like the member name,
> - some vary slightly, e.g. ".allocm = driver_alloc,"
> - some read vague, e.g. ".round_buffersize = driver_round," where
> .round_blocksize also exists
> - some have the member name as comment, on both functions and NULL lines
> - some use 0 not NULL
>
> Use C99 style everywhere for consistency and clarity, getting rid of all
> annoying differences and allow for clear member removals/additions:
>
> - dont change current order of members
> - no explicit NULL members
> - no comments or blank lines
> - trailing comma in last member line
>
> GENERIC.MP builds fine with this diff on arm64, amd64, i386 and sparc64.
> I should be able to test macppc soon.
>
> Feedback? Objection? OK?
>
> NB: `git diff --word-diff --color' really helps seeing actual change.
> 14 files changed, 228 insertions(+), 348 deletions(-)
Here's the actual complete diff that was tested as per above,
the previous did not include a bunch of files.
I can also send/commit this in smaller per driver/arch/whatever chunks
if that's preferred.
---
sys/arch/hppa/gsc/harmony.c | 38 ++-
sys/arch/luna88k/cbus/nec86.c | 5 --
sys/arch/macppc/dev/aoa.c | 36 ++
sys/arch/macppc/dev/awacs.c | 36 ++
sys/arch/macppc/dev/daca.c| 36 ++
sys/arch/macppc/dev/onyx.c| 36 ++
sys/arch/macppc/dev/snapper.c | 36 ++
sys/arch/macppc/dev/tumbler.c | 36 ++
sys/arch/sparc64/dev/ce4231.c | 37 ++
sys/dev/isa/ess.c | 76 -
sys/dev/isa/gus.c | 91 +--
sys/dev/isa/pas.c | 38 ++-
sys/dev/isa/sb.c | 38 ++-
sys/dev/pci/auacer.c | 37 ++
sys/dev/pci/auglx.c | 37 ++
sys/dev/pci/auich.c | 37 ++
sys/dev/pci/auixp.c | 37 ++
sys/dev/pci/autri.c | 36 ++
sys/dev/pci/auvia.c | 37 ++
sys/dev/pci/azalia.c | 42 ++--
sys/dev/pci/cmpci.c | 37 ++
sys/dev/pci/cs4280.c | 36 ++
sys/dev/pci/cs4281.c | 37 ++
sys/dev/pci/eap.c | 72 +++
sys/dev/pci/emuxki.c | 37 ++
sys/dev/pci/envy.c| 36 ++
sys/dev/pci/esa.c | 38 ++-
sys/dev/pci/eso.c | 37 ++
sys/dev/pci/fms.c | 36 ++
sys/dev/pci/maestro.c | 36 ++
sys/dev/pci/neo.c | 41 ++--
sys/dev/pci/sv.c | 38 ++-
sys/dev/pci/yds.c | 37 ++
sys/dev/sbus/cs4231.c | 37 ++
sys/dev/tc/bba.c | 38 ++-
sys/dev/usb/uaudio.c | 39 ++-
sys/dev/usb/utvfu.c | 33 +
37 files changed, 580 insertions(+), 887 deletions(-)
diff --git a/sys/arch/hppa/gsc/harmony.c b/sys/arch/hppa/gsc/harmony.c
index ade991c17c2..6823cda936e 100644
--- a/sys/arch/hppa/gsc/harmony.c
+++ b/sys/arch/hppa/gsc/harmony.c
@@ -74,28 +74,22 @@ int harmony_trigger_input(void *, void *, void *, int,
void (*intr)(void *), void *, struct audio_params *);
const struct audio_hw_if harmony_sa_hw_if = {
- harmony_open,
- harmony_close,
- harmony_set_params,
- harmony_round_blocksize,
- harmony_commit_settings,
- NULL,
- NULL,
- NULL,
- NULL,
- harmony_halt_output,
- harmony_halt_input,
- NULL,
- NULL,
- harmony_set_port,
- harmony_get_port,
- harmony_query_devinfo,
- harmony_allocm,
- harmony_freem,
- harmony_round_buffersize,
- harmony_get_props,
- harmony_trigger_output,
- harmony_trigger_input
+ .open = harmony_open,
+ .close = harmony_close,
+ .set_params = harmony_set_params,
+ .round_blocksize = harmony_round_blocksize,
+ .commit_settings = harmony_commit_settings,
+ .halt_output = harmony_halt_output,
+ .halt_input = harmony_halt_input,
+ .set_port = harmony_set_port,
+ .get_port = harmony_get_port,
+ .query_devinfo = harmony_query_devinfo,
+ .allocm = harmony_allocm,
+ .freem = harmony_freem,
+ .round_buffersize = harmony_round_buffersize,
+ .get_props = harmony_get_props,
+ .trigger_output = harmony_trigger_output,
+ .trigger_input = harmony_trigger_input,
};
int harmony_match(struct
Re: sys: use C99 struct init for struct audio_hw_if
On Tue, Oct 18, 2022 at 02:17:54PM +, Klemens Nanni wrote: > audio(9) cleanup demands removing a member from this struct, which is > cumbersome in our current tree as drivers initialise it inconsistently, > i.e. a simple removal of ".member_name = driver_func," is not always > possible. > > Most drivers call their driver_*() functions like the member name, > - some vary slightly, e.g. ".allocm = driver_alloc," > - some read vague, e.g. ".round_buffersize = driver_round," where > .round_blocksize also exists > - some have the member name as comment, on both functions and NULL lines > - some use 0 not NULL > > Use C99 style everywhere for consistency and clarity, getting rid of all > annoying differences and allow for clear member removals/additions: > > - dont change current order of members > - no explicit NULL members > - no comments or blank lines > - trailing comma in last member line > > GENERIC.MP builds fine with this diff on arm64, amd64, i386 and sparc64. > I should be able to test macppc soon. > > Feedback? Objection? OK? > AFAICS, this will make life easier and the diff looks good ok ratchov
Re: em(4) IPv4, TCP, UDP checksum offloading
On 15.10.2022. 22:01, Moritz Buhl wrote: > With the previous diffs I am seeing sporadic connection problems in tcpbench > via IPv6 on Intel 82546GB. > The diff was too big anyways. Here is a smaller diff that introduces > checksum offloading for the controllers that use the advanced descriptors. > > I tested this diff on i350 and 82571EB, receive, send, forwarding, > icmp4, icmp6, tcp4, tcp6, udp4, udp6, also over vlan and veb. Hi, I've tried same thing on i210 and 82576.
arm64: aplmca: constify _hw_if struct
The only driver with a non-const struct.
Seen while looking at it for audio(9).
OK?
diff --git a/sys/arch/arm64/dev/aplmca.c b/sys/arch/arm64/dev/aplmca.c
index 07a53103be4..03ade86fee7 100644
--- a/sys/arch/arm64/dev/aplmca.c
+++ b/sys/arch/arm64/dev/aplmca.c
@@ -135,7 +135,7 @@ int aplmca_trigger_input(void *, void *, void *, int,
intaplmca_halt_output(void *);
intaplmca_halt_input(void *);
-struct audio_hw_if aplmca_hw_if = {
+const struct audio_hw_if aplmca_hw_if = {
.set_params = aplmca_set_params,
.get_props = aplmca_get_props,
.allocm = aplmca_allocm,
dev/isa: ess, pas: constify string table
Each only used once for a printf() call in *_attach().
Seen while tweaking their *_hw_if struct.
OK?
Index: dev/isa/ess.c
===
RCS file: /cvs/src/sys/dev/isa/ess.c,v
retrieving revision 1.28
diff -u -p -r1.28 ess.c
--- dev/isa/ess.c 21 Mar 2022 19:22:40 - 1.28
+++ dev/isa/ess.c 18 Oct 2022 14:21:45 -
@@ -181,7 +181,7 @@ voidess_clear_mreg_bits(struct ess_soft
void ess_set_mreg_bits(struct ess_softc *, u_char, u_char);
void ess_read_multi_mix_reg(struct ess_softc *, u_char, u_int8_t *,
bus_size_t);
-static char *essmodel[] = {
+static const char *essmodel[] = {
"unsupported",
"1888",
"1887",
Index: dev/isa/pas.c
===
RCS file: /cvs/src/sys/dev/isa/pas.c,v
retrieving revision 1.33
diff -u -p -r1.33 pas.c
--- dev/isa/pas.c 6 Apr 2022 18:59:28 - 1.33
+++ dev/isa/pas.c 18 Oct 2022 14:21:37 -
@@ -136,7 +136,7 @@ const struct audio_hw_if pas_hw_if = {
/* The Address Translation code is used to convert I/O register addresses to
be relative to the given base -register */
-static char *pasnames[] = {
+static const char *pasnames[] = {
"",
"Plus",
"CDPC",
sys: use C99 struct init for struct audio_hw_if
audio(9) cleanup demands removing a member from this struct, which is
cumbersome in our current tree as drivers initialise it inconsistently,
i.e. a simple removal of ".member_name = driver_func," is not always
possible.
Most drivers call their driver_*() functions like the member name,
- some vary slightly, e.g. ".allocm = driver_alloc,"
- some read vague, e.g. ".round_buffersize = driver_round," where
.round_blocksize also exists
- some have the member name as comment, on both functions and NULL lines
- some use 0 not NULL
Use C99 style everywhere for consistency and clarity, getting rid of all
annoying differences and allow for clear member removals/additions:
- dont change current order of members
- no explicit NULL members
- no comments or blank lines
- trailing comma in last member line
GENERIC.MP builds fine with this diff on arm64, amd64, i386 and sparc64.
I should be able to test macppc soon.
Feedback? Objection? OK?
NB: `git diff --word-diff --color' really helps seeing actual change.
---
sys/arch/hppa/gsc/harmony.c | 38 ++-
sys/arch/luna88k/cbus/nec86.c | 5 --
sys/arch/macppc/dev/aoa.c | 36 ++
sys/arch/macppc/dev/awacs.c | 36 ++
sys/arch/macppc/dev/daca.c| 36 ++
sys/arch/macppc/dev/onyx.c| 36 ++
sys/arch/macppc/dev/snapper.c | 36 ++
sys/arch/macppc/dev/tumbler.c | 36 ++
sys/arch/sparc64/dev/ce4231.c | 37 ++
sys/dev/isa/ess.c | 76 -
sys/dev/isa/gus.c | 91 +--
sys/dev/isa/pas.c | 38 ++-
sys/dev/isa/sb.c | 38 ++-
sys/dev/pci/auacer.c | 37 ++
14 files changed, 228 insertions(+), 348 deletions(-)
diff --git a/sys/arch/hppa/gsc/harmony.c b/sys/arch/hppa/gsc/harmony.c
index ade991c17c2..6823cda936e 100644
--- a/sys/arch/hppa/gsc/harmony.c
+++ b/sys/arch/hppa/gsc/harmony.c
@@ -74,28 +74,22 @@ int harmony_trigger_input(void *, void *, void *, int,
void (*intr)(void *), void *, struct audio_params *);
const struct audio_hw_if harmony_sa_hw_if = {
- harmony_open,
- harmony_close,
- harmony_set_params,
- harmony_round_blocksize,
- harmony_commit_settings,
- NULL,
- NULL,
- NULL,
- NULL,
- harmony_halt_output,
- harmony_halt_input,
- NULL,
- NULL,
- harmony_set_port,
- harmony_get_port,
- harmony_query_devinfo,
- harmony_allocm,
- harmony_freem,
- harmony_round_buffersize,
- harmony_get_props,
- harmony_trigger_output,
- harmony_trigger_input
+ .open = harmony_open,
+ .close = harmony_close,
+ .set_params = harmony_set_params,
+ .round_blocksize = harmony_round_blocksize,
+ .commit_settings = harmony_commit_settings,
+ .halt_output = harmony_halt_output,
+ .halt_input = harmony_halt_input,
+ .set_port = harmony_set_port,
+ .get_port = harmony_get_port,
+ .query_devinfo = harmony_query_devinfo,
+ .allocm = harmony_allocm,
+ .freem = harmony_freem,
+ .round_buffersize = harmony_round_buffersize,
+ .get_props = harmony_get_props,
+ .trigger_output = harmony_trigger_output,
+ .trigger_input = harmony_trigger_input,
};
int harmony_match(struct device *, void *, void *);
diff --git a/sys/arch/luna88k/cbus/nec86.c b/sys/arch/luna88k/cbus/nec86.c
index eec75a8f4ac..2d2d965d1bc 100644
--- a/sys/arch/luna88k/cbus/nec86.c
+++ b/sys/arch/luna88k/cbus/nec86.c
@@ -84,12 +84,7 @@ const struct audio_hw_if nec86_hw_if = {
.set_port = nec86hw_mixer_set_port,
.get_port = nec86hw_mixer_get_port,
.query_devinfo = nec86hw_mixer_query_devinfo,
- .allocm = NULL,
- .freem = NULL,
- .round_buffersize = NULL,
.get_props = nec86_get_props,
- .trigger_output = NULL,
- .trigger_input = NULL
};
/*
diff --git a/sys/arch/macppc/dev/aoa.c b/sys/arch/macppc/dev/aoa.c
index f6372876a4d..39dfaa526c3 100644
--- a/sys/arch/macppc/dev/aoa.c
+++ b/sys/arch/macppc/dev/aoa.c
@@ -66,28 +66,20 @@ struct cfdriver aoa_cd = {
};
const struct audio_hw_if aoa_hw_if = {
- i2s_open,
- i2s_close,
- i2s_set_params,
- i2s_round_blocksize,
- NULL,
- NULL,
- NULL,
- NULL,
- NULL,
- i2s_halt_output,
- i2s_halt_input,
- NULL,
- NULL,
- i2s_set_port,
- i2s_get_port,
- i2s_query_devinfo,
- i2s_allocm,
- NULL,
- i2s_round_buffersize,
- i2s_get_props,
- i2s_trigger_output,
- i2s_trigger_input
+ .open = i2s_open,
+ .close = i2s_close,
+ .set_params = i2s_set_params,
+ .round_blocksize = i2s_round_blocksize,
+ .halt_output = i2s_halt_output,
+ .halt_input =
Re: fix use after free in proxy_parse_uri()
On Tue, Oct 18, 2022 at 03:25:36PM +0200, Claudio Jeker wrote: > With rev 1.65 proxy_parse_uri() can assign a pointer to proxyport > that is part of fullhost and so points to freed memory (once that function > returns). The fix is to copy the port as well. > > This should be a fix for > https://github.com/rpki-client/rpki-client-portable/issues/74 ugh, sorry about that. ok tb > -- > :wq Claudio > > Index: http.c > === > RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v > retrieving revision 1.69 > diff -u -p -r1.69 http.c > --- http.c20 Sep 2022 08:53:27 - 1.69 > +++ http.c18 Oct 2022 13:15:58 - > @@ -408,7 +408,8 @@ proxy_parse_uri(char *uri) > > if ((proxy.proxyhost = strdup(host)) == NULL) > err(1, NULL); > - proxy.proxyport = port; > + if ((proxy.proxyport = strdup(port)) == NULL) > + err(1, NULL); > proxy.proxyauth = cookie; > > free(fullhost); >
fix use after free in proxy_parse_uri()
With rev 1.65 proxy_parse_uri() can assign a pointer to proxyport that is part of fullhost and so points to freed memory (once that function returns). The fix is to copy the port as well. This should be a fix for https://github.com/rpki-client/rpki-client-portable/issues/74 -- :wq Claudio Index: http.c === RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v retrieving revision 1.69 diff -u -p -r1.69 http.c --- http.c 20 Sep 2022 08:53:27 - 1.69 +++ http.c 18 Oct 2022 13:15:58 - @@ -408,7 +408,8 @@ proxy_parse_uri(char *uri) if ((proxy.proxyhost = strdup(host)) == NULL) err(1, NULL); - proxy.proxyport = port; + if ((proxy.proxyport = strdup(port)) == NULL) + err(1, NULL); proxy.proxyauth = cookie; free(fullhost);
Re: smtpd bug in Received: header with one recipient
Ping.
On Sun, Oct 09, 2022 at 08:01:01AM +0200, Martijn van Duren wrote:
> Bit focused on other things atm and not familiar with this part of the
> code, but some comments.
>
> On Sat, 2022-10-08 at 12:18 -0600, Chris Waddey wrote:
>> A message with a single successful recipient but with a failed
>> rcpt to: command afterward generates an incorrect Received: header.
...
>> The following patch fixes the problem:
>> Index: smtp_session.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
>> retrieving revision 1.432
>> diff -u -p -r1.432 smtp_session.c
>> --- smtp_session.c 1 Jul 2021 07:42:16 - 1.432
>> +++ smtp_session.c 8 Oct 2022 18:04:51 -
>> @@ -2732,6 +2732,7 @@ static void
>> smtp_message_begin(struct smtp_tx *tx)
>> {
>>struct smtp_session *s;
>> + struct smtp_rcpt *srfp;
>>int (*m_printf)(struct smtp_tx *, const char *, ...);
>>m_printf = smtp_message_printf;
>> @@ -2780,10 +2781,13 @@ smtp_message_begin(struct smtp_tx *tx)
>>}
>>}
>> + /* If we get failed "RCPT TO" commands that modify tx->evp, then
>> +* make sure we use the real recipient for the "for" clause */
> See style(9) how comments should be formatted:
> /*
>* Multi-line comments look like this. Make them real sentences.
>* Fill them so they look like real paragraphs.
>*/
Got it.
>>if (tx->rcptcount == 1) {
>> + srfp = TAILQ_FIRST(>rcpts);
>>m_printf(tx, "\n\tfor <%s@%s>",
>> - tx->evp.rcpt.user,
>> - tx->evp.rcpt.domain);
>> + srfp->maddr.user,
>> + srfp->maddr.domain);
> I don't see anything wrong with this, but does the code still do the correct
> thing with multiple recipients?
> RCPT TO:
> RCPT TO:
> RCPT TO:
> or
> RCPT TO:
> RCPT TO:
> RCPT TO:
For reference, testing the two above scenarios with the original gives
the following Received: header (except date and message id
differences):
Received: by thief.private (OpenSMTPD) with ESMTP id 4f5144f3; Sun, 9 Oct 2022
12:09:22 -0600 (MDT)
With the patch it still gives the following for both scenarios:
Received: by thief.private (OpenSMTPD) with ESMTP id 50bf7075; Sun, 9 Oct 2022
12:14:32 -0600 (MDT)
Modified patch with comment fix (feel free to remove the comment
entirely if you think it's not needed):
Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.432
diff -u -p -r1.432 smtp_session.c
--- smtp_session.c1 Jul 2021 07:42:16 -1.432
+++ smtp_session.c9 Oct 2022 18:15:53 -
@@ -2732,6 +2732,7 @@ static void
smtp_message_begin(struct smtp_tx *tx)
{
struct smtp_session *s;
+struct smtp_rcpt *srfp;
int(*m_printf)(struct smtp_tx *, const char *, ...);
m_printf = smtp_message_printf;
@@ -2780,10 +2781,15 @@ smtp_message_begin(struct smtp_tx *tx)
}
}
+/*
+ * If we get failed "RCPT TO" commands that modify tx->evp, then
+ * make sure we use the real recipient for the "for" clause
+ */
if (tx->rcptcount == 1) {
+srfp = TAILQ_FIRST(>rcpts);
m_printf(tx, "\n\tfor <%s@%s>",
-tx->evp.rcpt.user,
-tx->evp.rcpt.domain);
+srfp->maddr.user,
+srfp->maddr.domain);
}
m_printf(tx, ";\n\t%s\n", time_to_text(time(>time)));
Patch with no comment:
Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.432
diff -u -p -r1.432 smtp_session.c
--- smtp_session.c1 Jul 2021 07:42:16 -1.432
+++ smtp_session.c9 Oct 2022 18:18:44 -
@@ -2732,6 +2732,7 @@ static void
smtp_message_begin(struct smtp_tx *tx)
{
struct smtp_session *s;
+struct smtp_rcpt *srfp;
int(*m_printf)(struct smtp_tx *, const char *, ...);
m_printf = smtp_message_printf;
@@ -2781,9 +2782,10 @@ smtp_message_begin(struct smtp_tx *tx)
}
if (tx->rcptcount == 1) {
+srfp = TAILQ_FIRST(>rcpts);
m_printf(tx, "\n\tfor <%s@%s>",
-tx->evp.rcpt.user,
-tx->evp.rcpt.domain);
+srfp->maddr.user,
+srfp->maddr.domain);
}
m_printf(tx, ";\n\t%s\n", time_to_text(time(>time)));
Re: bgpctl show metric up/down time
On Tue, Oct 18, 2022 at 01:08:55PM +0200, Claudio Jeker wrote:
> As mentioned I think having metric values that depend on session state is
> not ideal. Introduce 'bgpd_peer_last_change_seconds' which is the last
> time the session up/down state changed. It does not track every state
> change only when a session goes in and out of ESTABLISHED state.
> This replaces bgpd_peer_up_seconds and bgpd_peer_down_seconds.
Makes sense.
ok tb
>
> --
> :wq Claudio
>
> Index: output_ometric.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/output_ometric.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 output_ometric.c
> --- output_ometric.c 17 Oct 2022 12:01:19 - 1.1
> +++ output_ometric.c 18 Oct 2022 11:03:04 -
> @@ -33,8 +33,8 @@
> #include "ometric.h"
>
> struct ometric *bgpd_info, *bgpd_scrape_time;
> -struct ometric *peer_info, *peer_state, *peer_state_raw, *peer_up_time,
> - *peer_down_time, *peer_last_read, *peer_last_write;
> +struct ometric *peer_info, *peer_state, *peer_state_raw, *peer_last_change,
> + *peer_last_read, *peer_last_write;
> struct ometric *peer_prefixes_transmit, *peer_prefixes_receive;
> struct ometric *peer_message_transmit, *peer_message_recieve;
> struct ometric *peer_update_transmit, *peer_update_pending,
> @@ -91,10 +91,9 @@ ometric_head(struct parse_result *arg)
> "peer session state");
> peer_state_raw = ometric_new(OMT_GAUGE, "bgpd_peer_state_raw",
> "peer session state raw int value");
> - peer_up_time = ometric_new(OMT_GAUGE, "bgpd_peer_up_seconds",
> - "peer session up time in seconds");
> - peer_down_time = ometric_new(OMT_GAUGE, "bgpd_peer_down_seconds",
> - "peer session down time in seconds");
> + peer_last_change = ometric_new(OMT_GAUGE,
> + "bgpd_peer_last_change_seconds",
> + "time in seconds since peer's last up/down state change");
> peer_last_read = ometric_new(OMT_GAUGE, "bgpd_peer_last_read_seconds",
> "peer time since last read in seconds");
> peer_last_write = ometric_new(OMT_GAUGE, "bgpd_peer_last_write_seconds",
> @@ -193,16 +192,15 @@ ometric_neighbor_stats(struct peer *p, s
> ometric_set_state(peer_state, statenames[p->state], ol);
> ometric_set_int(peer_state_raw, p->state, ol);
>
> + ometric_set_int(peer_last_change, get_monotime(p->stats.last_updown),
> + ol);
> +
> if (p->state == STATE_ESTABLISHED) {
> - ometric_set_int(peer_up_time,
> - get_monotime(p->stats.last_updown), ol);
> ometric_set_int(peer_last_read,
> get_monotime(p->stats.last_read), ol);
> ometric_set_int(peer_last_write,
> get_monotime(p->stats.last_write), ol);
> - } else if (p->stats.last_updown != 0)
> - ometric_set_int(peer_down_time,
> - get_monotime(p->stats.last_updown), ol);
> + }
>
> ometric_set_int(peer_prefixes_transmit, p->stats.prefix_out_cnt, ol);
> ometric_set_int(peer_prefixes_receive, p->stats.prefix_cnt, ol);
>
Re: initialize peer last_updown when peer is created
On Tue, Oct 18, 2022 at 01:13:27PM +0200, Claudio Jeker wrote: > Currently the last_updown stat is not initalized and 0 when a peer is > initially added. If the peer is passive or is unable to establish a > connection last_updown remains 0 which is not ideal. > > Initalize the last_updown timestamp in init_peer() which is called when a > new peer is added or cloned. With this the time is shown properly in > bgpctl outputs. ok tb > > -- > :wq Claudio > > Index: session.c > === > RCS file: /cvs/src/usr.sbin/bgpd/session.c,v > retrieving revision 1.435 > diff -u -p -r1.435 session.c > --- session.c 31 Aug 2022 15:51:44 - 1.435 > +++ session.c 18 Oct 2022 10:59:39 - > @@ -582,6 +582,8 @@ init_peer(struct peer *p) > else > timer_set(>timers, Timer_IdleHold, 0); /* start ASAP */ > > + p->stats.last_updown = getmonotime(); > + > /* >* on startup, demote if requested. >* do not handle new peers. they must reach ESTABLISHED beforehands. >
initialize peer last_updown when peer is created
Currently the last_updown stat is not initalized and 0 when a peer is initially added. If the peer is passive or is unable to establish a connection last_updown remains 0 which is not ideal. Initalize the last_updown timestamp in init_peer() which is called when a new peer is added or cloned. With this the time is shown properly in bgpctl outputs. -- :wq Claudio Index: session.c === RCS file: /cvs/src/usr.sbin/bgpd/session.c,v retrieving revision 1.435 diff -u -p -r1.435 session.c --- session.c 31 Aug 2022 15:51:44 - 1.435 +++ session.c 18 Oct 2022 10:59:39 - @@ -582,6 +582,8 @@ init_peer(struct peer *p) else timer_set(>timers, Timer_IdleHold, 0); /* start ASAP */ + p->stats.last_updown = getmonotime(); + /* * on startup, demote if requested. * do not handle new peers. they must reach ESTABLISHED beforehands.
bgpctl show metric up/down time
As mentioned I think having metric values that depend on session state is
not ideal. Introduce 'bgpd_peer_last_change_seconds' which is the last
time the session up/down state changed. It does not track every state
change only when a session goes in and out of ESTABLISHED state.
This replaces bgpd_peer_up_seconds and bgpd_peer_down_seconds.
--
:wq Claudio
Index: output_ometric.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/output_ometric.c,v
retrieving revision 1.1
diff -u -p -r1.1 output_ometric.c
--- output_ometric.c17 Oct 2022 12:01:19 - 1.1
+++ output_ometric.c18 Oct 2022 11:03:04 -
@@ -33,8 +33,8 @@
#include "ometric.h"
struct ometric *bgpd_info, *bgpd_scrape_time;
-struct ometric *peer_info, *peer_state, *peer_state_raw, *peer_up_time,
- *peer_down_time, *peer_last_read, *peer_last_write;
+struct ometric *peer_info, *peer_state, *peer_state_raw, *peer_last_change,
+ *peer_last_read, *peer_last_write;
struct ometric *peer_prefixes_transmit, *peer_prefixes_receive;
struct ometric *peer_message_transmit, *peer_message_recieve;
struct ometric *peer_update_transmit, *peer_update_pending,
@@ -91,10 +91,9 @@ ometric_head(struct parse_result *arg)
"peer session state");
peer_state_raw = ometric_new(OMT_GAUGE, "bgpd_peer_state_raw",
"peer session state raw int value");
- peer_up_time = ometric_new(OMT_GAUGE, "bgpd_peer_up_seconds",
- "peer session up time in seconds");
- peer_down_time = ometric_new(OMT_GAUGE, "bgpd_peer_down_seconds",
- "peer session down time in seconds");
+ peer_last_change = ometric_new(OMT_GAUGE,
+ "bgpd_peer_last_change_seconds",
+ "time in seconds since peer's last up/down state change");
peer_last_read = ometric_new(OMT_GAUGE, "bgpd_peer_last_read_seconds",
"peer time since last read in seconds");
peer_last_write = ometric_new(OMT_GAUGE, "bgpd_peer_last_write_seconds",
@@ -193,16 +192,15 @@ ometric_neighbor_stats(struct peer *p, s
ometric_set_state(peer_state, statenames[p->state], ol);
ometric_set_int(peer_state_raw, p->state, ol);
+ ometric_set_int(peer_last_change, get_monotime(p->stats.last_updown),
+ ol);
+
if (p->state == STATE_ESTABLISHED) {
- ometric_set_int(peer_up_time,
- get_monotime(p->stats.last_updown), ol);
ometric_set_int(peer_last_read,
get_monotime(p->stats.last_read), ol);
ometric_set_int(peer_last_write,
get_monotime(p->stats.last_write), ol);
- } else if (p->stats.last_updown != 0)
- ometric_set_int(peer_down_time,
- get_monotime(p->stats.last_updown), ol);
+ }
ometric_set_int(peer_prefixes_transmit, p->stats.prefix_out_cnt, ol);
ometric_set_int(peer_prefixes_receive, p->stats.prefix_cnt, ol);
Re: audio: remove unused AUDIO_PROP_{MMAP,INDEPENDENT}
On Tue, Oct 18, 2022 at 09:32:27AM +0200, Alexandre Ratchov wrote: > On Mon, Oct 17, 2022 at 07:45:33PM +, Klemens Nanni wrote: > > I was looking at one particular driver and needed to know what those > > flags are, turns out AUDIO_PROP_FULLDUPLEX is the only audio(9) property > > that is in use. > > Even AUDIO_PROP_FULLDUPLEX (and the associated get_props() and setfd() > routines) should go. FWIW, an audio device has only these modes: I was wondering about the usefulness of AUDIO_PROP_FULLDUPLEX... > > AUDIO_MODE_PLAY -> play only > AUDIO_MODE_RECORD -> record only > AUDIO_MODE_PLAY | AUDIO_MODE_RECORD -> play and record (in sync) > > But this requires touching the isa(4) drivers, bba(4/alpha), arcofi(4) > and alike. > > > > > Feedback? Objection? OK? > > > > ok ratchov, thanks! > > As we,re at it, last point: audio_if.h and audioio.h are private > interfaces and the risk of breaking a port close to zero. > > distfiles contain the "sys/audioio.h" string because it's still used > on SunOS and NetBSD but these are #ifdef'd out on OpenBSD. Thanks for clarifying that.
Re: audio: remove unused AUDIO_PROP_{MMAP,INDEPENDENT}
On Mon, Oct 17, 2022 at 07:45:33PM +, Klemens Nanni wrote: > I was looking at one particular driver and needed to know what those > flags are, turns out AUDIO_PROP_FULLDUPLEX is the only audio(9) property > that is in use. Even AUDIO_PROP_FULLDUPLEX (and the associated get_props() and setfd() routines) should go. FWIW, an audio device has only these modes: AUDIO_MODE_PLAY -> play only AUDIO_MODE_RECORD -> record only AUDIO_MODE_PLAY | AUDIO_MODE_RECORD -> play and record (in sync) But this requires touching the isa(4) drivers, bba(4/alpha), arcofi(4) and alike. > > Feedback? Objection? OK? > ok ratchov, thanks! As we,re at it, last point: audio_if.h and audioio.h are private interfaces and the risk of breaking a port close to zero. distfiles contain the "sys/audioio.h" string because it's still used on SunOS and NetBSD but these are #ifdef'd out on OpenBSD.
Re: mpii(4): document/handle RAID 1E, bioctl(8): print it as such
On Wed, Oct 05, 2022 at 12:10:26AM +, Klemens Nanni wrote:
> On Sun, Sep 25, 2022 at 12:59:27AM +, Klemens Nanni wrote:
> > On a sparc64 T4-2 I boot OpenBSD from hardware RAID 1E using the
> > built-in controller:
> >
> > mpii0 at pci15 dev 0 function 0 "Symbios Logic SAS2008" rev 0x03: msi
> > mpii0: Solana On-Board, firmware 9.0.0.0 IR, MPI 2.0
> > scsibus1 at mpii0: 834 targets
> > sd0 at scsibus1 targ 0 lun 0:
> > naa.600508e06cd1dcd59022a30a
> > sd0: 713824MB, 512 bytes/sector, 1461911552 sectors
> > root on sd0a (efde5b2c6ab7b8ac.a) swap on sd0b dump on sd0b
> >
> > But OpenBSD shows it as RAID10, which is misleading:
> >
> > # bioctl mpii0
> > Volume Status Size Device
> > mpii0 0 Online 748498714112 sd0 RAID10
> > 0 Online 500107861504 0:2.0 noencl > CT500MX500SSD1>
> > 1 Online 500107861504 0:1.0 noencl > CT500MX500SSD1>
> > 2 Online 500107861504 0:0.0 noencl > CT500MX500SSD1>
> >
> > Turns out the driver reports it like that and adapting the tool is
> > trivial.
> >
> > Document 1E support, report 1E as 0x1E (like softraid(4) 1C being 0x1C)
> > and print it as such.
> >
> > This merely supports printing it; softraid code has nothing to do with
> > what hardware supports and the create/write path remains unchanged in
> > driver/softraid/bioctl land.
> >
> > Format strings for different RAID levels start identical up to the level
> > specifics, so hoist the identical part while here to make this clearer
> > and simplify/shorten it.
> >
> > # ./obj/bioctl mpii0
> > Volume Status Size Device
> > mpii0 0 Online 748498714112 sd0 RAID1E
> > 0 Online 500107861504 0:2.0 noencl > CT500MX500SSD1>
> > 1 Online 500107861504 0:1.0 noencl > CT500MX500SSD1>
> > 2 Online 500107861504 0:0.0 noencl > CT500MX500SSD1>
> >
> > Feedback? OK? (for after release)
>
> Ping.
Anyone?
Index: sys/dev/pci/mpii.c
===
RCS file: /cvs/src/sys/dev/pci/mpii.c,v
retrieving revision 1.143
diff -u -p -r1.143 mpii.c
--- sys/dev/pci/mpii.c 16 Apr 2022 19:19:59 - 1.143
+++ sys/dev/pci/mpii.c 5 Oct 2022 00:09:17 -
@@ -3423,6 +3423,8 @@ mpii_ioctl_vol(struct mpii_softc *sc, st
bv->bv_level = 1;
break;
case MPII_CFG_RAID_VOL_0_TYPE_RAID1E:
+ bv->bv_level = 0x1E;
+ break;
case MPII_CFG_RAID_VOL_0_TYPE_RAID10:
bv->bv_level = 10;
break;
Index: share/man/man4/mpii.4
===
RCS file: /cvs/src/share/man/man4/mpii.4,v
retrieving revision 1.15
diff -u -p -r1.15 mpii.4
--- share/man/man4/mpii.4 19 Jun 2018 10:43:21 - 1.15
+++ share/man/man4/mpii.4 5 Oct 2022 00:09:17 -
@@ -59,11 +59,12 @@ Lenovo N2215, ThinkSystem 430
LSI SAS 9200-8e, SAS 9207-8i, SAS 9211-4i, SAS 9211-8i
.It
Broadcom SAS 9300, HBA 9400
+.It
+Oracle SPARC T Series Platforms
.El
.Pp
Some models of these controllers carry an Integrated RAID (IR) firmware
-providing support for RAID 0, RAID 1, RAID10 or RAID5 using SAS or SATA
-drives.
+providing support for RAID levels 0, 1, 1E, 5 or 10 using SAS or SATA drives.
All RAID configuration is done through the controllers' BIOSes.
.Sh SEE ALSO
.Xr bio 4 ,
Index: sbin/bioctl/bioctl.c
===
RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
retrieving revision 1.149
diff -u -p -r1.149 bioctl.c
--- sbin/bioctl/bioctl.c26 Aug 2022 09:14:00 - 1.149
+++ sbin/bioctl/bioctl.c18 Oct 2022 06:04:25 -
@@ -492,25 +492,23 @@ bio_inq(char *name)
else
snprintf(size, sizeof size, "%14llu",
bv.bv_size);
+ printf("%11s %-10s %14s %-7s ",
+ volname, status, size, bv.bv_dev);
switch (bv.bv_level) {
case 'C':
- printf("%11s %-10s %14s %-7s CRYPTO%s%s\n",
- volname, status, size, bv.bv_dev,
+ printf("CRYPTO%s%s\n",
percent, seconds);
break;
case 'c':
- printf("%11s %-10s %14s %-7s CONCAT%s%s\n",
- volname, status, size, bv.bv_dev,
+ printf("CONCAT%s%s\n",
percent, seconds);
break;
case 0x1C:
- printf("%11s %-10s %14s %-7s RAID%X%s%s %s\n",
-
