Re: pipex(4): remove PIPEXCSESSION ioctl(2) command
ok yasuoka
On Mon, 11 Jul 2022 01:11:22 +0300
Vitaliy Makkoveev wrote:
> Long time ago pipex(4) session can't be deleted until both pipex(4)
> input and output queues become empty. Dead sessions were linked to the
> stack and the `ip_forward' flag was used to prevent packets forwarding.
> npppd(8) marked such sessions by doing PIPEXCSESSION ioctl(2) call.
>
> But since we started to unlink close session from the stack, this logic
> became unnecessary. Also pipex(4) session could be closed just after
> close request.
>
> npppd(8) was the only userland program which did PIPEXCSESSION ioctl(2)
> call, and we removed it week ago. It's time to remove the remains from
> kernel and pipex(4) man page.
>
> Now the `flags' member of 'pipex_session' structure became immutable.
>
> Index: share/man/man4/pipex.4
> ===
> RCS file: /cvs/src/share/man/man4/pipex.4,v
> retrieving revision 1.14
> diff -u -p -r1.14 pipex.4
> --- share/man/man4/pipex.42 Jan 2021 13:15:15 - 1.14
> +++ share/man/man4/pipex.410 Jul 2022 21:59:42 -
> @@ -179,18 +179,6 @@ See
> section for a description of the
> .Vt pipex_statistics
> structure.
> -.It Dv PIPEXCSESSION Fa "struct pipex_session_config_req *"
> -Change the configuration of the specified session.
> -The session and configuration are specified by a
> -.Vt pipex_session_config_req
> -structure, which has the following definition:
> -.Bd -literal
> -struct pipex_session_config_req {
> -int pcr_protocol; /* tunnel protocol */
> -uint16_t pcr_session_id; /* session-id */
> -int pcr_ip_forward; /* ip_forwarding on/off */
> -};
> -.Ed
> .It Dv PIPEXGSTATFa "struct pipex_session_stat_req *"
> Get statistics for the specified session.
> Specify the session using a
> Index: sys/net/pipex.c
> ===
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.144
> diff -u -p -r1.144 pipex.c
> --- sys/net/pipex.c 10 Jul 2022 21:28:10 - 1.144
> +++ sys/net/pipex.c 10 Jul 2022 21:59:56 -
> @@ -173,11 +173,6 @@ pipex_ioctl(void *ownersc, u_long cmd, c
>
> NET_ASSERT_LOCKED();
> switch (cmd) {
> - case PIPEXCSESSION:
> - ret = pipex_config_session(
> - (struct pipex_session_config_req *)data, ownersc);
> - break;
> -
> case PIPEXGSTAT:
> ret = pipex_get_stat((struct pipex_session_stat_req *)data,
> ownersc);
> @@ -323,8 +318,6 @@ pipex_init_session(struct pipex_session
> session->ppp_flags = req->pr_ppp_flags;
> session->ppp_id = req->pr_ppp_id;
>
> - session->flags |= PIPEX_SFLAGS_IP_FORWARD;
> -
> session->stat_counters = counters_alloc(pxc_ncounters);
>
> session->ip_address.sin_family = AF_INET;
> @@ -569,32 +562,6 @@ pipex_export_session_stats(struct pipex_
> }
>
> Static int
> -pipex_config_session(struct pipex_session_config_req *req, void *ownersc)
> -{
> - struct pipex_session *session;
> - int error = 0;
> -
> - NET_ASSERT_LOCKED();
> -
> - session = pipex_lookup_by_session_id(req->pcr_protocol,
> - req->pcr_session_id);
> - if (session == NULL)
> - return (EINVAL);
> -
> - if (session->ownersc == ownersc) {
> - if (req->pcr_ip_forward != 0)
> - session->flags |= PIPEX_SFLAGS_IP_FORWARD;
> - else
> - session->flags &= ~PIPEX_SFLAGS_IP_FORWARD;
> - } else
> - error = EINVAL;
> -
> - pipex_rele_session(session);
> -
> - return error;
> -}
> -
> -Static int
> pipex_get_stat(struct pipex_session_stat_req *req, void *ownersc)
> {
> struct pipex_session *session;
> @@ -810,9 +777,7 @@ pipex_ip_output(struct mbuf *m0, struct
> /*
>* Multicast packet is a idle packet and it's not TCP.
>*/
> - if ((session->flags & (PIPEX_SFLAGS_IP_FORWARD |
> - PIPEX_SFLAGS_IP6_FORWARD)) == 0)
> - goto drop;
> +
> /* reset idle timer */
> if (session->timeout_sec != 0) {
> is_idle = 0;
> @@ -850,9 +815,6 @@ pipex_ip_output(struct mbuf *m0, struct
>
> if (session_tmp->ownersc != session->ownersc)
> goto next;
> - if ((session->flags & (PIPEX_SFLAGS_IP_FORWARD |
> - PIPEX_SFLAGS_IP6_FORWARD)) == 0)
> - goto next;
>
> refcnt_take(&session_tmp->pxs_refcnt);
> mtx_leave(&pipex_list_mtx);
> @@ -878,8 +840,6 @@ next:
> }
>
> return;
> -drop:
> - m_freem(m0);
> dropped:
> counters_inc(session->stat_counters, pxc_oerrors);
> }
> @@ -989,8 +949,6 @@ pipex_ppp_input(struct mbuf *m0, struc
Re: interface media list mutex
> On 11 Jul 2022, at 13:02, Alexander Bluhm wrote:
>
> Hi,
>
> Customer complains that routing video stream over OpenBSD stutters
> when snmpd is runnning. It looks like ioctl(SIOCGIFMEDIA) may be
> the culprit, so I want to get it out of the netlock.
>
> Start with a mutex around interface media list.
>
> ok?
>
ok
> bluhm
>
> Index: dev/ic/gem.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/ic/gem.c,v
> retrieving revision 1.126
> diff -u -p -r1.126 gem.c
> --- dev/ic/gem.c 12 Dec 2020 11:48:52 - 1.126
> +++ dev/ic/gem.c 11 Jul 2022 06:16:15 -
> @@ -332,6 +332,7 @@ gem_config(struct gem_softc *sc)
> }
>
> /* Check if we support GigE media. */
> + mtx_enter(&ifmedia_mtx);
> TAILQ_FOREACH(ifm, &sc->sc_media.ifm_list, ifm_list) {
> if (IFM_SUBTYPE(ifm->ifm_media) == IFM_1000_T ||
> IFM_SUBTYPE(ifm->ifm_media) == IFM_1000_SX ||
> @@ -341,6 +342,7 @@ gem_config(struct gem_softc *sc)
> break;
> }
> }
> + mtx_leave(&ifmedia_mtx);
>
> /* Attach the interface. */
> if_attach(ifp);
> Index: net/if_media.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_media.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 if_media.c
> --- net/if_media.c10 Jul 2022 21:13:41 - 1.33
> +++ net/if_media.c11 Jul 2022 06:24:28 -
> @@ -82,6 +82,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #ifdef IFMEDIA_DEBUG
> @@ -102,6 +103,8 @@ int ifmedia_debug = 0;
> staticvoid ifmedia_printword(uint64_t);
> #endif
>
> +struct mutex ifmedia_mtx = MUTEX_INITIALIZER(IPL_NET);
> +
> /*
> * Initialize if_media struct for a specific interface instance.
> */
> @@ -110,6 +113,7 @@ ifmedia_init(struct ifmedia *ifm, uint64
> ifm_change_cb_t change_callback, ifm_stat_cb_t status_callback)
> {
> TAILQ_INIT(&ifm->ifm_list);
> + ifm->ifm_nwords = 0;
> ifm->ifm_cur = NULL;
> ifm->ifm_media = 0;
> ifm->ifm_mask = dontcare_mask; /* IF don't-care bits */
> @@ -145,7 +149,10 @@ ifmedia_add(struct ifmedia *ifm, uint64_
> entry->ifm_data = data;
> entry->ifm_aux = aux;
>
> + mtx_enter(&ifmedia_mtx);
> TAILQ_INSERT_TAIL(&ifm->ifm_list, entry, ifm_list);
> + ifm->ifm_nwords++;
> + mtx_leave(&ifmedia_mtx);
> }
>
> /*
> @@ -290,7 +297,6 @@ ifmedia_ioctl(struct ifnet *ifp, struct
> case SIOCGIFMEDIA:
> {
> struct ifmediareq *ifmr = (struct ifmediareq *) ifr;
> - struct ifmedia_entry *ep;
> size_t nwords;
>
> if (ifmr->ifm_count < 0)
> @@ -302,37 +308,54 @@ ifmedia_ioctl(struct ifnet *ifp, struct
> ifmr->ifm_status = 0;
> (*ifm->ifm_status_cb)(ifp, ifmr);
>
> - /*
> - * Count them so we know a-priori how much is the max we'll
> - * need.
> - */
> - ep = TAILQ_FIRST(&ifm->ifm_list);
> - for (nwords = 0; ep != NULL; ep = TAILQ_NEXT(ep, ifm_list))
> - nwords++;
> + mtx_enter(&ifmedia_mtx);
> + nwords = ifm->ifm_nwords;
> + mtx_leave(&ifmedia_mtx);
>
> - if (ifmr->ifm_count != 0) {
> - size_t minwords, ksiz;
> + if (ifmr->ifm_count == 0) {
> + ifmr->ifm_count = nwords;
> + return (0);
> + }
> +
> + do {
> + struct ifmedia_entry *ife;
> uint64_t *kptr;
> + size_t ksiz;
>
> - minwords = nwords > (size_t)ifmr->ifm_count ?
> - (size_t)ifmr->ifm_count : nwords;
> kptr = mallocarray(nwords, sizeof(*kptr), M_TEMP,
> M_WAITOK | M_ZERO);
> ksiz = nwords * sizeof(*kptr);
> +
> + mtx_enter(&ifmedia_mtx);
> + /* Madia list might grow during malloc(). */
> + if (nwords < ifm->ifm_nwords) {
> + nwords = ifm->ifm_nwords;
> + mtx_leave(&ifmedia_mtx);
> + free(kptr, M_TEMP, ksiz);
> + continue;
> + }
> + /* Request memory too small, set error and ifm_count. */
> + if (ifmr->ifm_count < ifm->ifm_nwords) {
> + nwords = ifm->ifm_nwords;
> + mtx_leave(&ifmedia_mtx);
> + free(kptr, M_TEMP, ksiz);
> + error = E2BIG;
> + break;
> + }
> /*
>* G
Re: Introduce uvm_pagewait()
> Date: Mon, 11 Jul 2022 13:33:39 +0200
> From: Martin Pieuchot
>
> On 28/06/22(Tue) 14:13, Martin Pieuchot wrote:
> > I'd like to abstract the use of PG_WANTED to start unifying & cleaning
> > the various cases where a code path is waiting for a busy page. Here's
> > the first step.
> >
> > ok?
>
> Anyone?
ok kettenis@
> > Index: uvm/uvm_amap.c
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
> > retrieving revision 1.90
> > diff -u -p -r1.90 uvm_amap.c
> > --- uvm/uvm_amap.c 30 Aug 2021 16:59:17 - 1.90
> > +++ uvm/uvm_amap.c 28 Jun 2022 11:53:08 -
> > @@ -781,9 +781,7 @@ ReStart:
> > * it and then restart.
> > */
> > if (pg->pg_flags & PG_BUSY) {
> > - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> > - rwsleep_nsec(pg, amap->am_lock, PVM | PNORELOCK,
> > - "cownow", INFSLP);
> > + uvm_pagewait(pg, amap->am_lock, "cownow");
> > goto ReStart;
> > }
> >
> > Index: uvm/uvm_aobj.c
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_aobj.c,v
> > retrieving revision 1.103
> > diff -u -p -r1.103 uvm_aobj.c
> > --- uvm/uvm_aobj.c 29 Dec 2021 20:22:06 - 1.103
> > +++ uvm/uvm_aobj.c 28 Jun 2022 11:53:08 -
> > @@ -835,9 +835,8 @@ uao_detach(struct uvm_object *uobj)
> > while ((pg = RBT_ROOT(uvm_objtree, &uobj->memt)) != NULL) {
> > pmap_page_protect(pg, PROT_NONE);
> > if (pg->pg_flags & PG_BUSY) {
> > - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> > - rwsleep_nsec(pg, uobj->vmobjlock, PVM, "uao_det",
> > - INFSLP);
> > + uvm_pagewait(pg, uobj->vmobjlock, "uao_det");
> > + rw_enter(uobj->vmobjlock, RW_WRITE);
> > continue;
> > }
> > uao_dropswap(&aobj->u_obj, pg->offset >> PAGE_SHIFT);
> > @@ -909,9 +908,8 @@ uao_flush(struct uvm_object *uobj, voff_
> >
> > /* Make sure page is unbusy, else wait for it. */
> > if (pg->pg_flags & PG_BUSY) {
> > - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> > - rwsleep_nsec(pg, uobj->vmobjlock, PVM, "uaoflsh",
> > - INFSLP);
> > + uvm_pagewait(pg, uobj->vmobjlock, "uaoflsh");
> > + rw_enter(uobj->vmobjlock, RW_WRITE);
> > curoff -= PAGE_SIZE;
> > continue;
> > }
> > @@ -1147,9 +1145,8 @@ uao_get(struct uvm_object *uobj, voff_t
> >
> > /* page is there, see if we need to wait on it */
> > if ((ptmp->pg_flags & PG_BUSY) != 0) {
> > - atomic_setbits_int(&ptmp->pg_flags, PG_WANTED);
> > - rwsleep_nsec(ptmp, uobj->vmobjlock, PVM,
> > - "uao_get", INFSLP);
> > + uvm_pagewait(ptmp, uobj->vmobjlock, "uao_get");
> > + rw_enter(uobj->vmobjlock, RW_WRITE);
> > continue; /* goto top of pps while loop */
> > }
> >
> > Index: uvm/uvm_km.c
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_km.c,v
> > retrieving revision 1.150
> > diff -u -p -r1.150 uvm_km.c
> > --- uvm/uvm_km.c7 Jun 2022 12:07:45 - 1.150
> > +++ uvm/uvm_km.c28 Jun 2022 11:53:08 -
> > @@ -255,9 +255,8 @@ uvm_km_pgremove(struct uvm_object *uobj,
> > for (curoff = start ; curoff < end ; curoff += PAGE_SIZE) {
> > pp = uvm_pagelookup(uobj, curoff);
> > if (pp && pp->pg_flags & PG_BUSY) {
> > - atomic_setbits_int(&pp->pg_flags, PG_WANTED);
> > - rwsleep_nsec(pp, uobj->vmobjlock, PVM, "km_pgrm",
> > - INFSLP);
> > + uvm_pagewait(pp, uobj->vmobjlock, "km_pgrm");
> > + rw_enter(uobj->vmobjlock, RW_WRITE);
> > curoff -= PAGE_SIZE; /* loop back to us */
> > continue;
> > }
> > Index: uvm/uvm_page.c
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> > retrieving revision 1.166
> > diff -u -p -r1.166 uvm_page.c
> > --- uvm/uvm_page.c 12 May 2022 12:48:36 - 1.166
> > +++ uvm/uvm_page.c 28 Jun 2022 11:57:42 -
> > @@ -1087,6 +1087,23 @@ uvm_page_unbusy(struct vm_page **pgs, in
> > }
> > }
> >
> > +/*
> > + * uvm_pagewait: wait for a busy page
> > + *
> > + * => page must be known PG_BUSY
> > + * => object must be locked
> > + * => object will be unlocked on return
> > + */
Re: bgpd document add-path send
On 2022/07/11 19:12, Claudio Jeker wrote: > This is my try at documenting the just added add-path bits. > > -- > :wq Claudio > > Index: bgpd.8 > === > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v > retrieving revision 1.74 > diff -u -p -r1.74 bgpd.8 > --- bgpd.828 Jun 2022 11:52:24 - 1.74 > +++ bgpd.811 Jul 2022 13:22:05 - > @@ -401,6 +401,16 @@ has been started. > .Re > .Pp > .Rs > +.%A D. Walton > +.%A A. Retana > +.%A E. Chen > +.%A J. Scudder > +.%D July 2016 > +.%R RFC 7911 > +.%T Advertisement of Multiple Paths in BGP > +.Re > +.Pp > +.Rs > .%A C. Petrie > .%A T. King > .%D May 2017 > Index: bgpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v > retrieving revision 1.223 > diff -u -p -r1.223 bgpd.conf.5 > --- bgpd.conf.5 28 Jun 2022 11:52:24 - 1.223 > +++ bgpd.conf.5 11 Jul 2022 14:08:24 - > @@ -843,6 +843,57 @@ The default is > .Ic no . > .Pp > .It Xo > +.Ic announce add-path send > +.Pq Ic no Ns | Ns Ic all > +.Xc > +.It Xo > +.Ic announce add-path send > +.Pq Ic best Ns | Ns Ic ecmp | Ns Ic as-wide-best > +.Op Ic plus Ar num > +.Op Ic max Ar num > +.Xc > +If set to > +.Ic all , > +.Ic best , > +.Ic ecmp , > +or > +.Ic as-wide-best , > +the send add-path capability is announced which allows to send multiple paths > +per prefix. > +Which paths are sent depends on the mode: > +.Pp > +.Bl -tag -width as-wide-best -compact > +.It Ic all > +send all valid prefixes Never used add-path so I might misunderstand things, but shouldn't this be "send all valid paths" (and same in other descriptions)? > +.It Ic best > +send the best prefix and maybe more perhaps s/more/others/ this makes me wonder exactly what "best" and "more" are > +.It Ic ecmp > +send prefixes with equal nexthop cost > +.It Ic as-wide-best > +send prefixes where the fist 8 check of the decision process match ...where the first 8 checks of the decision process... > +.El > +.Pp > +.Ic plus > +allows to include additional routes and works for > +.Ic best , > +.Ic ecmp , > +and > +.Ic as-wide-best . > +.Ic max > +can be used to limit the total amount of routes announced for > +.Ic ecmp > +and > +.Ic as-wide-best . > +The default is > +.Ic no . > +.Pp > +Right now > +.Ic ecmp > +and > +.Ic as-wide-best > +are equivalent. > +.Pp > +.It Xo > .Ic announce as-4byte > .Pq Ic yes Ns | Ns Ic no > .Xc >
Re: What is ieee80211com.ic_max_nnodes?
On Mon, Jul 11, 2022, at 2:24 PM, Stefan Sperling wrote: Thank you so much for your prompt reply! > In AP mode, max nodes effectively sets a limit on how many clients can > be connected concurrently. Because athn(4) USB firmware has an internal > limit of 8 stations in its station table, and one entry is reserved for > other use, the USB device can only serve 7 clients at a time. Can you please show me where that limit is set to 7 or 8? > In the line of code you linked to, a similar cap is applied based on the > number of pairwise WPA key slots available in hardware. This should be > larger than the USB device limit, since the driver queries the device for > this limit and caps it at 128. > As this paragraph says, rather than 7 or 8, sc->kc_entries is 128. And lastly, how is ic_max_nnodes used by the net80211 stack? Thank you! -- Farhan Khan PGP Fingerprint: 1312 89CE 663E 1EB2 179C 1C83 C41D 2281 F8DA C0DE
Re: echo(1): check for stdio errors
On Mon, Jul 11, 2022 at 08:31:04AM -0600, Todd C. Miller wrote:
> On Sun, 10 Jul 2022 20:58:35 -0900, Philip Guenther wrote:
>
> > Three thoughts:
> > 1) Since stdio errors are sticky, is there any real advantage to checking
> > each call instead of just checking the final fclose()?
My thinking was that we have no idea how many arguments we're going to
print, so we may as well fail as soon as possible.
Maybe in more complex programs there would be a code-length or
complexity-reducing upside to deferring the ferror(3) check until,
say, the end of a subroutine or something.
> > [...]
>
> Will that really catch all errors? From what I can tell, fclose(3)
> can succeed even if the error flag was set. The pattern I prefer
> is to use a final fflush(3) followed by a call to ferror(3) before
> the fclose(3).
That's weird, I was under the impression POSIX mandated an error case
for the implicit fflush(3) done by fclose(3). But I'm looking at the
standard and seeing nothing specific.
So, yes? It is probably more portable to check fflush(3) explicitly?
This feels redundant though. Like, obviously I want to flush the
descriptor when we close the stream, and obviously I would want to
know if the flush failed. That's why I'm using stdio.
Index: echo.c
===
RCS file: /cvs/src/bin/echo/echo.c,v
retrieving revision 1.10
diff -u -p -r1.10 echo.c
--- echo.c 9 Oct 2015 01:37:06 - 1.10
+++ echo.c 11 Jul 2022 18:19:39 -
@@ -53,12 +53,15 @@ main(int argc, char *argv[])
nflag = 0;
while (*argv) {
- (void)fputs(*argv, stdout);
- if (*++argv)
- putchar(' ');
+ if (fputs(*argv, stdout) == EOF)
+ err(1, "stdout");
+ if (*++argv && putchar(' ') == EOF)
+ err(1, "stdout");
}
- if (!nflag)
- putchar('\n');
+ if (!nflag && putchar('\n') == EOF)
+ err(1, "stdout");
+ if (fflush(stdout) == EOF || fclose(stdout) == EOF)
+ err(1, "stdout");
return 0;
}
Re: What is ieee80211com.ic_max_nnodes?
On Mon, Jul 11, 2022 at 11:32:12AM -0400, Farhan Khan wrote: > Hi all, > > I am reading through the athn(4) driver and see a reference to > ieee80211com.ic_max_nnodes here > (http://bxr.su/OpenBSD/sys/dev/ic/athn.c#294). What is this variable? The > comment immediately above says "In HostAP mode, the number of STAs that we > can handle...". What does "handle" this mean in this context? What is it used > for? > > Broader question: I am trying to identify what the FreeBSD equivalent of this > is, but that exact question might be out of scope of this email list. > > Preliminary Research: > * Looking through the code, I see that it is used only by athn(4) and rtwn(4). > * The ieee80211 subsystem assigns it to ieee80211_cache_size aka > IEEE80211_CACHE_SIZE (512) by default. So does the athn driver. > * The variable is consumed in sys/net80211/ieee80211_node.c in the functions > ieee80211_alloc_node_helper ieee80211_clean_nodes, but I do not these > functions (yet). > * NetBSD seems to call the the variable ic_max_aid, but I could be mistaken. > Their wi(4), rtwn(4) and rt(4) drivers uses it. > > Thanks! It is the maximum allowed number of entries in ic_tree. struct ieee80211_tree ic_tree; int ic_nnodes; /* length of ic_nnodes */ int ic_max_nnodes; /* max length of ic_nnodes */ This tree represents our view of all other the wlan devices we can see and potentially interact with. (The comments say 'length' because at some point in time, ic_tree was a list instead of a tree; someone forgot to update/remove these comments when that change was made.) Nodes in this tree are keyed on MAC address, and correspond to a "station" in 802.11 standard terminology. In client mode, max nodes says how many APs can show up in scan results. In AP mode, max nodes effectively sets a limit on how many clients can be connected concurrently. Because athn(4) USB firmware has an internal limit of 8 stations in its station table, and one entry is reserved for other use, the USB device can only serve 7 clients at a time. In the line of code you linked to, a similar cap is applied based on the number of pairwise WPA key slots available in hardware. This should be larger than the USB device limit, since the driver queries the device for this limit and caps it at 128.
Re: bgpd document add-path send
On Mon, Jul 11, 2022 at 07:12:11PM +0200, Claudio Jeker wrote: > This is my try at documenting the just added add-path bits. > > -- > :wq Claudio > some comments: > Index: bgpd.8 > === > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v > retrieving revision 1.74 > diff -u -p -r1.74 bgpd.8 > --- bgpd.828 Jun 2022 11:52:24 - 1.74 > +++ bgpd.811 Jul 2022 13:22:05 - > @@ -401,6 +401,16 @@ has been started. > .Re > .Pp > .Rs > +.%A D. Walton > +.%A A. Retana > +.%A E. Chen > +.%A J. Scudder > +.%D July 2016 > +.%R RFC 7911 > +.%T Advertisement of Multiple Paths in BGP > +.Re > +.Pp > +.Rs > .%A C. Petrie > .%A T. King > .%D May 2017 > Index: bgpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v > retrieving revision 1.223 > diff -u -p -r1.223 bgpd.conf.5 > --- bgpd.conf.5 28 Jun 2022 11:52:24 - 1.223 > +++ bgpd.conf.5 11 Jul 2022 14:08:24 - > @@ -843,6 +843,57 @@ The default is > .Ic no . > .Pp > .It Xo > +.Ic announce add-path send > +.Pq Ic no Ns | Ns Ic all > +.Xc > +.It Xo > +.Ic announce add-path send > +.Pq Ic best Ns | Ns Ic ecmp | Ns Ic as-wide-best > +.Op Ic plus Ar num > +.Op Ic max Ar num > +.Xc > +If set to > +.Ic all , > +.Ic best , > +.Ic ecmp , > +or > +.Ic as-wide-best , > +the send add-path capability is announced which allows to send multiple paths > +per prefix. "allows to" is not great. i'd go for: ...announced, which allows sending multiple paths... or ...announced, which allows multiple paths... to be sent. > +Which paths are sent depends on the mode: > +.Pp > +.Bl -tag -width as-wide-best -compact > +.It Ic all > +send all valid prefixes > +.It Ic best > +send the best prefix and maybe more > +.It Ic ecmp > +send prefixes with equal nexthop cost > +.It Ic as-wide-best > +send prefixes where the fist 8 check of the decision process match > +.El > +.Pp > +.Ic plus > +allows to include additional routes and works for again: allows the inclusion of additional routes... > +.Ic best , > +.Ic ecmp , > +and > +.Ic as-wide-best . > +.Ic max > +can be used to limit the total amount of routes announced for > +.Ic ecmp > +and > +.Ic as-wide-best . > +The default is > +.Ic no . > +.Pp > +Right now > +.Ic ecmp > +and > +.Ic as-wide-best > +are equivalent. > +.Pp > +.It Xo > .Ic announce as-4byte > .Pq Ic yes Ns | Ns Ic no > .Xc > jmc
Re: bgpd document add-path send
On Mon, Jul 11, 2022 at 07:12:11PM +0200, Claudio Jeker wrote: > This is my try at documenting the just added add-path bits. > > -- > :wq Claudio > some comments inline: > Index: bgpd.8 > === > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v > retrieving revision 1.74 > diff -u -p -r1.74 bgpd.8 > --- bgpd.828 Jun 2022 11:52:24 - 1.74 > +++ bgpd.811 Jul 2022 13:22:05 - > @@ -401,6 +401,16 @@ has been started. > .Re > .Pp > .Rs > +.%A D. Walton > +.%A A. Retana > +.%A E. Chen > +.%A J. Scudder > +.%D July 2016 > +.%R RFC 7911 > +.%T Advertisement of Multiple Paths in BGP > +.Re > +.Pp > +.Rs > .%A C. Petrie > .%A T. King > .%D May 2017 > Index: bgpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v > retrieving revision 1.223 > diff -u -p -r1.223 bgpd.conf.5 > --- bgpd.conf.5 28 Jun 2022 11:52:24 - 1.223 > +++ bgpd.conf.5 11 Jul 2022 14:08:24 - > @@ -843,6 +843,57 @@ The default is > .Ic no . > .Pp > .It Xo > +.Ic announce add-path send > +.Pq Ic no Ns | Ns Ic all > +.Xc > +.It Xo > +.Ic announce add-path send > +.Pq Ic best Ns | Ns Ic ecmp | Ns Ic as-wide-best > +.Op Ic plus Ar num > +.Op Ic max Ar num > +.Xc > +If set to > +.Ic all , > +.Ic best , > +.Ic ecmp , > +or > +.Ic as-wide-best , > +the send add-path capability is announced which allows to send multiple paths ... announced, which allows sending ... or ... announced, which allows multiple paths ... to be sent. > +per prefix. > +Which paths are sent depends on the mode: > +.Pp > +.Bl -tag -width as-wide-best -compact > +.It Ic all > +send all valid prefixes > +.It Ic best > +send the best prefix and maybe more > +.It Ic ecmp > +send prefixes with equal nexthop cost > +.It Ic as-wide-best > +send prefixes where the fist 8 check of the decision process match > +.El > +.Pp > +.Ic plus > +allows to include additional routes and works for ... allows additional routes ... or ... allows additional routes ... to be included. > +.Ic best , > +.Ic ecmp , > +and > +.Ic as-wide-best . > +.Ic max > +can be used to limit the total amount of routes announced for > +.Ic ecmp > +and > +.Ic as-wide-best . > +The default is > +.Ic no . > +.Pp > +Right now > +.Ic ecmp > +and > +.Ic as-wide-best > +are equivalent. > +.Pp > +.It Xo > .Ic announce as-4byte > .Pq Ic yes Ns | Ns Ic no > .Xc > jmc
bgpd document add-path send
This is my try at documenting the just added add-path bits. -- :wq Claudio Index: bgpd.8 === RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v retrieving revision 1.74 diff -u -p -r1.74 bgpd.8 --- bgpd.8 28 Jun 2022 11:52:24 - 1.74 +++ bgpd.8 11 Jul 2022 13:22:05 - @@ -401,6 +401,16 @@ has been started. .Re .Pp .Rs +.%A D. Walton +.%A A. Retana +.%A E. Chen +.%A J. Scudder +.%D July 2016 +.%R RFC 7911 +.%T Advertisement of Multiple Paths in BGP +.Re +.Pp +.Rs .%A C. Petrie .%A T. King .%D May 2017 Index: bgpd.conf.5 === RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v retrieving revision 1.223 diff -u -p -r1.223 bgpd.conf.5 --- bgpd.conf.5 28 Jun 2022 11:52:24 - 1.223 +++ bgpd.conf.5 11 Jul 2022 14:08:24 - @@ -843,6 +843,57 @@ The default is .Ic no . .Pp .It Xo +.Ic announce add-path send +.Pq Ic no Ns | Ns Ic all +.Xc +.It Xo +.Ic announce add-path send +.Pq Ic best Ns | Ns Ic ecmp | Ns Ic as-wide-best +.Op Ic plus Ar num +.Op Ic max Ar num +.Xc +If set to +.Ic all , +.Ic best , +.Ic ecmp , +or +.Ic as-wide-best , +the send add-path capability is announced which allows to send multiple paths +per prefix. +Which paths are sent depends on the mode: +.Pp +.Bl -tag -width as-wide-best -compact +.It Ic all +send all valid prefixes +.It Ic best +send the best prefix and maybe more +.It Ic ecmp +send prefixes with equal nexthop cost +.It Ic as-wide-best +send prefixes where the fist 8 check of the decision process match +.El +.Pp +.Ic plus +allows to include additional routes and works for +.Ic best , +.Ic ecmp , +and +.Ic as-wide-best . +.Ic max +can be used to limit the total amount of routes announced for +.Ic ecmp +and +.Ic as-wide-best . +The default is +.Ic no . +.Pp +Right now +.Ic ecmp +and +.Ic as-wide-best +are equivalent. +.Pp +.It Xo .Ic announce as-4byte .Pq Ic yes Ns | Ns Ic no .Xc
What is ieee80211com.ic_max_nnodes?
Hi all, I am reading through the athn(4) driver and see a reference to ieee80211com.ic_max_nnodes here (http://bxr.su/OpenBSD/sys/dev/ic/athn.c#294). What is this variable? The comment immediately above says "In HostAP mode, the number of STAs that we can handle...". What does "handle" this mean in this context? What is it used for? Broader question: I am trying to identify what the FreeBSD equivalent of this is, but that exact question might be out of scope of this email list. Preliminary Research: * Looking through the code, I see that it is used only by athn(4) and rtwn(4). * The ieee80211 subsystem assigns it to ieee80211_cache_size aka IEEE80211_CACHE_SIZE (512) by default. So does the athn driver. * The variable is consumed in sys/net80211/ieee80211_node.c in the functions ieee80211_alloc_node_helper ieee80211_clean_nodes, but I do not these functions (yet). * NetBSD seems to call the the variable ic_max_aid, but I could be mistaken. Their wi(4), rtwn(4) and rt(4) drivers uses it. Thanks! -- Farhan Khan PGP Fingerprint: 1312 89CE 663E 1EB2 179C 1C83 C41D 2281 F8DA C0DE
Re: bgpd: add add-path send support
On Fri, Jul 08, 2022 at 06:43:17PM +0200, Claudio Jeker wrote:
> Add the missing bits for add-path send support.
> The config options allows for a fair amount of configuration and not all
> have been tested:
> announce add-path send best [ plus X ]
> announce add-path send ecmp [ plus X ] [ max Y ]
> announce add-path send as-wide-best [ plus X ] [ max Y ]
> announce add-path send all
>
> Right now ECMP and as-wide-best are the same. This is because of missing
> nexthop metrics. The heavy lifting is done by up_generate_addpath()
> and is based on the same trick as 'rde evaluate all'. This is not optimal
> but it should work and the code is somewhat easy to follow.
> Also the reload behaviour is a bit special. You may want to tune the
> settings on an active session but removing the option should keep
> the last settings until session reset. Still need to think about this
> a bit more.
>
> A few bits like the minor cleanup in rde_decide.c can be committed
> independently.
Please do.
>
> If add-path send is not set then the system should behave as before.
> Which is the important bit of this.
The diff reads fine to me and I agree that without setting add-path send
nothing changes. I have a couple of nits inline, but I'm fine with the
diff as it is.
ok tb
> --
> :wq Claudio
>
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.440
> diff -u -p -r1.440 bgpd.h
> --- bgpd.h7 Jul 2022 12:16:04 - 1.440
> +++ bgpd.h8 Jul 2022 12:32:49 -
> @@ -307,6 +307,20 @@ struct bgpd_config {
>
> extern int cmd_opts;
>
> +enum addpath_mode {
> + ADDPATH_EVAL_NONE,
> + ADDPATH_EVAL_BEST,
> + ADDPATH_EVAL_ECMP,
> + ADDPATH_EVAL_AS_WIDE,
> + ADDPATH_EVAL_ALL,
> +};
> +
> +struct addpath_eval {
> + enum addpath_mode mode;
> + int extrapaths;
> + int maxpaths;
> +};
> +
> enum export_type {
> EXPORT_UNSET,
> EXPORT_NONE,
> @@ -402,6 +416,7 @@ struct peer_config {
> struct bgpd_addr local_addr_v6;
> struct peer_auth auth;
> struct capabilities capabilities;
> + struct addpath_eval eval;
> char group[PEER_DESCR_LEN];
> char descr[PEER_DESCR_LEN];
> char reason[REASON_LEN];
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.431
> diff -u -p -r1.431 parse.y
> --- parse.y 27 Jun 2022 13:26:51 - 1.431
> +++ parse.y 8 Jul 2022 12:41:23 -
> @@ -210,7 +210,7 @@ typedef struct {
> %token EBGP IBGP
> %token LOCALAS REMOTEAS DESCR LOCALADDR MULTIHOP PASSIVE MAXPREFIX
> RESTART
> %token ANNOUNCE CAPABILITIES REFRESH AS4BYTE CONNECTRETRY ENHANCED
> ADDPATH
> -%token SEND RECV POLICY
> +%token SEND RECV PLUS POLICY
> %token DEMOTE ENFORCE NEIGHBORAS ASOVERRIDE REFLECTOR DEPEND DOWN
> %token DUMP IN OUT SOCKET RESTRICTED
> %token LOG TRANSPARENT
> @@ -230,12 +230,13 @@ typedef struct {
> %token IPSEC ESP AH SPI IKE
> %token IPV4 IPV6
> %token QUALIFY VIA
> -%token NE LE GE XRANGE LONGER MAXLEN
> +%token NE LE GE XRANGE LONGER MAXLEN MAX
> %token STRING
> %token NUMBER
> %type asnumber as4number as4number_any
> optnumber
> %type espah family safi restart origincode
> nettype
> %type yesno inout restricted validity expires
> enforce
> +%type addpathopt addpathmax
Maybe addpathextra would match the 'extra' used everywhere else better than
addpathopt. Or even addextrapaths and addmaxpaths.
> %type string
> %typeaddress
> %type prefix addrspec
> @@ -1356,6 +1357,28 @@ groupopts_l: /* empty */
> | groupopts_l error '\n'
> ;
>
> +addpathopt : /* empty */ { $$ = 0; }
> + | PLUS NUMBER {
> + if ($2 < 1 || $2 > USHRT_MAX) {
> + yyerror("additional paths must be between "
> + "%u and %u", 1, USHRT_MAX);
> + YYERROR;
> + }
> + $$ = $2;
> + }
> + ;
> +
> +addpathmax : /* empty */ { $$ = 0; }
> + | MAX NUMBER{
> + if ($2 < 1 || $2 > USHRT_MAX) {
> + yyerror("maximum additional paths must be "
> + "between %u and %u", 1, USHRT_MAX);
> + YYERROR;
> + }
> + $
Replace selwakeup() with KNOTE() in audio(4)
Replace selwakeup() with KNOTE() in audio(4).
KNOTE() can be used up to IPL_SCHED. IPL_SCHED is higher than IPL_AUDIO,
so the deferring through soft interrupts is no longer necessary.
In audio_detach(), the selwakeup() calls should not need replacing.
Any remaining kevent/poll/select waiters are woken up by
klist_invalidate().
OK?
Index: dev/audio.c
===
RCS file: src/sys/dev/audio.c,v
retrieving revision 1.199
diff -u -p -r1.199 audio.c
--- dev/audio.c 2 Jul 2022 08:50:41 - 1.199
+++ dev/audio.c 11 Jul 2022 15:14:09 -
@@ -20,6 +20,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -75,8 +77,7 @@ struct audio_buf {
size_t used;/* bytes used in the FIFO */
size_t blksz; /* DMA block size */
unsigned int nblks; /* number of blocks */
- struct selinfo sel; /* to record & wakeup poll(2) */
- void *softintr; /* context to call selwakeup() */
+ struct klist klist; /* list of knotes */
unsigned int pos; /* bytes transferred */
unsigned int xrun; /* bytes lost by xruns */
int blocking; /* read/write blocking */
@@ -136,10 +137,9 @@ struct audio_softc {
int mix_nent; /* size of mixer state */
int mix_isopen; /* mixer open for reading */
int mix_blocking; /* read() blocking */
- struct selinfo mix_sel; /* wakeup poll(2) */
+ struct klist mix_klist; /* list of knotes */
struct mixer_ev *mix_evbuf; /* per mixer-control event */
struct mixer_ev *mix_pending; /* list of changed controls */
- void *mix_softintr; /* context to call selwakeup() */
#if NWSKBD > 0
struct wskbd_vol spkr, mic;
struct task wskbd_task;
@@ -153,6 +153,8 @@ int audio_activate(struct device *, int)
int audio_detach(struct device *, int);
void audio_pintr(void *);
void audio_rintr(void *);
+void audio_buf_wakeup(struct audio_buf *);
+void audio_mixer_wakeup(struct audio_softc *);
#if NWSKBD > 0
void wskbd_mixer_init(struct audio_softc *);
void wskbd_mixer_cb(void *);
@@ -275,53 +277,33 @@ audio_blksz_bytes(int mode,
}
void
-audio_mixer_wakeup(void *addr)
+audio_mixer_wakeup(struct audio_softc *sc)
{
- struct audio_softc *sc = addr;
+ MUTEX_ASSERT_LOCKED(&audio_lock);
if (sc->mix_blocking) {
wakeup(&sc->mix_blocking);
sc->mix_blocking = 0;
}
- /*
-* As long as selwakeup() grabs the KERNEL_LOCK() make sure it is
-* already held here to avoid lock ordering problems with `audio_lock'
-*/
- KERNEL_ASSERT_LOCKED();
- mtx_enter(&audio_lock);
- selwakeup(&sc->mix_sel);
- mtx_leave(&audio_lock);
+ KNOTE(&sc->mix_klist, 0);
}
void
-audio_buf_wakeup(void *addr)
+audio_buf_wakeup(struct audio_buf *buf)
{
- struct audio_buf *buf = addr;
+ MUTEX_ASSERT_LOCKED(&audio_lock);
if (buf->blocking) {
wakeup(&buf->blocking);
buf->blocking = 0;
}
- /*
-* As long as selwakeup() grabs the KERNEL_LOCK() make sure it is
-* already held here to avoid lock ordering problems with `audio_lock'
-*/
- KERNEL_ASSERT_LOCKED();
- mtx_enter(&audio_lock);
- selwakeup(&buf->sel);
- mtx_leave(&audio_lock);
+ KNOTE(&buf->klist, 0);
}
int
audio_buf_init(struct audio_softc *sc, struct audio_buf *buf, int dir)
{
- klist_init_mutex(&buf->sel.si_note, &audio_lock);
- buf->softintr = softintr_establish(IPL_SOFTAUDIO,
- audio_buf_wakeup, buf);
- if (buf->softintr == NULL) {
- printf("%s: can't establish softintr\n", DEVNAME(sc));
- goto bad;
- }
+ klist_init_mutex(&buf->klist, &audio_lock);
if (sc->ops->round_buffersize) {
buf->datalen = sc->ops->round_buffersize(sc->arg,
dir, AUDIO_BUFSZ);
@@ -333,13 +315,10 @@ audio_buf_init(struct audio_softc *sc, s
} else
buf->data = malloc(buf->datalen, M_DEVBUF, M_WAITOK);
if (buf->data == NULL) {
- softintr_disestablish(buf->softintr);
- goto bad;
+ klist_free(&buf->klist);
+ return ENOMEM;
}
return 0;
-bad:
- klist_free(&buf->sel.si_note);
- return ENOMEM;
}
void
@@ -349,8 +328,7 @@ audio_buf_done(struct audio_softc *sc, s
sc->ops->freem(sc->arg, buf->data, M_DEVBUF);
else
free(buf->data, M_DEVBUF, buf->datalen);
- softintr_disestablish(buf->softintr);
- klist_free(&buf->sel.si_note);
+ klist_free(&buf->klist);
}
/*
@@ -563,13 +541
Re: echo(1): check for stdio errors
On Sun, 10 Jul 2022 20:58:35 -0900, Philip Guenther wrote: > Three thoughts: > 1) Since stdio errors are sticky, is there any real advantage to checking > each call instead of just checking the final fclose()? Will that really catch all errors? From what I can tell, fclose(3) can succeed even if the error flag was set. The pattern I prefer is to use a final fflush(3) followed by a call to ferror(3) before the fclose(3). - todd
Re: ampintc: minor nits
On Mon, Jul 11, 2022 at 09:02:20AM +0200, Mark Kettenis wrote: > > Date: Mon, 11 Jul 2022 08:35:44 +0200 > > From: Anton Lindqvist > > > > Hi, > > Addressing the following nits made it easier for me to grasp the ampintc > > implementation while reading the specification. There's also one missing > > NULL check included. > > > > Too verbose or does anyone else consider this an improvement? > > Your timing is a but unfortunate since I just posted the > suspend/resume diff that will conflict with this. > > Some/all of this may be useful, but not right now :(. That was actually the thing that reminded me about this diff :) I can wait until things have stabilized and send out a new diff.
Re: Introduce uvm_pagewait()
On Tue, Jun 28, 2022 at 02:13:04PM +0200, Martin Pieuchot wrote:
> I'd like to abstract the use of PG_WANTED to start unifying & cleaning
> the various cases where a code path is waiting for a busy page. Here's
> the first step.
>
> ok?
if it helps you, yes it is fine. ok semarie@
just one nit in uvm_pagewait() regarding one KASSERT() which could be avoided
(see below).
I also wonder about PNORELOCK usage: we replace only one `PVM | PNORELOCK`
call,
and all others needs a rw_enter() after calling uvm_pagewait(). but I don't
have
strong opinion about that.
> Index: uvm/uvm_amap.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
> retrieving revision 1.90
> diff -u -p -r1.90 uvm_amap.c
> --- uvm/uvm_amap.c30 Aug 2021 16:59:17 - 1.90
> +++ uvm/uvm_amap.c28 Jun 2022 11:53:08 -
> @@ -781,9 +781,7 @@ ReStart:
>* it and then restart.
>*/
> if (pg->pg_flags & PG_BUSY) {
> - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> - rwsleep_nsec(pg, amap->am_lock, PVM | PNORELOCK,
> - "cownow", INFSLP);
> + uvm_pagewait(pg, amap->am_lock, "cownow");
> goto ReStart;
> }
>
> Index: uvm/uvm_aobj.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_aobj.c,v
> retrieving revision 1.103
> diff -u -p -r1.103 uvm_aobj.c
> --- uvm/uvm_aobj.c29 Dec 2021 20:22:06 - 1.103
> +++ uvm/uvm_aobj.c28 Jun 2022 11:53:08 -
> @@ -835,9 +835,8 @@ uao_detach(struct uvm_object *uobj)
> while ((pg = RBT_ROOT(uvm_objtree, &uobj->memt)) != NULL) {
> pmap_page_protect(pg, PROT_NONE);
> if (pg->pg_flags & PG_BUSY) {
> - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> - rwsleep_nsec(pg, uobj->vmobjlock, PVM, "uao_det",
> - INFSLP);
> + uvm_pagewait(pg, uobj->vmobjlock, "uao_det");
> + rw_enter(uobj->vmobjlock, RW_WRITE);
> continue;
> }
> uao_dropswap(&aobj->u_obj, pg->offset >> PAGE_SHIFT);
> @@ -909,9 +908,8 @@ uao_flush(struct uvm_object *uobj, voff_
>
> /* Make sure page is unbusy, else wait for it. */
> if (pg->pg_flags & PG_BUSY) {
> - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> - rwsleep_nsec(pg, uobj->vmobjlock, PVM, "uaoflsh",
> - INFSLP);
> + uvm_pagewait(pg, uobj->vmobjlock, "uaoflsh");
> + rw_enter(uobj->vmobjlock, RW_WRITE);
> curoff -= PAGE_SIZE;
> continue;
> }
> @@ -1147,9 +1145,8 @@ uao_get(struct uvm_object *uobj, voff_t
>
> /* page is there, see if we need to wait on it */
> if ((ptmp->pg_flags & PG_BUSY) != 0) {
> - atomic_setbits_int(&ptmp->pg_flags, PG_WANTED);
> - rwsleep_nsec(ptmp, uobj->vmobjlock, PVM,
> - "uao_get", INFSLP);
> + uvm_pagewait(ptmp, uobj->vmobjlock, "uao_get");
> + rw_enter(uobj->vmobjlock, RW_WRITE);
> continue; /* goto top of pps while loop */
> }
>
> Index: uvm/uvm_km.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_km.c,v
> retrieving revision 1.150
> diff -u -p -r1.150 uvm_km.c
> --- uvm/uvm_km.c 7 Jun 2022 12:07:45 - 1.150
> +++ uvm/uvm_km.c 28 Jun 2022 11:53:08 -
> @@ -255,9 +255,8 @@ uvm_km_pgremove(struct uvm_object *uobj,
> for (curoff = start ; curoff < end ; curoff += PAGE_SIZE) {
> pp = uvm_pagelookup(uobj, curoff);
> if (pp && pp->pg_flags & PG_BUSY) {
> - atomic_setbits_int(&pp->pg_flags, PG_WANTED);
> - rwsleep_nsec(pp, uobj->vmobjlock, PVM, "km_pgrm",
> - INFSLP);
> + uvm_pagewait(pp, uobj->vmobjlock, "km_pgrm");
> + rw_enter(uobj->vmobjlock, RW_WRITE);
> curoff -= PAGE_SIZE; /* loop back to us */
> continue;
> }
> Index: uvm/uvm_page.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> retrieving revision 1.166
> diff -u -p -r1.166 uvm_page.c
> --- uvm/uvm_page.c12 May 2022 12:48:36 - 1.166
> +++ uvm/uvm_page.c28 Jun 2022 11:57:42 -
> @@ -1087,6 +1087,23 @@ uvm_page_unbusy(struct vm_page **pgs, in
> }
> }
>
> +/
Re: Introduce uvm_pagewait()
On 28/06/22(Tue) 14:13, Martin Pieuchot wrote:
> I'd like to abstract the use of PG_WANTED to start unifying & cleaning
> the various cases where a code path is waiting for a busy page. Here's
> the first step.
>
> ok?
Anyone?
> Index: uvm/uvm_amap.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
> retrieving revision 1.90
> diff -u -p -r1.90 uvm_amap.c
> --- uvm/uvm_amap.c30 Aug 2021 16:59:17 - 1.90
> +++ uvm/uvm_amap.c28 Jun 2022 11:53:08 -
> @@ -781,9 +781,7 @@ ReStart:
>* it and then restart.
>*/
> if (pg->pg_flags & PG_BUSY) {
> - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> - rwsleep_nsec(pg, amap->am_lock, PVM | PNORELOCK,
> - "cownow", INFSLP);
> + uvm_pagewait(pg, amap->am_lock, "cownow");
> goto ReStart;
> }
>
> Index: uvm/uvm_aobj.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_aobj.c,v
> retrieving revision 1.103
> diff -u -p -r1.103 uvm_aobj.c
> --- uvm/uvm_aobj.c29 Dec 2021 20:22:06 - 1.103
> +++ uvm/uvm_aobj.c28 Jun 2022 11:53:08 -
> @@ -835,9 +835,8 @@ uao_detach(struct uvm_object *uobj)
> while ((pg = RBT_ROOT(uvm_objtree, &uobj->memt)) != NULL) {
> pmap_page_protect(pg, PROT_NONE);
> if (pg->pg_flags & PG_BUSY) {
> - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> - rwsleep_nsec(pg, uobj->vmobjlock, PVM, "uao_det",
> - INFSLP);
> + uvm_pagewait(pg, uobj->vmobjlock, "uao_det");
> + rw_enter(uobj->vmobjlock, RW_WRITE);
> continue;
> }
> uao_dropswap(&aobj->u_obj, pg->offset >> PAGE_SHIFT);
> @@ -909,9 +908,8 @@ uao_flush(struct uvm_object *uobj, voff_
>
> /* Make sure page is unbusy, else wait for it. */
> if (pg->pg_flags & PG_BUSY) {
> - atomic_setbits_int(&pg->pg_flags, PG_WANTED);
> - rwsleep_nsec(pg, uobj->vmobjlock, PVM, "uaoflsh",
> - INFSLP);
> + uvm_pagewait(pg, uobj->vmobjlock, "uaoflsh");
> + rw_enter(uobj->vmobjlock, RW_WRITE);
> curoff -= PAGE_SIZE;
> continue;
> }
> @@ -1147,9 +1145,8 @@ uao_get(struct uvm_object *uobj, voff_t
>
> /* page is there, see if we need to wait on it */
> if ((ptmp->pg_flags & PG_BUSY) != 0) {
> - atomic_setbits_int(&ptmp->pg_flags, PG_WANTED);
> - rwsleep_nsec(ptmp, uobj->vmobjlock, PVM,
> - "uao_get", INFSLP);
> + uvm_pagewait(ptmp, uobj->vmobjlock, "uao_get");
> + rw_enter(uobj->vmobjlock, RW_WRITE);
> continue; /* goto top of pps while loop */
> }
>
> Index: uvm/uvm_km.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_km.c,v
> retrieving revision 1.150
> diff -u -p -r1.150 uvm_km.c
> --- uvm/uvm_km.c 7 Jun 2022 12:07:45 - 1.150
> +++ uvm/uvm_km.c 28 Jun 2022 11:53:08 -
> @@ -255,9 +255,8 @@ uvm_km_pgremove(struct uvm_object *uobj,
> for (curoff = start ; curoff < end ; curoff += PAGE_SIZE) {
> pp = uvm_pagelookup(uobj, curoff);
> if (pp && pp->pg_flags & PG_BUSY) {
> - atomic_setbits_int(&pp->pg_flags, PG_WANTED);
> - rwsleep_nsec(pp, uobj->vmobjlock, PVM, "km_pgrm",
> - INFSLP);
> + uvm_pagewait(pp, uobj->vmobjlock, "km_pgrm");
> + rw_enter(uobj->vmobjlock, RW_WRITE);
> curoff -= PAGE_SIZE; /* loop back to us */
> continue;
> }
> Index: uvm/uvm_page.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> retrieving revision 1.166
> diff -u -p -r1.166 uvm_page.c
> --- uvm/uvm_page.c12 May 2022 12:48:36 - 1.166
> +++ uvm/uvm_page.c28 Jun 2022 11:57:42 -
> @@ -1087,6 +1087,23 @@ uvm_page_unbusy(struct vm_page **pgs, in
> }
> }
>
> +/*
> + * uvm_pagewait: wait for a busy page
> + *
> + * => page must be known PG_BUSY
> + * => object must be locked
> + * => object will be unlocked on return
> + */
> +void
> +uvm_pagewait(struct vm_page *pg, struct rwlock *lock, const char *wmesg)
> +{
> + KASSERT(rw_lock_held(lock));
> + KASSERT((pg->pg_flags & PG_BUSY) != 0);
>
interface media list mutex
Hi,
Customer complains that routing video stream over OpenBSD stutters
when snmpd is runnning. It looks like ioctl(SIOCGIFMEDIA) may be
the culprit, so I want to get it out of the netlock.
Start with a mutex around interface media list.
ok?
bluhm
Index: dev/ic/gem.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/ic/gem.c,v
retrieving revision 1.126
diff -u -p -r1.126 gem.c
--- dev/ic/gem.c12 Dec 2020 11:48:52 - 1.126
+++ dev/ic/gem.c11 Jul 2022 06:16:15 -
@@ -332,6 +332,7 @@ gem_config(struct gem_softc *sc)
}
/* Check if we support GigE media. */
+ mtx_enter(&ifmedia_mtx);
TAILQ_FOREACH(ifm, &sc->sc_media.ifm_list, ifm_list) {
if (IFM_SUBTYPE(ifm->ifm_media) == IFM_1000_T ||
IFM_SUBTYPE(ifm->ifm_media) == IFM_1000_SX ||
@@ -341,6 +342,7 @@ gem_config(struct gem_softc *sc)
break;
}
}
+ mtx_leave(&ifmedia_mtx);
/* Attach the interface. */
if_attach(ifp);
Index: net/if_media.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_media.c,v
retrieving revision 1.33
diff -u -p -r1.33 if_media.c
--- net/if_media.c 10 Jul 2022 21:13:41 - 1.33
+++ net/if_media.c 11 Jul 2022 06:24:28 -
@@ -82,6 +82,7 @@
#include
#include
#include
+#include
#include
#ifdef IFMEDIA_DEBUG
@@ -102,6 +103,8 @@ int ifmedia_debug = 0;
static void ifmedia_printword(uint64_t);
#endif
+struct mutex ifmedia_mtx = MUTEX_INITIALIZER(IPL_NET);
+
/*
* Initialize if_media struct for a specific interface instance.
*/
@@ -110,6 +113,7 @@ ifmedia_init(struct ifmedia *ifm, uint64
ifm_change_cb_t change_callback, ifm_stat_cb_t status_callback)
{
TAILQ_INIT(&ifm->ifm_list);
+ ifm->ifm_nwords = 0;
ifm->ifm_cur = NULL;
ifm->ifm_media = 0;
ifm->ifm_mask = dontcare_mask; /* IF don't-care bits */
@@ -145,7 +149,10 @@ ifmedia_add(struct ifmedia *ifm, uint64_
entry->ifm_data = data;
entry->ifm_aux = aux;
+ mtx_enter(&ifmedia_mtx);
TAILQ_INSERT_TAIL(&ifm->ifm_list, entry, ifm_list);
+ ifm->ifm_nwords++;
+ mtx_leave(&ifmedia_mtx);
}
/*
@@ -290,7 +297,6 @@ ifmedia_ioctl(struct ifnet *ifp, struct
case SIOCGIFMEDIA:
{
struct ifmediareq *ifmr = (struct ifmediareq *) ifr;
- struct ifmedia_entry *ep;
size_t nwords;
if (ifmr->ifm_count < 0)
@@ -302,37 +308,54 @@ ifmedia_ioctl(struct ifnet *ifp, struct
ifmr->ifm_status = 0;
(*ifm->ifm_status_cb)(ifp, ifmr);
- /*
-* Count them so we know a-priori how much is the max we'll
-* need.
-*/
- ep = TAILQ_FIRST(&ifm->ifm_list);
- for (nwords = 0; ep != NULL; ep = TAILQ_NEXT(ep, ifm_list))
- nwords++;
+ mtx_enter(&ifmedia_mtx);
+ nwords = ifm->ifm_nwords;
+ mtx_leave(&ifmedia_mtx);
- if (ifmr->ifm_count != 0) {
- size_t minwords, ksiz;
+ if (ifmr->ifm_count == 0) {
+ ifmr->ifm_count = nwords;
+ return (0);
+ }
+
+ do {
+ struct ifmedia_entry *ife;
uint64_t *kptr;
+ size_t ksiz;
- minwords = nwords > (size_t)ifmr->ifm_count ?
- (size_t)ifmr->ifm_count : nwords;
kptr = mallocarray(nwords, sizeof(*kptr), M_TEMP,
M_WAITOK | M_ZERO);
ksiz = nwords * sizeof(*kptr);
+
+ mtx_enter(&ifmedia_mtx);
+ /* Madia list might grow during malloc(). */
+ if (nwords < ifm->ifm_nwords) {
+ nwords = ifm->ifm_nwords;
+ mtx_leave(&ifmedia_mtx);
+ free(kptr, M_TEMP, ksiz);
+ continue;
+ }
+ /* Request memory too small, set error and ifm_count. */
+ if (ifmr->ifm_count < ifm->ifm_nwords) {
+ nwords = ifm->ifm_nwords;
+ mtx_leave(&ifmedia_mtx);
+ free(kptr, M_TEMP, ksiz);
+ error = E2BIG;
+ break;
+ }
/*
* Get the media words from the interface's list.
*/
- ep = TAILQ_FIRST(&ifm->ifm_list);
- for (nwords
Re: ampintc: minor nits
> Date: Mon, 11 Jul 2022 08:35:44 +0200
> From: Anton Lindqvist
>
> Hi,
> Addressing the following nits made it easier for me to grasp the ampintc
> implementation while reading the specification. There's also one missing
> NULL check included.
>
> Too verbose or does anyone else consider this an improvement?
Your timing is a but unfortunate since I just posted the
suspend/resume diff that will conflict with this.
Some/all of this may be useful, but not right now :(.
> diff --git sys/arch/arm64/dev/ampintc.c sys/arch/arm64/dev/ampintc.c
> index 3983aa5ed08..6407793e9b8 100644
> --- sys/arch/arm64/dev/ampintc.c
> +++ sys/arch/arm64/dev/ampintc.c
> @@ -125,6 +125,13 @@
> #define ICPHPIR 0x18
> #define ICPIIR 0xFC
>
> +/* Sofware Generated Interrupts (SGI) */
> +#define SGI_MIN 0
> +#define SGI_MAX 15
> +/* Private Peripheral Interrupts (PPI) */
> +#define PPI_MIN 16
> +#define PPI_MAX 31
> +
> /*
> * what about periph_id and component_id
> */
> @@ -233,7 +240,7 @@ ampintc_attach(struct device *parent, struct device
> *self, void *aux)
> {
> struct ampintc_softc *sc = (struct ampintc_softc *)self;
> struct fdt_attach_args *faa = aux;
> - int i, nintr, ncpu;
> + int i, ncpu, nconfigs, nintr, nlines;
> uint32_t ictr;
> #ifdef MULTIPROCESSOR
> int nipi, ipiirq[2];
> @@ -258,8 +265,9 @@ ampintc_attach(struct device *parent, struct device
> *self, void *aux)
> evcount_attach(&sc->sc_spur, "irq1023/spur", NULL);
>
> ictr = bus_space_read_4(sc->sc_iot, sc->sc_d_ioh, ICD_ICTR);
> - nintr = 32 * ((ictr >> ICD_ICTR_ITL_SH) & ICD_ICTR_ITL_M);
> - nintr += 32; /* ICD_ICTR + 1, irq 0-31 is SGI, 32+ is PPI */
> + nlines = (ictr >> ICD_ICTR_ITL_SH) & ICD_ICTR_ITL_M;
> + /* Account for SGI + PPI = 16 + 16 = 32 */
> + nintr = 32 * (nlines + 1);
> sc->sc_nintr = nintr;
> ncpu = ((ictr >> ICD_ICTR_CPU_SH) & ICD_ICTR_CPU_M) + 1;
> printf(" nirq %d, ncpu %d", nintr, ncpu);
> @@ -281,10 +289,19 @@ ampintc_attach(struct device *parent, struct device
> *self, void *aux)
> /* target no cpus */
> bus_space_write_1(sc->sc_iot, sc->sc_d_ioh, ICD_IPTRn(i), 0);
> }
> - for (i = 2; i < nintr/16; i++) {
> - /* irq 32 - N */
> +
> + /*
> + * Reset edge/level configuration for all interrupts where each
> + * interrupt is represented using 2 bits and each configuration register
> + * being 32 bits wide. Each configuration register therefore covers 16
> + * configurations.
> + *
> + * Since SGI are read only and PPI might not be programmable skip the
> + * first 2*(16 + 16)/32 = 2 configuration registers.
> + */
> + nconfigs = 2 * (nlines + 1);
> + for (i = 2; i < nconfigs; i++)
> bus_space_write_4(sc->sc_iot, sc->sc_d_ioh, ICD_ICRn(i*16), 0);
> - }
>
> /* software reset of the part? */
> /* set protection bit (kernel only)? */
> @@ -293,6 +310,8 @@ ampintc_attach(struct device *parent, struct device
> *self, void *aux)
>
> sc->sc_handler = mallocarray(nintr, sizeof(*sc->sc_handler), M_DEVBUF,
> M_ZERO | M_NOWAIT);
> + if (sc->sc_handler == NULL)
> + panic("%s: cannot allocate irq handlers", __func__);
> for (i = 0; i < nintr; i++) {
> TAILQ_INIT(&sc->sc_handler[i].iq_list);
> }
> @@ -313,7 +332,7 @@ ampintc_attach(struct device *parent, struct device
> *self, void *aux)
>* possible that most are not available to the non-secure OS.
>*/
> nipi = 0;
> - for (i = 0; i < 16; i++) {
> + for (i = 0; i <= SGI_MAX; i++) {
> int reg, oldreg;
>
> oldreg = bus_space_read_1(sc->sc_iot, sc->sc_d_ioh,
> @@ -762,10 +781,10 @@ ampintc_intr_establish(int irqno, int type, int level,
> struct cpu_info *ci,
> if (ci == NULL)
> ci = &cpu_info_primary;
>
> - if (irqno < 16) {
> + if (irqno <= SGI_MAX) {
> /* SGI are only EDGE */
> type = IST_EDGE_RISING;
> - } else if (irqno < 32) {
> + } else if (irqno <= PPI_MAX) {
> /* PPI are only LEVEL */
> type = IST_LEVEL_HIGH;
> }
>
>
