Re: pppac(4): don't grab netlock within pppacioctl()
ok yasuoka
On Mon, 18 Jul 2022 13:50:37 +0300
Vitaliy Makkoveev wrote:
> pipex(4) doesn't rely on netlock anymore.
>
> Index: sys/net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.119
> diff -u -p -r1.119 if_pppx.c
> --- sys/net/if_pppx.c 15 Jul 2022 22:56:13 - 1.119
> +++ sys/net/if_pppx.c 18 Jul 2022 10:48:31 -
> @@ -1161,7 +1161,6 @@ pppacioctl(dev_t dev, u_long cmd, caddr_
> struct pppac_softc *sc = pppac_lookup(dev);
> int error = 0;
>
> - NET_LOCK();
> switch (cmd) {
> case FIONBIO:
> break;
> @@ -1180,7 +1179,6 @@ pppacioctl(dev_t dev, u_long cmd, caddr_
> error = pipex_ioctl(sc, cmd, data);
> break;
> }
> - NET_UNLOCK();
>
> return (error);
> }
>
Re: pipex(4): kill "Static" keyword
ok yasuoka
On Mon, 18 Jul 2022 12:31:47 +0300
Vitaliy Makkoveev wrote:
> We don't use "static" keyword for functions declaration to allow ddb(4)
> debug. Also, many "Static" functions are called by pppx(4) layer outside
> pipex(4) layer.
>
> This is the mostly mechanic diff, except the `pipex_pppoe_padding' which
> should be "static const".
>
> Index: sys/net/pipex.c
> ===
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.146
> diff -u -p -r1.146 pipex.c
> --- sys/net/pipex.c 15 Jul 2022 22:56:13 - 1.146
> +++ sys/net/pipex.c 18 Jul 2022 09:30:49 -
> @@ -74,9 +74,6 @@
> #include
> #include
>
> -/* drop static for ddb debuggability */
> -#define Static
> -
> #include
> #include "pipex_local.h"
>
> @@ -559,7 +556,7 @@ pipex_export_session_stats(struct pipex_
> stats->idle_time = session->idle_time;
> }
>
> -Static int
> +int
> pipex_get_stat(struct pipex_session_stat_req *req, void *ownersc)
> {
> struct pipex_session *session;
> @@ -580,7 +577,7 @@ pipex_get_stat(struct pipex_session_stat
> return error;
> }
>
> -Static int
> +int
> pipex_get_closed(struct pipex_session_list_req *req, void *ownersc)
> {
> struct pipex_session *session, *session_tmp;
> @@ -608,7 +605,7 @@ pipex_get_closed(struct pipex_session_li
> return (0);
> }
>
> -Static struct pipex_session *
> +struct pipex_session *
> pipex_lookup_by_ip_address_locked(struct in_addr addr)
> {
> struct pipex_session *session;
> @@ -660,7 +657,7 @@ pipex_lookup_by_ip_address(struct in_add
> }
>
>
> -Static struct pipex_session *
> +struct pipex_session *
> pipex_lookup_by_session_id_locked(int protocol, int session_id)
> {
> struct pipex_hash_head *list;
> @@ -704,20 +701,20 @@ pipex_lookup_by_session_id(int protocol,
> /***
> * Timer functions
> ***/
> -Static void
> +void
> pipex_timer_start(void)
> {
> timeout_set_proc(_timer_ch, pipex_timer, NULL);
> timeout_add_sec(_timer_ch, pipex_prune);
> }
>
> -Static void
> +void
> pipex_timer_stop(void)
> {
> timeout_del(_timer_ch);
> }
>
> -Static void
> +void
> pipex_timer(void *ignored_arg)
> {
> struct pipex_session *session, *session_tmp;
> @@ -764,7 +761,7 @@ pipex_timer(void *ignored_arg)
> /***
> * Common network I/O functions. (tunnel protocol independent)
> ***/
> -Static void
> +void
> pipex_ip_output(struct mbuf *m0, struct pipex_session *session)
> {
> int is_idle;
> @@ -840,7 +837,7 @@ dropped:
> counters_inc(session->stat_counters, pxc_oerrors);
> }
>
> -Static void
> +void
> pipex_ppp_output(struct mbuf *m0, struct pipex_session *session, int proto)
> {
> u_char *cp, hdr[16];
> @@ -897,7 +894,7 @@ drop:
> counters_inc(session->stat_counters, pxc_oerrors);
> }
>
> -Static void
> +void
> pipex_ppp_input(struct mbuf *m0, struct pipex_session *session, int
> decrypted)
> {
> int proto, hlen = 0;
> @@ -990,7 +987,7 @@ drop:
> counters_inc(session->stat_counters, pxc_ierrors);
> }
>
> -Static void
> +void
> pipex_ip_input(struct mbuf *m0, struct pipex_session *session)
> {
> struct ifnet *ifp;
> @@ -1067,7 +1064,7 @@ drop:
> }
>
> #ifdef INET6
> -Static void
> +void
> pipex_ip6_input(struct mbuf *m0, struct pipex_session *session)
> {
> struct ifnet *ifp;
> @@ -1115,7 +1112,7 @@ drop:
> }
> #endif
>
> -Static struct mbuf *
> +struct mbuf *
> pipex_common_input(struct pipex_session *session, struct mbuf *m0, int hlen,
> int plen, int locked)
> {
> @@ -1187,7 +1184,7 @@ not_ours:
> /*
> * pipex_ppp_proto
> */
> -Static int
> +int
> pipex_ppp_proto(struct mbuf *m0, struct pipex_session *session, int off,
> int *hlenp)
> {
> @@ -1228,7 +1225,7 @@ pipex_ppp_proto(struct mbuf *m0, struct
> /***
> * PPPoE
> ***/
> -Static u_charpipex_pppoe_padding[ETHERMIN];
> +static const u_char pipex_pppoe_padding[ETHERMIN];
> /*
> * pipex_pppoe_lookup_session
> */
> @@ -1286,7 +1283,7 @@ pipex_pppoe_input(struct mbuf *m0, struc
> /*
> * pipex_ppope_output
> */
> -Static void
> +void
> pipex_pppoe_output(struct mbuf *m0, struct pipex_session *session)
> {
> struct pipex_pppoe_header *pppoe;
> @@ -1332,7 +1329,7 @@ pipex_pppoe_output(struct mbuf *m0, stru
> /***
> * PPTP
> ***/
> -Static void
> +void
> pipex_pptp_output(struct mbuf
Re: checksum offloading for em
Oh, thanks so much for doing that... I had gotten about halfway through writing this same patch and had to abandon the effort a few months ago because life got crazy. BTW, there is a section in there that disables the CRC stripping for i350/i210 claiming there's a bug causing it to strip whether it's wanted or not. I did some digging in the official Intel driver, which FreeBSD uses more or less verbatim, and it looks like it's not a bug so much as difficult documentation: the per-queue setting for the stripping overrides the global one. Intel fixes it here: http://git.dpdk.org/dpdk-stable/commit/?h=v2.0.0=c872db9ea56413142e12c1477fa49b86f4f6740e If that's not included in this patch (haven't read through it in detail yet), you might want to look into it, I had put it in on mine and it seemed to work. FWIW, this fix went in not long after the OpenBSD version was forked from the Intel source. Unfortunate timing! I'll see if I can give this a try this weekend, I definitely have i210/i211/i350 hardware as well as legacy hardware that I *think* falls under the old regime. - Dave > On Jul 22, 2022, at 4:09 PM, Moritz Buhl wrote: > > On Mon, Jun 27, 2022 at 08:07:32AM +1000, Jonathan Gray wrote: >> On Sun, Jun 26, 2022 at 04:43:59PM +0200, Moritz Buhl wrote: >>> On Sun, Jun 26, 2022 at 12:23:58PM +0200, Moritz Buhl wrote: Hi, I noticed that for some offloading-capable em controllers checksum offloading is still disabled and I couldn't find a reason for that. >> >> There are two descriptor formats on 82575/82576/i350/i354/i210/i211. >> The older one we use and the newer igb/ix style one we don't use in em. >> A lot of the offloading options are in the newer descriptor format >> from memory. > > Thanks that helped a lot. Below is a diff that implements offloading > for i350 and i210 (I haven't checked it for the others you mentioned). > It also does tcp and upd checksums for the older ones also ipv4. > > I didn't feel confident in the diff but I currently cannot cause > any misbehavoir. > > Using VLAN with i350 and i210 should still break because > IFCAP_VLAN_HWTAGGING is not fully implemented yet. > > I would appreciate any feedback and comments. > > mbuhl
ipv4 reassemble in parallel
Hi,
The IPv6 reassembly code looks MP safe. So we can run it in parallel.
Note that ip_ours() runs with shared netlock, while ip_local() has
exclusive netlock after queuing.
ok?
bluhm
Index: netinet/ip_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.372
diff -u -p -r1.372 ip_input.c
--- netinet/ip_input.c 29 Jun 2022 09:01:48 - 1.372
+++ netinet/ip_input.c 22 Jul 2022 21:59:38 -
@@ -142,6 +142,11 @@ intip_local(struct mbuf **, int *, int,
intip_dooptions(struct mbuf *, struct ifnet *);
intin_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
+intip_fragcheck(struct mbuf **, int *);
+struct mbuf * ip_reass(struct ipqent *, struct ipq *);
+void ip_freef(struct ipq *);
+void ip_flush(void);
+
static void ip_send_dispatch(void *);
static void ip_sendraw_dispatch(void *);
static struct task ipsend_task = TASK_INITIALIZER(ip_send_dispatch,
_mq);
@@ -234,6 +239,10 @@ ip_init(void)
int
ip_ours(struct mbuf **mp, int *offp, int nxt, int af)
{
+ nxt = ip_fragcheck(mp, offp);
+ if (nxt == IPPROTO_DONE)
+ return IPPROTO_DONE;
+
/* We are already in a IPv4/IPv6 local deliver loop. */
if (af != AF_UNSPEC)
return ip_local(mp, offp, nxt, af);
@@ -550,14 +559,29 @@ ip_input_if(struct mbuf **mp, int *offp,
int
ip_local(struct mbuf **mp, int *offp, int nxt, int af)
{
- struct mbuf *m = *mp;
- struct ip *ip = mtod(m, struct ip *);
+ struct ip *ip;
+
+ NET_ASSERT_WLOCKED();
+
+ ip = mtod(*mp, struct ip *);
+ *offp = ip->ip_hl << 2;
+ nxt = ip->ip_p;
+
+ /* Check whether we are already in a IPv4/IPv6 local deliver loop. */
+ if (af == AF_UNSPEC)
+ nxt = ip_deliver(mp, offp, nxt, AF_INET);
+ return nxt;
+}
+
+int
+ip_fragcheck(struct mbuf **mp, int *offp)
+{
+ struct ip *ip;
struct ipq *fp;
struct ipqent *ipqe;
int mff, hlen;
- NET_ASSERT_WLOCKED();
-
+ ip = mtod(*mp, struct ip *);
hlen = ip->ip_hl << 2;
/*
@@ -568,12 +592,12 @@ ip_local(struct mbuf **mp, int *offp, in
* but it's not worth the time; just let them time out.)
*/
if (ip->ip_off &~ htons(IP_DF | IP_RF)) {
- if (m->m_flags & M_EXT) { /* XXX */
- if ((m = *mp = m_pullup(m, hlen)) == NULL) {
+ if ((*mp)->m_flags & M_EXT) { /* XXX */
+ if ((*mp = m_pullup(*mp, hlen)) == NULL) {
ipstat_inc(ips_toosmall);
return IPPROTO_DONE;
}
- ip = mtod(m, struct ip *);
+ ip = mtod(*mp, struct ip *);
}
mtx_enter(_mutex);
@@ -630,28 +654,26 @@ ip_local(struct mbuf **mp, int *offp, in
}
ip_frags++;
ipqe->ipqe_mff = mff;
- ipqe->ipqe_m = m;
+ ipqe->ipqe_m = *mp;
ipqe->ipqe_ip = ip;
- m = *mp = ip_reass(ipqe, fp);
- if (m == NULL)
+ *mp = ip_reass(ipqe, fp);
+ if (*mp == NULL)
goto bad;
ipstat_inc(ips_reassembled);
- ip = mtod(m, struct ip *);
+ ip = mtod(*mp, struct ip *);
hlen = ip->ip_hl << 2;
ip->ip_len = htons(ntohs(ip->ip_len) + hlen);
- } else
- if (fp)
+ } else {
+ if (fp != NULL)
ip_freef(fp);
+ }
mtx_leave(_mutex);
}
*offp = hlen;
- nxt = ip->ip_p;
- /* Check whether we are already in a IPv4/IPv6 local deliver loop. */
- if (af == AF_UNSPEC)
- nxt = ip_deliver(mp, offp, nxt, AF_INET);
- return nxt;
+ return ip->ip_p;
+
bad:
mtx_leave(_mutex);
m_freemp(mp);
Index: netinet/ip_var.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.93
diff -u -p -r1.93 ip_var.h
--- netinet/ip_var.h5 May 2022 13:57:40 - 1.93
+++ netinet/ip_var.h22 Jul 2022 21:39:01 -
@@ -223,9 +223,7 @@ struct route;
struct inpcb;
int ip_ctloutput(int, struct socket *, int, int, struct mbuf *);
-voidip_flush(void);
int ip_fragment(struct mbuf *, struct mbuf_list *, struct ifnet *, u_long);
-voidip_freef(struct ipq *);
voidip_freemoptions(struct ip_moptions *);
int ip_getmoptions(int, struct ip_moptions *,
Re: checksum offloading for em
On Mon, Jun 27, 2022 at 08:07:32AM +1000, Jonathan Gray wrote:
> On Sun, Jun 26, 2022 at 04:43:59PM +0200, Moritz Buhl wrote:
> > On Sun, Jun 26, 2022 at 12:23:58PM +0200, Moritz Buhl wrote:
> > > Hi,
> > >
> > > I noticed that for some offloading-capable em controllers checksum
> > > offloading is still disabled and I couldn't find a reason for that.
>
> There are two descriptor formats on 82575/82576/i350/i354/i210/i211.
> The older one we use and the newer igb/ix style one we don't use in em.
> A lot of the offloading options are in the newer descriptor format
> from memory.
Thanks that helped a lot. Below is a diff that implements offloading
for i350 and i210 (I haven't checked it for the others you mentioned).
It also does tcp and upd checksums for the older ones also ipv4.
I didn't feel confident in the diff but I currently cannot cause
any misbehavoir.
Using VLAN with i350 and i210 should still break because
IFCAP_VLAN_HWTAGGING is not fully implemented yet.
I would appreciate any feedback and comments.
mbuhl
Index: dev/pci/if_em.c
===
RCS file: /cvs/src/sys/dev/pci/if_em.c,v
retrieving revision 1.362
diff -u -p -r1.362 if_em.c
--- dev/pci/if_em.c 23 Jun 2022 09:38:28 - 1.362
+++ dev/pci/if_em.c 22 Jul 2022 12:37:27 -
@@ -37,6 +37,8 @@ POSSIBILITY OF SUCH DAMAGE.
#include
#include
+#include
+
/*
* Driver version
*/
@@ -278,6 +280,8 @@ void em_receive_checksum(struct em_softc
struct mbuf *);
u_int em_transmit_checksum_setup(struct em_queue *, struct mbuf *, u_int,
u_int32_t *, u_int32_t *);
+u_int em_tx_ctx_setup(struct em_queue *, struct mbuf *, u_int, u_int32_t *,
+ u_int32_t *);
void em_iff(struct em_softc *);
void em_update_link_status(struct em_softc *);
int em_get_buf(struct em_queue *, int);
@@ -1221,11 +1225,12 @@ em_encap(struct em_queue *que, struct mb
}
if (sc->hw.mac_type >= em_82543 && sc->hw.mac_type != em_82575 &&
- sc->hw.mac_type != em_82576 &&
- sc->hw.mac_type != em_82580 && sc->hw.mac_type != em_i210 &&
- sc->hw.mac_type != em_i350) {
+ sc->hw.mac_type != em_82576 && sc->hw.mac_type != em_82580 &&
+ sc->hw.mac_type != em_i210 && sc->hw.mac_type != em_i350) {
used += em_transmit_checksum_setup(que, m, head,
_upper, _lower);
+ } else if (sc->hw.mac_type == em_i210 || sc->hw.mac_type == em_i350) {
+ used += em_tx_ctx_setup(que, m, head, _upper, _lower);
} else {
txd_upper = txd_lower = 0;
}
@@ -1971,10 +1976,11 @@ em_setup_interface(struct em_softc *sc)
#endif
if (sc->hw.mac_type >= em_82543 && sc->hw.mac_type != em_82575 &&
- sc->hw.mac_type != em_82576 &&
- sc->hw.mac_type != em_82580 && sc->hw.mac_type != em_i210 &&
- sc->hw.mac_type != em_i350)
+ sc->hw.mac_type != em_82576 && sc->hw.mac_type != em_82580) {
+ ifp->if_capabilities |= IFCAP_CSUM_IPv4;
ifp->if_capabilities |= IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4;
+ ifp->if_capabilities |= IFCAP_CSUM_TCPv6 | IFCAP_CSUM_UDPv6;
+ }
/*
* Specify the media types supported by this adapter and register
@@ -2391,6 +2397,96 @@ em_free_transmit_structures(struct em_so
}
}
+u_int
+em_tx_ctx_setup(struct em_queue *que, struct mbuf *mp, u_int head,
+u_int32_t *olinfo_status, u_int32_t *cmd_type_len)
+{
+ struct e1000_adv_tx_context_desc *TD;
+ struct ether_header *eh = mtod(mp, struct ether_header *);
+ struct mbuf *m;
+ uint32_t vlan_macip_lens = 0, type_tucmd_mlhl = 0;
+ uint32_t iphlen;
+ int off = 0, hoff;
+ uint8_t ipproto;
+
+ *olinfo_status = 0;
+ *cmd_type_len = 0;
+ TD = (struct e1000_adv_tx_context_desc *)>tx.sc_tx_desc_ring[head];
+
+ // XXX: VLAN TAGGING
+
+ vlan_macip_lens |= (sizeof(*eh) << E1000_ADVTXD_MACLEN_SHIFT);
+
+ switch (ntohs(eh->ether_type)) {
+ case ETHERTYPE_IP: {
+ struct ip *ip;
+
+ m = m_getptr(mp, sizeof(*eh), );
+ ip = (struct ip *)(mtod(m, caddr_t) + hoff);
+
+ iphlen = ip->ip_hl << 2;
+ ipproto = ip->ip_p;
+
+ type_tucmd_mlhl |= E1000_ADVTXD_TUCMD_IPV4;
+ if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) {
+ *olinfo_status |= E1000_TXD_POPTS_IXSM << 8;
+ off = 1;
+ }
+
+ break;
+ }
+#ifdef INET6
+ case ETHERTYPE_IPV6: {
+ struct ip6_hdr *ip6;
+
+ m = m_getptr(mp, sizeof(*eh), );
+ ip6 = (struct ip6_hdr *)(mtod(m,
ipv6 local deliver net lock
Hi,
During regress testing I found this bug.
splassert: rip6_input: want 1 have 2
Starting stack trace...
rip6_input(1,2,d0c6b7ad,f57ff9fc) at rip6_input+0x166
rip6_input(f57ffbfc,f57ffbe8,3a,18) at rip6_input+0x166
icmp6_input(f57ffbfc,f57ffbe8,3a,18) at icmp6_input+0x66d
ip_deliver(f57ffbfc,f57ffbe8,3a,18) at ip_deliver+0xf4
ip6_input_if(f57ffbfc,f57ffbe8,29,0,d7066830) at ip6_input_if+0x88a
ipv6_input(d7066830,dafe5400) at ipv6_input+0x2b
ether_input(d7066830,dafe5400) at ether_input+0x3a9
if_input_process(d7066830,f57ffc54) at if_input_process+0x5d
ifiq_process(d7066ae0) at ifiq_process+0x57
taskq_thread(d6ff1040) at taskq_thread+0x69
End of stack trace.
ip6_input() has shared net lock. ip_deliver() needs exclusive net
lock. Use ip6_ours() to queue the packet. Move the write lock
assertion into ip_deliver() to catch such bugs earlier.
The assertion is only triggered with IPv6 multicast forwarding or
router alert hop by hop option. So nobody noticed it.
ok?
bluhm
Index: netinet/ip_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.372
diff -u -p -r1.372 ip_input.c
--- netinet/ip_input.c 29 Jun 2022 09:01:48 - 1.372
+++ netinet/ip_input.c 22 Jul 2022 19:23:47 -
@@ -556,8 +556,6 @@ ip_local(struct mbuf **mp, int *offp, in
struct ipqent *ipqe;
int mff, hlen;
- NET_ASSERT_WLOCKED();
-
hlen = ip->ip_hl << 2;
/*
@@ -673,6 +671,8 @@ ip_deliver(struct mbuf **mp, int *offp,
#ifdef INET6
int nest = 0;
#endif /* INET6 */
+
+ NET_ASSERT_WLOCKED();
/* pf might have modified stuff, might have to chksum */
switch (af) {
Index: netinet6/ip6_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.248
diff -u -p -r1.248 ip6_input.c
--- netinet6/ip6_input.c29 Jun 2022 22:45:24 - 1.248
+++ netinet6/ip6_input.c22 Jul 2022 19:23:17 -
@@ -448,8 +448,7 @@ ip6_input_if(struct mbuf **mp, int *offp
if (ours) {
if (af == AF_UNSPEC)
- nxt = ip_deliver(mp, offp, nxt,
- AF_INET6);
+ nxt = ip6_ours(mp, offp, nxt, af);
goto out;
}
goto bad;
@@ -550,7 +549,7 @@ ip6_input_if(struct mbuf **mp, int *offp
if (ours) {
if (af == AF_UNSPEC)
- nxt = ip_deliver(mp, offp, nxt, AF_INET6);
+ nxt = ip6_ours(mp, offp, nxt, af);
goto out;
}
@@ -584,8 +583,6 @@ ip6_input_if(struct mbuf **mp, int *offp
int
ip6_local(struct mbuf **mp, int *offp, int nxt, int af)
{
- NET_ASSERT_WLOCKED();
-
nxt = ip6_hbhchcheck(mp, offp, NULL);
if (nxt == IPPROTO_DONE)
return IPPROTO_DONE;
[v2] timeout.9: rewrite
Hi, As promised, here is the timeout.9 manpage rewrite I've been sitting on. I am pretty sure jmc@ (and maybe schwarze@) read an earlier version of this. It has drifted a bit since then, but not much. My main goal here is to make all the "gotchas" in the timeout API more explicit. The API is large, so the manpage is necessarily longer than the average manpage. We're also stuck in the midst of an API transition, so there is some overlap in the API coverage. Hopefully most of that redundancy can be consolidated in the future after I finish the clock interrupt work. -Scott Index: share/man/man9/timeout.9 === RCS file: /cvs/src/share/man/man9/timeout.9,v retrieving revision 1.55 diff -u -p -r1.55 timeout.9 --- share/man/man9/timeout.922 Jun 2022 14:10:49 - 1.55 +++ share/man/man9/timeout.922 Jul 2022 18:34:14 - @@ -1,6 +1,7 @@ .\"$OpenBSD: timeout.9,v 1.55 2022/06/22 14:10:49 visa Exp $ .\" .\" Copyright (c) 2000 Artur Grabowski +.\" Copyright (c) 2021, 2022 Scott Cheloha .\" All rights reserved. .\" .\" Redistribution and use in source and binary forms, with or without @@ -36,6 +37,8 @@ .Nm timeout_add_nsec , .Nm timeout_add_usec , .Nm timeout_add_tv , +.Nm timeout_rel_nsec , +.Nm timeout_abs_ts , .Nm timeout_del , .Nm timeout_del_barrier , .Nm timeout_barrier , @@ -44,281 +47,375 @@ .Nm timeout_triggered , .Nm TIMEOUT_INITIALIZER , .Nm TIMEOUT_INITIALIZER_FLAGS -.Nd execute a function after a specified period of time +.Nd execute a function in the future .Sh SYNOPSIS .In sys/types.h .In sys/timeout.h .Ft void -.Fn timeout_set "struct timeout *to" "void (*fn)(void *)" "void *arg" +.Fo timeout_set +.Fa "struct timeout *to" +.Fa "void (*fn)(void *)" +.Fa "void *arg" +.Fc .Ft void .Fo timeout_set_flags .Fa "struct timeout *to" .Fa "void (*fn)(void *)" .Fa "void *arg" +.Fa "int kclock" .Fa "int flags" .Fc .Ft void -.Fn timeout_set_proc "struct timeout *to" "void (*fn)(void *)" "void *arg" +.Fo timeout_set_proc +.Fa "struct timeout *to" +.Fa "void (*fn)(void *)" +.Fa "void *arg" +.Fc .Ft int -.Fn timeout_add "struct timeout *to" "int ticks" +.Fo timeout_add +.Fa "struct timeout *to" +.Fa "int nticks" +.Fc .Ft int -.Fn timeout_del "struct timeout *to" +.Fo timeout_add_sec +.Fa "struct timeout *to" +.Fa "int secs" +.Fc .Ft int -.Fn timeout_del_barrier "struct timeout *to" -.Ft void -.Fn timeout_barrier "struct timeout *to" +.Fo timeout_add_msec +.Fa "struct timeout *to" +.Fa "int msecs" +.Fc .Ft int -.Fn timeout_pending "struct timeout *to" +.Fo timeout_add_usec +.Fa "struct timeout *to" +.Fa "int usecs" +.Fc .Ft int -.Fn timeout_initialized "struct timeout *to" +.Fo timeout_add_nsec +.Fa "struct timeout *to" +.Fa "int nsecs" +.Fc .Ft int -.Fn timeout_triggered "struct timeout *to" +.Fo timeout_add_tv +.Fa "struct timeout *to" +.Fa "struct timeval *tv" +.Fc .Ft int -.Fn timeout_add_tv "struct timeout *to" "struct timeval *" +.Fo timeout_rel_nsec +.Fa "struct timeout *to" +.Fa "uint64_t nsecs" +.Fc .Ft int -.Fn timeout_add_sec "struct timeout *to" "int sec" +.Fo timeout_abs_ts +.Fa "struct timeout *to" +.Fa "const struct timespec *abs" +.Fc .Ft int -.Fn timeout_add_msec "struct timeout *to" "int msec" +.Fo timeout_del +.Fa "struct timeout *to" +.Fc +.Ft int +.Fo timeout_del_barrier +.Fa "struct timeout *to" +.Fc +.Ft void +.Fo timeout_barrier +.Fa "struct timeout *to" +.Fc +.Ft int +.Fo timeout_pending +.Fa "struct timeout *to" +.Fc .Ft int -.Fn timeout_add_usec "struct timeout *to" "int usec" +.Fo timeout_initialized +.Fa "struct timeout *to" +.Fc .Ft int -.Fn timeout_add_nsec "struct timeout *to" "int nsec" -.Fn TIMEOUT_INITIALIZER "void (*fn)(void *)" "void *arg" -.Fn TIMEOUT_INITIALIZER_FLAGS "void (*fn)(void *)" "void *arg" "int flags" +.Fo timeout_triggered +.Fa "struct timeout *to" +.Fc +.Fo TIMEOUT_INITIALIZER +.Fa "void (*fn)(void *)" +.Fa "void *arg" +.Fc +.Fo TIMEOUT_INITIALIZER_FLAGS +.Fa "void (*fn)(void *)" +.Fa "void *arg" +.Fa "int kclock" +.Fa "int flags" +.Fc .Sh DESCRIPTION The .Nm timeout -API provides a mechanism to execute a function at a given time. -The granularity of the time is limited by the granularity of the -.Xr hardclock 9 -timer which executes -.Xr hz 9 -times a second. +API provides a mechanism to schedule a function for asynchronous +execution in the future. .Pp -It is the responsibility of the caller to provide these functions with -pre-allocated timeout structures. +All state is encapsulated in a caller-allocated timeout structure +.Pq hereafter, a Qo timeout Qc . +A timeout must be initialized before it may be used as input to other +functions in the API. .Pp The .Fn timeout_set -function prepares the timeout structure -.Fa to -to be used in future calls to -.Fn timeout_add -and -.Fn timeout_del . -The timeout will be prepared to call the function specified by the +function initializes the timeout +.Fa to . +When the timeout is executed,
Re: nd6: Zap nd6_recalc_reachtm_interval indirection
On 2022-07-22 14:27 +02, Claudio Jeker wrote: > On Fri, Jul 22, 2022 at 12:18:34PM +, Klemens Nanni wrote: >> Only used once, so use the macro directly like ND6_SLOWTIMER_INTERVAL >> is used in many places. >> >> OK? > > Is that a value that should be adjustable? I don't think so, this is the amount of time that has to elapse until a new random time for the reachability timer is calculated. This is like three layers deep in nd6, this is not a knob that needs twiddling. It's also not a knob that RFC 4861 specifies, let alone specifies as twiddleable. OK florian > >> diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c >> index ff679bcb151..3decec947c4 100644 >> --- a/sys/netinet6/nd6.c >> +++ b/sys/netinet6/nd6.c >> @@ -88,8 +88,6 @@ TAILQ_HEAD(llinfo_nd6_head, llinfo_nd6) nd6_list; >> struct pool nd6_pool; /* pool for llinfo_nd6 structures */ >> int nd6_inuse; >> >> -int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL; >> - >> void nd6_timer(void *); >> void nd6_slowtimo(void *); >> void nd6_expire(void *); >> @@ -1318,7 +1316,7 @@ nd6_slowtimo(void *ignored_arg) >> * value gets recomputed at least once every few hours. >> * (RFC 2461, 6.3.4) >> */ >> -nd6if->recalctm = nd6_recalc_reachtm_interval; >> +nd6if->recalctm = ND6_RECALC_REACHTM_INTERVAL; >> nd6if->reachable = >> ND_COMPUTE_RTIME(nd6if->basereachable); >> } >> } >> > > -- > :wq Claudio > -- I'm not entirely sure you are real.
Re: pf: DIOCXCOMMIT and copyin
On Thu, Jul 21, 2022 at 11:13:28AM +0200, Moritz Buhl wrote:
> Hi tech,
>
> for the other two DIOCX ioctls syzkaller showed that it is possible
> to grab netlock while doing copyin.
>
> The same problem should exist for DIOCXCOMMIT but syzkaller didn't
> find it yet.
>
> In case anybody can reproduce the witness lock order reversals the
> syzkaller can produce, the diff below might address the problem.
>
> mbuhl
OK bluhm@
> Index: sys/net/pf_ioctl.c
> ===
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> retrieving revision 1.383
> diff -u -p -r1.383 pf_ioctl.c
> --- sys/net/pf_ioctl.c20 Jul 2022 09:33:11 - 1.383
> +++ sys/net/pf_ioctl.c20 Jul 2022 18:17:45 -
> @@ -2621,13 +2621,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> }
> ioe = malloc(sizeof(*ioe), M_TEMP, M_WAITOK);
> table = malloc(sizeof(*table), M_TEMP, M_WAITOK);
> - NET_LOCK();
> - PF_LOCK();
> /* first makes sure everything will succeed */
> for (i = 0; i < io->size; i++) {
> if (copyin(io->array+i, ioe, sizeof(*ioe))) {
> - PF_UNLOCK();
> - NET_UNLOCK();
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> error = EFAULT;
> @@ -2635,13 +2631,13 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> }
> if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
> sizeof(ioe->anchor)) {
> - PF_UNLOCK();
> - NET_UNLOCK();
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> error = ENAMETOOLONG;
> goto fail;
> }
> + NET_LOCK();
> + PF_LOCK();
> switch (ioe->type) {
> case PF_TRANS_TABLE:
> rs = pf_find_ruleset(ioe->anchor);
> @@ -2677,7 +2673,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> error = EINVAL;
> goto fail;
> }
> + PF_UNLOCK();
> + NET_UNLOCK();
> }
> + NET_LOCK();
> + PF_LOCK();
>
> /*
>* Checked already in DIOCSETLIMIT, but check again as the
> @@ -2696,9 +2696,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> }
> /* now do the commit - no errors should happen here */
> for (i = 0; i < io->size; i++) {
> + PF_UNLOCK();
> + NET_UNLOCK();
> if (copyin(io->array+i, ioe, sizeof(*ioe))) {
> - PF_UNLOCK();
> - NET_UNLOCK();
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> error = EFAULT;
> @@ -2706,13 +2706,13 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> }
> if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
> sizeof(ioe->anchor)) {
> - PF_UNLOCK();
> - NET_UNLOCK();
> free(table, M_TEMP, sizeof(*table));
> free(ioe, M_TEMP, sizeof(*ioe));
> error = ENAMETOOLONG;
> goto fail;
> }
> + NET_LOCK();
> + PF_LOCK();
> switch (ioe->type) {
> case PF_TRANS_TABLE:
> memset(table, 0, sizeof(*table));
Re: interface media current data
On Fri, Jul 22, 2022 at 05:44:52PM +0200, Mark Kettenis wrote:
> > Date: Fri, 22 Jul 2022 17:01:20 +0200
> > From: Alexander Bluhm
> >
> > On Fri, Jul 15, 2022 at 04:51:25PM +0200, Alexander Bluhm wrote:
> > > On Fri, Jul 15, 2022 at 02:05:40AM +0200, Alexander Bluhm wrote:
> > > > If anyone has some of these old network drivers, please test setting
> > > > media type with ifconfig.
> > >
> > > Updated diff. I have compile tested it on amd64, i386, octeon,
> > > macppc, sparc64. Setting media for gem(4) and bge(4) works.
> >
> > Also compiles and boots on powerpc64 and arm64.
> >
> > How do we proceed here? The diff touches 70 drivers, that cannot
> > be tested due to missing hardware. But I need the MP safe interface
> > in ifmedia. Should I commit the API and the few drivers for the
> > hardware I have? And then slowly commit the remaining drivers.
> > Or put everything in and see what happens?
> >
> > ok?
>
> Maybe this approach is too invasive?
>
> A lot of the changes are in attach functions, where concurrencey is
> *not* a problem.
Most of the drivers run with kernel lock, so there is currently no
problem. And most of them will keep kernel lock for the next 20
years. Many of them are not used anymore.
The idea of an API is to make things simple. Certain data fields
should be protected by a mutex, always. Alternative is to review
70 drivers each time MP work is done in ifmedia to ensure that
struct fields are not used concurrently.
But I am fine with committing ifmedia, gem(4) and bge(4) now. Then
we can decide on a per driver basis. As long kernel lock is not
removed from the ifmedia layer, this diff is not strictly necessary.
I want to commit it anyway to show how MP in ifmedia should work.
Note that ixl(4) has already messed up, whether kernel or net lock
is needed. So something has to be done. A clean MP safe ifmedia
layer would help.
bluhm
> > > Index: arch/octeon/dev/cn30xxgmx.c
> > > ===
> > > RCS file: /data/mirror/openbsd/cvs/src/sys/arch/octeon/dev/cn30xxgmx.c,v
> > > retrieving revision 1.52
> > > diff -u -p -r1.52 cn30xxgmx.c
> > > --- arch/octeon/dev/cn30xxgmx.c 6 Apr 2022 18:59:26 - 1.52
> > > +++ arch/octeon/dev/cn30xxgmx.c 14 Jul 2022 22:46:20 -
> > > @@ -725,19 +725,21 @@ cn30xxgmx_reset_timing(struct cn30xxgmx_
> > > int
> > > cn30xxgmx_reset_flowctl(struct cn30xxgmx_port_softc *sc)
> > > {
> > > - struct ifmedia_entry *ife = sc->sc_port_mii->mii_media.ifm_cur;
> > > + uint64_t media;
> > > +
> > > + ifmedia_current(>sc_port_mii->mii_media, , NULL);
> > >
> > > /*
> > >* Get flow control negotiation result.
> > >*/
> > > #ifdef GMX_802_3X_DISABLE_AUTONEG
> > > /* Tentative support for SEIL-compat.. */
> > > - if (IFM_SUBTYPE(ife->ifm_media) == IFM_AUTO) {
> > > + if (IFM_SUBTYPE(media) == IFM_AUTO) {
> > > sc->sc_port_flowflags &= ~IFM_ETH_FMASK;
> > > }
> > > #else
> > > /* Default configuration of NetBSD */
> > > - if (IFM_SUBTYPE(ife->ifm_media) == IFM_AUTO &&
> > > + if (IFM_SUBTYPE(media) == IFM_AUTO &&
> > > (sc->sc_port_mii->mii_media_active & IFM_ETH_FMASK) !=
> > > sc->sc_port_flowflags) {
> > > sc->sc_port_flowflags =
> > > Index: dev/ic/an.c
> > > ===
> > > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/ic/an.c,v
> > > retrieving revision 1.79
> > > diff -u -p -r1.79 an.c
> > > --- dev/ic/an.c 21 Apr 2022 21:03:02 - 1.79
> > > +++ dev/ic/an.c 14 Jul 2022 22:30:51 -
> > > @@ -1277,17 +1277,18 @@ an_media_change(struct ifnet *ifp)
> > > {
> > > struct an_softc *sc = ifp->if_softc;
> > > struct ieee80211com *ic = >sc_ic;
> > > - struct ifmedia_entry *ime;
> > > + uint64_t media;
> > > enum ieee80211_opmode newmode;
> > > int i, rate, error = 0;
> > >
> > > - ime = ic->ic_media.ifm_cur;
> > > - if (IFM_SUBTYPE(ime->ifm_media) == IFM_AUTO) {
> > > + ifmedia_current(>ic_media, , NULL);
> > > +
> > > + if (IFM_SUBTYPE(media) == IFM_AUTO) {
> > > i = -1;
> > > } else {
> > > struct ieee80211_rateset *rs =
> > > >ic_sup_rates[IEEE80211_MODE_11B];
> > > - rate = ieee80211_media2rate(ime->ifm_media);
> > > + rate = ieee80211_media2rate(media);
> > > if (rate == 0)
> > > return EINVAL;
> > > for (i = 0; i < rs->rs_nrates; i++) {
> > > @@ -1303,13 +1304,13 @@ an_media_change(struct ifnet *ifp)
> > > }
> > >
> > > #ifndef IEEE80211_STA_ONLY
> > > - if (ime->ifm_media & IFM_IEEE80211_ADHOC)
> > > + if (media & IFM_IEEE80211_ADHOC)
> > > newmode = IEEE80211_M_IBSS;
> > > - else if (ime->ifm_media & IFM_IEEE80211_HOSTAP)
> > > + else if (media & IFM_IEEE80211_HOSTAP)
> > > newmode = IEEE80211_M_HOSTAP;
> > > else
> > > #endif
> > > - if (ime->ifm_media & IFM_IEEE80211_MONITOR)
> > > + if (media &
nd6: statically initialise global timeouts and tasks
No need to to this at runtime if the compiler can do it for us. Also makes it easier to know what's going on at a glance, imho. Feedback? OK? diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c index c8720fb275c..035e65874ea 100644 --- a/sys/netinet6/nd6.c +++ b/sys/netinet6/nd6.c @@ -84,7 +84,8 @@ int nd6_debug = 1; int nd6_debug = 0; #endif -TAILQ_HEAD(llinfo_nd6_head, llinfo_nd6) nd6_list; +TAILQ_HEAD(llinfo_nd6_head, llinfo_nd6) nd6_list = +TAILQ_HEAD_INITIALIZER(nd6_list); struct pool nd6_pool; /* pool for llinfo_nd6 structures */ intnd6_inuse; @@ -96,10 +97,13 @@ void nd6_invalidate(struct rtentry *); void nd6_free(struct rtentry *); int nd6_llinfo_timer(struct rtentry *); -struct timeout nd6_timer_to; -struct timeout nd6_slowtimo_ch; -struct timeout nd6_expire_timeout; -struct task nd6_expire_task; +struct timeout nd6_timer_to = TIMEOUT_INITIALIZER_FLAGS(nd6_timer, NULL, +TIMEOUT_PROC); +struct timeout nd6_slowtimo_ch = TIMEOUT_INITIALIZER_FLAGS(nd6_slowtimo, NULL, +TIMEOUT_PROC); +struct timeout nd6_expire_timeout = TIMEOUT_INITIALIZER(nd6_slowtimo, NULL); + +struct task nd6_expire_task = TASK_INITIALIZER(nd6_expire, NULL); void nd6_init(void) @@ -111,19 +115,13 @@ nd6_init(void) return; } - TAILQ_INIT(_list); pool_init(_pool, sizeof(struct llinfo_nd6), 0, IPL_SOFTNET, 0, "nd6", NULL); - task_set(_expire_task, nd6_expire, NULL); - nd6_init_done = 1; /* start timer */ - timeout_set_proc(_timer_to, nd6_timer, NULL); - timeout_set_proc(_slowtimo_ch, nd6_slowtimo, NULL); timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL); - timeout_set(_expire_timeout, nd6_expire_timer, NULL); } struct nd_ifinfo *
Re: interface media current data
> Date: Fri, 22 Jul 2022 17:01:20 +0200
> From: Alexander Bluhm
>
> On Fri, Jul 15, 2022 at 04:51:25PM +0200, Alexander Bluhm wrote:
> > On Fri, Jul 15, 2022 at 02:05:40AM +0200, Alexander Bluhm wrote:
> > > If anyone has some of these old network drivers, please test setting
> > > media type with ifconfig.
> >
> > Updated diff. I have compile tested it on amd64, i386, octeon,
> > macppc, sparc64. Setting media for gem(4) and bge(4) works.
>
> Also compiles and boots on powerpc64 and arm64.
>
> How do we proceed here? The diff touches 70 drivers, that cannot
> be tested due to missing hardware. But I need the MP safe interface
> in ifmedia. Should I commit the API and the few drivers for the
> hardware I have? And then slowly commit the remaining drivers.
> Or put everything in and see what happens?
>
> ok?
Maybe this approach is too invasive?
A lot of the changes are in attach functions, where concurrencey is
*not* a problem.
> > Index: arch/octeon/dev/cn30xxgmx.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/arch/octeon/dev/cn30xxgmx.c,v
> > retrieving revision 1.52
> > diff -u -p -r1.52 cn30xxgmx.c
> > --- arch/octeon/dev/cn30xxgmx.c 6 Apr 2022 18:59:26 - 1.52
> > +++ arch/octeon/dev/cn30xxgmx.c 14 Jul 2022 22:46:20 -
> > @@ -725,19 +725,21 @@ cn30xxgmx_reset_timing(struct cn30xxgmx_
> > int
> > cn30xxgmx_reset_flowctl(struct cn30xxgmx_port_softc *sc)
> > {
> > - struct ifmedia_entry *ife = sc->sc_port_mii->mii_media.ifm_cur;
> > + uint64_t media;
> > +
> > + ifmedia_current(>sc_port_mii->mii_media, , NULL);
> >
> > /*
> > * Get flow control negotiation result.
> > */
> > #ifdef GMX_802_3X_DISABLE_AUTONEG
> > /* Tentative support for SEIL-compat.. */
> > - if (IFM_SUBTYPE(ife->ifm_media) == IFM_AUTO) {
> > + if (IFM_SUBTYPE(media) == IFM_AUTO) {
> > sc->sc_port_flowflags &= ~IFM_ETH_FMASK;
> > }
> > #else
> > /* Default configuration of NetBSD */
> > - if (IFM_SUBTYPE(ife->ifm_media) == IFM_AUTO &&
> > + if (IFM_SUBTYPE(media) == IFM_AUTO &&
> > (sc->sc_port_mii->mii_media_active & IFM_ETH_FMASK) !=
> > sc->sc_port_flowflags) {
> > sc->sc_port_flowflags =
> > Index: dev/ic/an.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/ic/an.c,v
> > retrieving revision 1.79
> > diff -u -p -r1.79 an.c
> > --- dev/ic/an.c 21 Apr 2022 21:03:02 - 1.79
> > +++ dev/ic/an.c 14 Jul 2022 22:30:51 -
> > @@ -1277,17 +1277,18 @@ an_media_change(struct ifnet *ifp)
> > {
> > struct an_softc *sc = ifp->if_softc;
> > struct ieee80211com *ic = >sc_ic;
> > - struct ifmedia_entry *ime;
> > + uint64_t media;
> > enum ieee80211_opmode newmode;
> > int i, rate, error = 0;
> >
> > - ime = ic->ic_media.ifm_cur;
> > - if (IFM_SUBTYPE(ime->ifm_media) == IFM_AUTO) {
> > + ifmedia_current(>ic_media, , NULL);
> > +
> > + if (IFM_SUBTYPE(media) == IFM_AUTO) {
> > i = -1;
> > } else {
> > struct ieee80211_rateset *rs =
> > >ic_sup_rates[IEEE80211_MODE_11B];
> > - rate = ieee80211_media2rate(ime->ifm_media);
> > + rate = ieee80211_media2rate(media);
> > if (rate == 0)
> > return EINVAL;
> > for (i = 0; i < rs->rs_nrates; i++) {
> > @@ -1303,13 +1304,13 @@ an_media_change(struct ifnet *ifp)
> > }
> >
> > #ifndef IEEE80211_STA_ONLY
> > - if (ime->ifm_media & IFM_IEEE80211_ADHOC)
> > + if (media & IFM_IEEE80211_ADHOC)
> > newmode = IEEE80211_M_IBSS;
> > - else if (ime->ifm_media & IFM_IEEE80211_HOSTAP)
> > + else if (media & IFM_IEEE80211_HOSTAP)
> > newmode = IEEE80211_M_HOSTAP;
> > else
> > #endif
> > - if (ime->ifm_media & IFM_IEEE80211_MONITOR)
> > + if (media & IFM_IEEE80211_MONITOR)
> > newmode = IEEE80211_M_MONITOR;
> > else
> > newmode = IEEE80211_M_STA;
> > @@ -1323,7 +1324,8 @@ an_media_change(struct ifnet *ifp)
> > else
> > error = 0;
> > }
> > - ifp->if_baudrate = ifmedia_baudrate(ic->ic_media.ifm_cur->ifm_media);
> > + ifmedia_current(>ic_media, , NULL);
> > + ifp->if_baudrate = ifmedia_baudrate(media);
> >
> > return error;
> > }
> > Index: dev/ic/elink3.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/ic/elink3.c,v
> > retrieving revision 1.98
> > diff -u -p -r1.98 elink3.c
> > --- dev/ic/elink3.c 12 Dec 2020 11:48:52 - 1.98
> > +++ dev/ic/elink3.c 14 Jul 2022 22:30:51 -
> > @@ -571,6 +571,7 @@ epinit(struct ep_softc *sc)
> > register struct ifnet *ifp = >sc_arpcom.ac_if;
> > bus_space_tag_t iot = sc->sc_iot;
> > bus_space_handle_t ioh = sc->sc_ioh;
> > + u_int data;
> >
Re: pppac(4): don't grab netlock within pppacioctl()
On Mon, Jul 18, 2022 at 01:50:37PM +0300, Vitaliy Makkoveev wrote:
> pipex(4) doesn't rely on netlock anymore.
OK bluhm@
> Index: sys/net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.119
> diff -u -p -r1.119 if_pppx.c
> --- sys/net/if_pppx.c 15 Jul 2022 22:56:13 - 1.119
> +++ sys/net/if_pppx.c 18 Jul 2022 10:48:31 -
> @@ -1161,7 +1161,6 @@ pppacioctl(dev_t dev, u_long cmd, caddr_
> struct pppac_softc *sc = pppac_lookup(dev);
> int error = 0;
>
> - NET_LOCK();
> switch (cmd) {
> case FIONBIO:
> break;
> @@ -1180,7 +1179,6 @@ pppacioctl(dev_t dev, u_long cmd, caddr_
> error = pipex_ioctl(sc, cmd, data);
> break;
> }
> - NET_UNLOCK();
>
> return (error);
> }
snmpd(8): don't traverse back in tree
When we have 2 overlapping regions within the same backend the current
code takes the OID of the parent region after the child region returned
an EOMV. This is of course wrong and creates an infinite loop.
OK?
martijn@
Index: application.c
===
RCS file: /cvs/src/usr.sbin/snmpd/application.c,v
retrieving revision 1.6
diff -u -p -r1.6 application.c
--- application.c 30 Jun 2022 11:28:36 - 1.6
+++ application.c 22 Jul 2022 15:07:53 -
@@ -1260,6 +1260,7 @@ appl_varbind_backend(struct appl_varbind
struct appl_request_upstream *ureq = ivb->avi_request_upstream;
struct appl_region search, *region, *pregion;
struct appl_varbind *vb = &(ivb->avi_varbind);
+ struct ber_oid oid;
int next, cmp;
next = ureq->aru_pdu->be_type == SNMP_C_GETNEXTREQ ||
@@ -1310,10 +1311,15 @@ appl_varbind_backend(struct appl_varbind
}
ivb->avi_region = region;
if (next) {
+ oid = region->ar_oid;
do {
pregion = region;
- region = appl_region_next(ureq->aru_ctx,
- &(region->ar_oid), pregion);
+ region = appl_region_next(ureq->aru_ctx, , pregion);
+ if (region != NULL &&
+ appl_region_cmp(region, pregion) > 0)
+ oid = region->ar_oid;
+ else
+ ober_oid_nextsibling();
} while (region != NULL &&
region->ar_backend == pregion->ar_backend);
if (region == NULL) {
Re: pipex(4): kill "Static" keyword
On Mon, Jul 18, 2022 at 12:31:47PM +0300, Vitaliy Makkoveev wrote:
> We don't use "static" keyword for functions declaration to allow ddb(4)
> debug. Also, many "Static" functions are called by pppx(4) layer outside
> pipex(4) layer.
>
> This is the mostly mechanic diff, except the `pipex_pppoe_padding' which
> should be "static const".
OK bluhm@
> Index: sys/net/pipex.c
> ===
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.146
> diff -u -p -r1.146 pipex.c
> --- sys/net/pipex.c 15 Jul 2022 22:56:13 - 1.146
> +++ sys/net/pipex.c 18 Jul 2022 09:30:49 -
> @@ -74,9 +74,6 @@
> #include
> #include
>
> -/* drop static for ddb debuggability */
> -#define Static
> -
> #include
> #include "pipex_local.h"
>
> @@ -559,7 +556,7 @@ pipex_export_session_stats(struct pipex_
> stats->idle_time = session->idle_time;
> }
>
> -Static int
> +int
> pipex_get_stat(struct pipex_session_stat_req *req, void *ownersc)
> {
> struct pipex_session *session;
> @@ -580,7 +577,7 @@ pipex_get_stat(struct pipex_session_stat
> return error;
> }
>
> -Static int
> +int
> pipex_get_closed(struct pipex_session_list_req *req, void *ownersc)
> {
> struct pipex_session *session, *session_tmp;
> @@ -608,7 +605,7 @@ pipex_get_closed(struct pipex_session_li
> return (0);
> }
>
> -Static struct pipex_session *
> +struct pipex_session *
> pipex_lookup_by_ip_address_locked(struct in_addr addr)
> {
> struct pipex_session *session;
> @@ -660,7 +657,7 @@ pipex_lookup_by_ip_address(struct in_add
> }
>
>
> -Static struct pipex_session *
> +struct pipex_session *
> pipex_lookup_by_session_id_locked(int protocol, int session_id)
> {
> struct pipex_hash_head *list;
> @@ -704,20 +701,20 @@ pipex_lookup_by_session_id(int protocol,
> /***
> * Timer functions
> ***/
> -Static void
> +void
> pipex_timer_start(void)
> {
> timeout_set_proc(_timer_ch, pipex_timer, NULL);
> timeout_add_sec(_timer_ch, pipex_prune);
> }
>
> -Static void
> +void
> pipex_timer_stop(void)
> {
> timeout_del(_timer_ch);
> }
>
> -Static void
> +void
> pipex_timer(void *ignored_arg)
> {
> struct pipex_session *session, *session_tmp;
> @@ -764,7 +761,7 @@ pipex_timer(void *ignored_arg)
> /***
> * Common network I/O functions. (tunnel protocol independent)
> ***/
> -Static void
> +void
> pipex_ip_output(struct mbuf *m0, struct pipex_session *session)
> {
> int is_idle;
> @@ -840,7 +837,7 @@ dropped:
> counters_inc(session->stat_counters, pxc_oerrors);
> }
>
> -Static void
> +void
> pipex_ppp_output(struct mbuf *m0, struct pipex_session *session, int proto)
> {
> u_char *cp, hdr[16];
> @@ -897,7 +894,7 @@ drop:
> counters_inc(session->stat_counters, pxc_oerrors);
> }
>
> -Static void
> +void
> pipex_ppp_input(struct mbuf *m0, struct pipex_session *session, int
> decrypted)
> {
> int proto, hlen = 0;
> @@ -990,7 +987,7 @@ drop:
> counters_inc(session->stat_counters, pxc_ierrors);
> }
>
> -Static void
> +void
> pipex_ip_input(struct mbuf *m0, struct pipex_session *session)
> {
> struct ifnet *ifp;
> @@ -1067,7 +1064,7 @@ drop:
> }
>
> #ifdef INET6
> -Static void
> +void
> pipex_ip6_input(struct mbuf *m0, struct pipex_session *session)
> {
> struct ifnet *ifp;
> @@ -1115,7 +1112,7 @@ drop:
> }
> #endif
>
> -Static struct mbuf *
> +struct mbuf *
> pipex_common_input(struct pipex_session *session, struct mbuf *m0, int hlen,
> int plen, int locked)
> {
> @@ -1187,7 +1184,7 @@ not_ours:
> /*
> * pipex_ppp_proto
> */
> -Static int
> +int
> pipex_ppp_proto(struct mbuf *m0, struct pipex_session *session, int off,
> int *hlenp)
> {
> @@ -1228,7 +1225,7 @@ pipex_ppp_proto(struct mbuf *m0, struct
> /***
> * PPPoE
> ***/
> -Static u_charpipex_pppoe_padding[ETHERMIN];
> +static const u_char pipex_pppoe_padding[ETHERMIN];
> /*
> * pipex_pppoe_lookup_session
> */
> @@ -1286,7 +1283,7 @@ pipex_pppoe_input(struct mbuf *m0, struc
> /*
> * pipex_ppope_output
> */
> -Static void
> +void
> pipex_pppoe_output(struct mbuf *m0, struct pipex_session *session)
> {
> struct pipex_pppoe_header *pppoe;
> @@ -1332,7 +1329,7 @@ pipex_pppoe_output(struct mbuf *m0, stru
> /***
> * PPTP
> ***/
> -Static void
> +void
> pipex_pptp_output(struct mbuf
Re: interface media current data
On Fri, Jul 15, 2022 at 04:51:25PM +0200, Alexander Bluhm wrote:
> On Fri, Jul 15, 2022 at 02:05:40AM +0200, Alexander Bluhm wrote:
> > If anyone has some of these old network drivers, please test setting
> > media type with ifconfig.
>
> Updated diff. I have compile tested it on amd64, i386, octeon,
> macppc, sparc64. Setting media for gem(4) and bge(4) works.
Also compiles and boots on powerpc64 and arm64.
How do we proceed here? The diff touches 70 drivers, that cannot
be tested due to missing hardware. But I need the MP safe interface
in ifmedia. Should I commit the API and the few drivers for the
hardware I have? And then slowly commit the remaining drivers.
Or put everything in and see what happens?
ok?
bluhm
> Index: arch/octeon/dev/cn30xxgmx.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/octeon/dev/cn30xxgmx.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 cn30xxgmx.c
> --- arch/octeon/dev/cn30xxgmx.c 6 Apr 2022 18:59:26 - 1.52
> +++ arch/octeon/dev/cn30xxgmx.c 14 Jul 2022 22:46:20 -
> @@ -725,19 +725,21 @@ cn30xxgmx_reset_timing(struct cn30xxgmx_
> int
> cn30xxgmx_reset_flowctl(struct cn30xxgmx_port_softc *sc)
> {
> - struct ifmedia_entry *ife = sc->sc_port_mii->mii_media.ifm_cur;
> + uint64_t media;
> +
> + ifmedia_current(>sc_port_mii->mii_media, , NULL);
>
> /*
>* Get flow control negotiation result.
>*/
> #ifdef GMX_802_3X_DISABLE_AUTONEG
> /* Tentative support for SEIL-compat.. */
> - if (IFM_SUBTYPE(ife->ifm_media) == IFM_AUTO) {
> + if (IFM_SUBTYPE(media) == IFM_AUTO) {
> sc->sc_port_flowflags &= ~IFM_ETH_FMASK;
> }
> #else
> /* Default configuration of NetBSD */
> - if (IFM_SUBTYPE(ife->ifm_media) == IFM_AUTO &&
> + if (IFM_SUBTYPE(media) == IFM_AUTO &&
> (sc->sc_port_mii->mii_media_active & IFM_ETH_FMASK) !=
> sc->sc_port_flowflags) {
> sc->sc_port_flowflags =
> Index: dev/ic/an.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/ic/an.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 an.c
> --- dev/ic/an.c 21 Apr 2022 21:03:02 - 1.79
> +++ dev/ic/an.c 14 Jul 2022 22:30:51 -
> @@ -1277,17 +1277,18 @@ an_media_change(struct ifnet *ifp)
> {
> struct an_softc *sc = ifp->if_softc;
> struct ieee80211com *ic = >sc_ic;
> - struct ifmedia_entry *ime;
> + uint64_t media;
> enum ieee80211_opmode newmode;
> int i, rate, error = 0;
>
> - ime = ic->ic_media.ifm_cur;
> - if (IFM_SUBTYPE(ime->ifm_media) == IFM_AUTO) {
> + ifmedia_current(>ic_media, , NULL);
> +
> + if (IFM_SUBTYPE(media) == IFM_AUTO) {
> i = -1;
> } else {
> struct ieee80211_rateset *rs =
> >ic_sup_rates[IEEE80211_MODE_11B];
> - rate = ieee80211_media2rate(ime->ifm_media);
> + rate = ieee80211_media2rate(media);
> if (rate == 0)
> return EINVAL;
> for (i = 0; i < rs->rs_nrates; i++) {
> @@ -1303,13 +1304,13 @@ an_media_change(struct ifnet *ifp)
> }
>
> #ifndef IEEE80211_STA_ONLY
> - if (ime->ifm_media & IFM_IEEE80211_ADHOC)
> + if (media & IFM_IEEE80211_ADHOC)
> newmode = IEEE80211_M_IBSS;
> - else if (ime->ifm_media & IFM_IEEE80211_HOSTAP)
> + else if (media & IFM_IEEE80211_HOSTAP)
> newmode = IEEE80211_M_HOSTAP;
> else
> #endif
> - if (ime->ifm_media & IFM_IEEE80211_MONITOR)
> + if (media & IFM_IEEE80211_MONITOR)
> newmode = IEEE80211_M_MONITOR;
> else
> newmode = IEEE80211_M_STA;
> @@ -1323,7 +1324,8 @@ an_media_change(struct ifnet *ifp)
> else
> error = 0;
> }
> - ifp->if_baudrate = ifmedia_baudrate(ic->ic_media.ifm_cur->ifm_media);
> + ifmedia_current(>ic_media, , NULL);
> + ifp->if_baudrate = ifmedia_baudrate(media);
>
> return error;
> }
> Index: dev/ic/elink3.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/ic/elink3.c,v
> retrieving revision 1.98
> diff -u -p -r1.98 elink3.c
> --- dev/ic/elink3.c 12 Dec 2020 11:48:52 - 1.98
> +++ dev/ic/elink3.c 14 Jul 2022 22:30:51 -
> @@ -571,6 +571,7 @@ epinit(struct ep_softc *sc)
> register struct ifnet *ifp = >sc_arpcom.ac_if;
> bus_space_tag_t iot = sc->sc_iot;
> bus_space_handle_t ioh = sc->sc_ioh;
> + u_int data;
> int i;
>
> /* make sure any pending reset has completed before touching board */
> @@ -650,7 +651,8 @@ epinit(struct ep_softc *sc)
> bus_space_write_2(iot, ioh, EP_COMMAND, ACK_INTR | 0xff);
>
> epsetfilter(sc);
> - epsetmedia(sc,
snmpd(8): restart requests where backend disappeared.
appl_request_downstream_free gets called from 3 locations:
- appl_request_upstream_free: called from appl_request_upstream_reply
and cleans up after a request has been answered, whether all the
downstream requests have been completed or not (when timed out)
- appl_response: When a downstream reqeuest has been completed
- appl_close: When a downstream backend has been closed.
appl_request_upstream_free is already done with the upstream request
and appl_response already automatically starts the next iteration of
missing varbinds.
The problem arrises when appl_close is called (e.g. a backend
disappears unexpected) while there are still pending downstream
requests. In that case all references to the upstream request are
lost and we have a memory leak.
Diff below resets the pending varbinds to new and restarts the
parsing procedure.
OK?
martijn@
Index: application.c
===
RCS file: /cvs/src/usr.sbin/snmpd/application.c,v
retrieving revision 1.6
diff -u -p -r1.6 application.c
--- application.c 30 Jun 2022 11:28:36 - 1.6
+++ application.c 22 Jul 2022 14:59:00 -
@@ -716,6 +716,7 @@ void
appl_request_downstream_free(struct appl_request_downstream *dreq)
{
struct appl_varbind_internal *vb;
+ int retry = 0;
if (dreq == NULL)
return;
@@ -723,9 +724,16 @@ appl_request_downstream_free(struct appl
RB_REMOVE(appl_requests, &(dreq->ard_backend->ab_requests), dreq);
evtimer_del(&(dreq->ard_timer));
- for (vb = dreq->ard_vblist; vb != NULL; vb = vb->avi_next)
+ for (vb = dreq->ard_vblist; vb != NULL; vb = vb->avi_next) {
vb->avi_request_downstream = NULL;
+ if (vb->avi_state == APPL_VBSTATE_PENDING) {
+ vb->avi_state = APPL_VBSTATE_NEW;
+ retry = 1;
+ }
+ }
+ if (retry)
+ appl_request_upstream_resolve(dreq->ard_request);
free(dreq);
}
Re: Zap unused ND6_IS_LLINFO_PROBREACH and MAX_REACHABLE_TIME
On Fri, Jul 22, 2022 at 12:55:42PM +, Klemens Nanni wrote:
> Leftovers from florian's RS/NA purge from the kernel in 2017.
>
> OK?
My grep tells me that can be deleted.
OK bluhm@
> diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
> index 82e674c5ecf..01ec60d02b9 100644
> --- a/sys/netinet6/nd6.h
> +++ b/sys/netinet6/nd6.h
> @@ -109,11 +109,9 @@ struct llinfo_nd6 {
> short ln_router; /* 2^0: ND6 router bit */
> };
>
> -#define ND6_IS_LLINFO_PROBREACH(n) ((n)->ln_state > ND6_LLINFO_INCOMPLETE)
> #define ND6_LLINFO_PERMANENT(n) ((n)->ln_rt->rt_expire == 0)
>
> /* node constants */
> -#define MAX_REACHABLE_TIME 360 /* msec */
> #define REACHABLE_TIME 3 /* msec */
> #define RETRANS_TIMER1000/* msec */
> #define MIN_RANDOM_FACTOR512 /* 1024 * 0.5 */
Re: ypldap without ypbind and portmap
> One concern I had while working on this was that changing the point at which > YP is enabled might affect the boot process. ypbind is started right after > ypldap, and ypldap becomes discoverable to ypbind before it daemonises, > so this doesn't change the ordering. Yes, it is a bit weird. There is still a way to make this more autoatic. ypldap could continue to create the ypbind RPC contact point, but if traffic happens it means "oh shit, I must abandon managing the binding file myself, ypbind will be doing it". But maybe it is better if ypldap has a special mode, which means it has no ypbind connection point which can act as a potential weakness. You don't want to delete the binding file upon exit? pledge/unveil reasons? Losing the lock is enough for libc to give up. If you run in this mode, how does ypbind react? It spins searching for a ypserv/ypldap which does not respond? Also we need this in /etc/rc. The domainname must be set for libc to talk to ypldap, so we should only start it in that circumstance. I'd rather fail to start it here, to make people think. Index: rc === RCS file: /cvs/src/etc/rc,v retrieving revision 1.561 diff -u -p -u -r1.561 rc --- rc 17 Jul 2022 03:17:37 - 1.561 +++ rc 22 Jul 2022 14:12:51 - @@ -498,9 +498,9 @@ if [[ $ipsec != NO && -f /etc/ipsec.conf fi echo -n 'starting RPC daemons:' -start_daemon portmap ypldap +start_daemon portmap if [[ -n $(domainname) ]]; then - start_daemon ypserv ypbind + start_daemon ypldap ypserv ypbind fi start_daemon mountd nfsd lockd statd amd echo '.'
Re: timeout.9: dont' repeat TIMEOUT_PROC sentence
On Fri, Jul 22, 2022 at 03:03:40PM +0200, Alexander Bluhm wrote: > On Fri, Jul 22, 2022 at 10:38:32AM +, Klemens Nanni wrote: > > timeout_set_flags() explains the flag and the next paragraph would simply > > repeat it. > > > > timeout_set_proc() can be described simpler, just like what the > > actual function does; this way I don't feel having just read the > > same sentence twice. > > The current text describes the intension of the function. You > change it to an implementation detail. The timeout_set_proc() is > a commonly used API, timeout_set_flags() is barely used. True. Anyways, Scott's timeout.9 revamp probably covers this. > Instead of adding a redirection in the man page to an unused function, > I would make timeout_set_flags() private to kern_timeout.c. Dthen > it needs no documentation. That's also possible. Either timeout_set_flags() is overkill and there won't be any flags soon or its work-in-progress and should stay as-is. I'd leave this to Scott, then.
Zap outdated nd6_free() comment about static
Added in 2002 r1.48 "sync with latest KAME [...]" along the attribute, but nd6_free() became a global void function in 2017 r1.212. Afaik static kernel functions are avoided to aid ddb'ugging and I presume the "significant changes in the kernel" bits of the comment stem from something 20 years ago no longer holding true today. Afterall, this change has been safe for five years. diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c index a596414a175..2e85d35c6f1 100644 --- a/sys/netinet6/nd6.c +++ b/sys/netinet6/nd6.c @@ -699,9 +699,6 @@ nd6_invalidate(struct rtentry *rt) /* * Free an nd6 llinfo entry. - * Since the function would cause significant changes in the kernel, DO NOT - * make it global, unless you have a strong reason for the change, and are sure - * that the change is safe. */ void nd6_free(struct rtentry *rt)
snmpd(8): honour searchrange end
This is the snmpd(8) part of searchrange end issue mentioned in my
libagentx diff from yesterday.[0]
Since searchranges are an agentx specific thing I implemented two
cases:
1) Backends that support searchranges set ab_range to 1
(application_agentx.c) and check the av_oid_end in
appl_varbind_valid. If it fails here we generate a GENERR
and disconnect the backend.
2) Backends that don't support searchranges set ab_range to 0 and
are not checked for overflow in appl_varbind_valid, but instead
treat the response as ENDOFMIBVIEW and continue searching at
av_oid_end.
Since this is already the 3rd EOMV case added I decided to move to
a simple variable and do the reset-logic in a single place.
While here, also fix an indent in appl_close.
OK?
martijn@
[0] https://marc.info/?l=openbsd-tech=165843466721371=2
Index: application.c
===
RCS file: /cvs/src/usr.sbin/snmpd/application.c,v
retrieving revision 1.6
diff -u -p -r1.6 application.c
--- application.c 30 Jun 2022 11:28:36 - 1.6
+++ application.c 22 Jul 2022 13:13:43 -
@@ -126,7 +126,7 @@ void appl_request_downstream_send(struct
void appl_request_downstream_timeout(int, short, void *);
void appl_request_upstream_reply(struct appl_request_upstream *);
int appl_varbind_valid(struct appl_varbind *, struct appl_varbind *, int, int,
-const char **);
+int, const char **);
int appl_varbind_backend(struct appl_varbind_internal *);
void appl_varbind_error(struct appl_varbind_internal *, enum appl_error);
void appl_report(struct snmp_message *, int32_t, struct ber_oid *,
@@ -541,7 +541,7 @@ appl_close(struct appl_backend *backend)
RB_FOREACH_SAFE(request, appl_requests,
&(backend->ab_requests), trequest)
- appl_request_downstream_free(request);
+ appl_request_downstream_free(request);
}
struct appl_region *
@@ -1025,14 +1025,13 @@ appl_response(struct appl_backend *backe
{
struct appl_request_downstream *dreq, search;
struct appl_request_upstream *ureq = NULL;
- struct appl_region *nregion;
const char *errstr;
char oidbuf[1024];
enum snmp_pdutype pdutype;
struct appl_varbind *vb;
struct appl_varbind_internal *origvb = NULL;
int invalid = 0;
- int next = 0;
+ int next = 0, eomv;
int32_t i;
appl_pdu_log(backend, SNMP_C_RESPONSE, requestid, error, index, vblist);
@@ -1055,7 +1054,7 @@ appl_response(struct appl_backend *backe
for (i = 1; vb != NULL; vb = vb->av_next, i++) {
if (!appl_varbind_valid(vb, origvb == NULL ?
NULL : &(origvb->avi_varbind), next,
-error != APPL_ERROR_NOERROR, )) {
+error != APPL_ERROR_NOERROR, backend->ab_range, )) {
smi_oid2string(&(vb->av_oid), oidbuf,
sizeof(oidbuf), 0);
log_warnx("%s: %"PRIu32" %s: %s",
@@ -1068,35 +1067,36 @@ appl_response(struct appl_backend *backe
appl_varbind_error(origvb, error);
origvb->avi_state = APPL_VBSTATE_DONE;
origvb->avi_varbind.av_oid = vb->av_oid;
- if (vb->av_value != NULL &&
+
+ eomv = vb->av_value != NULL &&
vb->av_value->be_class == BER_CLASS_CONTEXT &&
- vb->av_value->be_type == APPL_EXC_ENDOFMIBVIEW) {
- nregion = appl_region_next(ureq->aru_ctx,
- &(vb->av_oid), origvb->avi_region);
- if (nregion != NULL) {
- ober_free_elements(vb->av_value);
- origvb->avi_varbind.av_oid =
- nregion->ar_oid;
- origvb->avi_varbind.av_include = 1;
- vb->av_value = NULL;
- origvb->avi_state = APPL_VBSTATE_NEW;
- }
- }
+ vb->av_value->be_type == APPL_EXC_ENDOFMIBVIEW;
+ /*
+* Treat results past av_oid_end for backends that
+* don't support searchranges as EOMV
+*/
+ eomv |= !backend->ab_range && next &&
+ ober_oid_cmp(&(vb->av_oid),
+ &(origvb->avi_varbind.av_oid_end)) > 0;
/* RFC 3584 section 4.2.2.1 */
if (ureq->aru_pduversion == SNMP_V1 &&
vb->av_value != NULL &&
vb->av_value->be_class == BER_CLASS_APPLICATION &&
vb->av_value->be_type
Re: nd6: zap dead store nd6_allocated
On Fri, Jul 22, 2022 at 12:17:11PM +, Klemens Nanni wrote:
> There since KAME IPv6 import in 1999.
>
> OK?
Pool statistics has this info already.
OK bluhm@
> diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
> index df6cf601b34..ff679bcb151 100644
> --- a/sys/netinet6/nd6.c
> +++ b/sys/netinet6/nd6.c
> @@ -86,7 +86,7 @@ int nd6_debug = 0;
>
> TAILQ_HEAD(llinfo_nd6_head, llinfo_nd6) nd6_list;
> struct pool nd6_pool; /* pool for llinfo_nd6 structures */
> -int nd6_inuse, nd6_allocated;
> +int nd6_inuse;
>
> int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
>
> @@ -885,7 +885,6 @@ nd6_rtrequest(struct ifnet *ifp, int req, struct rtentry
> *rt)
> break;
> }
> nd6_inuse++;
> - nd6_allocated++;
> ln->ln_rt = rt;
> /* this is required for "ndp" command. - shin */
> if (req == RTM_ADD) {
Re: nd6: call nd6_timer() without argument
On Fri, Jul 22, 2022 at 11:18:14AM +, Klemens Nanni wrote:
> nd6_timer_to is a global struct and nd6_timer() accesses it as such,
> thereby ignoring its function argument.
>
> Make that clear when setting the timeout, which now goes like the other
> two timeouts.
>
> OK?
OK bluhm@
> Index: nd6.c
> ===
> RCS file: /cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.238
> diff -u -p -r1.238 nd6.c
> --- nd6.c 22 Feb 2022 01:15:02 - 1.238
> +++ nd6.c 22 Jul 2022 11:14:25 -
> @@ -122,7 +122,7 @@ nd6_init(void)
> nd6_init_done = 1;
>
> /* start timer */
> - timeout_set_proc(_timer_to, nd6_timer, _timer_to);
> + timeout_set_proc(_timer_to, nd6_timer, NULL);
> timeout_set_proc(_slowtimo_ch, nd6_slowtimo, NULL);
> timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
> timeout_set(_expire_timeout, nd6_expire_timer, NULL);
> @@ -318,7 +318,7 @@ nd6_llinfo_settimer(struct llinfo_nd6 *l
> }
>
> void
> -nd6_timer(void *arg)
> +nd6_timer(void *unused)
> {
> struct llinfo_nd6 *ln, *nln;
> time_t expire = getuptime() + nd6_gctimer;
Re: timeout.9: dont' repeat TIMEOUT_PROC sentence
On Fri, Jul 22, 2022 at 10:38:32AM +, Klemens Nanni wrote:
> timeout_set_flags() explains the flag and the next paragraph would simply
> repeat it.
>
> timeout_set_proc() can be described simpler, just like what the
> actual function does; this way I don't feel having just read the
> same sentence twice.
The current text describes the intension of the function. You
change it to an implementation detail. The timeout_set_proc() is
a commonly used API, timeout_set_flags() is barely used.
Instead of adding a redirection in the man page to an unused function,
I would make timeout_set_flags() private to kern_timeout.c. Dthen
it needs no documentation.
> sys/kern/kern_timeout.c:
> void
> timeout_set_flags(struct timeout *to, void (*fn)(void *), void *arg, int
> flags)
> {
> _timeout_set(to, fn, arg, KCLOCK_NONE, flags);
> }
>
> void
> timeout_set_proc(struct timeout *new, void (*fn)(void *), void *arg)
> {
> _timeout_set(new, fn, arg, KCLOCK_NONE, TIMEOUT_PROC);
> }
>
> Index: timeout.9
> ===
> RCS file: /cvs/src/share/man/man9/timeout.9,v
> retrieving revision 1.55
> diff -u -p -r1.55 timeout.9
> --- timeout.9 22 Jun 2022 14:10:49 - 1.55
> +++ timeout.9 22 Jul 2022 10:34:22 -
> @@ -138,11 +138,12 @@ interrupt context.
> .Pp
> The
> .Fn timeout_set_proc
> -function is similar to
> -.Fn timeout_set
> -but it runs the timeout in a process context instead of the default
> -.Dv IPL_SOFTCLOCK
> -interrupt context.
> +function is a shorthand for
> +.Fn timeout_set_flags
> +with a
> +.Fa flags
> +value of
> +.Dv TIMEOUT_PROC .
> .Pp
> The function
> .Fn timeout_add
Zap unused ND6_IS_LLINFO_PROBREACH and MAX_REACHABLE_TIME
Leftovers from florian's RS/NA purge from the kernel in 2017.
OK?
diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
index 82e674c5ecf..01ec60d02b9 100644
--- a/sys/netinet6/nd6.h
+++ b/sys/netinet6/nd6.h
@@ -109,11 +109,9 @@ struct llinfo_nd6 {
short ln_router; /* 2^0: ND6 router bit */
};
-#define ND6_IS_LLINFO_PROBREACH(n) ((n)->ln_state > ND6_LLINFO_INCOMPLETE)
#define ND6_LLINFO_PERMANENT(n)((n)->ln_rt->rt_expire == 0)
/* node constants */
-#define MAX_REACHABLE_TIME 360 /* msec */
#define REACHABLE_TIME 3 /* msec */
#define RETRANS_TIMER 1000/* msec */
#define MIN_RANDOM_FACTOR 512 /* 1024 * 0.5 */
Re: nd6: Zap nd6_recalc_reachtm_interval indirection
On Fri, Jul 22, 2022 at 02:27:38PM +0200, Claudio Jeker wrote: > On Fri, Jul 22, 2022 at 12:18:34PM +, Klemens Nanni wrote: > > Only used once, so use the macro directly like ND6_SLOWTIMER_INTERVAL > > is used in many places. > > > > OK? > > Is that a value that should be adjustable? We can think about a sysctl, similar to nd6.c:nd6_maxnudhint aka. net.inet6.icmp6.nd6_maxnudhint. At the moment, this looks like dead weight in ND6 and less useless code helps making progress.
Re: nd6: Zap nd6_recalc_reachtm_interval indirection
On Fri, Jul 22, 2022 at 12:18:34PM +, Klemens Nanni wrote: > Only used once, so use the macro directly like ND6_SLOWTIMER_INTERVAL > is used in many places. > > OK? Is that a value that should be adjustable? > diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c > index ff679bcb151..3decec947c4 100644 > --- a/sys/netinet6/nd6.c > +++ b/sys/netinet6/nd6.c > @@ -88,8 +88,6 @@ TAILQ_HEAD(llinfo_nd6_head, llinfo_nd6) nd6_list; > struct pool nd6_pool; /* pool for llinfo_nd6 structures */ > int nd6_inuse; > > -int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL; > - > void nd6_timer(void *); > void nd6_slowtimo(void *); > void nd6_expire(void *); > @@ -1318,7 +1316,7 @@ nd6_slowtimo(void *ignored_arg) >* value gets recomputed at least once every few hours. >* (RFC 2461, 6.3.4) >*/ > - nd6if->recalctm = nd6_recalc_reachtm_interval; > + nd6if->recalctm = ND6_RECALC_REACHTM_INTERVAL; > nd6if->reachable = > ND_COMPUTE_RTIME(nd6if->basereachable); > } > } > -- :wq Claudio
nd6: Zap nd6_recalc_reachtm_interval indirection
Only used once, so use the macro directly like ND6_SLOWTIMER_INTERVAL is used in many places. OK? diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c index ff679bcb151..3decec947c4 100644 --- a/sys/netinet6/nd6.c +++ b/sys/netinet6/nd6.c @@ -88,8 +88,6 @@ TAILQ_HEAD(llinfo_nd6_head, llinfo_nd6) nd6_list; struct pool nd6_pool; /* pool for llinfo_nd6 structures */ intnd6_inuse; -int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL; - void nd6_timer(void *); void nd6_slowtimo(void *); void nd6_expire(void *); @@ -1318,7 +1316,7 @@ nd6_slowtimo(void *ignored_arg) * value gets recomputed at least once every few hours. * (RFC 2461, 6.3.4) */ - nd6if->recalctm = nd6_recalc_reachtm_interval; + nd6if->recalctm = ND6_RECALC_REACHTM_INTERVAL; nd6if->reachable = ND_COMPUTE_RTIME(nd6if->basereachable); } }
nd6: zap dead store nd6_allocated
There since KAME IPv6 import in 1999.
OK?
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index df6cf601b34..ff679bcb151 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -86,7 +86,7 @@ int nd6_debug = 0;
TAILQ_HEAD(llinfo_nd6_head, llinfo_nd6) nd6_list;
struct pool nd6_pool; /* pool for llinfo_nd6 structures */
-intnd6_inuse, nd6_allocated;
+intnd6_inuse;
int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
@@ -885,7 +885,6 @@ nd6_rtrequest(struct ifnet *ifp, int req, struct rtentry
*rt)
break;
}
nd6_inuse++;
- nd6_allocated++;
ln->ln_rt = rt;
/* this is required for "ndp" command. - shin */
if (req == RTM_ADD) {
Re: timeout.9: fix description
On Fri, Jul 22, 2022 at 06:16:35AM -0500, Scott Cheloha wrote: > I rewrote this page a year or so ago but I > think I dropped the patch due to lack of > developer input. If you give me 12 hours I > will send it out for your consideration. Sure, no hurry, I'll look at it. > I would send it sooner, but I accidentally cut > the fiber-optic cable with a shovel last night, > so we all need to wait until this afternoon for > AT to replace it. Just don't dig your own grave!
nd6: call nd6_timer() without argument
nd6_timer_to is a global struct and nd6_timer() accesses it as such,
thereby ignoring its function argument.
Make that clear when setting the timeout, which now goes like the other
two timeouts.
OK?
Index: nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.238
diff -u -p -r1.238 nd6.c
--- nd6.c 22 Feb 2022 01:15:02 - 1.238
+++ nd6.c 22 Jul 2022 11:14:25 -
@@ -122,7 +122,7 @@ nd6_init(void)
nd6_init_done = 1;
/* start timer */
- timeout_set_proc(_timer_to, nd6_timer, _timer_to);
+ timeout_set_proc(_timer_to, nd6_timer, NULL);
timeout_set_proc(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
timeout_set(_expire_timeout, nd6_expire_timer, NULL);
@@ -318,7 +318,7 @@ nd6_llinfo_settimer(struct llinfo_nd6 *l
}
void
-nd6_timer(void *arg)
+nd6_timer(void *unused)
{
struct llinfo_nd6 *ln, *nln;
time_t expire = getuptime() + nd6_gctimer;
Re: timeout.9: fix description
> On Jul 22, 2022, at 05:50, Klemens Nanni wrote: > > NAME has it right: >... – execute a function after a specified period of time > > but DESCRIPTION says something else: >The timeout API provides a mechanism to execute a function >at a given time. > > The latter reads as if I could pass a specific point in time, e.g. > Fri Jul 22 16:00:00 UTC 2022, at which a function should be run. > > But this API is about timeouts, i.e. a duration of time like 10s, > which does not my understanding of "at a given time". > > > So reuse NAME's wording and make it a proper sentence. > > Feedback? OK? I rewrote this page a year or so ago but I think I dropped the patch due to lack of developer input. If you give me 12 hours I will send it out for your consideration. I would send it sooner, but I accidentally cut the fiber-optic cable with a shovel last night, so we all need to wait until this afternoon for AT to replace it.
Re: bgpd kroute F_KERNEL flag
On Fri, Jul 22, 2022 at 12:36:04PM +0200, Claudio Jeker wrote: > There is no need to use F_KERNEL to tag routes from the kernel. > All this can be done by priority (RTP_MINE vs anything else). > The conversion is simple in most cases. > > In kr_fib_delete() and kr_fib_change() check if the route is a bgpd owned > route and in that case remove the F_BGPD_INSERTED flag. The original route > is no longer in the FIB and so that flag should be cleared. Makes sense and reads fine. ok
timeout.9: fix description
NAME has it right: ... – execute a function after a specified period of time but DESCRIPTION says something else: The timeout API provides a mechanism to execute a function at a given time. The latter reads as if I could pass a specific point in time, e.g. Fri Jul 22 16:00:00 UTC 2022, at which a function should be run. But this API is about timeouts, i.e. a duration of time like 10s, which does not my understanding of "at a given time". So reuse NAME's wording and make it a proper sentence. Feedback? OK? Index: timeout.9 === RCS file: /cvs/src/share/man/man9/timeout.9,v retrieving revision 1.55 diff -u -p -r1.55 timeout.9 --- timeout.9 22 Jun 2022 14:10:49 - 1.55 +++ timeout.9 22 Jul 2022 10:41:54 - @@ -88,7 +88,8 @@ .Sh DESCRIPTION The .Nm timeout -API provides a mechanism to execute a function at a given time. +API provides a mechanism to execute a function after a specified period of +time has elapsed. The granularity of the time is limited by the granularity of the .Xr hardclock 9 timer which executes
timeout.9: dont' repeat TIMEOUT_PROC sentence
timeout_set_flags() explains the flag and the next paragraph would simply
repeat it.
timeout_set_proc() can be described simpler, just like what the
actual function does; this way I don't feel having just read the
same sentence twice.
sys/kern/kern_timeout.c:
void
timeout_set_flags(struct timeout *to, void (*fn)(void *), void *arg, int flags)
{
_timeout_set(to, fn, arg, KCLOCK_NONE, flags);
}
void
timeout_set_proc(struct timeout *new, void (*fn)(void *), void *arg)
{
_timeout_set(new, fn, arg, KCLOCK_NONE, TIMEOUT_PROC);
}
Index: timeout.9
===
RCS file: /cvs/src/share/man/man9/timeout.9,v
retrieving revision 1.55
diff -u -p -r1.55 timeout.9
--- timeout.9 22 Jun 2022 14:10:49 - 1.55
+++ timeout.9 22 Jul 2022 10:34:22 -
@@ -138,11 +138,12 @@ interrupt context.
.Pp
The
.Fn timeout_set_proc
-function is similar to
-.Fn timeout_set
-but it runs the timeout in a process context instead of the default
-.Dv IPL_SOFTCLOCK
-interrupt context.
+function is a shorthand for
+.Fn timeout_set_flags
+with a
+.Fa flags
+value of
+.Dv TIMEOUT_PROC .
.Pp
The function
.Fn timeout_add
bgpd kroute F_KERNEL flag
There is no need to use F_KERNEL to tag routes from the kernel.
All this can be done by priority (RTP_MINE vs anything else).
The conversion is simple in most cases.
In kr_fib_delete() and kr_fib_change() check if the route is a bgpd owned
route and in that case remove the F_BGPD_INSERTED flag. The original route
is no longer in the FIB and so that flag should be cleared.
--
:wq Claudio
Index: bgpd.c
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
retrieving revision 1.249
diff -u -p -r1.249 bgpd.c
--- bgpd.c 20 Jul 2022 12:43:27 - 1.249
+++ bgpd.c 22 Jul 2022 08:10:01 -
@@ -1119,7 +1119,7 @@ int
bgpd_filternexthop(struct kroute_full *kf)
{
/* kernel routes are never filtered */
- if (kf->flags & F_KERNEL && kf->prefixlen != 0)
+ if (kf->priority != RTP_MINE && kf->prefixlen != 0)
return (0);
if (cflags & BGPD_FLAG_NEXTHOP_BGP) {
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.442
diff -u -p -r1.442 bgpd.h
--- bgpd.h 20 Jul 2022 12:43:27 - 1.442
+++ bgpd.h 22 Jul 2022 08:15:45 -
@@ -54,6 +54,8 @@
#defineMAX_RTSOCK_BUF (2 * 1024 * 1024)
#defineMAX_COMM_MATCH 3
+#defineRTP_MINE0xff/* internal route
priority */
+
#defineBGPD_OPT_VERBOSE0x0001
#defineBGPD_OPT_VERBOSE2 0x0002
#defineBGPD_OPT_NOACTION 0x0004
@@ -74,12 +76,11 @@
#defineSOCKET_NAME "/var/run/bgpd.sock"
#defineF_BGPD 0x0001
-#defineF_KERNEL0x0002
+#defineF_BGPD_INSERTED 0x0002
#defineF_CONNECTED 0x0004
#defineF_NEXTHOP 0x0008
#defineF_DOWN 0x0010
#defineF_STATIC0x0020
-#defineF_BGPD_INSERTED 0x0040
#defineF_REJECT0x0080
#defineF_BLACKHOLE 0x0100
#defineF_LONGER0x0200
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.276
diff -u -p -r1.276 kroute.c
--- kroute.c21 Jul 2022 10:22:43 - 1.276
+++ kroute.c22 Jul 2022 09:12:06 -
@@ -42,8 +42,6 @@
#include "bgpd.h"
#include "log.h"
-#defineRTP_MINE0xff
-
struct ktable **krt;
u_intkrt_size;
@@ -1339,7 +1337,7 @@ kr_redistribute(int type, struct ktable
return;
}
- if (!(kf->flags & F_KERNEL))
+ if (kf->priority == RTP_MINE)
return;
switch (kf->prefix.aid) {
@@ -1760,8 +1758,7 @@ kroute_insert(struct ktable *kt, struct
krm->next = kr;
}
- if ((kr->flags & (F_KERNEL | F_CONNECTED)) ==
- (F_KERNEL | F_CONNECTED))
+ if (kf->priority != RTP_MINE && kf->flags & F_CONNECTED)
if (kif_kr_insert(kr) == -1)
return (-1);
@@ -1801,8 +1798,7 @@ kroute_insert(struct ktable *kt, struct
kr6m->next = kr6;
}
- if ((kr6->flags & (F_KERNEL | F_CONNECTED)) ==
- (F_KERNEL | F_CONNECTED))
+ if (kf->priority != RTP_MINE && kf->flags & F_CONNECTED)
if (kif_kr6_insert(kr6) == -1)
return (-1);
@@ -1813,7 +1809,7 @@ kroute_insert(struct ktable *kt, struct
}
/* XXX this is wrong for nexthop validated via BGP */
- if (kf->flags & F_KERNEL) {
+ if (kf->priority != RTP_MINE) {
RB_FOREACH(h, knexthop_tree, KT2KNT(kt))
if (prefix_compare(>prefix, >nexthop,
kf->prefixlen) == 0)
@@ -1873,7 +1869,7 @@ kroute_remove(struct ktable *kt, struct
if (s->kroute == kr)
knexthop_validate(kt, s);
- if (kr->flags & F_KERNEL && kr == krm && kr->next == NULL)
+ if (kr->priority != RTP_MINE && kr == krm && kr->next == NULL)
/* again remove only once */
kr_redistribute(IMSG_NETWORK_REMOVE, kt, kr_tofull(kr));
@@ -1991,7 +1987,7 @@ kroute6_remove(struct ktable *kt, struct
if (s->kroute == kr)
knexthop_validate(kt, s);
- if (kr->flags & F_KERNEL && kr == krm && kr->next == NULL)
+ if (kr->priority != RTP_MINE && kr == krm && kr->next == NULL)
/* again remove only once */
kr_redistribute(IMSG_NETWORK_REMOVE, kt, kr6_tofull(kr));
@@ -3186,7 +3182,6 @@
libc/yp internals mop up
Following the switch to ypconnect(), several fields in the dom_binding
struct used internally are no longer needed. The following diff removes
them.
Index: yp/ypinternal.h
===
RCS file: /OpenBSD/src/lib/libc/yp/ypinternal.h,v
retrieving revision 1.13
diff -u -p -r1.13 ypinternal.h
--- yp/ypinternal.h 17 Jul 2022 03:08:58 - 1.13
+++ yp/ypinternal.h 22 Jul 2022 06:43:15 -
@@ -31,14 +31,9 @@
* yp_prot.h and yp.h.
*/
struct dom_binding {
- struct dom_binding *dom_pnext;
- char dom_domain[YPMAXDOMAIN + 1];
struct sockaddr_in dom_server_addr;
- u_short dom_server_port;
int dom_socket;
CLIENT *dom_client;
- u_short dom_local_port;
- long dom_vers;
};
#define BINDINGDIR "/var/yp/binding"
Index: yp/yp_first.c
===
RCS file: /OpenBSD/src/lib/libc/yp/yp_first.c,v
retrieving revision 1.11
diff -u -p -r1.11 yp_first.c
--- yp/yp_first.c 13 Sep 2015 20:57:28 - 1.11
+++ yp/yp_first.c 22 Jul 2022 06:43:15 -
@@ -69,7 +69,6 @@ again:
if (r != RPC_SUCCESS) {
if (tries++)
clnt_perror(ysd->dom_client, "yp_first: clnt_call");
- ysd->dom_vers = -1;
goto again;
}
if (!(r = ypprot_err(yprkv.stat))) {
Index: yp/yp_maplist.c
===
RCS file: /OpenBSD/src/lib/libc/yp/yp_maplist.c,v
retrieving revision 1.9
diff -u -p -r1.9 yp_maplist.c
--- yp/yp_maplist.c 16 Jan 2015 16:48:51 - 1.9
+++ yp/yp_maplist.c 22 Jul 2022 06:43:15 -
@@ -56,7 +56,6 @@ again:
if (r != RPC_SUCCESS) {
if (tries++)
clnt_perror(ysd->dom_client, "yp_maplist: clnt_call");
- ysd->dom_vers = -1;
goto again;
}
*outmaplist = ypml.maps;
Index: yp/yp_master.c
===
RCS file: /OpenBSD/src/lib/libc/yp/yp_master.c,v
retrieving revision 1.9
diff -u -p -r1.9 yp_master.c
--- yp/yp_master.c 16 Jan 2015 16:48:51 - 1.9
+++ yp/yp_master.c 22 Jul 2022 06:43:15 -
@@ -65,7 +65,6 @@ again:
if (r != RPC_SUCCESS) {
if (tries++)
clnt_perror(ysd->dom_client, "yp_master: clnt_call");
- ysd->dom_vers = -1;
goto again;
}
if (!(r = ypprot_err(yprm.stat))) {
Index: yp/yp_order.c
===
RCS file: /OpenBSD/src/lib/libc/yp/yp_order.c,v
retrieving revision 1.10
diff -u -p -r1.10 yp_order.c
--- yp/yp_order.c 16 Jan 2015 16:48:51 - 1.10
+++ yp/yp_order.c 22 Jul 2022 06:43:15 -
@@ -72,7 +72,6 @@ again:
}
if (r != RPC_SUCCESS) {
clnt_perror(ysd->dom_client, "yp_order: clnt_call");
- ysd->dom_vers = -1;
goto again;
}
*outorder = ypro.ordernum;
Index: yp/ypmatch_cache.c
===
RCS file: /OpenBSD/src/lib/libc/yp/ypmatch_cache.c,v
retrieving revision 1.17
diff -u -p -r1.17 ypmatch_cache.c
--- yp/ypmatch_cache.c 13 Sep 2015 20:57:28 - 1.17
+++ yp/ypmatch_cache.c 22 Jul 2022 06:43:15 -
@@ -187,7 +187,6 @@ again:
if (r != RPC_SUCCESS) {
if (tries++)
clnt_perror(ysd->dom_client, "yp_match: clnt_call");
- ysd->dom_vers = -1;
goto again;
}
if (!(r = ypprot_err(yprv.stat))) {
@@ -248,7 +247,6 @@ again:
if (r != RPC_SUCCESS) {
if (tries++)
clnt_perror(ysd->dom_client, "yp_next: clnt_call");
- ysd->dom_vers = -1;
goto again;
}
if (!(r = ypprot_err(yprkv.stat))) {
