Re: Checking MAC address of incoming unicast packets
As there isn't any consensus for one of the other solutions, I am going to
commit the patch that fixes this inside vio(4).
On Thu, 14 Jan 2016, Stefan Fritsch wrote:
> On Mon, 4 Jan 2016, Stefan Fritsch wrote:
> > On Sun, 3 Jan 2016, Theo de Raadt wrote:
> > > >> dlg writes:
> > > >> > should we just do it unconditionally? is there a downside to that?
> > > >
> > > >It may decrease performance a tiny bit. Since such bits tend to add
> > > >up, I would be hesitant to enable the check for drivers that don't
> > > >need it. OTOH, freebsd and linux seem to always do the check.
> > >
> > > How does it decrease performance?
> >
> > I am talking about this diff in ether_input():
> >
> > diff --git sys/net/if_ethersubr.c sys/net/if_ethersubr.c
> > --- sys/net/if_ethersubr.c
> > +++ sys/net/if_ethersubr.c
> > @@ -353,8 +353,7 @@ ether_input(struct ifnet *ifp, struct mbuf *m, void
> > *cookie)
> > * If packet is unicast and we're in promiscuous mode, make sure it
> > * is for us. Drop otherwise.
> > */
> > - if ((m->m_flags & (M_BCAST|M_MCAST)) == 0 &&
> > - (ifp->if_flags & IFF_PROMISC)) {
> > + if ((m->m_flags & (M_BCAST|M_MCAST)) == 0) {
> > if (memcmp(ac->ac_enaddr, eh->ether_dhost, ETHER_ADDR_LEN)) {
> > m_freem(m);
> > return (1);
> >
> > For other drivers than vio this means an additional 6 byte memcmp where
> > one operand is already in the cache, and the other operand may or may not
> > be in the cache. 'tiny performance impact' seems about right.
>
> Hrvoje Popovski was very helpful and did some performance tests with the
> patch above. There is only measurable difference at very high package
> rates, but I must admit that there the difference is a bit larger than I
> expected:
>
> > i'm sending 64byte packets from host connected to ix2 and counting then
> > on host connected to ix3
>
> > without your patch
>
> sending receiving
> 400kpps 400kpps
> 600kpps 600kpps
> 800kpps 690kpps
> 1Mpps 610kpps
> 1.4Mpps 580kpps
> 12Mpps 580kpps
>
>
> > with your patch
>
> sending receiving
> 400kpps 400kpps - 0%
> 600kpps 600kpps - 0%
> 800kpps 680kpps - 1.4%
> 1Mpps 600kpps - 1.6%
> 1.4Mpps 560kpps - 3.4%
> 12Mpps 560kpps - 3.4%
>
>
>
> So, which variant should I commit?
>
> - do the check unconditionally
> - add some flag so that ether_input() does the check for vio(4)
> unconditionally
> - do the check in vio(4)
>
> mpi fixed a similar bug in trunk(4) in July, but that does not use
> ether_input() and would not profit from the flag approach.
>
>
>
> > > Maybe you should show the diff which does the check in the driver
> > > itself, then then it will be easier to see the performance cost. Right
> > > now I can't perceive the problem.
> >
> > It's not very expensive. But it's more code duplication than I like:
> >
> > diff --git sys/dev/pci/if_vio.c sys/dev/pci/if_vio.c
> > --- sys/dev/pci/if_vio.c
> > +++ sys/dev/pci/if_vio.c
> > @@ -1005,6 +1005,9 @@ vio_rxeof(struct vio_softc *sc)
> > int r = 0;
> > int slot, len, bufs_left;
> > struct virtio_net_hdr *hdr;
> > + int promisc = (ifp->if_flags & IFF_PROMISC);
> > + u_int8_t *enaddr = sc->sc_ac.ac_enaddr;
> > + struct ether_header *eh;
> >
> > while (virtio_dequeue(vsc, vq, &slot, &len) == 0) {
> > r = 1;
> > @@ -1035,10 +1038,33 @@ vio_rxeof(struct vio_softc *sc)
> > bufs_left--;
> > }
> >
> > - if (bufs_left == 0) {
> > - ml_enqueue(&ml, m0);
> > - m0 = NULL;
> > + if (bufs_left > 0)
> > + continue;
> > +
> > + /*
> > +* Unfortunately, mac filtering is only best effort
> > +* in virtio-net. Unwanted packets may still arrive.
> > +* If we are not promiscous, we have to check if the
> > +* packet is actually for us.
> > +*/
> > + if (!promisc) {
> > + m0 = m_pullup(m0, ETHER_HDR_LEN);
> > + /*
> > +* no need to check m0 == NULL, we know m0 has enough
> > +* space
> > +*/
> > +
> > + eh = mtod(m0, struct ether_header *);
> > + if (!ETHER_IS_MULTICAST(eh->ether_dhost) &&
> > + memcmp(enaddr, eh->ether_dhost, ETHER_ADDR_LEN) !=
> > 0) {
> > + m_freem(m0);
> > + m0 = NULL;
> > + continue;
> > + }
> > }
> > +
> > + ml_enqueue(&ml, m0);
> > + m0 = NULL;
> > }
> > if (m0 != NULL) {
> > DBGPRINT("expected %d buffers, got %d", (int)hdr->num_buffers,
> >
> >
>
>
typo in OpenBSD 5.7 errata 22 and OpenBSD 5.8 errata 10
Hello, Thank you for early warning by Theo and quick solution. I noticed typo in http://ftp.openbsd.org/pub/OpenBSD/patches/5.7/common/022_ssh.patch.sig http://ftp.openbsd.org/pub/OpenBSD/patches/5.8/common/010_ssh.patch.sig on line 16 is "And then rebuild and install sshd:" there shall be ssh not sshd Best regards, -- Jiri Navratil, http://kouc.navratil.cz, +420 222 767 131
Re: mg: display wide characters
On 2016-01-21T20:31:05-0500, Kjell Wooding wrote: > Oh goodness. My recollection is that 1 byte per character is assumed all > over the place. This is going to take a *while* to test properly. > > Maybe this is a good time for me to start looking at mg again... Some things that were were challenging, or that were significantly edge-case-y: o extended lines with wide characters (For example, open http://sprunge.us/dRdU on a 80x24 terminal emulator, then press C-e a couple times, or just hold down M-f. Current mg has some problems doing this.) o making sure the dot lines up with the actual insert position in cases involving tabs, non-ASCII, wide non-ASCII, non-printable non-ASCII, extended lines, and mixtures of all of the above o potential for buffer overflow whenever ncol was used to mean `maximum number of bytes we'll ever need' o sanely placing the `$' to denote extended lines, depending on what type of character is the character that goes beyond ncol If there's something that seems suspiciously absent from that list, perhaps it's something I've overlooked and haven't tested myself. Hopefully that helps somewhat. -- S. Gilles
Re: 11n: fix BA timeout value in ADDBA requests/responses
> Date: Thu, 21 Jan 2016 19:06:51 +0100 > From: Stefan Sperling > > The ADDBA frames use a timeout value in units of TU (802.11 time unit). > ba->ba_timeout_val is in usec and has already been multiplied by TU > (e.h. in ieee80211_recv_addba_req()). > We need to divide by TU when copying out to the frame. > > ok? ok kettenis@ > Index: ieee80211_output.c > === > RCS file: /cvs/src/sys/net80211/ieee80211_output.c,v > retrieving revision 1.107 > diff -u -p -r1.107 ieee80211_output.c > --- ieee80211_output.c12 Jan 2016 09:28:09 - 1.107 > +++ ieee80211_output.c21 Jan 2016 17:49:40 - > @@ -1430,7 +1430,7 @@ ieee80211_get_addba_req(struct ieee80211 > if ((ic->ic_htcaps & IEEE80211_HTCAP_DELAYEDBA) == 0) > params |= IEEE80211_ADDBA_BA_POLICY; /* use immediate BA */ > LE_WRITE_2(frm, params); frm += 2; > - LE_WRITE_2(frm, ba->ba_timeout_val); frm += 2; > + LE_WRITE_2(frm, ba->ba_timeout_val / IEEE80211_DUR_TU); frm += 2; > LE_WRITE_2(frm, ba->ba_winstart); frm += 2; > > m->m_pkthdr.len = m->m_len = frm - mtod(m, u_int8_t *); > @@ -1470,7 +1470,7 @@ ieee80211_get_addba_resp(struct ieee8021 > params |= ba->ba_winsize << 6; > LE_WRITE_2(frm, params); frm += 2; > if (status == 0) > - LE_WRITE_2(frm, ba->ba_timeout_val); > + LE_WRITE_2(frm, ba->ba_timeout_val / IEEE80211_DUR_TU); > else > LE_WRITE_2(frm, 0); > frm += 2; > >
11n: fix BA timeout value in ADDBA requests/responses
The ADDBA frames use a timeout value in units of TU (802.11 time unit). ba->ba_timeout_val is in usec and has already been multiplied by TU (e.h. in ieee80211_recv_addba_req()). We need to divide by TU when copying out to the frame. ok? Index: ieee80211_output.c === RCS file: /cvs/src/sys/net80211/ieee80211_output.c,v retrieving revision 1.107 diff -u -p -r1.107 ieee80211_output.c --- ieee80211_output.c 12 Jan 2016 09:28:09 - 1.107 +++ ieee80211_output.c 21 Jan 2016 17:49:40 - @@ -1430,7 +1430,7 @@ ieee80211_get_addba_req(struct ieee80211 if ((ic->ic_htcaps & IEEE80211_HTCAP_DELAYEDBA) == 0) params |= IEEE80211_ADDBA_BA_POLICY; /* use immediate BA */ LE_WRITE_2(frm, params); frm += 2; - LE_WRITE_2(frm, ba->ba_timeout_val); frm += 2; + LE_WRITE_2(frm, ba->ba_timeout_val / IEEE80211_DUR_TU); frm += 2; LE_WRITE_2(frm, ba->ba_winstart); frm += 2; m->m_pkthdr.len = m->m_len = frm - mtod(m, u_int8_t *); @@ -1470,7 +1470,7 @@ ieee80211_get_addba_resp(struct ieee8021 params |= ba->ba_winsize << 6; LE_WRITE_2(frm, params); frm += 2; if (status == 0) - LE_WRITE_2(frm, ba->ba_timeout_val); + LE_WRITE_2(frm, ba->ba_timeout_val / IEEE80211_DUR_TU); else LE_WRITE_2(frm, 0); frm += 2;
Re: I have a mirror testing program for you. - Mirror down
Hi all, On Wed, Jan 20, 2016 at 10:27:44AM +, Stuart Henderson wrote: > On 2016/01/20 10:38, Benjamin Baier wrote: > > Important thing first, the mirror http://openbsd.cs.fau.de/pub/OpenBSD/ > > seems to be down. > > +cc maintainer, could you take a look please Simon? Down for v4+v6, > traceroute stops at informatik.gate.uni-erlangen.de (131.188.20.38 / > 2001:638:a000::3341:41) with !A on v6. I was about to write a mail to mirrors-discuss@ since Monday, sorry... Seems like one of the disks gave up and the hardware RAID controller messed up. I need to look at this on Saturday, when I have physical access to the machine. Stuart, can you maybe remove it from the list for now until I know how bad it really is? Regards, Simon
Re: mg: display wide characters
I am busy for the next month or so, but will try and look at your diff afterwards. Hopefully some others will be able to test/comment as well, in the mean time. Mark
msdosfs uiomove() conversion
Below the uiomove() conversion for msdosfs. This diff prevents
truncation of uio_resid in both msdosfs_read() and msdosfs_write().
Index: msdosfs/msdosfs_vnops.c
===
RCS file: /cvs/src/sys/msdosfs/msdosfs_vnops.c,v
retrieving revision 1.105
diff -u -p -u -r1.105 msdosfs_vnops.c
--- msdosfs/msdosfs_vnops.c 13 Jan 2016 10:00:55 - 1.105
+++ msdosfs/msdosfs_vnops.c 21 Jan 2016 15:52:04 -
@@ -516,10 +516,9 @@ msdosfs_read(void *v)
struct msdosfsmount *pmp = dep->de_pmp;
struct uio *uio = ap->a_uio;
int isadir, error = 0;
- uint32_t n, diff, size;
+ uint32_t n, diff, size, on;
struct buf *bp;
daddr_t cn;
- long on;
/*
* If they didn't ask for any data, then we are done.
@@ -537,7 +536,7 @@ msdosfs_read(void *v)
cn = de_cluster(pmp, uio->uio_offset);
size = pmp->pm_bpcluster;
on = uio->uio_offset & pmp->pm_crbomask;
- n = min((uint32_t) (pmp->pm_bpcluster - on), uio->uio_resid);
+ n = ulmin(pmp->pm_bpcluster - on, uio->uio_resid);
/*
* de_FileSize is uint32_t, and we know that uio_offset <
@@ -569,7 +568,7 @@ msdosfs_read(void *v)
brelse(bp);
return (error);
}
- error = uiomovei(bp->b_data + on, (int) n, uio);
+ error = uiomove(bp->b_data + on, n, uio);
brelse(bp);
} while (error == 0 && uio->uio_resid > 0 && n != 0);
if (!isadir && !(vp->v_mount->mnt_flag & MNT_NOATIME))
@@ -584,9 +583,8 @@ int
msdosfs_write(void *v)
{
struct vop_write_args *ap = v;
- int n;
- int croffset;
- int resid;
+ uint32_t n, croffset;
+ size_t resid;
ssize_t overrun;
int extended = 0;
uint32_t osize;
@@ -715,7 +713,7 @@ msdosfs_write(void *v)
}
}
- n = min(uio->uio_resid, pmp->pm_bpcluster - croffset);
+ n = ulmin(uio->uio_resid, pmp->pm_bpcluster - croffset);
if (uio->uio_offset + n > dep->de_FileSize) {
dep->de_FileSize = uio->uio_offset + n;
uvm_vnp_setsize(vp, dep->de_FileSize);
@@ -729,7 +727,7 @@ msdosfs_write(void *v)
/*
* Copy the data from user space into the buf header.
*/
- error = uiomovei(bp->b_data + croffset, n, uio);
+ error = uiomove(bp->b_data + croffset, n, uio);
/*
* If they want this synchronous then write it and wait for
@@ -1554,7 +1552,7 @@ msdosfs_readdir(void *v)
sizeof(struct direntry);
if (uio->uio_resid < dirbuf.d_reclen)
goto out;
- error = uiomovei(&dirbuf, dirbuf.d_reclen, uio);
+ error = uiomove(&dirbuf, dirbuf.d_reclen, uio);
if (error)
goto out;
offset = dirbuf.d_off;
@@ -1682,7 +1680,7 @@ msdosfs_readdir(void *v)
goto out;
}
wlast = -1;
- error = uiomovei(&dirbuf, dirbuf.d_reclen, uio);
+ error = uiomove(&dirbuf, dirbuf.d_reclen, uio);
if (error) {
brelse(bp);
goto out;
cheers,
natano
mg: display wide characters
Change handling of wide characters so that they are displayed if
mbrtowc(3) can parse them (completely determined by the current
LC_CTYPE). Characters which are not parsed are still displayed as \OOO
sequences, and this wide handling can be disabled by `show-raw-mode'
or `set-default-mode "raw"'.
With multibyte characters displayable, also add insert-char, which
behaves as a restricted version of its emacs counterpart: accepts a
codepoint and inserts the appropriate multibyte sequence. The
codepoint number is whatever wchar_t uses. Modify what-cursor-position
to display this information if the dot is on a byte that is part of a
wide character.
--
This is an admittedly rather large patch: the one byte:one thing to
display identity was subtly ingrained in places, and I tried to avoid
doing a complete char * -> wchar_t * -> char * conversion on the
code. I've tried to keep the logic naive and to match existing style,
but I'm under no illusions. Still, it passes my testing, and fixes a
few bugs that 20160118 displays with extended lines of non-printable
characters.
I should note that this patch introduces warnings, at least with 5.8,
because gcc still (erroneously) complains about initializing to { 0 }.
If that's a serious concern, I can send another version using memset.
I don't have commit access, so if this goes through someone else will
have to finalize it.
Thanks,
S. Gilles
Index: usr.bin/mg/basic.c
===
RCS file: /cvs/src/usr.bin/mg/basic.c,v
retrieving revision 1.47
diff -u -r1.47 basic.c
--- usr.bin/mg/basic.c 10 Oct 2015 09:13:14 - 1.47
+++ usr.bin/mg/basic.c 21 Jan 2016 14:20:52 -
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include "def.h"
@@ -269,12 +270,25 @@
int
getgoal(struct line *dlp)
{
- int c, i, col = 0;
- char tmp[5];
+ return getbyteofcol(dlp, 0, curgoal);
+}
+/*
+ * Return the byte offset within lp that is targetcol columns beyond
+ * startbyte
+ */
+size_t
+getbyteofcol(const struct line *lp, const size_t startbyte,
+ const size_t targetcol)
+{
+ int c;
+ size_t i, col = 0;
+ char tmp[5];
+ size_t advance_by = 1;
- for (i = 0; i < llength(dlp); i++) {
- c = lgetc(dlp, i);
+ for (i = startbyte; i < llength(lp); i += advance_by) {
+ advance_by = 1;
+ c = lgetc(lp, i);
if (c == '\t'
#ifdef NOTAB
&& !(curbp->b_flag & BFNOTAB)
@@ -284,18 +298,86 @@
col++;
} else if (ISCTRL(c) != FALSE) {
col += 2;
- } else if (isprint(c))
+ } else if (isprint(c)) {
col++;
- else {
+ } else if (!(curbp->b_flag & BFSHOWRAW)) {
+ mbstate_t mbs = { 0 };
+ wchar_t wc = 0;
+ size_t consumed = mbrtowc(&wc, &lp->l_text[i],
+ llength(lp) - i, &mbs);
+ int width = -1;
+ if (consumed < (size_t) -2) {
+ width = wcwidth(wc);
+ }
+ if (width >= 0) {
+ col += width;
+ advance_by = consumed;
+ } else {
+ col += snprintf(tmp, sizeof(tmp), "\\%o", c);
+ }
+ } else {
col += snprintf(tmp, sizeof(tmp), "\\%o", c);
}
- if (col > curgoal)
+ if (col > targetcol)
break;
}
return (i);
}
/*
+ * Return the column at which specified offset byte would appear, if
+ * this were part of a longer string printed by vtputs, starting at
+ * intial_col
+ */
+size_t
+getcolofbyte(const struct line *lp, const size_t startbyte,
+ const size_t initial_col, const size_t targetoffset)
+{
+ int c;
+ size_t i, col = initial_col;
+ char tmp[5];
+ size_t advance_by = 1;
+
+ for (i = startbyte; i < llength(lp); i += advance_by) {
+ if (i >= targetoffset)
+ break;
+ advance_by = 1;
+ c = lgetc(lp, i);
+ if (c == '\t'
+#ifdef NOTAB
+ && !(curbp->b_flag & BFNOTAB)
+#endif
+ ) {
+ col |= 0x07;
+ col++;
+ } else if (ISCTRL(c) != FALSE) {
+ col += 2;
+ } else if (isprint(c)) {
+ col++;
+ } else if (!(curbp->b_flag & BFSHOWRAW)) {
+ mbstate_t mbs = { 0 };
+ wchar_t wc = 0;
+ size_t consumed = mbrtowc(&wc, &lp->l_text[i],
+
audioctl: simpler output
The following attributes are always the same for play
and record directions (this is a kernel constraint):
- sample rate
- encoding (including precision, msb and bps)
- block size (in frames per block, not bytes)
- pause
- active
The diff below makes audioctl display a single attribute (which
makes the output shorter and nicer). We don't maintain backward
compatibility of field names as this is mostly a developper tool.
As we're at it drop unused/duplicate fields.
OK?
Index: audioctl.c
===
RCS file: /cvs/src/usr.bin/audioctl/audioctl.c,v
retrieving revision 1.29
diff -u -p -u -p -r1.29 audioctl.c
--- audioctl.c 28 Jul 2015 20:51:10 - 1.29
+++ audioctl.c 21 Jan 2016 07:48:13 -
@@ -63,6 +63,8 @@ int properties, fullduplex;
struct audio_pos getpos;
+unsigned int block_size;
+
struct field {
const char *name;
void *valp;
@@ -85,30 +87,19 @@ struct field {
{ "encodings", encbuf, STRING, READONLY },
{ "properties", &properties,PROPS, READONLY },
{ "hiwat", &info.hiwat,UINT, 0 },
- { "lowat", &info.lowat,UINT, 0 },
{ "mode", &info.mode, P_R,READONLY },
- { "play.rate", &info.play.sample_rate, UINT, 0 },
- { "play.sample_rate", &info.play.sample_rate, UINT, ALIAS },
+ { "rate", &info.play.sample_rate, UINT, 0 },
+ { "precision", &info.play.precision, UINT, 0 },
+ { "bps",&info.play.bps, UINT, 0 },
+ { "msb",&info.play.msb, UINT, 0 },
+ { "encoding", &info.play.encoding,ENC,0 },
+ { "pause", &info.play.pause, UCHAR, 0 },
+ { "active", &info.play.active, UCHAR, READONLY },
+ { "block_size", &block_size,UINT, 0 },
{ "play.channels", &info.play.channels,UINT, 0 },
- { "play.precision", &info.play.precision, UINT, 0 },
- { "play.bps", &info.play.bps, UINT, 0 },
- { "play.msb", &info.play.msb, UINT, 0 },
- { "play.encoding", &info.play.encoding,ENC,0 },
- { "play.pause", &info.play.pause, UCHAR, 0 },
- { "play.active",&info.play.active, UCHAR, READONLY },
- { "play.block_size",&info.play.block_size, UINT, 0 },
{ "play.bytes", &getpos.play_pos, UINT, READONLY },
{ "play.errors",&getpos.play_xrun, UINT, READONLY },
- { "record.rate",&info.record.sample_rate,UINT, 0 },
- { "record.sample_rate", &info.record.sample_rate,UINT, ALIAS },
{ "record.channels",&info.record.channels, UINT, 0 },
- { "record.precision", &info.record.precision, UINT, 0 },
- { "record.bps", &info.record.bps, UINT, 0 },
- { "record.msb", &info.record.msb, UINT, 0 },
- { "record.encoding",&info.record.encoding, ENC,0 },
- { "record.pause", &info.record.pause, UCHAR, 0 },
- { "record.active", &info.record.active,UCHAR, READONLY },
- { "record.block_size", &info.record.block_size,UINT, 0 },
{ "record.bytes", &getpos.rec_pos,UINT, READONLY },
{ "record.errors", &getpos.rec_xrun, UINT, READONLY },
{ 0 }
@@ -303,6 +294,8 @@ getinfo(int fd)
err(1, "AUDIO_GETINFO");
if (ioctl(fd, AUDIO_GETPOS, &getpos) < 0)
err(1, "AUDIO_GETPOS");
+ block_size = info.play.block_size /
+ (info.play.channels * info.play.bps);
}
void
@@ -419,8 +412,19 @@ main(int argc, char **argv)
}
argv++;
}
- if (writeinfo && ioctl(fd, AUDIO_SETINFO, &info) < 0)
- err(1, "set failed");
+ if (writeinfo) {
+ info.record.sample_rate = info.play.sample_rate;
+ info.record.encoding = info.play.encoding;
+ info.record.precision = info.play.precision;
+ info.record.bps = info.play.bps;
+ info.record.msb = info.play.msb;
+ info.record.block_size = block_size *
+ info.record.bps * info.record.channels;
+ info.play.block_size = block_size *
+ info.play.bps * info.play.channels;
+ if (ioctl(fd, AUDIO_SETINFO, &info) < 0)
+ err(1, "set failed");
+ }
getinfo(fd);
for (i = 0; fields[i].name; i++) {
if
