Re: iwn: improve processing of block ack notifications
On Fri, Apr 17, 2020 at 06:30:22PM +0200, Stefan Sperling wrote:
> On Fri, Apr 17, 2020 at 04:03:30PM +0200, Stefan Sperling wrote:
> > In particular, this fixes wrong assumptions about what the data in the
> > firmware's "compressed block ack" notifications is supposed to represent,
> > and actually pieces information about individual subframes of aggregated
> > frames (A-MPDUs) back together when reporting to MiRA, rather than reporting
> > unrelated subframes to MiRA individually. (Recall that MiRA was primarily
> > designed to operate on units of entire A-MDPUs.)
>
> Unfortunately, this diff can trigger a page fault. I am looking for a fix.
This new version has been tested by cwen and Josh Grosse and seems to
fix the problems they found with the previous diff. Thanks!
diff 24c5886569df7f9428f24e6e74c71e02cba8c34c
719fff35cc167c0814926c547e92b317aad32553
blob - 8476916e4337cb6e681c11f26fc1fca5f4a48334
blob + b8beb56aa1112e65c2907b2955694b6a8907d9ae
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -2282,7 +2282,11 @@ iwn_mira_choose(struct iwn_softc *sc, struct ieee80211
iwn_set_link_quality(sc, ni);
}
-/* Process an incoming Compressed BlockAck. */
+/*
+ * Process an incoming Compressed BlockAck.
+ * Note that these block ack notifications are generated by firmware and do
+ * not necessarily correspond to contents of block ack frames seen on the air.
+ */
void
iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc,
struct iwn_rx_data *data)
@@ -2293,7 +2297,8 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_
struct ieee80211_tx_ba *ba;
struct iwn_node *wn;
struct iwn_tx_ring *txq;
- uint16_t ssn, idx;
+ uint16_t seq, ssn, idx, end_idx;
+ int min_ampdu_id, max_ampdu_id, id;
int qid;
if (ic->ic_state != IEEE80211_S_RUN)
@@ -2322,66 +2327,103 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_
if (ba->ba_state != IEEE80211_BA_AGREED)
return;
- ssn = le16toh(cba->ssn); /* BA window starting sequence number */
- if (!SEQ_LT(ssn, ba->ba_winstart)) {
- ieee80211_output_ba_move_window(ic, ni, cba->tid, ssn);
+ /*
+* The first bit in cba->bitmap corresponds to the sequence number
+* stored in the sequence control field cba->seq.
+* Any frames older than this can now be discarded; they should
+* already have been reported as failures or been acknowledged.
+*
+* Multiple BA notifications in a row may be using this number, with
+* additional bits being set in cba->bitmap. It is unclear how the
+* firmware decides to shift this window forward.
+*/
+ seq = le16toh(cba->seq) >> IEEE80211_SEQ_SEQ_SHIFT;
+ if (!SEQ_LT(seq, ba->ba_winstart)) {
+ ieee80211_output_ba_move_window(ic, ni, cba->tid, seq);
iwn_ampdu_txq_advance(sc, txq, qid,
- IWN_AGG_SSN_TO_TXQ_IDX(ssn));
+ IWN_AGG_SSN_TO_TXQ_IDX(seq));
iwn_clear_oactive(sc, txq);
}
+ /* Our BA window should now correspond to the bitmap. */
+ if (ba->ba_winstart != seq)
+ return;
- /* ba->ba_winstart should now correspond to cba->ssn */
- if (ba->ba_winstart != cba->ssn)
+ /* Skip rate control if our Tx rate is fixed. */
+ if (ic->ic_fixed_mcs != -1)
return;
/*
-* Update Tx rate statistics.
-* Skip rate control if our Tx rate is fixed.
+* The firmware's new BA window starting sequence number
+* corresponds to the first hole in cba->bitmap, implying
+* that all frames between 'seq' and 'ssn' have been acked.
*/
- if (ic->ic_fixed_mcs == -1 && cba->nframes_sent > 0) {
- int end_idx = IWN_AGG_SSN_TO_TXQ_IDX(ba->ba_winend);
- int bit = 0, nsent = cba->nframes_sent;
+ ssn = le16toh(cba->ssn);
+ /* Determine the min/max IDs we assigned to AMPDUs in this range. */
+ idx = IWN_AGG_SSN_TO_TXQ_IDX(seq);
+ end_idx = IWN_AGG_SSN_TO_TXQ_IDX(ssn);
+ min_ampdu_id = txq->data[idx].ampdu_id;
+ max_ampdu_id = min_ampdu_id;
+ while (idx != end_idx) {
+ struct iwn_tx_data *txdata = &txq->data[idx];
+
+ if (txdata->m != NULL) {
+ if (min_ampdu_id > txdata->ampdu_id)
+ min_ampdu_id = txdata->ampdu_id;
+ if (max_ampdu_id < txdata->ampdu_id)
+ max_ampdu_id = txdata->ampdu_id;
+ }
+
+ idx = (idx + 1) % IWN_TX_RING_COUNT;
+ }
+
+ /*
+* Update Tx rate statistics for A-MPDUs before firmware's BA window.
+*/
+ for (id = min_ampdu_id; id <= max_ampdu_id; id++) {
+ int have_ack = 0, bit = 0;
+ idx = IWN_AGG_SSN_TO_TXQ_IDX(seq);
+
Re: iwn: improve processing of block ack notifications
On Fri, Apr 17, 2020 at 04:03:30PM +0200, Stefan Sperling wrote: > In particular, this fixes wrong assumptions about what the data in the > firmware's "compressed block ack" notifications is supposed to represent, > and actually pieces information about individual subframes of aggregated > frames (A-MPDUs) back together when reporting to MiRA, rather than reporting > unrelated subframes to MiRA individually. (Recall that MiRA was primarily > designed to operate on units of entire A-MDPUs.) Unfortunately, this diff can trigger a page fault. I am looking for a fix.
Re: suggest to run rpki-client hourly
Todd C. Miller wrote: > On Thu, 16 Apr 2020 17:18:15 -0600, "Theo de Raadt" wrote: > > > I agree we should try 1 hour. > > > > > +#~ * * * * -s -n rpki-client -v && bgpctl > > > reload > > > > I would prefer if you use -ns rather than the two seperate options. > > The option parsing doesn't currently support bundling flags like > that. I think this should work like getopt. I'll take a look a bit later.
Re: implement locale(1) charmap argument
On Fri, Apr 17, 2020 at 03:05:06PM +0200, Ingo Schwarze wrote: > Naively, it does seem like it would make sense to have "locale -m" > print a list of possible output values of "locale chardef", so i'm > not opposed to adding "US-ASCII" to it. But that doesn't appear to > be how it works elsewhere, at least not everywhere. I found no > documentation stating clearly what it is supposed to do, POSIX feels > murky at best. Good grief! Well, we can leave good enough alone then, I suppose :) Thank you for doing such elaborate research.
iwn: improve processing of block ack notifications
This applies on top of a previous diff I sent, the one to make use of
the entire firmware retry table.
Recently I have spent a good amount of time trying to reverse-engineer
the sparsely documented behaviour of both iwn(4) and iwm(4) firmware with
respect to Tx aggregation. For details, see my comments in the diff.
I have also put some thought into how we can reconcile the firmware's
representation of relevant information with what MiRA expects to see.
This diff is the best I could come up with so far. There were at lot of
earlier attempts and I'm still not sure if this is the best possible
solution, but it seems more correct than what we have now.
In particular, this fixes wrong assumptions about what the data in the
firmware's "compressed block ack" notifications is supposed to represent,
and actually pieces information about individual subframes of aggregated
frames (A-MPDUs) back together when reporting to MiRA, rather than reporting
unrelated subframes to MiRA individually. (Recall that MiRA was primarily
designed to operate on units of entire A-MDPUs.)
diff 24c5886569df7f9428f24e6e74c71e02cba8c34c
636d6d139ade79b967dc9eb8b553e48dcd939227
blob - 8476916e4337cb6e681c11f26fc1fca5f4a48334
blob + f1a30eea896c8502e8f15bb3d1133b74120253f4
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -2282,7 +2282,11 @@ iwn_mira_choose(struct iwn_softc *sc, struct ieee80211
iwn_set_link_quality(sc, ni);
}
-/* Process an incoming Compressed BlockAck. */
+/*
+ * Process an incoming Compressed BlockAck.
+ * Note that these block ack notifications are generated by firmware and do
+ * not necessarily correspond to contents of block ack frames seen on the air.
+ */
void
iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc,
struct iwn_rx_data *data)
@@ -2293,7 +2297,8 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_
struct ieee80211_tx_ba *ba;
struct iwn_node *wn;
struct iwn_tx_ring *txq;
- uint16_t ssn, idx;
+ uint16_t seq, ssn, idx, end_idx;
+ int min_ampdu_id, max_ampdu_id, id;
int qid;
if (ic->ic_state != IEEE80211_S_RUN)
@@ -2322,66 +2327,98 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_
if (ba->ba_state != IEEE80211_BA_AGREED)
return;
- ssn = le16toh(cba->ssn); /* BA window starting sequence number */
- if (!SEQ_LT(ssn, ba->ba_winstart)) {
- ieee80211_output_ba_move_window(ic, ni, cba->tid, ssn);
+ /*
+* The first bit in cba->bitmap corresponds to the sequence number
+* stored in the sequence control field cba->seq.
+* Any frames older than this can now be discarded; they should
+* already have been reported as failures or been acknowledged.
+*
+* Multiple BA notifications in a row may be using this number, with
+* additional bits being set in cba->bitmap. It is unclear how the
+* firmware decides to shift this window forward.
+*/
+ seq = le16toh(cba->seq) >> IEEE80211_SEQ_SEQ_SHIFT;
+ if (!SEQ_LT(seq, ba->ba_winstart)) {
+ ieee80211_output_ba_move_window(ic, ni, cba->tid, seq);
iwn_ampdu_txq_advance(sc, txq, qid,
- IWN_AGG_SSN_TO_TXQ_IDX(ssn));
+ IWN_AGG_SSN_TO_TXQ_IDX(seq));
iwn_clear_oactive(sc, txq);
}
+ /* Our BA window should now correspond to the bitmap. */
+ if (ba->ba_winstart != seq)
+ return;
- /* ba->ba_winstart should now correspond to cba->ssn */
- if (ba->ba_winstart != cba->ssn)
+ /* Skip rate control if our Tx rate is fixed. */
+ if (ic->ic_fixed_mcs != -1)
return;
/*
-* Update Tx rate statistics.
-* Skip rate control if our Tx rate is fixed.
+* The firmware's new BA window starting sequence number
+* corresponds to the first hole in cba->bitmap, implying
+* that all frames between 'seq' and 'ssn' have been acked.
*/
- if (ic->ic_fixed_mcs == -1 && cba->nframes_sent > 0) {
- int end_idx = IWN_AGG_SSN_TO_TXQ_IDX(ba->ba_winend);
- int bit = 0, nsent = cba->nframes_sent;
+ ssn = le16toh(cba->ssn);
+ /* Determine the min/max IDs we assigned to AMPDUs in this range. */
+ idx = IWN_AGG_SSN_TO_TXQ_IDX(seq);
+ end_idx = IWN_AGG_SSN_TO_TXQ_IDX(ssn);
+ min_ampdu_id = txq->data[idx].ampdu_id;
+ max_ampdu_id = min_ampdu_id;
+ while (idx != end_idx) {
+ struct iwn_tx_data *txdata = &txq->data[idx];
+
+ if (min_ampdu_id > txdata->ampdu_id)
+ min_ampdu_id = txdata->ampdu_id;
+ if (max_ampdu_id < txdata->ampdu_id)
+ max_ampdu_id = txdata->ampdu_id;
+
+ idx = (idx + 1) % IWN_TX_RING_COUNT;
+ }
+
+ /*
+* Update Tx rate statistics for A-MPDUs befor
iwn(4): always traverse the entire multi-rate retry table
This adapts the use of the firmware multi-rate retry table to always
make use of all available slots.
The table specifies 16 Tx rates the firmware can use, one per Tx attempt.
For example, on a 5 GHz channel the current code fills this table with
static values like this:
MCS-7 MCS-6 MCS-5 MCS-4 MCS-3 MCS-2 MCS-1 MCS-0 OFDM6 OFDM6 OFDM6 ...
We then point the firmware at the entry in this table where Tx attempts
should begin. If we're currently sending at, say, MCS-5, then the rate
at the 3rd entry will be used first, then the 4th, and so on.
This is in fact what a comment in the Linux driver suggests doing for
non-11n operation, and I guess that's why Damien implemented it this way.
When I added 11n mode to the driver, with MCS 0 to 7, I kept this design.
With MIMO this approach won't work because then we'll have two ranges of
continous rates: MCS 0 to 7 (single antenna) and MCS 8 to 15 (dual antenna).
MiRA never mixes Tx rates using different amounts of antennas.
So we will need an approach that looks more like iwm(4) does it, where the
table is continuously updated and our the Tx current rate is at the front.
While we're transmitting at MCS 5, the table now looks like this:
MCS-5 MCS-4 MCS-3 MCS-2 MCS-1 MCS-0 OFDM6 OFDM6 OFDM6 OFDM6 OFDM6 OFDM6 ...
Once we support MIMO, the table would look like this while we're using MCS-9:
MCS-9 MCS-8 OFDM6 OFDM6 OFDM6 OFDM6 OFDM6 OFDM6 ...
So this change prepares for MIMO and makes sure that the firmware can
always make use of the full amount of Tx attempts.
diff 6f793971788fd7061f66330336cbeb5103b717c3
24c5886569df7f9428f24e6e74c71e02cba8c34c
blob - 110bbe97b980b338acd8aa61fbabd33926c207be
blob + 8476916e4337cb6e681c11f26fc1fca5f4a48334
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -157,6 +157,7 @@ voidiwn_rx_phy(struct iwn_softc *, struct
iwn_rx_des
struct iwn_rx_data *);
void iwn_rx_done(struct iwn_softc *, struct iwn_rx_desc *,
struct iwn_rx_data *, struct mbuf_list *);
+void iwn_mira_choose(struct iwn_softc *, struct ieee80211_node *);
void iwn_rx_compressed_ba(struct iwn_softc *, struct iwn_rx_desc *,
struct iwn_rx_data *);
void iwn5000_rx_calib_results(struct iwn_softc *,
@@ -1864,8 +1865,12 @@ iwn_iter_func(void *arg, struct ieee80211_node *ni)
struct iwn_softc *sc = arg;
struct iwn_node *wn = (void *)ni;
- if ((ni->ni_flags & IEEE80211_NODE_HT) == 0)
+ if ((ni->ni_flags & IEEE80211_NODE_HT) == 0) {
+ int old_txrate = ni->ni_txrate;
ieee80211_amrr_choose(&sc->amrr, ni, &wn->amn);
+ if (old_txrate != ni->ni_txrate)
+ iwn_set_link_quality(sc, ni);
+ }
}
void
@@ -2254,6 +2259,29 @@ iwn_rx_done(struct iwn_softc *sc, struct iwn_rx_desc *
ieee80211_release_node(ic, ni);
}
+void
+iwn_mira_choose(struct iwn_softc *sc, struct ieee80211_node *ni)
+{
+ struct ieee80211com *ic = &sc->sc_ic;
+ struct iwn_node *wn = (void *)ni;
+ int best_mcs = ieee80211_mira_get_best_mcs(&wn->mn);
+
+ ieee80211_mira_choose(&wn->mn, ic, ni);
+
+ /*
+* Update firmware's LQ retry table if MiRA has chosen a new MCS.
+*
+* We only need to do this if the best MCS has changed because
+* we ask firmware to use a fixed MCS while MiRA is probing a
+* candidate MCS.
+* While not probing we ask firmware to retry at lower rates in case
+* Tx at the newly chosen best MCS ends up failing, and then report
+* any resulting Tx retries to MiRA in order to trigger probing.
+*/
+ if (best_mcs != ieee80211_mira_get_best_mcs(&wn->mn))
+ iwn_set_link_quality(sc, ni);
+}
+
/* Process an incoming Compressed BlockAck. */
void
iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc,
@@ -2352,6 +2380,8 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_
if (wn->mn.ampdu_size > 0)
ieee80211_mira_choose(&wn->mn, ic, ni);
}
+ if (wn->mn.ampdu_size > 0)
+ iwn_mira_choose(sc, ni);
}
/*
@@ -2584,7 +2614,7 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, struct iwn_tx_
wn->mn.retries++;
if (txfail)
wn->mn.txfail++;
- ieee80211_mira_choose(&wn->mn, ic, ni);
+ iwn_mira_choose(sc, ni);
}
if (txfail)
@@ -2778,7 +2808,7 @@ iwn_tx_done(struct iwn_softc *sc, struct iwn_rx_desc *
wn->mn.retries++;
if (txfail)
wn->mn.txfail++;
- ieee80211_mira_choose(&wn->mn, ic, data->ni);
+ iwn_mira_choose(sc, data->ni);
}
} else if (data->txrate == data->ni->ni_txrate) {
wn->amn.amn_txcnt++;
@@ -3526,11 +3556,
iwn: work around fifo underrun Tx errors
This diff works around FIFO_UNDERRUN (0x84) Tx errors being reported
by iwn(4) firmware. When this error occurs, it tends to occur multiple
times in a row. Affected frames are lost and never get transmitted, and
traffic stalls for a while. This affects tcpbench very visibly.
I don't understand what is causing this. I have found that it only
occurs when we ask the firmware to use its multi-rate retry table.
If we send frames at a fixed rate, it does not happen.
This error is particularly problematic with block ack, because the
failed frames disappear and leave a hole in the receivers block ack
window. The receiver will then have to wait a while for the lost frames
until it eventually decides to skip them.
This problem effectively makes Tx aggegration unusable on iwn(4).
My workaround is to always use a fixed Tx rate for aggregation queues.
This is not ideal for single frames which also get send from such queues
when traffic is low and may now be more likely to fail.
But if the firmware decides to aggregate frames during traffic bursts,
all frames contained in an aggregate are always sent together at the
same rate anyway, so in this case we don't loose anything.
I would like to find a better fix, but this allows me to proceed with
additional fixes for aggregation support and together with those fixes
this seems better than the lossy behaviour we have now.
diff 0eca04344da7ad4deb76485dcef00cdf88803be4
6f793971788fd7061f66330336cbeb5103b717c3
blob - 14c2d9a35e2973feb1ec2347eeef3d6041864291
blob + 110bbe97b980b338acd8aa61fbabd33926c207be
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -3513,10 +3513,12 @@ iwn_tx(struct iwn_softc *sc, struct mbuf *m, struct ie
else
tx->rflags = rinfo->flags;
/*
-* Skip rate control if our Tx rate is fixed.
-* Keep the Tx rate constant while mira is probing.
+* Keep the Tx rate constant while mira is probing, or if this is
+* an aggregation queue in which case a fixed Tx rate works around
+* FIFO_UNDERRUN Tx errors.
*/
if (tx->id == sc->broadcast_id || ieee80211_mira_is_probing(&wn->mn) ||
+ qid >= sc->first_agg_txq ||
ic->ic_fixed_mcs != -1 || ic->ic_fixed_rate != -1) {
/* Group or management frame, or probing, or fixed Tx rate. */
tx->linkq = 0;
Re: suggest to run rpki-client hourly
On Thu, 16 Apr 2020 17:18:15 -0600, "Theo de Raadt" wrote: > I agree we should try 1 hour. > > > +#~ * * * * -s -n rpki-client -v && bgpctl reload > > I would prefer if you use -ns rather than the two seperate options. The option parsing doesn't currently support bundling flags like that. - todd
iwn(4): TID fix for block ack request frames
Make iwn(4) put the correct traffic identifier (TID) into the Tx command
when sending block ack request (BAR) frames.
This is not critical but occasionally I have seen the firmware send a
BAR frame with a bogus TID value, which the receiver will simply discard.
Writing the correct TID into the Tx command seems to fix this.
The bogus value was always 8, which matches IWN_NONQOS_TID and is the
default value for the variable 'tid' seen in the diff below; 'tid' will
be copied to the Tx command later.
diff 807e4f4b605ac66e215f436cbcf9b39590b82cab
0eca04344da7ad4deb76485dcef00cdf88803be4
blob - 2a1f1c261914ed847c02c9e9f39d9acb1dabbd0e
blob + 14c2d9a35e2973feb1ec2347eeef3d6041864291
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -3422,8 +3422,17 @@ iwn_tx(struct iwn_softc *sc, struct mbuf *m, struct ie
flags |= IWN_TX_NEED_ACK;
}
if (type == IEEE80211_FC0_TYPE_CTL &&
- subtype == IEEE80211_FC0_SUBTYPE_BAR)
+ subtype == IEEE80211_FC0_SUBTYPE_BAR) {
+ struct ieee80211_frame_min *mwh;
+ uint8_t *barfrm;
+ uint16_t ctl;
+ mwh = mtod(m, struct ieee80211_frame_min *);
+ barfrm = (uint8_t *)&mwh[1];
+ ctl = LE_READ_2(barfrm);
+ tid = (ctl & IEEE80211_BA_TID_INFO_MASK) >>
+ IEEE80211_BA_TID_INFO_SHIFT;
flags |= (IWN_TX_NEED_ACK | IWN_TX_IMM_BA);
+ }
if (wh->i_fc[1] & IEEE80211_FC1_MORE_FRAG)
flags |= IWN_TX_MORE_FRAG; /* Cannot happen yet. */
Re: implement locale(1) charmap argument
Hi Stefan and Todd, Stefan Sperling wrote on Fri, Apr 17, 2020 at 08:55:29AM +0200: > On Thu, Apr 16, 2020 at 09:35:18PM +0200, Ingo Schwarze wrote: >>$ locale -m >> UTF-8 >>$ locale charmap >> UTF-8 >>$ LC_ALL=C locale charmap >> US-ASCII >>$ LC_ALL=POSIX locale charmap >> US-ASCII > I am OK with your diff, Thanks to both of you for checking, i have put it in. > and noticed a separate issue with -m which > is exposed by this change: > > If US-ASCII is an available charmap, shouldn't locale -m list "US-ASCII" > in addition to "UTF-8"? I'm not completely sure what "available charmaps" is supposed to mean in the POSIX standard. Testing on an old Debian system, is see this: $ locale -m > charmaps.loc $ wc -l charmaps.loc 235 $ ls /usr/share/i18n/charmaps | sed 's/.gz$//' | sort > charmaps.ls $ diff -u charmaps.ls charmaps.loc | grep '^[+-][^+-]' +MAC_CENTRALEUROPE +NF_Z_62-010_(1973) +WIN-SAMI-2 $ locale charmap UTF-8 $ locale -m | grep UTF UTF-8 $ LC_CTYPE=C locale charmap ANSI_X3.4-1968 $ locale -m | grep 1968 ANSI_X3.4-1968 So "locale -m" gives almost a directory listing, but not quite; it produces a few additional entries that aren't in the directory. The return values from "locale charset" appear in "locale -m". Then again, Linux is not a certified UNIX system. So let's try with something certified: > uname -a SunOS unstable11s 5.11 11.3 sun4u sparc SUNW,SPARC-Enterprise > locale charmap UTF-8 > LC_CTYPE=C locale charmap 646 > locale -m | wc 0 0 0 It's a bit difficult because Solaris 11 does not provide locate(1), but i failed to find any charmap files there. Both UTF-8 and US-ASCII work (i tested that by compiling and running mandoc) but still "locale -m" returns nothing. > uname -a SunOS unstable10s 5.10 Generic_150400-17 sun4v sparc SUNW,SPARC-Enterprise-T5220 > locale charmap 646 > LC_CTYPE=en_US.UTF-8 locale charmap UTF-8 > locale -m iso_8859_1/charmap.src > ls -F /usr/lib/localedef/src/ charmaps/ en_US.UTF-8/ extensions/ iso_8859_1/ locales/ > ls -F /usr/lib/localedef/src/charmaps/ charmap.ANSI1251.bz2 charmap.ISO8859-9.bz2 charmap.iso-8859-5.bz2 charmap.ISO8859-1.bz2 charmap.KOI8-R.bz2charmap.iso-8859-6.bz2 charmap.ISO8859-13.bz2charmap.UTF-8.bz2 charmap.iso-8859-7.bz2 charmap.ISO8859-15.bz2charmap.ansi-1251.bz2 charmap.iso-8859-8.bz2 charmap.ISO8859-2.bz2 charmap.ar.bz2@ charmap.iso-8859-9.bz2 charmap.ISO8859-4.bz2 charmap.he.bz2@ charmap.koi8-r.bz2 charmap.ISO8859-5.bz2 charmap.iso-8859-1.bz2charmap.utf-8.bz2 charmap.ISO8859-6.bz2 charmap.iso-8859-13.bz2 charmap.utf8.bz2@ charmap.ISO8859-7.bz2 charmap.iso-8859-15.bz2 charmap.ISO8859-8.bz2 charmap.iso-8859-2.bz2 Same vendor, different version, different behaviour. Again, both UTF-8 and US-ASCII work, and there are several charmap files, but "locale -m" returns something that is neither a charmap name nor a filename for any of the locales, nor a list of anything. Frankly, i doubt the usefulness of "locale -m" in general, and even more so on OpenBSD: if i understand correctly, it is supposed to be used to determine valid input for the -f option of the localedef(1) utility, which we don't even have. Naively, it does seem like it would make sense to have "locale -m" print a list of possible output values of "locale chardef", so i'm not opposed to adding "US-ASCII" to it. But that doesn't appear to be how it works elsewhere, at least not everywhere. I found no documentation stating clearly what it is supposed to do, POSIX feels murky at best. Also, look at this: http://man.bsd.lv/FreeBSD-12.0/locale#BUGS http://man.bsd.lv/NetBSD-8.1/locale#BUGS "BUGS Since FreeBSD does not support charmaps in their POSIX meaning, locale emulates the -m option using the CODESETs listing of all available locales." That does look somehwat similar to what you are suggesting, but *they* call it a bug! Feel free to add "US-ASCII\n" if you like, it does feel as if it might add some minor clarity, but i hardly expect any real practical benefit. Yours, Ingo
