Re: ibuf free fd on close
On Tue, Oct 24, 2023 at 04:00:42PM +0200, Claudio Jeker wrote: > On Tue, Oct 24, 2023 at 03:50:47PM +0200, Theo Buehler wrote: > > On Tue, Oct 24, 2023 at 03:01:26PM +0200, Claudio Jeker wrote: > > > When I added ibuf_get_fd() the idea was to make sure that ibuf_free() will > > > close any fd still on the buffer. This way even if a fd is unexpectedly > > > passed nothing will happen. > > > > > > That code was disabled at start because userland was not fully ready. In > > > particular rpki-client did not handle that well. All of this is to my > > > knowledge fixed so there is no reason to keep the NOTYET :) > > > > > > With this users need to use ibuf_fd_get() to take the fd off the ibuf. > > > Code not doing so will break because ibuf_free() will close the fd which > > > is probably still in use somewhere else. > > > > Nothing in base outside of libutil seems to reach directly for the fd > > (checked by compiling with that struct member renamed in the public > > header). > > > > The internal uses are addressed by this diff, so > > > > ok tb > > > > I can put the fd rename through a bulk to catch some ports in a couple > > of days but I don't think there is a need to wait. > > Thanks. Do we have a list of ports that use ibuf / imsg? mdnsd sets an ibuf fd to -1, which is harmless: https://github.com/haesbaert/mdnsd/blob/master/libmdns/mdnsl.c#L513 got's lib/privsep.c code has a similar pattern of setting the fd before imsg_close(). This won't be affected directly by this change as far as I can see, but the privsep code probably needs an fd audit, ibuf related and otherwise. Just skimming the code I saw a few potential fd leaks, e.g. request_tree() leaks if got_privsep_send_tree_req() exits early. PS: It is quite easy to hit "unexpected end of file" if one clicks on links in the web view. Clicking on "i can't count" here: https://got.gameoftrees.org/?action=summary=got.git
Re: malloc: micro optimizations
Hi,
It works fine for me.
ok asou@
--
ASOU Masato
From: Otto Moerbeek
Date: Wed, 25 Oct 2023 11:04:01 +0200
> Hi,
>
> a few micro-optimization, including getting rid of some statistics
> that are not actualy very interesting.
>
> Speedup amounts to a few tenths of percents to a few percents,
> depending on how biased the benchmark is.
>
> -Otto
>
> Index: stdlib/malloc.c
> ===
> RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.291
> diff -u -p -r1.291 malloc.c
> --- stdlib/malloc.c 22 Oct 2023 12:19:26 - 1.291
> +++ stdlib/malloc.c 24 Oct 2023 14:05:37 -
> @@ -169,16 +169,12 @@ struct dir_info {
> void *caller;
> size_t inserts;
> size_t insert_collisions;
> - size_t finds;
> - size_t find_collisions;
> size_t deletes;
> size_t delete_moves;
> size_t cheap_realloc_tries;
> size_t cheap_reallocs;
> size_t malloc_used; /* bytes allocated */
> size_t malloc_guarded; /* bytes used for guards */
> - size_t pool_searches; /* searches for pool */
> - size_t other_pool; /* searches in other pool */
> #define STATS_ADD(x,y) ((x) += (y))
> #define STATS_SUB(x,y) ((x) -= (y))
> #define STATS_INC(x) ((x)++)
> @@ -209,12 +205,14 @@ static void unmap(struct dir_info *d, vo
> struct chunk_info {
> LIST_ENTRY(chunk_info) entries;
> void *page; /* pointer to the page */
> + /* number of shorts should add up to 8, check alloc_chunk_info() */
> u_short canary;
> u_short bucket;
> u_short free; /* how many free chunks */
> u_short total; /* how many chunks */
> u_short offset; /* requested size table offset */
> - u_short bits[1];/* which chunks are free */
> +#define CHUNK_INFO_TAIL 3
> + u_short bits[CHUNK_INFO_TAIL]; /* which chunks are free */
> };
>
> #define CHUNK_FREE(i, n) ((i)->bits[(n) / MALLOC_BITS] & (1U << ((n) %
> MALLOC_BITS)))
> @@ -656,12 +654,10 @@ find(struct dir_info *d, void *p)
> index = hash(p) & mask;
> r = d->r[index].p;
> q = MASK_POINTER(r);
> - STATS_INC(d->finds);
> while (q != p && r != NULL) {
> index = (index - 1) & mask;
> r = d->r[index].p;
> q = MASK_POINTER(r);
> - STATS_INC(d->find_collisions);
> }
> return (q == p && r != NULL) ? >r[index] : NULL;
> }
> @@ -949,7 +945,7 @@ init_chunk_info(struct dir_info *d, stru
>
> p->bucket = bucket;
> p->total = p->free = MALLOC_PAGESIZE / B2ALLOC(bucket);
> - p->offset = bucket == 0 ? 0xdead : howmany(p->total, MALLOC_BITS);
> + p->offset = howmany(p->total, MALLOC_BITS);
> p->canary = (u_short)d->canary1;
>
> /* set all valid bits in the bitmap */
> @@ -971,8 +967,13 @@ alloc_chunk_info(struct dir_info *d, u_i
> count = MALLOC_PAGESIZE / B2ALLOC(bucket);
>
> size = howmany(count, MALLOC_BITS);
> - size = sizeof(struct chunk_info) + (size - 1) * sizeof(u_short);
> - if (mopts.chunk_canaries)
> + /* see declaration of struct chunk_info */
> + if (size <= CHUNK_INFO_TAIL)
> + size = 0;
> + else
> + size -= CHUNK_INFO_TAIL;
> + size = sizeof(struct chunk_info) + size * sizeof(u_short);
> + if (mopts.chunk_canaries && bucket > 0)
> size += count * sizeof(u_short);
> size = _ALIGN(size);
> count = MALLOC_PAGESIZE / size;
> @@ -1129,8 +1130,7 @@ fill_canary(char *ptr, size_t sz, size_t
> static void *
> malloc_bytes(struct dir_info *d, size_t size)
> {
> - u_int i, r, bucket, listnum;
> - size_t k;
> + u_int i, k, r, bucket, listnum;
> u_short *lp;
> struct chunk_info *bp;
> void *p;
> @@ -1170,7 +1170,7 @@ malloc_bytes(struct dir_info *d, size_t
> /* no bit halfway, go to next full short */
> i /= MALLOC_BITS;
> for (;;) {
> - if (++i >= howmany(bp->total, MALLOC_BITS))
> + if (++i >= bp->offset)
> i = 0;
> lp = >bits[i];
> if (*lp) {
> @@ -1228,7 +1228,7 @@ validate_canary(struct dir_info *d, u_ch
> }
> }
>
> -static uint32_t
> +static inline uint32_t
> find_chunknum(struct dir_info *d, struct chunk_info *info, void *ptr, int
> check)
> {
> uint32_t chunknum;
> @@ -1532,12 +1532,10 @@ findpool(void *p, struct dir_info *argpo
> struct dir_info *pool = argpool;
> struct region_info *r = find(pool, p);
>
> - STATS_INC(pool->pool_searches);
> if (r == NULL) {
> u_int i, nmutexes;
>
> nmutexes =
Re: dhcpd(8): Parse lease database after dropping privileges
Hello,
I just wanted to double check if there is any additional feedback on the patch.
I am happy to make changes or discuss alternative approaches.
Thank you,
Stephen
On Tue, Oct 3, 2023 at 11:33 PM Stephen Fox wrote:
>
> Hello,
>
> I received feedback from deraadt that the first two unveil(2) calls were
> unnecessary because pledge(2) automatically unveils "/usr/share/zoneinfo".
>
> This updated patch removes the unneeded unveil(2) calls.
>
> Best regards,
> Stephen
> ---
> usr.sbin/dhcpd/confpars.c | 41 ++-
> usr.sbin/dhcpd/db.c | 19 ++
> usr.sbin/dhcpd/dhcpd.c| 30 ++--
> usr.sbin/dhcpd/dhcpd.h| 2 ++
> 4 files changed, 76 insertions(+), 16 deletions(-)
>
> diff --git usr.sbin/dhcpd/confpars.c usr.sbin/dhcpd/confpars.c
> index 1bdffb38842..bb245fc663e 100644
> --- usr.sbin/dhcpd/confpars.c
> +++ usr.sbin/dhcpd/confpars.c
> @@ -57,6 +57,8 @@
> #include "dhctoken.h"
> #include "log.h"
>
> +FILE *db_file_ro;
> +
> intparse_cidr(FILE *, unsigned char *, unsigned char *);
>
> /*
> @@ -106,18 +108,11 @@ readconf(void)
> }
>
> /*
> - * lease-file :== lease-declarations EOF
> - * lease-statments :==
> - *| lease-declaration
> - *| lease-declarations lease-declaration
> + * Open and retain a read-only file descriptor to the lease database file.
> */
> void
> -read_leases(void)
> +open_leases(void)
> {
> - FILE *cfile;
> - char *val;
> - int token;
> -
> new_parse(path_dhcpd_db);
>
> /*
> @@ -131,23 +126,40 @@ read_leases(void)
> * thinking that no leases have been assigned to anybody, which
> * could create severe network chaos.
> */
> - if ((cfile = fopen(path_dhcpd_db, "r")) == NULL) {
> + if ((db_file_ro = fopen(path_dhcpd_db, "r")) == NULL) {
> log_warn("Can't open lease database (%s)", path_dhcpd_db);
> log_warnx("check for failed database rewrite attempt!");
> log_warnx("Please read the dhcpd.leases manual page if you");
> fatalx("don't know what to do about this.");
> }
> +}
> +
> +/*
> + * lease-file :== lease-declarations EOF
> + * lease-statments :==
> + *| lease-declaration
> + *| lease-declarations lease-declaration
> + */
> +void
> +read_leases(void)
> +{
> + char *val;
> + int token;
> +
> + if (!db_file_ro) {
> + fatalx("db_file_ro is NULL");
> + }
>
> do {
> - token = next_token(, cfile);
> + token = next_token(, db_file_ro);
> if (token == EOF)
> break;
> if (token != TOK_LEASE) {
> log_warnx("Corrupt lease file - possible data loss!");
> - skip_to_semi(cfile);
> + skip_to_semi(db_file_ro);
> } else {
> struct lease *lease;
> - lease = parse_lease_declaration(cfile);
> + lease = parse_lease_declaration(db_file_ro);
> if (lease)
> enter_lease(lease);
> else
> @@ -155,7 +167,8 @@ read_leases(void)
> }
>
> } while (1);
> - fclose(cfile);
> + fclose(db_file_ro);
> + db_file_ro = NULL;
> }
>
> /*
> diff --git usr.sbin/dhcpd/db.c usr.sbin/dhcpd/db.c
> index 295e522b1ce..634ec8a593a 100644
> --- usr.sbin/dhcpd/db.c
> +++ usr.sbin/dhcpd/db.c
> @@ -190,6 +190,16 @@ commit_leases(void)
> return (1);
> }
>
> +/*
> + * Open and retain two file descriptors to the lease database file:
> + *
> + * - A read-only fd for learning existing leases
> + * - A write-only fd for writing new leases
> + *
> + * Reading and parsing data from the read-only fd is done separately.
> + * This allows privilege drop to happen *before* parsing untrusted
> + * client data from the lease database file.
> + */
> void
> db_startup(void)
> {
> @@ -202,6 +212,15 @@ db_startup(void)
> if ((db_file = fdopen(db_fd, "w")) == NULL)
> fatalx("Can't fdopen new lease file!");
>
> + open_leases();
> +}
> +
> +/*
> + * Read and parse the existing leases from the lease database file.
> + */
> +void
> +db_parse(void)
> +{
> /* Read in the existing lease file... */
> read_leases();
> time(_time);
> diff --git usr.sbin/dhcpd/dhcpd.c usr.sbin/dhcpd/dhcpd.c
> index b3562dce34f..7759f7839e0 100644
> --- usr.sbin/dhcpd/dhcpd.c
> +++ usr.sbin/dhcpd/dhcpd.c
> @@ -244,8 +244,6 @@ main(int argc, char *argv[])
>
> icmp_startup(1, lease_pinged);
>
> - if (chroot(pw->pw_dir) == -1)
> - fatal("chroot %s", pw->pw_dir);
> if (chdir("/") == -1)
> fatal("chdir(\"/\")");
> if (setgroups(1, >pw_gid) ||
> @@ -253,6 +251,34
Re: patch unveil fail
On Wed, Oct 25, 2023 at 07:00:28PM +0200, Omar Polo wrote:
> On 2023/10/25 13:38:37 +0200, Alexander Bluhm wrote:
> > @@ -213,11 +214,27 @@ main(int argc, char *argv[])
> > perror("unveil");
> > my_exit(2);
> > }
> > - if (filearg[0] != NULL)
> > + if (filearg[0] != NULL) {
> > + char *origdir;
> > +
> > if (unveil(filearg[0], "rwc") == -1) {
> > perror("unveil");
> > my_exit(2);
> > }
> > + if ((origdir = dirname(filearg[0])) == NULL) {
>
> Not sure if we're interested in it, but dirname(3) theoretically alter
> the passed string. our dirname doesn't do it, but per posix it can,
> IIUC. This could cause issues since filearg[0] is used later.
>
> If we care about portability here, we should pass a copy to dirname.
> don't know if we care thought.
unveil(2) is not portable code anyway. And dirname(3) is only used
for that.
> > + perror("dirname");
> > + my_exit(2);
> > + }
> > + if (unveil(origdir, "rwc") == -1) {
> > + perror("unveil");
> > + my_exit(2);
> > + }
> > + } else {
> > + if (unveil(".", "rwc") == -1) {
> > + perror("unveil");
> > + my_exit(2);
> > + }
> > + }
> > if (filearg[1] != NULL)
> > if (unveil(filearg[1], "r") == -1) {
> > perror("unveil");
Re: patch unveil fail
On 2023/10/25 13:38:37 +0200, Alexander Bluhm wrote:
> Index: patch.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/patch/patch.c,v
> diff -u -p -r1.74 patch.c
> --- patch.c 19 Jul 2023 13:26:20 - 1.74
> +++ patch.c 24 Oct 2023 17:13:28 -
> @@ -32,6 +32,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -213,11 +214,27 @@ main(int argc, char *argv[])
> perror("unveil");
> my_exit(2);
> }
> - if (filearg[0] != NULL)
> + if (filearg[0] != NULL) {
> + char *origdir;
> +
> if (unveil(filearg[0], "rwc") == -1) {
> perror("unveil");
> my_exit(2);
> }
> + if ((origdir = dirname(filearg[0])) == NULL) {
Not sure if we're interested in it, but dirname(3) theoretically alter
the passed string. our dirname doesn't do it, but per posix it can,
IIUC. This could cause issues since filearg[0] is used later.
If we care about portability here, we should pass a copy to dirname.
don't know if we care thought.
> + perror("dirname");
> + my_exit(2);
> + }
> + if (unveil(origdir, "rwc") == -1) {
> + perror("unveil");
> + my_exit(2);
> + }
> + } else {
> + if (unveil(".", "rwc") == -1) {
> + perror("unveil");
> + my_exit(2);
> + }
> + }
> if (filearg[1] != NULL)
> if (unveil(filearg[1], "r") == -1) {
> perror("unveil");
Re: patch unveil fail
On Wed, 25 Oct 2023 13:38:37 +0200, Alexander Bluhm wrote: > Since 7.4 patch(1) does not work if an explicit patchfile is given on > command line. > > https://marc.info/?l=openbsd-cvs=168941770509379=2 OK millert@ - todd
Re: patch unveil fail
reads correct, OK florian
On 2023-10-25 13:38 +02, Alexander Bluhm wrote:
> Hi,
>
> Since 7.4 patch(1) does not work if an explicit patchfile is given on
> command line.
>
> https://marc.info/?l=openbsd-cvs=168941770509379=2
>
> root@ot14:.../~# patch /usr/src/usr.bin/patch/patch.c patch-unveil.diff
> Hmm... Looks like a unified diff to me...
> The text leading up to this was:
> --
> |Index: patch.c
> |===
> |RCS file: /data/mirror/openbsd/cvs/src/usr.bin/patch/patch.c,v
> |diff -u -p -r1.74 patch.c
> |--- patch.c19 Jul 2023 13:26:20 - 1.74
> |+++ patch.c24 Oct 2023 17:13:28 -
> --
> Patching file /usr/src/usr.bin/patch/patch.c using Plan A...
> Hunk #1 succeeded at 32.
> Hunk #2 succeeded at 214.
> Hunk #3 succeeded at 245.
> Can't backup /usr/src/usr.bin/patch/patch.c, output is in
> /tmp/patchoorjYymLKcM: No such file or directory
> done
>
> A backup file should be created in the directory of the original
> file, but only the current directory is unveiled. Then the patched
> file is created in /tmp and does not replace the original patchfile
> in place.
>
> Diff below fixes it.
>
> ok?
>
> bluhm
>
> Index: patch.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/patch/patch.c,v
> diff -u -p -r1.74 patch.c
> --- patch.c 19 Jul 2023 13:26:20 - 1.74
> +++ patch.c 24 Oct 2023 17:13:28 -
> @@ -32,6 +32,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -213,11 +214,27 @@ main(int argc, char *argv[])
> perror("unveil");
> my_exit(2);
> }
> - if (filearg[0] != NULL)
> + if (filearg[0] != NULL) {
> + char *origdir;
> +
> if (unveil(filearg[0], "rwc") == -1) {
> perror("unveil");
> my_exit(2);
> }
> + if ((origdir = dirname(filearg[0])) == NULL) {
> + perror("dirname");
> + my_exit(2);
> + }
> + if (unveil(origdir, "rwc") == -1) {
> + perror("unveil");
> + my_exit(2);
> + }
> + } else {
> + if (unveil(".", "rwc") == -1) {
> + perror("unveil");
> + my_exit(2);
> + }
> + }
> if (filearg[1] != NULL)
> if (unveil(filearg[1], "r") == -1) {
> perror("unveil");
> @@ -228,10 +245,6 @@ main(int argc, char *argv[])
> perror("unveil");
> my_exit(2);
> }
> - if (unveil(".", "rwc") == -1) {
> - perror("unveil");
> - my_exit(2);
> - }
> if (*rejname != '\0')
> if (unveil(rejname, "rwc") == -1) {
> perror("unveil");
>
--
In my defence, I have been left unsupervised.
installer: suggest CDN if ftplist1 doesn't work
Dear all,
In case no mirror was previously configured (aka, this is a fresh
install) and ftplist1.openbsd.org is unreachable, perhaps it's useful to
suggest a default mirror to expedite the installation process.
Terminal transcript on a machine that's not able to reach ftplist1,
doing 'enter enter enter enter':
Let's install the sets!
Location of sets? (disk http nfs or 'done') [http]
HTTP proxy URL? (e.g. 'http://proxy:8080', or 'none') [none]
(Unable to get list from openbsd.org, but that is OK)
HTTP Server? (hostname or 'done') [cdn.openbsd.org]
Server directory? [pub/OpenBSD/snapshots/amd64]
The below diff is joint work with kn@
OK?
Kind regards,
Job
Index: distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
diff -u -p -r1.1257 install.sub
--- distrib/miniroot/install.sub24 Oct 2023 18:03:53 - 1.1257
+++ distrib/miniroot/install.sub25 Oct 2023 13:42:28 -
@@ -1868,6 +1868,7 @@ install_http() {
else
echo "(Unable to get list from openbsd.org, but that is OK)"
_prompt="HTTP Server? (hostname or 'done')"
+ HTTP_SERVER=$DEFAULT_MIRROR
fi
# Use information from /etc/installurl as defaults for upgrades.
@@ -2935,7 +2936,7 @@ finish_up() {
# Create /etc/installurl if it does not yet exist.
if [[ ! -f /mnt/etc/installurl ]]; then
- echo "${INSTALL_URL:-https://cdn.openbsd.org/pub/OpenBSD}; \
+ echo "${INSTALL_URL:-https://${DEFAULT_MIRROR}/pub/OpenBSD}; \
>/mnt/etc/installurl
fi
@@ -3621,6 +3622,7 @@ UPGRADE_BSDRD=false
V4_AUTOCONF=false
V6_AUTOCONF=false
WLANLIST=/tmp/i/wlanlist
+DEFAULT_MIRROR=cdn.openbsd.org
# Save one boot's worth of dmesg.
dmesgtail >/var/run/dmesg.boot
Re: IPv4 on ix(4) slow/nothing - 7.4
On Thu, Oct 19, 2023 at 04:04:26PM +0200, Jan Klemkow wrote:
> On Wed, Oct 18, 2023 at 08:53:44PM +0200, Alexander Bluhm wrote:
> > On Wed, Oct 18, 2023 at 08:19:29PM +0200, Mischa wrote:
> > > It's indeed something like that: ix -> vlan (tagged) -> veb
> >
> > When vlan is added to veb, kernel should disable LRO on ix.
> > All testing before release did not find this code path :-(
> >
> > Is it possible to add vlan to veb first, and then add or change the
> > vlan parent to ix? If it works, that should also disable LRO.
> >
> > Jan said he will have a look tomorrow.
> >
> > trunk, carp, ... in veb or bridge might have the same issue.
>
> First round of fixes for vlan(4), vxlan(4), nvgre(4) and bpe(4).
Don't know much about nvgre(4) and bpe(4).
Maybe we should call ifsetlro(ifp0, 0) unconditionally in
vxlan_set_parent(). Otherwise we may get large UDP packets with
large TCP frames.
For vlan(4) this diff is correct. For the others it is at least
an improvement.
OK bluhm@
> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.708
> diff -u -p -r1.708 if.c
> --- net/if.c 16 Sep 2023 09:33:27 - 1.708
> +++ net/if.c 19 Oct 2023 13:03:33 -
> @@ -3243,6 +3243,17 @@ ifsetlro(struct ifnet *ifp, int on)
> struct ifreq ifrq;
> int error = 0;
> int s = splnet();
> + struct if_parent parent;
> +
> + memset(, 0, sizeof(parent));
> + if ((*ifp->if_ioctl)(ifp, SIOCGIFPARENT, (caddr_t)) != -1) {
> + struct ifnet *ifp0 = if_unit(parent.ifp_parent);
> +
> + if (ifp0 != NULL) {
> + ifsetlro(ifp0, on);
> + if_put(ifp0);
> + }
> + }
>
> if (!ISSET(ifp->if_capabilities, IFCAP_LRO)) {
> error = ENOTSUP;
> Index: net/if_bpe.c
> ===
> RCS file: /cvs/src/sys/net/if_bpe.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 if_bpe.c
> --- net/if_bpe.c 8 Nov 2021 04:54:44 - 1.19
> +++ net/if_bpe.c 19 Oct 2023 13:20:18 -
> @@ -631,6 +631,9 @@ bpe_set_parent(struct bpe_softc *sc, con
> goto put;
> }
>
> + if (ether_brport_isset(ifp))
> + ifsetlro(ifp0, 0);
> +
> /* commit */
> sc->sc_key.k_if = ifp0->if_index;
> etherbridge_flush(>sc_eb, IFBF_FLUSHALL);
> Index: net/if_gre.c
> ===
> RCS file: /cvs/src/sys/net/if_gre.c,v
> retrieving revision 1.174
> diff -u -p -r1.174 if_gre.c
> --- net/if_gre.c 13 May 2023 13:35:17 - 1.174
> +++ net/if_gre.c 19 Oct 2023 13:24:56 -
> @@ -3544,6 +3544,9 @@ nvgre_set_parent(struct nvgre_softc *sc,
> return (EPROTONOSUPPORT);
> }
>
> + if (ether_brport_isset(>sc_ac.ac_if))
> + ifsetlro(ifp0, 0);
> +
> /* commit */
> sc->sc_ifp0 = ifp0->if_index;
> if_put(ifp0);
> Index: net/if_vlan.c
> ===
> RCS file: /cvs/src/sys/net/if_vlan.c,v
> retrieving revision 1.215
> diff -u -p -r1.215 if_vlan.c
> --- net/if_vlan.c 16 May 2023 14:32:54 - 1.215
> +++ net/if_vlan.c 19 Oct 2023 11:08:23 -
> @@ -937,6 +937,9 @@ vlan_set_parent(struct vlan_softc *sc, c
> if (error != 0)
> goto put;
>
> + if (ether_brport_isset(ifp))
> + ifsetlro(ifp0, 0);
> +
> /* commit */
> sc->sc_ifidx0 = ifp0->if_index;
> if (!ISSET(sc->sc_flags, IFVF_LLADDR))
> Index: net/if_vxlan.c
> ===
> RCS file: /cvs/src/sys/net/if_vxlan.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 if_vxlan.c
> --- net/if_vxlan.c3 Aug 2023 09:49:08 - 1.93
> +++ net/if_vxlan.c19 Oct 2023 13:18:47 -
> @@ -1582,6 +1582,9 @@ vxlan_set_parent(struct vxlan_softc *sc,
> goto put;
> }
>
> + if (ether_brport_isset(ifp))
> + ifsetlro(ifp0, 0);
> +
> /* commit */
> sc->sc_if_index0 = ifp0->if_index;
> etherbridge_flush(>sc_eb, IFBF_FLUSHALL);
patch unveil fail
Hi,
Since 7.4 patch(1) does not work if an explicit patchfile is given on
command line.
https://marc.info/?l=openbsd-cvs=168941770509379=2
root@ot14:.../~# patch /usr/src/usr.bin/patch/patch.c patch-unveil.diff
Hmm... Looks like a unified diff to me...
The text leading up to this was:
--
|Index: patch.c
|===
|RCS file: /data/mirror/openbsd/cvs/src/usr.bin/patch/patch.c,v
|diff -u -p -r1.74 patch.c
|--- patch.c19 Jul 2023 13:26:20 - 1.74
|+++ patch.c24 Oct 2023 17:13:28 -
--
Patching file /usr/src/usr.bin/patch/patch.c using Plan A...
Hunk #1 succeeded at 32.
Hunk #2 succeeded at 214.
Hunk #3 succeeded at 245.
Can't backup /usr/src/usr.bin/patch/patch.c, output is in
/tmp/patchoorjYymLKcM: No such file or directory
done
A backup file should be created in the directory of the original
file, but only the current directory is unveiled. Then the patched
file is created in /tmp and does not replace the original patchfile
in place.
Diff below fixes it.
ok?
bluhm
Index: patch.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/patch/patch.c,v
diff -u -p -r1.74 patch.c
--- patch.c 19 Jul 2023 13:26:20 - 1.74
+++ patch.c 24 Oct 2023 17:13:28 -
@@ -32,6 +32,7 @@
#include
#include
+#include
#include
#include
#include
@@ -213,11 +214,27 @@ main(int argc, char *argv[])
perror("unveil");
my_exit(2);
}
- if (filearg[0] != NULL)
+ if (filearg[0] != NULL) {
+ char *origdir;
+
if (unveil(filearg[0], "rwc") == -1) {
perror("unveil");
my_exit(2);
}
+ if ((origdir = dirname(filearg[0])) == NULL) {
+ perror("dirname");
+ my_exit(2);
+ }
+ if (unveil(origdir, "rwc") == -1) {
+ perror("unveil");
+ my_exit(2);
+ }
+ } else {
+ if (unveil(".", "rwc") == -1) {
+ perror("unveil");
+ my_exit(2);
+ }
+ }
if (filearg[1] != NULL)
if (unveil(filearg[1], "r") == -1) {
perror("unveil");
@@ -228,10 +245,6 @@ main(int argc, char *argv[])
perror("unveil");
my_exit(2);
}
- if (unveil(".", "rwc") == -1) {
- perror("unveil");
- my_exit(2);
- }
if (*rejname != '\0')
if (unveil(rejname, "rwc") == -1) {
perror("unveil");
make(1): document VPATH variable
I just noticed that our make(1) supports VPATH, but is not documented. OK? Index: make.1 === RCS file: /cvs/src/usr.bin/make/make.1,v diff -u -p -r1.141 make.1 --- make.1 10 Aug 2023 10:56:34 - 1.141 +++ make.1 25 Oct 2023 10:45:32 - @@ -832,6 +832,10 @@ It should not be used; see the section below. .It Va .VARIABLES List of all the names of global variables that have been set. +.It Va VPATH +A colon separated list of directories appended to the search path (see +.Va .PATH ) . +Supported for compatibility with other implementations. .El .Pp Variable expansion may be modified to select or modify each word of the
malloc: micro optimizations
Hi,
a few micro-optimization, including getting rid of some statistics
that are not actualy very interesting.
Speedup amounts to a few tenths of percents to a few percents,
depending on how biased the benchmark is.
-Otto
Index: stdlib/malloc.c
===
RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.291
diff -u -p -r1.291 malloc.c
--- stdlib/malloc.c 22 Oct 2023 12:19:26 - 1.291
+++ stdlib/malloc.c 24 Oct 2023 14:05:37 -
@@ -169,16 +169,12 @@ struct dir_info {
void *caller;
size_t inserts;
size_t insert_collisions;
- size_t finds;
- size_t find_collisions;
size_t deletes;
size_t delete_moves;
size_t cheap_realloc_tries;
size_t cheap_reallocs;
size_t malloc_used; /* bytes allocated */
size_t malloc_guarded; /* bytes used for guards */
- size_t pool_searches; /* searches for pool */
- size_t other_pool; /* searches in other pool */
#define STATS_ADD(x,y) ((x) += (y))
#define STATS_SUB(x,y) ((x) -= (y))
#define STATS_INC(x) ((x)++)
@@ -209,12 +205,14 @@ static void unmap(struct dir_info *d, vo
struct chunk_info {
LIST_ENTRY(chunk_info) entries;
void *page; /* pointer to the page */
+ /* number of shorts should add up to 8, check alloc_chunk_info() */
u_short canary;
u_short bucket;
u_short free; /* how many free chunks */
u_short total; /* how many chunks */
u_short offset; /* requested size table offset */
- u_short bits[1];/* which chunks are free */
+#define CHUNK_INFO_TAIL3
+ u_short bits[CHUNK_INFO_TAIL]; /* which chunks are free */
};
#define CHUNK_FREE(i, n) ((i)->bits[(n) / MALLOC_BITS] & (1U << ((n) %
MALLOC_BITS)))
@@ -656,12 +654,10 @@ find(struct dir_info *d, void *p)
index = hash(p) & mask;
r = d->r[index].p;
q = MASK_POINTER(r);
- STATS_INC(d->finds);
while (q != p && r != NULL) {
index = (index - 1) & mask;
r = d->r[index].p;
q = MASK_POINTER(r);
- STATS_INC(d->find_collisions);
}
return (q == p && r != NULL) ? >r[index] : NULL;
}
@@ -949,7 +945,7 @@ init_chunk_info(struct dir_info *d, stru
p->bucket = bucket;
p->total = p->free = MALLOC_PAGESIZE / B2ALLOC(bucket);
- p->offset = bucket == 0 ? 0xdead : howmany(p->total, MALLOC_BITS);
+ p->offset = howmany(p->total, MALLOC_BITS);
p->canary = (u_short)d->canary1;
/* set all valid bits in the bitmap */
@@ -971,8 +967,13 @@ alloc_chunk_info(struct dir_info *d, u_i
count = MALLOC_PAGESIZE / B2ALLOC(bucket);
size = howmany(count, MALLOC_BITS);
- size = sizeof(struct chunk_info) + (size - 1) * sizeof(u_short);
- if (mopts.chunk_canaries)
+ /* see declaration of struct chunk_info */
+ if (size <= CHUNK_INFO_TAIL)
+ size = 0;
+ else
+ size -= CHUNK_INFO_TAIL;
+ size = sizeof(struct chunk_info) + size * sizeof(u_short);
+ if (mopts.chunk_canaries && bucket > 0)
size += count * sizeof(u_short);
size = _ALIGN(size);
count = MALLOC_PAGESIZE / size;
@@ -1129,8 +1130,7 @@ fill_canary(char *ptr, size_t sz, size_t
static void *
malloc_bytes(struct dir_info *d, size_t size)
{
- u_int i, r, bucket, listnum;
- size_t k;
+ u_int i, k, r, bucket, listnum;
u_short *lp;
struct chunk_info *bp;
void *p;
@@ -1170,7 +1170,7 @@ malloc_bytes(struct dir_info *d, size_t
/* no bit halfway, go to next full short */
i /= MALLOC_BITS;
for (;;) {
- if (++i >= howmany(bp->total, MALLOC_BITS))
+ if (++i >= bp->offset)
i = 0;
lp = >bits[i];
if (*lp) {
@@ -1228,7 +1228,7 @@ validate_canary(struct dir_info *d, u_ch
}
}
-static uint32_t
+static inline uint32_t
find_chunknum(struct dir_info *d, struct chunk_info *info, void *ptr, int
check)
{
uint32_t chunknum;
@@ -1532,12 +1532,10 @@ findpool(void *p, struct dir_info *argpo
struct dir_info *pool = argpool;
struct region_info *r = find(pool, p);
- STATS_INC(pool->pool_searches);
if (r == NULL) {
u_int i, nmutexes;
nmutexes = mopts.malloc_pool[1]->malloc_mt ?
mopts.malloc_mutexes : 2;
- STATS_INC(pool->other_pool);
for (i = 1; i < nmutexes; i++) {
u_int j = (argpool->mutex + i) & (nmutexes - 1);
@@
Re: boot loaders: softraid volumes must be on RAID partitions
10/24/23 14:03, Crystal Kolipe пишет:
> On Tue, Oct 24, 2023 at 01:44:08AM +, Klemens Nanni wrote:
>> Rereading the code, I now question why it checks the 'a' label type at all.
>>
>> Taking your sd0d example through devboot():
>>
>> |#ifdef SOFTRAID
>> |/*
>> | * Determine the partition type for the 'a' partition of the
>> | * boot device.
>> | */
>> |TAILQ_FOREACH(dip, , list)
>> |if (dip->bios_info.bios_number == bootdev &&
>> |(dip->bios_info.flags & BDI_BADLABEL) == 0)
>> |part_type = dip->disklabel.d_partitions[0].p_fstype;
>>
>> Whatever sd0a's type is...
>>
>> |
>> |/*
>> | * See if we booted from a disk that is a member of a bootable
>> | * softraid volume.
>> | */
>> |SLIST_FOREACH(bv, _volumes, sbv_link) {
>> |if (bv->sbv_flags & BIOC_SCBOOTABLE)
>> |SLIST_FOREACH(bc, >sbv_chunks, sbc_link)
>> |if (bc->sbc_disk == bootdev)
>>
>> ... the boot loader sees we booted from a bootable softraid chunk,
>> matching disk sd0 by number and not partition.
>>
>> |sr_boot_vol = bv->sbv_unit;
>> |if (sr_boot_vol != -1)
>> |break;
>> |}
>> |#endif
>> |
>> |if (sr_boot_vol != -1 && part_type != FS_BSDFFS) {
>>
>> With softraid chunk on sd0d, sd0a happens to be not FFS (likely unused),
>> but that's still enough for the boot loader to stick to softraid on sd0!
>>
>> |*p++ = 's';
>> |*p++ = 'r';
>> |*p++ = '0' + sr_boot_vol;
>> |} else if (bootdev & 0x100) {
>>
>> So why check the 'a' label type if that's not relevant?
>
> The only reason I can see for that check is to better support the unusual
> configuration of booting from a disk which happens to be part of a bootable
> softraid volume, but which also has a regular FFS partition outside of the
> RAID, and for whatever reason the user wants to automatically boot from that
> instead.
>
> Maybe that was useful before support for booting from softraid crypto volumes
> was introduced, (when it was raid-1 only). It doesn't seem very useful now.
>
>> It is confusing and
>> Doubling down on the assumption that bootable chunks are always on 'a'
>> like my diff did shows that's a mistake and dropping the check actually
>> makes more sense to me now.
>
> I agree it is confusing.
>
> Effectively the code is there to treat part_type == FS_BSDFFS an 'edge case'
> as described above.
>
>> This boots with root on softraid on sd0a and sd0d.
>>
>> Thoughts?
>> Am I missing something?
>
> It works OK here, so I'm fine with removing this.
>
That matches sys/arch/amd64/stand/libsa/dev_i386.c r1.12 from 2012
which introduced this code; my second diff would be a revert:
If we have booted from a disk that is a member of a bootable
softraid volume, always select the srX device unless the 'a'
partition of the disk is FFS.
(Other architectures copied that, except sparc64 checking FS_RAID.)
OpenBSD Errata: October 25, 2023 (xserver msplit)
Errata patches for X11 server and kernel mbuf handling have been released for OpenBSD 7.3 and 7.4. Binary updates for the amd64, arm64 and i386 platform are available via the syspatch utility. Source code patches can be found on the respective errata page: https://www.openbsd.org/errata73.html https://www.openbsd.org/errata74.html
