FDDI/ATM leftovers
Since we don't support any FDDI or ATM interfaces anymore, remove some special cases for such interface types in our kernel. ok? Index: arch//amd64/amd64/autoconf.c === RCS file: /home/ncvs/src/sys/arch/amd64/amd64/autoconf.c,v retrieving revision 1.39 diff -u -p -r1.39 autoconf.c --- arch//amd64/amd64/autoconf.c19 Sep 2012 20:19:31 - 1.39 +++ arch//amd64/amd64/autoconf.c15 Nov 2013 14:08:14 - @@ -198,8 +198,7 @@ diskconf(void) for (ifp = TAILQ_FIRST(ifnet); ifp != NULL; ifp = TAILQ_NEXT(ifp, if_list)) { - if ((ifp-if_type == IFT_ETHER || - ifp-if_type == IFT_FDDI) + if (ifp-if_type == IFT_ETHER bcmp(bios_bootmac-mac, ((struct arpcom *)ifp)-ac_enaddr, ETHER_ADDR_LEN) == 0) Index: arch//i386/i386/autoconf.c === RCS file: /home/ncvs/src/sys/arch/i386/i386/autoconf.c,v retrieving revision 1.91 diff -u -p -r1.91 autoconf.c --- arch//i386/i386/autoconf.c 19 Sep 2012 20:19:31 - 1.91 +++ arch//i386/i386/autoconf.c 15 Nov 2013 14:08:14 - @@ -196,8 +196,7 @@ diskconf(void) for (ifp = TAILQ_FIRST(ifnet); ifp != NULL; ifp = TAILQ_NEXT(ifp, if_list)) { - if ((ifp-if_type == IFT_ETHER || - ifp-if_type == IFT_FDDI) + if (ifp-if_type == IFT_ETHER bcmp(bios_bootmac-mac, ((struct arpcom *)ifp)-ac_enaddr, ETHER_ADDR_LEN) == 0) Index: net/if.c === RCS file: /home/ncvs/src/sys/net/if.c,v retrieving revision 1.275 diff -u -p -r1.275 if.c --- net/if.c11 Nov 2013 09:15:34 - 1.275 +++ net/if.c15 Nov 2013 14:08:15 - @@ -1569,7 +1569,6 @@ ifioctl(struct socket *so, u_long cmd, c switch (ifp-if_type) { case IFT_ETHER: case IFT_CARP: - case IFT_FDDI: case IFT_XETHER: case IFT_ISO88025: case IFT_L2VLAN: Index: netinet6/in6_ifattach.c === RCS file: /home/ncvs/src/sys/netinet6/in6_ifattach.c,v retrieving revision 1.62 diff -u -p -r1.62 in6_ifattach.c --- netinet6/in6_ifattach.c 17 Oct 2013 16:27:45 - 1.62 +++ netinet6/in6_ifattach.c 15 Nov 2013 14:08:15 - @@ -189,8 +189,6 @@ found: /* IEEE802/EUI64 cases - what others? */ case IFT_ETHER: case IFT_CARP: - case IFT_FDDI: - case IFT_ATM: case IFT_IEEE1394: case IFT_IEEE80211: /* look at IEEE802/EUI64 only */ Index: netinet6/nd6_nbr.c === RCS file: /home/ncvs/src/sys/netinet6/nd6_nbr.c,v retrieving revision 1.71 diff -u -p -r1.71 nd6_nbr.c --- netinet6/nd6_nbr.c 11 Nov 2013 09:15:35 - 1.71 +++ netinet6/nd6_nbr.c 15 Nov 2013 14:08:15 - @@ -1061,7 +1061,6 @@ nd6_ifptomac(struct ifnet *ifp) { switch (ifp-if_type) { case IFT_ETHER: - case IFT_FDDI: case IFT_IEEE1394: case IFT_PROPVIRTUAL: case IFT_CARP:
Re: inteldrm diff
On Mon, Nov 18, 2013 at 01:22:20AM +0200, ja...@cieti.lv wrote: I tried this diff and at least one thing changed -- neverball now works (previously it immediately hung the GPU on start). There is still corruption and GPU hanging in chromium. There is corruption in mplayer -vo gl too (and it is still much slower than it is on 5.4). With a lot of help from Stefan Sperling I tried to do experiments. Since I am too unskilled I was not able to do it properly and build xenocara from the same point in time (date-based CVS checkouts do not compile for me) It's unfortunate that CVS messes up like this :( I just tested date-based checkouts with opencvs and it's also broken. but I built kernel on 5.3 from 2013-07-06 with just this patch applied: http://freshbsd.org/commit/openbsd/304417ea27d0874895cc4e65c30324b7bd14ac22 (as I am 80% sure it's when problems started) and the screen became totally corrupted in X but computer did not hang. It looked very similar to what is seen now. If that helps that is all I could find out. Well, that is the patch that requires a newer libdrm to work. It changes the userland-kernel interface that libdrm is using to talk to the kernel to configure graphics. So unless you also updated libdrm I don't think that it is surprising that graphics stopped working in this configuration. You could try updating libdrm (/usr/xenocara/lib/libdrm) to the same time frame.
Don't link multicast records to the first address
Diff below changes the way protocol multicast addresses are linked to an interface. Right now they are added to a list attached to the first protocol address of an interface. That makes this address descriptor and its position in the global list special. Plus in the IPv6 case, a special kludge is used to move multicast records from one address to another. So this diff reuse the design of the protocol agnostic struct ifaddr and adds a new list of multicast addresses, struct ifmaddr, to the interface descriptor. It solves the problems described previously and as a bonus properly free the IPv4 multicast records of an interface when it is detached, thus plugging some more leaks. I tested it with a carp setup, I appreciate any multicast related tests. Comments, ok? Index: net/if.c === RCS file: /home/ncvs/src/sys/net/if.c,v retrieving revision 1.275 diff -u -p -r1.275 if.c --- net/if.c11 Nov 2013 09:15:34 - 1.275 +++ net/if.c18 Nov 2013 10:43:00 - @@ -441,8 +441,9 @@ if_attach(struct ifnet *ifp) void if_attach_common(struct ifnet *ifp) { - TAILQ_INIT(ifp-if_addrlist); + TAILQ_INIT(ifp-if_maddrlist); + ifp-if_addrhooks = malloc(sizeof(*ifp-if_addrhooks), M_TEMP, M_WAITOK); TAILQ_INIT(ifp-if_addrhooks); Index: net/if.h === RCS file: /home/ncvs/src/sys/net/if.h,v retrieving revision 1.152 diff -u -p -r1.152 if.h --- net/if.h9 Nov 2013 06:38:42 - 1.152 +++ net/if.h18 Nov 2013 10:43:00 - @@ -262,6 +262,7 @@ struct ifnet { /* and the entries */ TAILQ_ENTRY(ifnet) if_list; /* all struct ifnets are chained */ TAILQ_ENTRY(ifnet) if_txlist; /* list of ifnets ready to tx */ TAILQ_HEAD(, ifaddr) if_addrlist; /* linked list of addresses per if */ + TAILQ_HEAD(, ifmaddr) if_maddrlist; /* list of multicast records */ TAILQ_HEAD(, ifg_list) if_groups; /* linked list of groups per if */ struct hook_desc_head *if_addrhooks; /* address change callbacks */ struct hook_desc_head *if_linkstatehooks; /* link change callbacks */ @@ -506,6 +507,16 @@ struct ifaddr_item { struct ifaddr *ifai_ifa; struct ifaddr_item *ifai_next; u_intifai_rdomain; +}; + +/* + * Interface multicast address. + */ +struct ifmaddr { + struct sockaddr *ifma_addr; /* Protocol address */ + struct ifnet*ifma_ifp; /* Back pointer to ifnet */ + unsigned int ifma_refcnt; /* Count of references */ + TAILQ_ENTRY(ifmaddr) ifma_list; /* Per-interface list */ }; /* Index: netinet/igmp.c === RCS file: /home/ncvs/src/sys/netinet/igmp.c,v retrieving revision 1.35 diff -u -p -r1.35 igmp.c --- netinet/igmp.c 18 Oct 2013 09:04:02 - 1.35 +++ netinet/igmp.c 18 Nov 2013 10:43:00 - @@ -193,6 +193,7 @@ igmp_input(struct mbuf *m, ...) struct igmp *igmp; int igmplen; int minlen; + struct ifmaddr *ifma; struct in_multi *inm; struct router_info *rti; struct in_ifaddr *ia; @@ -266,7 +267,10 @@ igmp_input(struct mbuf *m, ...) * except those that are already running and those * that belong to a local group (224.0.0.X). */ - IN_FOREACH_MULTI(ia, ifp, inm) { + TAILQ_FOREACH(ifma, ifp-if_maddrlist, ifma_list) { + if (ifma-ifma_addr-sa_family != AF_INET) + continue; + inm = ifmatoinm(ifma); if (inm-inm_timer == 0 !IN_LOCAL_GROUP(inm-inm_addr.s_addr)) { inm-inm_state = IGMP_DELAYING_MEMBER; @@ -294,7 +298,10 @@ igmp_input(struct mbuf *m, ...) * timers already running, check if they need to be * reset. */ - IN_FOREACH_MULTI(ia, ifp, inm) { + TAILQ_FOREACH(ifma, ifp-if_maddrlist, ifma_list) { + if (ifma-ifma_addr-sa_family != AF_INET) + continue; + inm = ifmatoinm(ifma); if (!IN_LOCAL_GROUP(inm-inm_addr.s_addr) (ip-ip_dst.s_addr == INADDR_ALLHOSTS_GROUP || ip-ip_dst.s_addr == inm-inm_addr.s_addr)) { @@ -521,11 +528,14 @@ void igmp_checktimer(struct ifnet *ifp) { struct in_multi *inm; - struct in_ifaddr *ia; + struct ifmaddr *ifma;
Re: FDDI/ATM leftovers
On 18 November 2013 11:24, Martin Pieuchot mpieuc...@nolizard.org wrote: Since we don't support any FDDI or ATM interfaces anymore, remove some special cases for such interface types in our kernel. ok? OK. looks like there's more stuff that can die in the fire...
Re: inteldrm diff
Date: Mon, 18 Nov 2013 11:26:31 +0100 From: Stefan Sperling s...@openbsd.org On Mon, Nov 18, 2013 at 01:22:20AM +0200, ja...@cieti.lv wrote: I tried this diff and at least one thing changed -- neverball now works (previously it immediately hung the GPU on start). There is still corruption and GPU hanging in chromium. There is corruption in mplayer -vo gl too (and it is still much slower than it is on 5.4). With a lot of help from Stefan Sperling I tried to do experiments. Since I am too unskilled I was not able to do it properly and build xenocara from the same point in time (date-based CVS checkouts do not compile for me) It's unfortunate that CVS messes up like this :( I just tested date-based checkouts with opencvs and it's also broken. but I built kernel on 5.3 from 2013-07-06 with just this patch applied: http://freshbsd.org/commit/openbsd/304417ea27d0874895cc4e65c30324b7bd14ac22 (as I am 80% sure it's when problems started) and the screen became totally corrupted in X but computer did not hang. It looked very similar to what is seen now. If that helps that is all I could find out. Well, that is the patch that requires a newer libdrm to work. It changes the userland-kernel interface that libdrm is using to talk to the kernel to configure graphics. So unless you also updated libdrm I don't think that it is surprising that graphics stopped working in this configuration. You could try updating libdrm (/usr/xenocara/lib/libdrm) to the same time frame. At this point it is probably pointless to figure out what broke things. Too many things have been piled on top of each otjher in the last 5 months. Even if you manage to figure out what broke things, we almost certainly can't revert the change now without breaking a lot of other stuff. Some of the problems you're seeing are simply caused by the fact that ports like chrome and firefox are now using graphics acceleration much more aggressively than they used to. You'll just have to hope that the changes I'm currently working on fix things eventually. Meanwhile the best you can do is avoiding the use of 3D acceleration, either by removing /usr/X11R6/lib/modules/dri/i965_dri.so from your system, changing permissions on /dev/drm0 such that only root can access it, or creating a /etc/drirc with the appropriate options. It's not really surprising that your GM45-based system has a lot of these issues. Neither jsg@ nor I have access to this hardware. If somebody has a laptop with this chipset that they're willing to donate and ship to Australia or the Netherlands, please contact us.
Re: Unexpected match set prio behaviour
On Mon, Nov 18, 2013 at 3:03 AM, Alexander Bluhm alexander.bl...@gmx.net wrote: On Thu, Nov 14, 2013 at 12:03:21AM +0200, Alexey Suslikov wrote: This is on 5.4-stable. vlan is only used to see what resulting prio is. #match on { $int_if } inet proto icmp all icmp-type echoreq set prio 5 pass quick on { $ext_if, $int_if } Can you test wether this diff matches your expected behaviour? Please try various combinations of pass and match rules. bluhm Index: net/pf.c === RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v retrieving revision 1.861 diff -u -p -r1.861 pf.c --- net/pf.c16 Nov 2013 00:36:01 - 1.861 +++ net/pf.c18 Nov 2013 00:56:55 - @@ -3110,8 +3110,10 @@ pf_rule_to_actions(struct pf_rule *r, st a-max_mss = r-max_mss; a-flags |= (r-scrub_flags (PFSTATE_NODF|PFSTATE_RANDOMID| PFSTATE_SETTOS|PFSTATE_SCRUB_TCP|PFSTATE_SETPRIO)); - a-set_prio[0] = r-set_prio[0]; - a-set_prio[1] = r-set_prio[1]; + if (r-scrub_flags PFSTATE_SETPRIO) { + a-set_prio[0] = r-set_prio[0]; + a-set_prio[1] = r-set_prio[1]; + } } #define PF_TEST_ATTRIB(t, a) \ well, it seems like now I have expected results. at least for following test cases. please tell if you need more. for a record, issue in question was discovered by Roman Kravchuk, I just assisted with analysis and reporting. Test 1 (default prio): # cat /etc/pf.conf ext_if=em0 int_if=vlan2525 set skip on { lo enc0 em1 } block log all #match on { $int_if } inet proto icmp all icmp-type echoreq set prio 6 #match on { $int_if } inet proto udp to port domain set prio 5 #match on { $int_if } inet proto tcp set prio (2, 4) pass quick on { $ext_if, $int_if } ICMP 12:45:57.293179 802.1Q vid 2525 pri 3 192.168.100.1 192.168.100.2: icmp: echo request 12:45:57.293491 802.1Q vid 2525 pri 3 192.168.100.2 192.168.100.1: icmp: echo reply TCP 12:46:39.953468 802.1Q vid 2525 pri 3 192.168.100.1.17637 192.168.100.2.80: S 370622106:370622106(0) win 16384 mss 1460,nop,nop,sackOK,nop,wscale 3,nop,nop,timestamp 1183962946 0 (DF) 12:46:39.953944 802.1Q vid 2525 pri 3 192.168.100.2.80 192.168.100.1.17637: S 3464733189:3464733189(0) ack 370622107 win 16384 mss 1460,nop,nop,sackOK,nop,wscale 3,nop,nop,timestamp 448817884 1183962946 (DF) 12:46:39.954024 802.1Q vid 2525 pri 3 192.168.100.1.17637 192.168.100.2.80: . ack 1 win 2048 nop,nop,timestamp 1183962946 448817884 (DF) 12:46:39.963421 802.1Q vid 2525 pri 3 192.168.100.1.17637 192.168.100.2.80: P 1:230(229) ack 1 win 2048 nop,nop,timestamp 1183962946 448817884 (DF) 12:46:39.970068 802.1Q vid 2525 pri 3 192.168.100.2.80 192.168.100.1.17637: . 1:1449(1448) ack 230 win 2172 nop,nop,timestamp 448817884 1183962946 (DF) 12:46:39.970095 802.1Q vid 2525 pri 3 192.168.100.2.80 192.168.100.1.17637: P 1449:2516(1067) ack 230 win 2172 nop,nop,timestamp 448817884 1183962946 (DF) 12:46:39.970172 802.1Q vid 2525 pri 3 192.168.100.1.17637 192.168.100.2.80: . ack 2516 win 1733 nop,nop,timestamp 1183962946 448817884 (DF) 12:46:39.970214 802.1Q vid 2525 pri 3 192.168.100.2.80 192.168.100.1.17637: F 2516:2516(0) ack 230 win 2172 nop,nop,timestamp 448817884 1183962946 (DF) 12:46:39.970280 802.1Q vid 2525 pri 3 192.168.100.1.17637 192.168.100.2.80: . ack 2517 win 1733 nop,nop,timestamp 1183962946 448817884 (DF) 12:46:39.993600 802.1Q vid 2525 pri 3 192.168.100.1.17637 192.168.100.2.80: F 230:230(0) ack 2517 win 2048 nop,nop,timestamp 1183962946 448817884 (DF) 12:46:39.993927 802.1Q vid 2525 pri 3 192.168.100.2.80 192.168.100.1.17637: . ack 231 win 2172 nop,nop,timestamp 448817884 1183962946 (DF) UDP 12:47:58.298665 802.1Q vid 2525 pri 3 192.168.100.1.39295 192.168.100.2.53: 36561+ A? i.ua. (22) 12:47:58.552804 802.1Q vid 2525 pri 3 192.168.100.2.53 192.168.100.1.39295: 36561 1/2/0 A 91.198.36.14 (74) Test 2 (match takes care of prio): # cat /etc/pf.conf ext_if=em0 int_if=vlan2525 set skip on { lo enc0 em1 } block log all match on { $int_if } inet proto icmp all icmp-type echoreq set prio 6 match on { $int_if } inet proto udp to port domain set prio 5 match on { $int_if } inet proto tcp set prio (2, 4) pass quick on { $ext_if, $int_if } ICMP 12:52:44.783107 802.1Q vid 2525 pri 6 192.168.100.1 192.168.100.2: icmp: echo request 12:52:44.783516 802.1Q vid 2525 pri 6 192.168.100.2 192.168.100.1: icmp: echo reply TCP 12:53:28.007629 802.1Q vid 2525 pri 2 192.168.100.1.49012 192.168.100.2.80: S 2694025614:2694025614(0) win 16384 mss 1460,nop,nop,sackOK,nop,wscale 3,nop,nop,timestamp 80976101 0 (DF) 12:53:28.007915 802.1Q vid 2525 pri 3 192.168.100.2.80 192.168.100.1.49012: S 704605823:704605823(0) ack 2694025615 win 16384 mss 1460,nop,nop,sackOK,nop,wscale 3,nop,nop,timestamp 281624921 80976101 (DF) 12:53:28.007990 802.1Q vid 2525 pri 4 192.168.100.1.49012 192.168.100.2.80: . ack 1 win 2048 nop,nop,timestamp 80976101
Re: FDDI/ATM leftovers
Martin Pieuchot mpieuchot at nolizard.org writes: - case IFT_FDDI: - case IFT_ATM: case IFT_IEEE1394: any plans for FireWire? :)
Re: convert sppp(4) to taskq
On 15/11/13(Fri) 15:45, Stefan Sperling wrote: On Fri, Nov 15, 2013 at 03:20:48PM +0100, Mike Belopuhov wrote: On 15 November 2013 15:13, Stefan Sperling s...@openbsd.org wrote: Is this done right? Works here with pppoe(4) for both IPv4 and IPv6. i think this diff might lack task_del's in the detach code. Ooops, good catch. have you tried destroying your pppoe interface? Yes, but evidently not while a task was scheduled. Same diff with task_del calls added. Even if right now calling task_del() is enough, do you know if there's an easy way to convert this code without putting the task storage in the chunk of memory it manipulates? In other words having the struct task outside of the softc? I'm asking because if you can do it, this will make the task totally independent and won't require any task_del(). The idea behind that is that tomorrow we will try to have more kernel threads running in parallel, and if your task is about to run or already running when your interface is destroyed you might end up freeing the memory the task is manipulating. Since the current code is already using allocating some memory for the arguments of the task, I'd argue that this is better than putting the task storage on the same memory chunk that it manipulates. But since this problem exists in other places in our tree, we might think of an alternative solution/api/whatever. Other than that your diff looks ok. Index: if_sppp.h === RCS file: /cvs/src/sys/net/if_sppp.h,v retrieving revision 1.19 diff -u -p -r1.19 if_sppp.h --- if_sppp.h 14 Nov 2013 16:52:33 - 1.19 +++ if_sppp.h 15 Nov 2013 13:48:47 - @@ -93,6 +93,12 @@ struct spppreq { #ifdef _KERNEL #include sys/timeout.h +#include sys/task.h + +#ifdef INET6 +#include netinet/in.h +#include netinet6/in6_var.h +#endif #define IDX_LCP 0/* idx into state table */ @@ -124,8 +130,13 @@ struct sipcp { #define IPV6CP_MYIFID_SEEN 2 /* have seen my suggested ifid */ u_int32_t saved_hisaddr; /* if hisaddr (IPv4) is dynamic, save * original one here, in network byte order */ - u_int32_t req_hisaddr; /* remote address requested */ - u_int32_t req_myaddr; /* local address requested */ + u_int32_t req_hisaddr; /* remote address requested (IPv4) */ + u_int32_t req_myaddr; /* local address requested (IPv4) */ +#ifdef INET6 + struct in6_aliasreq req_ifid; /* local ifid requested (IPv6) */ +#endif + struct task set_addr_task; /* set address from process context */ + struct task clear_addr_task;/* clear address from process context */ }; struct sauth { Index: if_spppsubr.c === RCS file: /cvs/src/sys/net/if_spppsubr.c,v retrieving revision 1.112 diff -u -p -r1.112 if_spppsubr.c --- if_spppsubr.c 14 Nov 2013 16:52:33 - 1.112 +++ if_spppsubr.c 15 Nov 2013 14:37:24 - @@ -46,7 +46,6 @@ #include sys/syslog.h #include sys/malloc.h #include sys/mbuf.h -#include sys/workq.h #include sys/timeout.h #include crypto/md5.h @@ -72,10 +71,6 @@ #include net/if_sppp.h -#ifdef INET6 -#include netinet6/in6_var.h -#endif - # define UNTIMEOUT(fun, arg, handle) \ timeout_del((handle)) @@ -291,6 +286,7 @@ HIDE void sppp_lcp_check_and_close(struc HIDE int sppp_ncp_check(struct sppp *sp); HIDE void sppp_ipcp_init(struct sppp *sp); +HIDE void sppp_ipcp_destroy(struct sppp *sp); HIDE void sppp_ipcp_up(struct sppp *sp); HIDE void sppp_ipcp_down(struct sppp *sp); HIDE void sppp_ipcp_open(struct sppp *sp); @@ -306,6 +302,7 @@ HIDE void sppp_ipcp_tlf(struct sppp *sp) HIDE void sppp_ipcp_scr(struct sppp *sp); HIDE void sppp_ipv6cp_init(struct sppp *sp); +HIDE void sppp_ipv6cp_destroy(struct sppp *sp); HIDE void sppp_ipv6cp_up(struct sppp *sp); HIDE void sppp_ipv6cp_down(struct sppp *sp); HIDE void sppp_ipv6cp_open(struct sppp *sp); @@ -902,6 +899,9 @@ sppp_detach(struct ifnet *ifp) struct sppp **q, *p, *sp = (struct sppp*) ifp; int i; + sppp_ipcp_destroy(sp); + sppp_ipv6cp_destroy(sp); + /* Remove the entry from the keepalive list. */ for (q = spppq; (p = *q); q = p-pp_next) if (p == sp) { @@ -2637,6 +2637,15 @@ sppp_ipcp_init(struct sppp *sp) sp-ipcp.flags = 0; sp-state[IDX_IPCP] = STATE_INITIAL; sp-fail_counter[IDX_IPCP] = 0; + task_set(sp-ipcp.set_addr_task, sppp_set_ip_addrs, sp, NULL); + task_set(sp-ipcp.clear_addr_task, sppp_clear_ip_addrs, sp, NULL); +} + +HIDE void +sppp_ipcp_destroy(struct sppp *sp) +{ + task_del(systq, sp-ipcp.set_addr_task); + task_del(systq, sp-ipcp.clear_addr_task); } HIDE void @@ -2955,38 +2964,11 @@ sppp_ipcp_RCN_nak(struct sppp *sp, struc addlog(\n);
Re: convert sppp(4) to taskq
On Mon, Nov 18, 2013 at 12:37:53PM +0100, Martin Pieuchot wrote: Even if right now calling task_del() is enough, do you know if there's an easy way to convert this code without putting the task storage in the chunk of memory it manipulates? In other words having the struct task outside of the softc? Well, tasks are stored in softc everywhere I looked for examples. The task storage and the storage for the task's arguments needs to be known when task_set() is called. As I understand, task_set() is called once, at attach time. Then the task gets scheduled or cancelled with task_add()/task_del(), and when the task runs it gets its arguments from the storage given to task_set(). If we wanted to store the task and its arguments outside the softc, I guess we'd need to malloc() task memory at attach time and free it on detach(). I.e. it has the same lifetime as the softc. So I don't see the point of uncoupling it from the softc. Or we would need to call task_set() and task_add() every time we want to schedule the task. I don't think that's what the tasqk API is intending. Or we would have the task itself free its own storage (task and arguments), instead of freeing it when the softc is destroyed. But I don't think that's what the API is intending either. I'm asking because if you can do it, this will make the task totally independent and won't require any task_del(). The idea behind that is that tomorrow we will try to have more kernel threads running in parallel, and if your task is about to run or already running when your interface is destroyed you might end up freeing the memory the task is manipulating. I'm assuming the task won't be interrupted in a way that would make the ifp storage go away while it runs. Perhaps in a new world with more kernel threads that won't hold. But that would cause quite a lot of problems everywhere, not just sppp(4). When the ifp goes away, the task must be not be allowed to run if already scheduled. Because a workq task cannot be cancelled, the bug where the interface is deleted before the workq task gets to run is present in the current code and fixed by switching to taskq, with the task_del() call at interface detach time. How can we fix this bug without task_del()? Would you have the task itself cope with an ifp that has disappeared? Since the current code is already using allocating some memory for the arguments of the task, I'd argue that this is better than putting the task storage on the same memory chunk that it manipulates. But since this problem exists in other places in our tree, we might think of an alternative solution/api/whatever. For now, I'd like to stick with the task in softc idiom I saw elsewhere. Generally, I think your concern is out of scope for my sppp(4) changes and should be discussed in a separate discussion thread because using a different idiom would affect many drivers.
Re: Don't link multicast records to the first address
Martin Pieuchot mpieuchot at nolizard.org writes: at at -1803,8 +1651,12 at at in6_delmulti(struct in6_multi *in6m) snip + s = splsoftnet(); + TAILQ_REMOVE(ifp-if_maddrlist, in6m-in6m_ifma, ifma_list); + splx(s); + free(in6m, M_IPMADDR); } splx(s); this splx seems wrong.
Re: convert sppp(4) to taskq
On 18/11/13(Mon) 13:35, Stefan Sperling wrote: On Mon, Nov 18, 2013 at 12:37:53PM +0100, Martin Pieuchot wrote: Even if right now calling task_del() is enough, do you know if there's an easy way to convert this code without putting the task storage in the chunk of memory it manipulates? In other words having the struct task outside of the softc? Well, tasks are stored in softc everywhere I looked for examples. I know :) But how many of these softc can be freed? I also introduced a similar situation in audio(4) if it is attached to uaudio(4), and I'm looking for an alternative way of doing it. The task storage and the storage for the task's arguments needs to be known when task_set() is called. As I understand, task_set() is called once, at attach time. Then the task gets scheduled or cancelled with task_add()/task_del(), and when the task runs it gets its arguments from the storage given to task_set(). If we wanted to store the task and its arguments outside the softc, I guess we'd need to malloc() task memory at attach time and free it on detach(). I.e. it has the same lifetime as the softc. So I don't see the point of uncoupling it from the softc. The point is that there's no way to check if the task queue still has a reference to your task when you free() your softc, right now big lock is protecting us™ :) Or we would need to call task_set() and task_add() every time we want to schedule the task. I don't think that's what the tasqk API is intending. Or we would have the task itself free its own storage (task and arguments), instead of freeing it when the softc is destroyed. But I don't think that's what the API is intending either. That's why I'm asking, maybe you or somebody else already has an idea. I'm asking because if you can do it, this will make the task totally independent and won't require any task_del(). The idea behind that is that tomorrow we will try to have more kernel threads running in parallel, and if your task is about to run or already running when your interface is destroyed you might end up freeing the memory the task is manipulating. I'm assuming the task won't be interrupted in a way that would make the ifp storage go away while it runs. Perhaps in a new world with more kernel threads that won't hold. But that would cause quite a lot of problems everywhere, not just sppp(4). When the ifp goes away, the task must be not be allowed to run if already scheduled. Because a workq task cannot be cancelled, the bug where the interface is deleted before the workq task gets to run is present in the current code and fixed by switching to taskq, with the task_del() call at interface detach time. How can we fix this bug without task_del()? Would you have the task itself cope with an ifp that has disappeared? Yep, that's another way to fix this problem. You can pass the index of the interface to the task and call if_get() when it runs. If the interface is gone don't do anything otherwise run the task. But this approach only makes sense if the task storage is not freed when the interface is destroyed. Since the current code is already using allocating some memory for the arguments of the task, I'd argue that this is better than putting the task storage on the same memory chunk that it manipulates. But since this problem exists in other places in our tree, we might think of an alternative solution/api/whatever. For now, I'd like to stick with the task in softc idiom I saw elsewhere. Fine with me. Generally, I think your concern is out of scope for my sppp(4) changes and should be discussed in a separate discussion thread because using a different idiom would affect many drivers. It is, clearly, but maybe you had an easy way to avoid it, apparently not ;) To be clear, my concern is only about drivers that can be destroyed/unplugged, that's hopefully not affecting so many drivers.
Re: initial i217/i218 Haswell Ethernet support for em(4)
On Sat, Nov 16, 2013 at 10:42:05AM +0100, Markus Hennecke wrote: On Sat, 9 Nov 2013, Jonathan Gray wrote: This adds the initial bits for the i217/i218 PHY and the Lynx Point PCH found in Haswell systems. Doesn't include the new workarounds yet and follows the pch2/82579 paths for now but this seems to be enough to make a desktop machine with I217-LM work This fails for the I217-V with an EEPROM invalid checksum error: Here is a revised diff with different eeprom bank logic from FreeBSD as suggested by Masanobu SAITOH: Index: if_em.c === RCS file: /cvs/src/sys/dev/pci/if_em.c,v retrieving revision 1.271 diff -u -p -r1.271 if_em.c --- if_em.c 15 Nov 2013 14:53:03 - 1.271 +++ if_em.c 16 Nov 2013 09:35:41 - @@ -130,6 +130,10 @@ const struct pci_matchid em_devices[] = { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_82578DM }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_82579LM }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_82579V }, + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_I217_LM }, + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_I217_V }, + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_I218_LM }, + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_I218_V }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_82580_COPPER }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_82580_FIBER }, { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_82580_SERDES }, @@ -788,6 +792,7 @@ em_init(void *arg) break; case em_pchlan: case em_pch2lan: + case em_pch_lpt: pba = E1000_PBA_26K; break; default: @@ -1626,7 +1631,8 @@ em_allocate_pci_resources(struct em_soft sc-hw.mac_type == em_ich9lan || sc-hw.mac_type == em_ich10lan || sc-hw.mac_type == em_pchlan || - sc-hw.mac_type == em_pch2lan) { + sc-hw.mac_type == em_pch2lan || + sc-hw.mac_type == em_pch_lpt) { val = pci_conf_read(pa-pa_pc, pa-pa_tag, EM_FLASH); if (PCI_MAPREG_TYPE(val) != PCI_MAPREG_TYPE_MEM) { printf(: flash is not mem space\n); Index: if_em_hw.c === RCS file: /cvs/src/sys/dev/pci/if_em_hw.c,v retrieving revision 1.73 diff -u -p -r1.73 if_em_hw.c --- if_em_hw.c 7 Aug 2013 01:06:34 - 1.73 +++ if_em_hw.c 18 Nov 2013 14:02:31 - @@ -179,6 +179,7 @@ int32_t em_get_pcs_speed_and_duplex_825 uint16_t *); int32_tem_set_eee_i350(struct em_hw *); int32_tem_set_eee_pchlan(struct em_hw *); +int32_tem_valid_nvm_bank_detect_ich8lan(struct em_hw *, uint32_t *); /* IGP cable length table */ @@ -255,6 +256,9 @@ em_set_phy_type(struct em_hw *hw) case I82579_E_PHY_ID: hw-phy_type = em_phy_82579; break; + case I217_E_PHY_ID: + hw-phy_type = em_phy_i217; + break; case I82580_I_PHY_ID: case I350_I_PHY_ID: hw-phy_type = em_phy_82580; @@ -568,6 +572,12 @@ em_set_mac_type(struct em_hw *hw) case E1000_DEV_ID_PCH2_LV_V: hw-mac_type = em_pch2lan; break; + case E1000_DEV_ID_PCH_LPT_I217_LM: + case E1000_DEV_ID_PCH_LPT_I217_V: + case E1000_DEV_ID_PCH_LPTLP_I218_LM: + case E1000_DEV_ID_PCH_LPTLP_I218_V: + hw-mac_type = em_pch_lpt; + break; case E1000_DEV_ID_EP80579_LAN_1: hw-mac_type = em_icp_; hw-icp__port_num = 0; @@ -591,6 +601,7 @@ em_set_mac_type(struct em_hw *hw) case em_ich10lan: case em_pchlan: case em_pch2lan: + case em_pch_lpt: hw-swfwhw_semaphore_present = TRUE; hw-asf_firmware_present = TRUE; break; @@ -682,6 +693,7 @@ em_set_media_type(struct em_hw *hw) case em_ich10lan: case em_pchlan: case em_pch2lan: + case em_pch_lpt: case em_82573: case em_82574: /* @@ -838,6 +850,7 @@ em_reset_hw(struct em_hw *hw) case em_ich10lan: case em_pchlan: case em_pch2lan: + case em_pch_lpt: if (!hw-phy_reset_disable em_check_phy_reset_block(hw) == E1000_SUCCESS) { /* @@ -850,7 +863,7 @@ em_reset_hw(struct em_hw *hw) * Gate automatic PHY configuration by hardware on * non-managed 82579 */ - if ((hw-mac_type == em_pch2lan) + if ((hw-mac_type == em_pch2lan || hw-mac_type == em_pch_lpt) !(E1000_READ_REG(hw, FWSM) E1000_FWSM_FW_VALID)) {
Re: FDDI/ATM leftovers
On Mon, Nov 18, 2013 at 11:28:56AM +, Alexey E. Suslikov wrote: Martin Pieuchot mpieuchot at nolizard.org writes: - case IFT_FDDI: - case IFT_ATM: case IFT_IEEE1394: any plans for FireWire? :) Nope. :-) Ken
hiding struct ifnet diff #1: queue.h
This guys rely on the fact that if.h includes queue.h, but they shouldn't really since lists are needed by the struct ifnet only. OK? diff --git sbin/dhclient/dhcpd.h sbin/dhclient/dhcpd.h index 53625fb..c03af36 100644 --- sbin/dhclient/dhcpd.h +++ sbin/dhclient/dhcpd.h @@ -43,10 +43,11 @@ #include sys/socket.h #include sys/sockio.h #include sys/stat.h #include sys/time.h #include sys/wait.h +#include sys/queue.h #include net/if.h #include net/if_dl.h #include net/route.h diff --git usr.sbin/pppoe/client.c usr.sbin/pppoe/client.c index 32cd0a1..deafd51 100644 --- usr.sbin/pppoe/client.c +++ usr.sbin/pppoe/client.c @@ -27,10 +27,11 @@ #include stdio.h #include sys/types.h #include sys/uio.h #include sys/socket.h +#include sys/queue.h #include net/if.h #include net/if_dl.h #include net/if_types.h #include netinet/in.h #include netinet/if_ether.h diff --git usr.sbin/pppoe/common.c usr.sbin/pppoe/common.c index 7f1b6b5..aec408f 100644 --- usr.sbin/pppoe/common.c +++ usr.sbin/pppoe/common.c @@ -27,10 +27,11 @@ #include stdio.h #include sys/types.h #include sys/uio.h #include sys/socket.h +#include sys/queue.h #include net/if.h #include net/if_dl.h #include net/if_types.h #include net/ppp_defs.h #include netinet/in.h diff --git usr.sbin/pppoe/pppoe.c usr.sbin/pppoe/pppoe.c index cb59379..ad30f9e 100644 --- usr.sbin/pppoe/pppoe.c +++ usr.sbin/pppoe/pppoe.c @@ -28,10 +28,11 @@ #include stdio.h #include sys/types.h #include sys/socket.h #include sys/ioctl.h #include sys/wait.h +#include sys/queue.h #include net/if.h #include net/if_dl.h #include net/if_types.h #include netinet/in.h #include netinet/if_ether.h diff --git usr.sbin/pppoe/server.c usr.sbin/pppoe/server.c index 410583a..fc3c76d 100644 --- usr.sbin/pppoe/server.c +++ usr.sbin/pppoe/server.c @@ -27,10 +27,11 @@ #include sys/types.h #include sys/uio.h #include sys/socket.h #include sys/param.h +#include sys/queue.h #include net/if.h #include net/if_dl.h #include net/if_types.h #include netinet/in.h #include netinet/if_ether.h diff --git usr.sbin/pppoe/session.c usr.sbin/pppoe/session.c index 8b9ba2c..5488a59 100644 --- usr.sbin/pppoe/session.c +++ usr.sbin/pppoe/session.c @@ -25,10 +25,11 @@ * POSSIBILITY OF SUCH DAMAGE. */ #include sys/types.h #include sys/socket.h +#include sys/queue.h #include net/if.h #include net/if_dl.h #include net/if_types.h #include netinet/in.h #include netinet/if_ether.h diff --git usr.sbin/pppoe/tag.c usr.sbin/pppoe/tag.c index b79a53c..e536846 100644 --- usr.sbin/pppoe/tag.c +++ usr.sbin/pppoe/tag.c @@ -25,10 +25,11 @@ * POSSIBILITY OF SUCH DAMAGE. */ #include sys/types.h #include sys/socket.h +#include sys/queue.h #include net/if.h #include net/if_dl.h #include net/if_types.h #include netinet/in.h #include netinet/if_ether.h
struct ifnet diff #2: ifnet in other structures
This diff hides a bunch of structures (namely arpcom, llinfo_arp, ethernet multicast macros along the way, and in_ifaddr the same way in6_ifaddr is hidden). OK? diff --git sys/netinet/if_ether.h sys/netinet/if_ether.h index 9ef9c12..459c3fa 100644 --- sys/netinet/if_ether.h +++ sys/netinet/if_ether.h @@ -144,10 +144,11 @@ structether_arp { #definearp_pro ea_hdr.ar_pro #definearp_hln ea_hdr.ar_hln #definearp_pln ea_hdr.ar_pln #definearp_op ea_hdr.ar_op +#ifdef _KERNEL /* * Structure shared between the ethernet driver modules and * the address resolution code. For example, each ec_softc or il_softc * begins with this structure. */ @@ -169,10 +170,11 @@ struct llinfo_arp { int la_hold_count; /* number of packets queued */ longla_asked; /* last time we QUERIED for this addr */ }; #define MAX_HOLD_QUEUE 10 #define MAX_HOLD_TOTAL 100 +#endif struct sockaddr_inarp { u_int8_t sin_len; u_int8_t sin_family; u_int16_t sin_port; @@ -204,11 +206,10 @@ void arp_ifinit(struct arpcom *, struct ifaddr *); void arp_rtrequest(int, struct rtentry *); intether_addmulti(struct ifreq *, struct arpcom *); intether_delmulti(struct ifreq *, struct arpcom *); intether_multiaddr(struct sockaddr *, u_int8_t[], u_int8_t[]); -#endif /* _KERNEL */ /* * Ethernet multicast address structure. There is one of these for each * multicast address or range of multicast addresses that we are supposed * to listen to on a particular interface. They are kept in a linked list, @@ -272,12 +273,10 @@ do { \ do { \ (step).e_enm = LIST_FIRST((ac)-ac_multiaddrs);\ ETHER_NEXT_MULTI((step), (enm));\ } while (/* CONSTCOND */ 0) -#ifdef _KERNEL - #ifdef NFSCLIENT extern struct ifnet *revarp_ifp; #endif /* NFSCLIENT */ void arprequest(struct ifnet *, u_int32_t *, u_int32_t *, u_int8_t *); diff --git sys/netinet/in_var.h sys/netinet/in_var.h index 249966b..88557a7 100644 --- sys/netinet/in_var.h +++ sys/netinet/in_var.h @@ -35,10 +35,11 @@ #ifndef _NETINET_IN_VAR_H_ #define _NETINET_IN_VAR_H_ #include sys/queue.h +#ifdef _KERNEL /* * Interface address, Internet version. One of these structures * is allocated for each interface with an Internet address. * The ifaddr structure contains the protocol-independent part * of the structure and is assumed to be first. @@ -57,10 +58,11 @@ struct in_ifaddr { struct sockaddr_in ia_sockmask; /* reserve space for general netmask */ LIST_HEAD(, in_multi) ia_multiaddrs; /* list of multicast addresses */ struct in_multi *ia_allhosts; /* multicast address record for the allhosts multicast group */ }; +#endif struct in_aliasreq { charifra_name[IFNAMSIZ];/* if name, e.g. en0 */ union { struct sockaddr_in ifrau_addr;
hiding struct ifnet diff #3: pfvar.h
pfctl shares some structures (namely pfi_kif) with the kernel but doesn't use the ifnet pointer so it gets a bunch of forward declarations for ifnet and interface group structures. OK? diff --git sys/net/pfvar.h sys/net/pfvar.h index 37f61e4..a05fc49 100644 --- sys/net/pfvar.h +++ sys/net/pfvar.h @@ -1190,10 +1190,13 @@ extern struct pf_state_tree pf_statetbl; /* keep synced with pfi_kif, used in RB_FIND */ struct pfi_kif_cmp { char pfik_name[IFNAMSIZ]; }; +struct ifnet; +struct ifg_group; + struct pfi_kif { char pfik_name[IFNAMSIZ]; RB_ENTRY(pfi_kif)pfik_tree; u_int64_tpfik_packets[2][2][2]; u_int64_tpfik_bytes[2][2][2];
hiding struct ifnet diff #4v1: if_var.h
This diff splits kernel visible parts away from if.h into a separate header if_var.h. As a compatibility goo for the kernel if.h will also include if_var.h under _KERNEL. The benefit of going this way is that we don't need to define _KERNEL in the netstat friends (a tradeoff is that they will have to include if_var.h directly) and that if.h doesn't become a mess. I will post a different diff later (v2) that will keep all the parts in one header under ifdefs. However the extent of changes will be essentially the same. Then we'll pick whichever we like best. diff --git sys/net/if.h sys/net/if.h index b7d1b3c..d11ace2 100644 --- sys/net/if.h +++ sys/net/if.h @@ -1,10 +1,9 @@ /* $OpenBSD: if.h,v 1.152 2013/11/09 06:38:42 dlg Exp $*/ /* $NetBSD: if.h,v 1.23 1996/05/07 02:40:27 thorpej Exp $ */ /* - * Copyright (c) 2012-2013 Henning Brauer henn...@openbsd.org * Copyright (c) 1982, 1986, 1989, 1993 * The Regents of the University of California. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -58,69 +57,10 @@ voidif_freenameindex(struct if_nameindex *); __END_DECLS #endif #if __BSD_VISIBLE -#include sys/queue.h -#include sys/tree.h -#include altq/if_altq.h -#ifdef _KERNEL -#include net/hfsc.h -#endif - -/* - * Structures defining a network interface, providing a packet - * transport mechanism (ala level 0 of the PUP protocols). - * - * Each interface accepts output datagrams of a specified maximum - * length, and provides higher level routines with input datagrams - * received from its medium. - * - * Output occurs when the routine if_output is called, with four parameters: - * (*ifp-if_output)(ifp, m, dst, rt) - * Here m is the mbuf chain to be sent and dst is the destination address. - * The output routine encapsulates the supplied datagram if necessary, - * and then transmits it on its medium. - * - * On input, each interface unwraps the data received by it, and either - * places it on the input queue of an internetwork datagram routine - * and posts the associated software interrupt, or passes the datagram to a raw - * packet input routine. - * - * Routines exist for locating interfaces by their addresses - * or for locating an interface on a certain network, as well as more general - * routing and gateway routines maintaining information used to locate - * interfaces. These routines live in the files if.c and route.c - */ - -#include sys/time.h - -struct mbuf; -struct proc; -struct rtentry; -struct socket; -struct ether_header; -struct arpcom; -struct rt_addrinfo; -struct ifnet; -struct hfsc_if; - -/* - * Structure describing a `cloning' interface. - */ -struct if_clone { - LIST_ENTRY(if_clone) ifc_list; /* on list of cloners */ - const char *ifc_name; /* name of device, e.g. `gif' */ - size_t ifc_namelen; /* length of name */ - - int (*ifc_create)(struct if_clone *, int); - int (*ifc_destroy)(struct ifnet *); -}; - -#defineIF_CLONE_INITIALIZER(name, create, destroy) \ - { { 0 }, name, sizeof(name) - 1, create, destroy } - /* * Structure used to query names of interface cloners. */ struct if_clonereq { int ifcr_total; /* total cloners (out) */ @@ -173,26 +113,10 @@ structif_data { #define IFQ_NQUEUES8 #define IFQ_MAXPRIOIFQ_NQUEUES - 1 #define IFQ_DEFPRIO3 /* - * Structure defining a queue for a network interface. - * XXX keep in sync with struct ifaltq. - */ -struct ifqueue { - struct { - struct mbuf *head; - struct mbuf *tail; - }ifq_q[IFQ_NQUEUES]; - int ifq_len; - int ifq_maxlen; - int ifq_drops; - struct hfsc_if *ifq_hfsc; - struct timeout *ifq_congestion; -}; - -/* * Values for if_link_state. */ #define LINK_STATE_UNKNOWN 0 /* link unknown */ #define LINK_STATE_INVALID 1 /* link invalid */ #define LINK_STATE_DOWN2 /* link is down */ @@ -214,12 +138,11 @@ struct if_status_description { }; #define LINK_STATE_DESC_MATCH(_ifs, _t, _s)\ (((_ifs)-ifs_type == (_t) || (_ifs)-ifs_type == 0) \ (_ifs)-ifs_state == (_s)) - - + #define LINK_STATE_DESCRIPTIONS { \ { IFT_ETHER, LINK_STATE_DOWN, no carrier }, \ \ { IFT_IEEE80211, LINK_STATE_DOWN, no network }, \ @@ -240,101 +163,18 @@ struct if_status_description { { 0, LINK_STATE_DOWN, down }, \ { 0, LINK_STATE_KALIVE_DOWN, keepalive down },
Re: inteldrm diff
It's not really surprising that your GM45-based system has a lot of these issues. Neither jsg@ nor I have access to this hardware. If somebody has a laptop with this chipset that they're willing to donate and ship to Australia or the Netherlands, please contact us.
hiding struct ifnet diff #4v2: _KERNEL
As promised here's a take on hiding ifnet via _KERNEL. This looks a bit simpler. I've tried not to toss things around that much and have only moved 'ifqueue'. diff --git sys/net/if.h sys/net/if.h index b7d1b3c..9a14117 100644 --- sys/net/if.h +++ sys/net/if.h @@ -58,16 +58,15 @@ voidif_freenameindex(struct if_nameindex *); __END_DECLS #endif #if __BSD_VISIBLE +#ifdef _KERNEL #include sys/queue.h #include sys/tree.h #include altq/if_altq.h -#ifdef _KERNEL #include net/hfsc.h -#endif /* * Structures defining a network interface, providing a packet * transport mechanism (ala level 0 of the PUP protocols). * @@ -116,10 +115,11 @@ struct if_clone { int (*ifc_destroy)(struct ifnet *); }; #defineIF_CLONE_INITIALIZER(name, create, destroy) \ { { 0 }, name, sizeof(name) - 1, create, destroy } +#endif /* _KERNEL */ /* * Structure used to query names of interface cloners. */ struct if_clonereq { @@ -168,30 +168,10 @@ structif_data { struct timeval ifi_lastchange; /* last operational state change */ struct mclpool ifi_mclpool[MCLPOOLS]; }; -#define IFQ_NQUEUES8 -#define IFQ_MAXPRIOIFQ_NQUEUES - 1 -#define IFQ_DEFPRIO3 - -/* - * Structure defining a queue for a network interface. - * XXX keep in sync with struct ifaltq. - */ -struct ifqueue { - struct { - struct mbuf *head; - struct mbuf *tail; - }ifq_q[IFQ_NQUEUES]; - int ifq_len; - int ifq_maxlen; - int ifq_drops; - struct hfsc_if *ifq_hfsc; - struct timeout *ifq_congestion; -}; - /* * Values for if_link_state. */ #define LINK_STATE_UNKNOWN 0 /* link unknown */ #define LINK_STATE_INVALID 1 /* link invalid */ @@ -214,12 +194,11 @@ struct if_status_description { }; #define LINK_STATE_DESC_MATCH(_ifs, _t, _s)\ (((_ifs)-ifs_type == (_t) || (_ifs)-ifs_type == 0) \ (_ifs)-ifs_state == (_s)) - - + #define LINK_STATE_DESCRIPTIONS { \ { IFT_ETHER, LINK_STATE_DOWN, no carrier }, \ \ { IFT_IEEE80211, LINK_STATE_DOWN, no network }, \ @@ -240,25 +219,46 @@ struct if_status_description { { 0, LINK_STATE_DOWN, down }, \ { 0, LINK_STATE_KALIVE_DOWN, keepalive down },\ { 0, 0, NULL } \ } -/* - * Structure defining a queue for a network interface. - * - * (Would like to call this struct ``if'', but C isn't PL/1.) - */ -TAILQ_HEAD(ifnet_head, ifnet); /* the actual queue head */ - /* Traditional BSD name for length of interface external name. */ #defineIFNAMSIZIF_NAMESIZE /* * Length of interface description, including terminating '\0'. */ #defineIFDESCRSIZE 64 +#defineIFQ_NQUEUES 8 +#defineIFQ_MAXPRIO IFQ_NQUEUES - 1 +#defineIFQ_DEFPRIO 3 + +#ifdef _KERNEL +/* + * Structure defining a queue for a network interface. + * XXX keep in sync with struct ifaltq. + */ +struct ifqueue { + struct { + struct mbuf *head; + struct mbuf *tail; + }ifq_q[IFQ_NQUEUES]; + int ifq_len; + int ifq_maxlen; + int ifq_drops; + struct hfsc_if *ifq_hfsc; + struct timeout *ifq_congestion; +}; + +/* + * Structure defining a queue for a network interface. + * + * (Would like to call this struct ``if'', but C isn't PL/1.) + */ +TAILQ_HEAD(ifnet_head, ifnet); /* the actual queue head */ + struct ifnet { /* and the entries */ void*if_softc; /* lower-level data for this if */ TAILQ_ENTRY(ifnet) if_list; /* all struct ifnets are chained */ TAILQ_ENTRY(ifnet) if_txlist; /* list of ifnets ready to tx */ TAILQ_HEAD(, ifaddr) if_addrlist; /* linked list of addresses per if */ @@ -330,10 +330,11 @@ struct ifnet {/* and the entries */ #defineif_omcasts if_data.ifi_omcasts #defineif_iqdrops if_data.ifi_iqdrops #defineif_noproto if_data.ifi_noproto #defineif_lastchange if_data.ifi_lastchange #defineif_capabilities if_data.ifi_capabilities +#endif /* _KERNEL */ #defineIFF_UP 0x1 /* interface is up */ #defineIFF_BROADCAST 0x2 /* broadcast address valid */ #defineIFF_DEBUG 0x4 /* turn on debugging */ #defineIFF_LOOPBACK
Support for External Random Number Server
Use Case I have several headless computers. Their only source of randomness is from the network. I also have a hardware true random number generator on another computer. I would like the headless computers to be able to access truly random numbers from a server - at the kernel level. I would like a standard, built into the OS, so I get this improved source of randomness right from the very first install. I want the random numbers encrypted as they transit the network. Ssh already does this. Possible Solutions: 1 Spawn a userland program or script which uses ssh, obtains the random numbers, and then calls add_true_randomness(). 2 Configure the kernel with the IP address of the server, and an account name, and the kernel can obtain truly random numbers whenever it wants. What is the best way to achieve my goal? Thanks, Ken Hendrickson
Re: Support for External Random Number Server
I would like the headless computers to be able to access truly random numbers from a server - at the kernel level. In addition, can a standard be developed and install scripts modified so that during the install, the install script obtains a bunch of truly random numbers from a server, to be used at first boot? Thanks, Ken Hendrickson
Re: hiding struct ifnet diff #4v2: _KERNEL
I prefer something this model of handling the situation, as long as the ports tree software handles it well. (But then again, the other way of doing it will cause effects there as well..) As promised here's a take on hiding ifnet via _KERNEL. This looks a bit simpler. I've tried not to toss things around that much and have only moved 'ifqueue'. diff --git sys/net/if.h sys/net/if.h index b7d1b3c..9a14117 100644 --- sys/net/if.h +++ sys/net/if.h @@ -58,16 +58,15 @@ void if_freenameindex(struct if_nameindex *); __END_DECLS #endif #if __BSD_VISIBLE +#ifdef _KERNEL #include sys/queue.h #include sys/tree.h #include altq/if_altq.h -#ifdef _KERNEL #include net/hfsc.h -#endif /* * Structures defining a network interface, providing a packet * transport mechanism (ala level 0 of the PUP protocols). * @@ -116,10 +115,11 @@ struct if_clone { int (*ifc_destroy)(struct ifnet *); }; #define IF_CLONE_INITIALIZER(name, create, destroy) \ { { 0 }, name, sizeof(name) - 1, create, destroy } +#endif /* _KERNEL */ /* * Structure used to query names of interface cloners. */ struct if_clonereq { @@ -168,30 +168,10 @@ struct if_data { struct timeval ifi_lastchange; /* last operational state change */ struct mclpool ifi_mclpool[MCLPOOLS]; }; -#define IFQ_NQUEUES 8 -#define IFQ_MAXPRIO IFQ_NQUEUES - 1 -#define IFQ_DEFPRIO 3 - -/* - * Structure defining a queue for a network interface. - * XXX keep in sync with struct ifaltq. - */ -struct ifqueue { - struct { - struct mbuf *head; - struct mbuf *tail; - }ifq_q[IFQ_NQUEUES]; - int ifq_len; - int ifq_maxlen; - int ifq_drops; - struct hfsc_if *ifq_hfsc; - struct timeout *ifq_congestion; -}; - /* * Values for if_link_state. */ #define LINK_STATE_UNKNOWN 0 /* link unknown */ #define LINK_STATE_INVALID 1 /* link invalid */ @@ -214,12 +194,11 @@ struct if_status_description { }; #define LINK_STATE_DESC_MATCH(_ifs, _t, _s) \ (((_ifs)-ifs_type == (_t) || (_ifs)-ifs_type == 0) \ (_ifs)-ifs_state == (_s)) - - + #define LINK_STATE_DESCRIPTIONS {\ { IFT_ETHER, LINK_STATE_DOWN, no carrier }, \ \ { IFT_IEEE80211, LINK_STATE_DOWN, no network }, \ @@ -240,25 +219,46 @@ struct if_status_description { { 0, LINK_STATE_DOWN, down }, \ { 0, LINK_STATE_KALIVE_DOWN, keepalive down },\ { 0, 0, NULL } \ } -/* - * Structure defining a queue for a network interface. - * - * (Would like to call this struct ``if'', but C isn't PL/1.) - */ -TAILQ_HEAD(ifnet_head, ifnet); /* the actual queue head */ - /* Traditional BSD name for length of interface external name. */ #define IFNAMSIZIF_NAMESIZE /* * Length of interface description, including terminating '\0'. */ #define IFDESCRSIZE 64 +#define IFQ_NQUEUES 8 +#define IFQ_MAXPRIO IFQ_NQUEUES - 1 +#define IFQ_DEFPRIO 3 + +#ifdef _KERNEL +/* + * Structure defining a queue for a network interface. + * XXX keep in sync with struct ifaltq. + */ +struct ifqueue { + struct { + struct mbuf *head; + struct mbuf *tail; + }ifq_q[IFQ_NQUEUES]; + int ifq_len; + int ifq_maxlen; + int ifq_drops; + struct hfsc_if *ifq_hfsc; + struct timeout *ifq_congestion; +}; + +/* + * Structure defining a queue for a network interface. + * + * (Would like to call this struct ``if'', but C isn't PL/1.) + */ +TAILQ_HEAD(ifnet_head, ifnet); /* the actual queue head */ + struct ifnet { /* and the entries */ void*if_softc; /* lower-level data for this if */ TAILQ_ENTRY(ifnet) if_list; /* all struct ifnets are chained */ TAILQ_ENTRY(ifnet) if_txlist; /* list of ifnets ready to tx */ TAILQ_HEAD(, ifaddr) if_addrlist; /* linked list of addresses per if */ @@ -330,10 +330,11 @@ struct ifnet { /* and the entries */ #define if_omcasts if_data.ifi_omcasts #define if_iqdrops if_data.ifi_iqdrops #define if_noproto if_data.ifi_noproto #define if_lastchange if_data.ifi_lastchange #define if_capabilities if_data.ifi_capabilities +#endif /* _KERNEL */
Re: Support for External Random Number Server
Check out /etc/rc, and look for random_seed() and writes into /dev/arandom On 2013 Nov 18 (Mon) at 19:23:55 + (+), Hendrickson, Kenneth wrote: :Use Case : :I have several headless computers. Their only source of randomness is from the network. I also have a hardware true random number generator on another computer. I would like the headless computers to be able to access truly random numbers from a server - at the kernel level. : :I would like a standard, built into the OS, so I get this improved source of randomness right from the very first install. : :I want the random numbers encrypted as they transit the network. Ssh already does this. : :Possible Solutions: : :1 Spawn a userland program or script which uses ssh, obtains the random numbers, and then calls add_true_randomness(). : :2 Configure the kernel with the IP address of the server, and an account name, and the kernel can obtain truly random numbers whenever it wants. : :What is the best way to achieve my goal? : :Thanks, :Ken Hendrickson : : -- Advice to young men: Be ascetic, and if you can't be ascetic, then at least be aseptic.
Re: Support for External Random Number Server
On Mon, Nov 18, 2013 at 07:23:55PM +, Hendrickson, Kenneth wrote: Use Case I have several headless computers. Their only source of randomness is from the network. I also have a hardware true random number generator on another computer. I would like the headless computers to be able to access truly random numbers from a server - at the kernel level. I would like a standard, built into the OS, so I get this improved source of randomness right from the very first install. I want the random numbers encrypted as they transit the network. Ssh already does this. Possible Solutions: 1 Spawn a userland program or script which uses ssh, obtains the random numbers, and then calls add_true_randomness(). 2 Configure the kernel with the IP address of the server, and an account name, and the kernel can obtain truly random numbers whenever it wants. What is the best way to achieve my goal? Thanks, Ken Hendrickson You can already do #1 out of the box, more or less: ssh user@randomhost dd if=/dev/random bs=1024 count=1 /dev/random As a bonus, it's something that you can perform during install after dumping out to the shell temporarily, and stick in a cronjob once the os is installed.
Re: Support for External Random Number Server
I would like a standard, built into the OS, so I get this improved source of randomness right from the very first install. You do get that already. It is not as bad as you might think. Basically the install script dumps 64K of its own randomness to the filesystem for the first boot, just like we do any other reboot. If you think that 64K of data from an install machine has zero entropy to it, you'd be wrong. We admit it is not great entropy. But entropy from the different moments in the reboot cycle do add up to something useable.
Re: CVS: cvs.openbsd.org: src
On Mon, Nov 18, 2013 at 3:19 PM, Ted Unangst t...@tedunangst.com wrote: On Mon, Nov 18, 2013 at 16:10, Ted Unangst wrote: CVSROOT: /cvs Module name: src Changes by: t...@cvs.openbsd.org2013/11/18 16:10:48 Modified files: lib/librthread : rthread.h rthread_sem.c Log message: interprocess semaphores ala sem_open. mostly following in the pattern of shm_open. with some additions and fixes from zhuk. btw, no library version change because the function stubs already existed. Hmm, since this is actually offering new functionality (by sem_open() and friends no longer always failing), I think it a minor bump would be appropriate. Consider that a program with an autoconf test of sem_open() will now return a different answer, just as if sem_open() was completely new. no? Philip Guenther
Re: CVS: cvs.openbsd.org: src
On Mon, Nov 18, 2013 at 3:19 PM, Ted Unangst t...@tedunangst.com wrote: On Mon, Nov 18, 2013 at 16:10, Ted Unangst wrote: CVSROOT: /cvs Module name: src Changes by: t...@cvs.openbsd.org2013/11/18 16:10:48 Modified files: lib/librthread : rthread.h rthread_sem.c Log message: interprocess semaphores ala sem_open. mostly following in the pattern of shm_open. with some additions and fixes from zhuk. btw, no library version change because the function stubs already existed. Hmm, since this is actually offering new functionality (by sem_open() and friends no longer always failing), I think it a minor bump would be appropriate. Consider that a program with an autoconf test of sem_open() will now return a different answer, just as if sem_open() was completely new. no? Well... considering recent library cranks, is that really a concern?
Small patch for fmt(1)
Hi all, clang gives a warning when building fmt(1): fmt.c:400:35: warning: passing 'char *' to parameter of type 'const unsigned char *' converts between pointers to integer types with different sign might_be_header takes a pointer to unsigned char. However there is no reason for this to be the case. The patch bellow fixes that. In addition it adds 'const' to the type of 'line'. [ http://people.freebsd.org/~eadler/files/o/0003.fmt.diff in case the patch below gets mangled ] Index: fmt.c === RCS file: /cvs/src/usr.bin/fmt/fmt.c,v retrieving revision 1.29 diff -u -r1.29 fmt.c --- fmt.c 17 Jan 2012 04:26:28 - 1.29 +++ fmt.c 19 Nov 2013 03:11:08 - @@ -233,7 +233,7 @@ static void process_named_file(const char *); static void process_stream(FILE *, const char *); static size_t indent_length(const char *, size_t); -static int might_be_header(const unsigned char *); +static int might_be_header(const char *); static void new_paragraph(size_t, size_t); static void output_word(size_t, size_t, const char *, size_t, size_t); static void output_indent(size_t); @@ -385,7 +385,7 @@ HdrType header_type; /* ^-- header_type of previous line; -1 at para start */ - char *line; + const char *line; size_t length; if (centerP) { @@ -486,7 +486,7 @@ * conservative to avoid mangling ordinary civilised text. */ static int -might_be_header(const unsigned char *line) +might_be_header(const char *line) { if (!isupper(*line++)) -- Eitan Adler
Re: Small patch for fmt(1)
On Mon, Nov 18, 2013 at 7:16 PM, Eitan Adler li...@eitanadler.com wrote: clang gives a warning when building fmt(1): fmt.c:400:35: warning: passing 'char *' to parameter of type 'const unsigned char *' converts between pointers to integer types with different sign might_be_header takes a pointer to unsigned char. However there is no reason for this to be the case. The patch bellow fixes that. In addition it adds 'const' to the type of 'line'. ... @@ -486,7 +486,7 @@ * conservative to avoid mangling ordinary civilised text. */ static int -might_be_header(const unsigned char *line) +might_be_header(const char *line) { if (!isupper(*line++)) The argument passed to isupper must be The argument to isupper() must be EOF or representable as an unsigned char; otherwise, the result is undefined. That's guaranteed by the current version, but the diff breaks that, so no, this isn't right, unless I missed a check somewhere that tosses all negative chars... Philip Guenther
Re: CVS: cvs.openbsd.org: src
On Mon, Nov 18, 2013 at 18:49, Philip Guenther wrote: btw, no library version change because the function stubs already existed. Hmm, since this is actually offering new functionality (by sem_open() and friends no longer always failing), I think it a minor bump would be appropriate. Consider that a program with an autoconf test of sem_open() will now return a different answer, just as if sem_open() was completely new. no? I hear you, but disagree. We fix program disabling bugs in libraries frequently without bumping. I have always thought of library versioning being more about program integrity, as in all the pieces you expect to find are all there, but it doesn't say anything about the inner workings of the pieces.
Re: disksort() vs bufqs
this one is for hd(4) on hp300s. again, i cant compile and therefore cant test. anyone able to compile this for me? does anyone have such hardware? On Sun, Nov 03, 2013 at 09:30:56AM +1000, David Gwynne wrote: once upon a time pretty much all block storage was on spinning disks, so all the drivers for disks tried to order queued io to better suit moving a head across a platter by calling disksort. between then and now a lot of the assumptions that disksort relied on are no longer true (eg, SSDs are a thing now) so it's usually more harmful than helpful. with beck@s addition of the nscan backend in bufqs it makes sense to try and unify all disk drivers behind bufqs and deprecate disksort. so, ive been working through the tree replacing the last direct users of the disksort() over to bufqs, but ive run out of code that i can compile for. there are now 4 drivers in 3 architectures that need conversion, so im asking if anyone is interested in taking some (or all) of them on. the code in question is: sys/arch/hp300/dev/hd.c sys/arch/sparc/dev/fd.c sys/arch/sparc/dev/xy.c sys/arch/vax/vsa/hdc9224.c if anyone has those archs and a some spare time, please feel free to have a go and cut them over. even if you dont have the hardware to test your change, making a compilable diff and sending it to tech@ for testing is better than i can do right now. cheers, dlg Index: hd.c === RCS file: /cvs/src/sys/arch/hp300/dev/hd.c,v retrieving revision 1.73 diff -u -p -r1.73 hd.c --- hd.c1 Nov 2013 17:36:19 - 1.73 +++ hd.c19 Nov 2013 05:22:44 - @@ -294,6 +294,7 @@ hdattach(parent, self, aux) */ sc-sc_dkdev.dk_name = sc-sc_dev.dv_xname; disk_attach(sc-sc_dev, sc-sc_dkdev); + bufq_init(sc-sc_bufq, BUFQ_DEFAULT); sc-sc_slave = ha-ha_slave; sc-sc_punit = ha-ha_punit; @@ -630,9 +631,9 @@ hdclose(dev, flag, mode, p) if (dk-dk_openmask == 0) { rs-sc_flags |= HDF_CLOSING; s = splbio(); - while (rs-sc_tab.b_active) { + while (rs-sc_bp == NULL) { rs-sc_flags |= HDF_WANTED; - tsleep((caddr_t)rs-sc_tab, PRIBIO, hdclose, 0); + tsleep((caddr_t)rs-sc_bp, PRIBIO, hdclose, 0); } splx(s); rs-sc_flags = ~(HDF_CLOSING); @@ -669,11 +670,11 @@ hdstrategy(bp) if (bounds_check_with_label(bp, rs-sc_dkdev.dk_label) == -1) goto done; + bufq_queue(rs-sc_bufq, bp); + s = splbio(); - dp = rs-sc_tab; - disksort(dp, bp); - if (dp-b_active == 0) { - dp-b_active = 1; + if (rs-b_bp == NULL) { + rs-sc_bp = bufq_dequeue(rs-sc_bufq); hdustart(rs); } splx(s); @@ -710,7 +711,7 @@ hdustart(rs) { struct buf *bp; - bp = rs-sc_tab.b_actf; + bp = rs-sc_bp; rs-sc_addr = bp-b_data; rs-sc_resid = bp-b_bcount; if (hpibreq(rs-sc_dev.dv_parent, rs-sc_hq)) @@ -722,24 +723,23 @@ hdfinish(rs, bp) struct hd_softc *rs; struct buf *bp; { - struct buf *dp = rs-sc_tab; int s; rs-sc_errcnt = 0; - dp-b_actf = bp-b_actf; bp-b_resid = 0; s = splbio(); biodone(bp); splx(s); + hpibfree(rs-sc_dev.dv_parent, rs-sc_hq); - if (dp-b_actf) - return (dp-b_actf); - dp-b_active = 0; - if (rs-sc_flags HDF_WANTED) { + rs-sc_bp = bufq_dequeue(rs-sc_bufq); + + if (rs-sc_bp == NULL rs-sc_flags HDF_WANTED) { rs-sc_flags = ~HDF_WANTED; - wakeup((caddr_t)dp); + wakeup((caddr_t)rs-sc_bp); } - return (NULL); + + return (rs-sc_bp); } void @@ -748,7 +748,7 @@ hdstart(arg) { struct hd_softc *rs = arg; struct disklabel *lp; - struct buf *bp = rs-sc_tab.b_actf; + struct buf *bp = rs-sc_bp; int ctlr, slave; daddr_t bn; @@ -831,7 +831,7 @@ hdgo(arg) void *arg; { struct hd_softc *rs = arg; - struct buf *bp = rs-sc_tab.b_actf; + struct buf *bp = rs-sc_bp; int rw, ctlr, slave; ctlr = rs-sc_dev.dv_parent-dv_unit; @@ -855,7 +855,7 @@ hdinterrupt(arg) { struct hd_softc *rs = arg; int unit = rs-sc_dev.dv_unit; - struct buf *bp = rs-sc_tab.b_actf; + struct buf *bp = rs-sc_bp; u_char stat = 13; /* in case hpibrecv fails */ int rv, restart, ctlr, slave; @@ -1025,7 +1025,7 @@ hderror(unit) * Note that not all errors report a block number, in that case * we just use b_blkno. */ - bp = rs-sc_tab.b_actf; + bp = rs-sc_bp; pbn = DL_GETPOFFSET(rs-sc_dkdev.dk_label-d_partitions[DISKPART(bp-b_dev)]); if
Re: disksort() vs bufqs
this one is for xy(4) on sparc. again, i cant test, so if anyone could do that for me it would be appreciated. does anyone have one of these? Index: xy.c === RCS file: /cvs/src/sys/arch/sparc/dev/xy.c,v retrieving revision 1.57 diff -u -p -r1.57 xy.c --- xy.c1 Nov 2013 17:36:19 - 1.57 +++ xy.c19 Nov 2013 05:54:05 - @@ -508,10 +508,7 @@ xyattach(parent, self, aux) xy-parent = xyc; /* init queue of waiting bufs */ - - xy-xyq.b_active = 0; - xy-xyq.b_actf = 0; - xy-xyq.b_actb = xy-xyq.b_actf; /* XXX b_actb: not used? */ + bufq_init(xy-xy_bufq, BUFQ_DEFAULT); xy-xyrq = xyc-reqs[xa-driveno]; @@ -1003,14 +1000,14 @@ xystrategy(bp) if (bounds_check_with_label(bp, xy-sc_dk.dk_label) == -1) goto done; + bufq_queue(xy-xy_bufq, bp); + /* * now we know we have a valid buf structure that we need to do I/O * on. */ s = splbio(); /* protect the queues */ - disksort(xy-xyq, bp); - /* start 'em up */ xyc_start(xy-parent, NULL); @@ -1604,7 +1601,6 @@ xyc_reset(xycsc, quiet, blastmode, error dvma_mapout((vaddr_t)iorq-dbufbase, (vaddr_t)iorq-buf-b_data, iorq-buf-b_bcount); - iorq-xy-xyq.b_actf = iorq-buf-b_actf; disk_unbusy(xycsc-reqs[lcv].xy-sc_dk, (xycsc-reqs[lcv].buf-b_bcount - xycsc-reqs[lcv].buf-b_resid), @@ -1651,9 +1647,9 @@ xyc_start(xycsc, iorq) if (iorq == NULL) { for (lcv = 0; lcv XYC_MAXDEV ; lcv++) { if ((xy = xycsc-sc_drives[lcv]) == NULL) continue; - if (xy-xyq.b_actf == NULL) continue; + if (!bufq_peek(xy-xy_bufq)) continue; if (xy-xyrq-mode != XY_SUB_FREE) continue; - xyc_startbuf(xycsc, xy, xy-xyq.b_actf); + xyc_startbuf(xycsc, xy, bufq_dequeue(xy-xy_bufq)); } } xyc_submit_iorq(xycsc, iorq, XY_SUB_NOQ); @@ -1783,7 +1779,6 @@ xyc_remove_iorq(xycsc) dvma_mapout((vaddr_t) iorq-dbufbase, (vaddr_t) bp-b_data, bp-b_bcount); - iorq-xy-xyq.b_actf = bp-b_actf; disk_unbusy(iorq-xy-sc_dk, (bp-b_bcount - bp-b_resid), (bp-b_flags B_READ)); Index: xyvar.h === RCS file: /cvs/src/sys/arch/sparc/dev/xyvar.h,v retrieving revision 1.7 diff -u -p -r1.7 xyvar.h --- xyvar.h 5 Jun 2011 18:40:33 - 1.7 +++ xyvar.h 19 Nov 2013 05:54:05 - @@ -115,7 +115,7 @@ struct xy_softc { u_char nsect;/* number of sectors per track */ u_char hw_spt; /* as above, but includes spare sectors */ struct xy_iorq *xyrq; /* this disk's ioreq structure */ - struct buf xyq; /* queue'd I/O requests */ + struct bufq xy_bufq;/* queue'd I/O requests */ struct dkbad dkb;/* bad144 sectors */ };
Re: Small patch for fmt(1)
On Mon, Nov 18, 2013 at 11:09 PM, Philip Guenther guent...@gmail.com wrote: On Mon, Nov 18, 2013 at 7:16 PM, Eitan Adler li...@eitanadler.com wrote: The argument passed to isupper must be The argument to isupper() must be EOF or representable as an unsigned char; otherwise, the result is undefined. That's guaranteed by the current version, but the diff breaks that, so no, this isn't right, unless I missed a check somewhere that tosses all negative chars... Of similar interest: get_line assigns the a value representable by an unsigned char to a buffer of type 'char*'. The following diff avoids the issue while also fixing the warning. An alternative fix would be to change the intermediate 'char* 's to 'unsigned char*'. Index: fmt.c === RCS file: /cvs/src/usr.bin/fmt/fmt.c,v retrieving revision 1.29 diff -u -r1.29 fmt.c --- fmt.c 17 Jan 2012 04:26:28 - 1.29 +++ fmt.c 19 Nov 2013 06:12:28 - @@ -233,7 +233,7 @@ static void process_named_file(const char *); static void process_stream(FILE *, const char *); static size_t indent_length(const char *, size_t); -static int might_be_header(const unsigned char *); +static int might_be_header(const char *); static void new_paragraph(size_t, size_t); static void output_word(size_t, size_t, const char *, size_t, size_t); static void output_indent(size_t); @@ -385,7 +385,7 @@ HdrType header_type; /* ^-- header_type of previous line; -1 at para start */ - char *line; + const char *line; size_t length; if (centerP) { @@ -486,14 +486,14 @@ * conservative to avoid mangling ordinary civilised text. */ static int -might_be_header(const unsigned char *line) +might_be_header(const char *line) { - if (!isupper(*line++)) + if (!isupper((unsigned char)*line++)) return 0; - while (isalnum(*line) || *line == '-') + while (isalnum((unsigned char)*line) || *line == '-') ++line; - return (*line == ':' isspace(line[1])); + return (*line == ':' isspace((unsigned char)line[1])); } /* Begin a new paragraph with an indent of |indent| spaces. -- Eitan Adler
Re: disksort() vs bufqs
and here's the last one, the hdc stuff on vax. can anyone compile this? test? Index: hdc9224.c === RCS file: /cvs/src/sys/arch/vax/vsa/hdc9224.c,v retrieving revision 1.41 diff -u -p -r1.41 hdc9224.c --- hdc9224.c 14 Oct 2013 23:26:22 - 1.41 +++ hdc9224.c 19 Nov 2013 06:43:24 - @@ -134,7 +134,7 @@ struct hdcsoftc { struct evcount sc_intrcnt; struct vsbus_dma sc_vd; vaddr_t sc_regs;/* register addresses */ - struct buf sc_buf_queue; + struct bufq sc_bufq; struct buf *sc_active; struct hdc9224_UDCreg sc_creg; /* (command) registers to be written */ struct hdc9224_UDCreg sc_sreg; /* (status) registers being read */ @@ -387,7 +387,7 @@ hdcintr(void *arg) struct buf *bp; sc-sc_status = HDC_RSTAT; - if (sc-sc_active == 0) + if (sc-sc_active == NULL) return; /* Complain? */ if ((sc-sc_status (DKC_ST_INTPEND | DKC_ST_DONE)) != @@ -395,7 +395,7 @@ hdcintr(void *arg) return; /* Why spurious ints sometimes??? */ bp = sc-sc_active; - sc-sc_active = 0; + sc-sc_active = NULL; if ((sc-sc_status DKC_ST_TERMCOD) != DKC_TC_SUCCESS) { int i; u_char *g = (u_char *)sc-sc_sreg; @@ -455,8 +455,9 @@ hdstrategy(struct buf *bp) if (bounds_check_with_label(bp, hd-sc_disk.dk_label) == -1) goto done; + bufq_queue(sc-sc_bufq, bp); + s = splbio(); - disksort(sc-sc_buf_queue, bp); if (inq == 0) { inq = 1; vsbus_dma_start(sc-sc_vd); @@ -479,8 +480,7 @@ hdc_qstart(void *arg) inq = 0; hdcstart(sc, NULL); - dp = sc-sc_buf_queue; - if (dp-b_actf != NULL) { + if (bufq_peek(sc-sc_bufq)) { vsbus_dma_start(sc-sc_vd); /* More to go */ inq = 1; } @@ -499,15 +499,14 @@ hdcstart(struct hdcsoftc *sc, struct buf splassert(IPL_BIO); - if (sc-sc_active) + if (sc-sc_active != NULL) return; /* Already doing something */ if (ob == NULL) { - dp = sc-sc_buf_queue; - if ((bp = dp-b_actf) == NULL) + bp = bufq_dequeue(sc-sc_bufq); + if (bp == NULL) return; /* Nothing to do */ - dp-b_actf = bp-b_actf; sc-sc_bufaddr = bp-b_data; sc-sc_bytecnt = bp-b_bcount; sc-sc_retries = 0;
Re: CVS: cvs.openbsd.org: src
On Tue, Nov 19, 2013 at 12:11:17AM -0500, Ted Unangst wrote: On Mon, Nov 18, 2013 at 18:49, Philip Guenther wrote: btw, no library version change because the function stubs already existed. Hmm, since this is actually offering new functionality (by sem_open() and friends no longer always failing), I think it a minor bump would be appropriate. Consider that a program with an autoconf test of sem_open() will now return a different answer, just as if sem_open() was completely new. no? I hear you, but disagree. We fix program disabling bugs in libraries frequently without bumping. I have always thought of library versioning being more about program integrity, as in all the pieces you expect to find are all there, but it doesn't say anything about the inner workings of the pieces. As theo says, there are other library bumps later, but you're wrong. Use-case: new packages, slightly older snapshots. New packages actually make use of sem_open, because of said added functionality. Without a bump, pkg_add will allow to add them, and they won't work, because the functionality wasn't there... It is added functionality. It's not a minor bugfix.
Re: CVS: cvs.openbsd.org: src
On Tue, Nov 19, 2013 at 12:11:17AM -0500, Ted Unangst wrote: On Mon, Nov 18, 2013 at 18:49, Philip Guenther wrote: btw, no library version change because the function stubs already existed. Hmm, since this is actually offering new functionality (by sem_open() and friends no longer always failing), I think it a minor bump would be appropriate. Consider that a program with an autoconf test of sem_open() will now return a different answer, just as if sem_open() was completely new. no? I hear you, but disagree. We fix program disabling bugs in libraries frequently without bumping. I have always thought of library versioning being more about program integrity, as in all the pieces you expect to find are all there, but it doesn't say anything about the inner workings of the pieces. As theo says, there are other library bumps later, but you're wrong. Use-case: new packages, slightly older snapshots. New packages actually make use of sem_open, because of said added functionality. Without a bump, pkg_add will allow to add them, and they won't work, because the functionality wasn't there... It is added functionality. It's not a minor bugfix. The current window you are arguying about is maybe from Oct 24, definately Oct 22. For many pkgs, a few other intermediate bumps mattered. A libc crank is coming uhm, rather shortly. Melodrama.