Re: installer: arm64: better disk variable handling
On Sat, Mar 25, 2023 at 07:13:13PM +, Klemens Nanni wrote: > On Sat, Mar 25, 2023 at 07:02:24PM +, Klemens Nanni wrote: > > Call the function argument `_disk' and construct full device paths where > > needed, so that other code can reuse the mnemonic variable. > > > > This matches install.sub's convention around ROOTDISK=sd0, ROOTDEV=sd0a and > > other disk/device code. > > > > Since it touches the installboot block, hoist it before the sysctl block > > which merely assigns `_plat'. > > > > A future diff then turns out much better. > > > - dd if=$_mdec/u-boot-sunxi-with-spl.bin of=${_disk}c \ > > + dd if=$_mdec/u-boot-sunxi-with-spl.bin of=/dev/${_disk}c \ > > Just noticed that this should use the raw device as done elsewhere across > distrib/ and explained in INSTALL.* docs. > > Easily missed before the diff, obvious with it. > > OK? This looks right to me, but I can't test it at the moment. > Index: ramdisk/install.md > === > RCS file: /cvs/src/distrib/arm64/ramdisk/install.md,v > retrieving revision 1.37 > diff -u -p -r1.37 install.md > --- ramdisk/install.md25 Mar 2023 18:29:37 - 1.37 > +++ ramdisk/install.md25 Mar 2023 19:06:43 - > @@ -36,7 +36,13 @@ NCPU=$(sysctl -n hw.ncpufound) > MOUNT_ARGS_msdos="-o-l" > > md_installboot() { > - local _disk=/dev/$1 _mdec _plat > + local _disk=$1 _mdec _plat > + > + if ! installboot -r /mnt $_disk; then > + echo "\nFailed to install bootblocks." > + echo "You will not be able to boot OpenBSD from ${_disk}." > + exit > + fi > > case $(sysctl -n machdep.compatible) in > apple,*)_plat=apple;; > @@ -44,12 +50,6 @@ md_installboot() { > raspberrypi,*) _plat=rpi; > esac > > - if ! installboot -r /mnt ${1}; then > - echo "\nFailed to install bootblocks." > - echo "You will not be able to boot OpenBSD from ${1}." > - exit > - fi > - > # Apply some final tweaks on selected platforms > _mdec=/usr/mdec/$_plat > > @@ -65,11 +65,11 @@ md_installboot() { > fi > ;; > pine64) > - dd if=$_mdec/u-boot-sunxi-with-spl.bin of=${_disk}c \ > + dd if=$_mdec/u-boot-sunxi-with-spl.bin of=/dev/r${_disk}c \ > bs=1024 seek=8 status=none > ;; > rpi) > - mount ${MOUNT_ARGS_msdos} ${_disk}i /mnt/mnt > + mount ${MOUNT_ARGS_msdos} /dev/${_disk}i /mnt/mnt > cp $_mdec/{bootcode.bin,start*.elf,fixup*.dat,*.dtb} /mnt/mnt/ > cp $_mdec/u-boot.bin /mnt/mnt/ > mkdir -p /mnt/mnt/overlays > -- andrew The programmer's national anthem is 'GH!!'.
Re: [installer] default answer to "Is the disk partition already mounted?"
On Wed, Apr 05, 2023 at 11:11:12AM +, Klemens Nanni wrote: > 04.04.2023 19:58, Mikhail пишет: > > In conversation with Klemens (kn@) new iteration has been born, it > > follows to what Omar (op@) has suggested - we're not trying to change > > the default answer for all cases, since no one complained for all these > > years, but just flip default for installation mode, which should save > > some keystrokes and back-and-forth movement for novice users. > > Feeback? > > This is OK kn if anyone wants to commit, else I'll take OKs. OK afresh1@ I can't recall the last time I installed off a disk, but the idea makes sense to me. > > > > Next attempt (with input from kn@): > > > > diff /usr/src > > commit - 3f266bdacd577948d2ae789cfe31cc2939a83cf9 > > path + /usr/src > > blob - 5919eeece326bed27a65b43e96be50e7c72471e1 > > file + distrib/miniroot/install.sub > > --- distrib/miniroot/install.sub > > +++ distrib/miniroot/install.sub > > @@ -2043,7 +2043,12 @@ install_disk() { > > # Install sets from disk. > > # Ask for the disk device containing the set files. > > install_disk() { > > - if ! ask_yn "Is the disk partition already mounted?" yes; then > > + local _ismounted=yes > > + > > + # No partitions are mounted prior to regular installation. > > + [[ $MODE == install ]] && _ismounted=no > > + > > + if ! ask_yn "Is the disk partition already mounted?" $_ismounted; then > > ask_which "disk" "contains the $MODE media" \ > > '$(bsort $(get_dkdevs))' \ > > '$(get_dkdevs_uninitialized)' > > > -- andrew Hey! It compiles! Ship it!
Re: route.8: markup route flags to get tags
05.04.2023 21:42, Jason McIntyre пишет: > i had originally thought to suggest Li (although i hate it). mdoc(7) .Dv says ", constant symbols," which fits well. > anyway, when the tags stuff just works, i don;t have any issues. but > then when we start adding markup just to get it working, it > feels less great. doesn;t just being an .It argument promote the item to > being tagged? (well, i guess not, but then shouldn;t it?) Automatic tagging already works great, my impression is that the number of manual tags is comparatively low; I guess mostly .Tg for too complicated markup like the 'route del[ete]' case. .Dv for those route flags isn't really manual tagging either, but just proper markup for fixed strings/implementation details, of which automatic tagging is a great byproduct. 'grep -r "^\.It [a-z]" /usr/share/man/" shows sentences and compound syntactical elements, for which a tag wouldn't make sense as-is, so I think .It's argument should not be tagged automatically (unless it is another macro like .Cm or .Dv): /usr/share/man/man1/dc.1:.It output base must be a number greater than 1 /usr/share/man/man1/ksh.1:.It unary +
Re: route.8: markup route flags to get tags
On Wed, Apr 05, 2023 at 10:20:40PM +0200, Ingo Schwarze wrote: > Hi Klemens and Jason, > hi. > Jason McIntyre wrote on Wed, Apr 05, 2023 at 07:12:05PM +0100: > > On Wed, Apr 05, 2023 at 01:07:18PM +, Klemens Nanni wrote: > > >> To find out what a particular letter means, you must arrive at this list > >> why generic means because the first column has no markup. > >> > >> As mdoc(7) 'constant symbol' the lookup is much quicker, e.g. > >>man -Otag=h route > >> makes this the first line in the pager: > >>MRTF_MODIFIED Modified dynamically (by redirect). > > > tag=h? is that a typo? > > > > anyway, i personally am not convinced that anyone would do this. > > I use "-O tag" occasionally, in particular with very long and complex > manual pages (and route(8) is complex and somewhat long), and i know > Klemens uses it often. Since implementing -O tag, i occasionally also > got feedback from others who like it, though there are certainly many > people who do not use it. > although i don;t use tags, i wasn;t trying to be dismissive of it for others. it was more that this example felt like overkill (because of the potential number of pages that we'd have to adjust to do this in a meaningful way). > On top of that, the feature is very useful when providing user support > for sending web links like > > https://man.openbsd.org/route.8#show > > which points to the same place as > > $ man -O tag=show route > yes, i get that. but per flag? for every page? > > i mean, it is easier just to type "man route" then page down. > > That works, too, but can become tedious in long and complex pages. > > > is the added complexity really worth it? > > The complexity feels minimal to me, it does nothing more than formally > announcing that these flags serve a syntactic purpose, exactly like we > already do it for the RTF_* flags on the same lines. > i was thinking of the complexity of making this work in general. > While the .Dv macro is not a perfect fit, i agree with Klemens that > it is the best one available. > i had originally thought to suggest Li (although i hate it). anyway, when the tags stuff just works, i don;t have any issues. but then when we start adding markup just to get it working, it feels less great. doesn;t just being an .It argument promote the item to being tagged? (well, i guess not, but then shouldn;t it?) kn, i don;t really want to ok this, but i don;t want to object either if people are genuinely using such stuff. jmc > > we must have a ton of pages with lists explaining flags. > > I see no need to hunt down similar cases in the tree, but when people > get annoyed with specific pages, i think it's fair to add tags > if it's logically sound and easily possible. > > There is one very minor gotcha with short tags like these. > With Klemens' patch, try > > $ man -l -O tag=n route.8 > > which brings you to the -n command line option; then hit the "t" key > once: it brings you to the RTF_CONNECTED ("n") flag... > > So short tags tend to clash with unrelated stuff marked up with > different macros, in this case .Fl and .Dv. > > But that's not a game-breaker. In a long and complex page, ":t nt" > may still be simpler than searching visually. > > I refrained from making the tagging feature more complex and > supporting stuff like "-O tag=Dv=n" vs. "-O tag=Fl=n", and i don't > think that was a mistake. Such clashes are not *that* common, so KISS. > > > like > > man -Otag=D ps > > that doesn;t work either. > > Probably wouldn't hurt either to make that work, but seems neither > urgent nor required to make Klemens' patch useful. > > >> Route flags that now produce an already existing tag don't conflict, i.e. > >> ":tT" first shows > >> -T rtable > >> Select an alternate routing table to modify or query. The > >> and then on second "t" > >>TRTF_MPLS MPLS route. > > Oops, right, you found that too. > > >> OK? > > FWIW, i'm OK with this patch. > > >> Index: route.8 > >> === > >> RCS file: /cvs/src/sbin/route/route.8,v > >> retrieving revision 1.117 > >> diff -u -p -r1.117 route.8 > >> --- route.818 Mar 2023 11:44:53 - 1.117 > >> +++ route.85 Apr 2023 12:56:12 - > >> @@ -401,27 +401,27 @@ Within the output of > >> the "Flags" column indicates what flags are set on the route. > >> The mapping between letters and flags is: > >> .Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1." > >> -.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1." > >> -.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2." > >> -.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3." > >> -.It B Ta Dv RTF_BLACKHOLE Ta "Just discard packets." > >> -.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address." > >> -.It C Ta Dv RTF_CLONING Ta "Generate new routes on use." > >> -.It c
Re: route.8: markup route flags to get tags
Hi Klemens and Jason, Jason McIntyre wrote on Wed, Apr 05, 2023 at 07:12:05PM +0100: > On Wed, Apr 05, 2023 at 01:07:18PM +, Klemens Nanni wrote: >> To find out what a particular letter means, you must arrive at this list >> why generic means because the first column has no markup. >> >> As mdoc(7) 'constant symbol' the lookup is much quicker, e.g. >> man -Otag=h route >> makes this the first line in the pager: >>MRTF_MODIFIED Modified dynamically (by redirect). > tag=h? is that a typo? > > anyway, i personally am not convinced that anyone would do this. I use "-O tag" occasionally, in particular with very long and complex manual pages (and route(8) is complex and somewhat long), and i know Klemens uses it often. Since implementing -O tag, i occasionally also got feedback from others who like it, though there are certainly many people who do not use it. On top of that, the feature is very useful when providing user support for sending web links like https://man.openbsd.org/route.8#show which points to the same place as $ man -O tag=show route > i mean, it is easier just to type "man route" then page down. That works, too, but can become tedious in long and complex pages. > is the added complexity really worth it? The complexity feels minimal to me, it does nothing more than formally announcing that these flags serve a syntactic purpose, exactly like we already do it for the RTF_* flags on the same lines. While the .Dv macro is not a perfect fit, i agree with Klemens that it is the best one available. > we must have a ton of pages with lists explaining flags. I see no need to hunt down similar cases in the tree, but when people get annoyed with specific pages, i think it's fair to add tags if it's logically sound and easily possible. There is one very minor gotcha with short tags like these. With Klemens' patch, try $ man -l -O tag=n route.8 which brings you to the -n command line option; then hit the "t" key once: it brings you to the RTF_CONNECTED ("n") flag... So short tags tend to clash with unrelated stuff marked up with different macros, in this case .Fl and .Dv. But that's not a game-breaker. In a long and complex page, ":t nt" may still be simpler than searching visually. I refrained from making the tagging feature more complex and supporting stuff like "-O tag=Dv=n" vs. "-O tag=Fl=n", and i don't think that was a mistake. Such clashes are not *that* common, so KISS. > like > man -Otag=D ps > that doesn;t work either. Probably wouldn't hurt either to make that work, but seems neither urgent nor required to make Klemens' patch useful. >> Route flags that now produce an already existing tag don't conflict, i.e. >> ":tT" first shows >> -T rtable >> Select an alternate routing table to modify or query. The >> and then on second "t" >>TRTF_MPLS MPLS route. Oops, right, you found that too. >> OK? FWIW, i'm OK with this patch. >> Index: route.8 >> === >> RCS file: /cvs/src/sbin/route/route.8,v >> retrieving revision 1.117 >> diff -u -p -r1.117 route.8 >> --- route.8 18 Mar 2023 11:44:53 - 1.117 >> +++ route.8 5 Apr 2023 12:56:12 - >> @@ -401,27 +401,27 @@ Within the output of >> the "Flags" column indicates what flags are set on the route. >> The mapping between letters and flags is: >> .Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1." >> -.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1." >> -.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2." >> -.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3." >> -.It B Ta Dv RTF_BLACKHOLE Ta "Just discard packets." >> -.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address." >> -.It C Ta Dv RTF_CLONING Ta "Generate new routes on use." >> -.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)." >> -.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)." >> -.It G Ta Dv RTF_GATEWAY Ta "Dest requires forwarding by intermediary." >> -.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)." >> -.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route." >> -.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation." >> -.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address." >> -.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)." >> -.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address." >> -.It n Ta Dv RTF_CONNECTED Ta "Interface route." >> -.It P Ta Dv RTF_MPATH Ta "Multipath route." >> -.It R Ta Dv RTF_REJECT Ta "Host or net unreachable." >> -.It S Ta Dv RTF_STATIC Ta "Manually added." >> -.It T Ta Dv RTF_MPLS Ta "MPLS route." >> -.It U Ta Dv RTF_UP Ta "Route usable." >> +.It Dv 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1." >> +.It Dv 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2." >> +.It Dv 3 Ta Dv RTF_PROTO3 Ta "Protocol
arp nd6 queue length sysctl
Hi, ARP has a sysctl to show the number of packets waiting for the arp response. ND6 should have the same to reduce places where mbufs can hide within the kernel. Atomic operations operate on unsigned int. Make the type of total hold queue length consistent. Use atomic load to read the value for the sysctl. That does not change much as int reads are always atomic, but it makes clear that we don't need a lock around sysctl_rdint(). ok? bluhm Index: netinet/icmp6.h === RCS file: /cvs/src/sys/netinet/icmp6.h,v retrieving revision 1.51 diff -u -p -r1.51 icmp6.h --- netinet/icmp6.h 11 Jan 2021 13:28:53 - 1.51 +++ netinet/icmp6.h 5 Apr 2023 19:38:25 - @@ -504,7 +504,8 @@ struct icmp6stat { #define ICMPV6CTL_REDIRTIMEOUT 3 /* redirect cache time */ #define ICMPV6CTL_ND6_DELAY8 #define ICMPV6CTL_ND6_UMAXTRIES9 -#define ICMPV6CTL_ND6_MMAXTRIES10 +#define ICMPV6CTL_ND6_MMAXTRIES10 +#define ICMPV6CTL_ND6_QUEUED 11 #define ICMPV6CTL_NODEINFO 13 #define ICMPV6CTL_ERRPPSLIMIT 14 /* ICMPv6 error pps limitation */ #define ICMPV6CTL_ND6_MAXNUDHINT 15 @@ -525,7 +526,7 @@ struct icmp6stat { { "nd6_delay", CTLTYPE_INT }, \ { "nd6_umaxtries", CTLTYPE_INT }, \ { "nd6_mmaxtries", CTLTYPE_INT }, \ - { 0, 0 }, \ + { "nd6_queued", CTLTYPE_INT }, \ { 0, 0 }, \ { 0, 0 }, \ { "errppslimit", CTLTYPE_INT }, \ Index: netinet/if_ether.c === RCS file: /cvs/src/sys/netinet/if_ether.c,v retrieving revision 1.259 diff -u -p -r1.259 if_ether.c --- netinet/if_ether.c 5 Apr 2023 19:35:23 - 1.259 +++ netinet/if_ether.c 5 Apr 2023 19:38:25 - @@ -109,7 +109,7 @@ LIST_HEAD(, llinfo_arp) arp_list = LIST_HEAD_INITIALIZER(arp_list); /* [mN] list of llinfo_arp structures */ struct pool arp_pool; /* [I] pool for llinfo_arp structures */ intarp_maxtries = 5; /* [I] arp requests before set to rejected */ -intla_hold_total; /* [a] packets currently in the arp queue */ +unsigned int la_hold_total; /* [a] packets currently in the arp queue */ #ifdef NFSCLIENT /* revarp state */ Index: netinet/ip_input.c === RCS file: /cvs/src/sys/netinet/ip_input.c,v retrieving revision 1.382 diff -u -p -r1.382 ip_input.c --- netinet/ip_input.c 8 Mar 2023 23:17:02 - 1.382 +++ netinet/ip_input.c 5 Apr 2023 19:38:26 - @@ -1698,7 +1698,8 @@ ip_sysctl(int *name, u_int namelen, void return (sysctl_niq(name + 1, namelen - 1, oldp, oldlenp, newp, newlen, )); case IPCTL_ARPQUEUED: - return (sysctl_rdint(oldp, oldlenp, newp, la_hold_total)); + return (sysctl_rdint(oldp, oldlenp, newp, + atomic_load_int(_hold_total))); case IPCTL_STATS: return (ip_sysctl_ipstat(oldp, oldlenp, newp)); #ifdef MROUTING Index: netinet/ip_var.h === RCS file: /cvs/src/sys/netinet/ip_var.h,v retrieving revision 1.108 diff -u -p -r1.108 ip_var.h --- netinet/ip_var.h17 Nov 2022 18:05:43 - 1.108 +++ netinet/ip_var.h5 Apr 2023 19:38:26 - @@ -217,7 +217,7 @@ extern int ipforwarding;/* enable IP f extern int ipmforwarding; /* enable multicast forwarding */ #endif extern int ipmultipath;/* enable multipath routing */ -extern int la_hold_total; +extern unsigned int la_hold_total; extern const struct pr_usrreqs rip_usrreqs; Index: netinet6/icmp6.c === RCS file: /cvs/src/sys/netinet6/icmp6.c,v retrieving revision 1.247 diff -u -p -r1.247 icmp6.c --- netinet6/icmp6.c10 Dec 2022 23:45:51 - 1.247 +++ netinet6/icmp6.c5 Apr 2023 19:38:26 - @@ -1902,6 +1902,11 @@ icmp6_sysctl(int *name, u_int namelen, v error = icmp6_sysctl_icmp6stat(oldp, oldlenp, newp); break; + case ICMPV6CTL_ND6_QUEUED: + error = sysctl_rdint(oldp, oldlenp, newp, + atomic_load_int(_hold_total)); + break; + default: NET_LOCK(); error = sysctl_bounded_arr(icmpv6ctl_vars, Index: netinet6/nd6.c === RCS file: /cvs/src/sys/netinet6/nd6.c,v retrieving revision 1.270 diff -u -p -r1.270 nd6.c --- netinet6/nd6.c 5 Apr 2023 19:35:23 - 1.270 +++ netinet6/nd6.c 5 Apr 2023 19:38:26 - @@ -87,7 +87,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; -intln_hold_total; /* [a] packets
Re: route.8: markup route flags to get tags
05.04.2023 18:12, Jason McIntyre пишет: > tag=h? is that a typo? Yes. > anyway, i personally am not convinced that anyone would do this. > i mean, it is easier just to type "man route" then page down. I use tags extensively and think they're both easier and faster if you already know what to look for. 'man route' then page down in whatever way still means you need to skim over the content and the size of the steps, i.e. the amount of mental overhead, depends on your window size. 'man route' and ":twhatever" brings you immediately to the thing you need, quick "q" and you're back to work with minimal distraction. > is the added complexity really worth it? we must have a ton of pages > with lists explaining flags. like > > man -Otag=D ps > > that doesn;t work either. Complexity seems low and is a one-time thing, which yields better accessibility not just in interative manual reading. There are certainly more pages, but manual tagging should still be done on a per-case basis. Your ps(1) example is quite similar, but I personally look way more often at 'route show' output than I do at 'ps -o state'.
Re: route.8: markup route flags to get tags
On Wed, Apr 05, 2023 at 01:07:18PM +, Klemens Nanni wrote: > To find out what a particular letter means, you must arrive at this list > why generic means because the first column has no markup. > > As mdoc(7) 'constant symbol' the lookup is much quicker, e.g. > man -Otag=h route > makes this the first line in the pager: >MRTF_MODIFIED Modified dynamically (by redirect). > tag=h? is that a typo? anyway, i personally am not convinced that anyone would do this. i mean, it is easier just to type "man route" then page down. is the added complexity really worth it? we must have a ton of pages with lists explaining flags. like man -Otag=D ps that doesn;t work either. jmc > Route flags that now produce an already existing tag don't conflict, i.e. > ":tT" first shows > -T rtable > Select an alternate routing table to modify or query. The > and then on second "t" >TRTF_MPLS MPLS route. > > > OK? > > Index: route.8 > === > RCS file: /cvs/src/sbin/route/route.8,v > retrieving revision 1.117 > diff -u -p -r1.117 route.8 > --- route.8 18 Mar 2023 11:44:53 - 1.117 > +++ route.8 5 Apr 2023 12:56:12 - > @@ -401,27 +401,27 @@ Within the output of > the "Flags" column indicates what flags are set on the route. > The mapping between letters and flags is: > .Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1." > -.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1." > -.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2." > -.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3." > -.It B Ta Dv RTF_BLACKHOLE Ta "Just discard packets." > -.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address." > -.It C Ta Dv RTF_CLONING Ta "Generate new routes on use." > -.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)." > -.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)." > -.It G Ta Dv RTF_GATEWAY Ta "Dest requires forwarding by intermediary." > -.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)." > -.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route." > -.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation." > -.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address." > -.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)." > -.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address." > -.It n Ta Dv RTF_CONNECTED Ta "Interface route." > -.It P Ta Dv RTF_MPATH Ta "Multipath route." > -.It R Ta Dv RTF_REJECT Ta "Host or net unreachable." > -.It S Ta Dv RTF_STATIC Ta "Manually added." > -.It T Ta Dv RTF_MPLS Ta "MPLS route." > -.It U Ta Dv RTF_UP Ta "Route usable." > +.It Dv 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1." > +.It Dv 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2." > +.It Dv 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3." > +.It Dv B Ta Dv RTF_BLACKHOLE Ta "Just discard packets." > +.It Dv b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address." > +.It Dv C Ta Dv RTF_CLONING Ta "Generate new routes on use." > +.It Dv c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)." > +.It Dv D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)." > +.It Dv G Ta Dv RTF_GATEWAY Ta "Dest requires forwarding by intermediary." > +.It Dv H Ta Dv RTF_HOST Ta "Host entry (net otherwise)." > +.It Dv h Ta Dv RTF_CACHED Ta "Referenced by gateway route." > +.It Dv L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation." > +.It Dv l Ta Dv RTF_LOCAL Ta "Correspond to a local address." > +.It Dv M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)." > +.It Dv m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address." > +.It Dv n Ta Dv RTF_CONNECTED Ta "Interface route." > +.It Dv P Ta Dv RTF_MPATH Ta "Multipath route." > +.It Dv R Ta Dv RTF_REJECT Ta "Host or net unreachable." > +.It Dv S Ta Dv RTF_STATIC Ta "Manually added." > +.It Dv T Ta Dv RTF_MPLS Ta "MPLS route." > +.It Dv U Ta Dv RTF_UP Ta "Route usable." > .El > .Pp > .It Xo >
route.8: markup route flags to get tags
To find out what a particular letter means, you must arrive at this list why generic means because the first column has no markup. As mdoc(7) 'constant symbol' the lookup is much quicker, e.g. man -Otag=h route makes this the first line in the pager: MRTF_MODIFIED Modified dynamically (by redirect). Route flags that now produce an already existing tag don't conflict, i.e. ":tT" first shows -T rtable Select an alternate routing table to modify or query. The and then on second "t" TRTF_MPLS MPLS route. OK? Index: route.8 === RCS file: /cvs/src/sbin/route/route.8,v retrieving revision 1.117 diff -u -p -r1.117 route.8 --- route.8 18 Mar 2023 11:44:53 - 1.117 +++ route.8 5 Apr 2023 12:56:12 - @@ -401,27 +401,27 @@ Within the output of the "Flags" column indicates what flags are set on the route. The mapping between letters and flags is: .Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1." -.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1." -.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2." -.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3." -.It B Ta Dv RTF_BLACKHOLE Ta "Just discard packets." -.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address." -.It C Ta Dv RTF_CLONING Ta "Generate new routes on use." -.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)." -.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)." -.It G Ta Dv RTF_GATEWAY Ta "Dest requires forwarding by intermediary." -.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)." -.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route." -.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation." -.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address." -.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)." -.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address." -.It n Ta Dv RTF_CONNECTED Ta "Interface route." -.It P Ta Dv RTF_MPATH Ta "Multipath route." -.It R Ta Dv RTF_REJECT Ta "Host or net unreachable." -.It S Ta Dv RTF_STATIC Ta "Manually added." -.It T Ta Dv RTF_MPLS Ta "MPLS route." -.It U Ta Dv RTF_UP Ta "Route usable." +.It Dv 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1." +.It Dv 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2." +.It Dv 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3." +.It Dv B Ta Dv RTF_BLACKHOLE Ta "Just discard packets." +.It Dv b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address." +.It Dv C Ta Dv RTF_CLONING Ta "Generate new routes on use." +.It Dv c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)." +.It Dv D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)." +.It Dv G Ta Dv RTF_GATEWAY Ta "Dest requires forwarding by intermediary." +.It Dv H Ta Dv RTF_HOST Ta "Host entry (net otherwise)." +.It Dv h Ta Dv RTF_CACHED Ta "Referenced by gateway route." +.It Dv L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation." +.It Dv l Ta Dv RTF_LOCAL Ta "Correspond to a local address." +.It Dv M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)." +.It Dv m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address." +.It Dv n Ta Dv RTF_CONNECTED Ta "Interface route." +.It Dv P Ta Dv RTF_MPATH Ta "Multipath route." +.It Dv R Ta Dv RTF_REJECT Ta "Host or net unreachable." +.It Dv S Ta Dv RTF_STATIC Ta "Manually added." +.It Dv T Ta Dv RTF_MPLS Ta "MPLS route." +.It Dv U Ta Dv RTF_UP Ta "Route usable." .El .Pp .It Xo
Re: fill out more rk356x dwqe phy-mode handling
> Date: Wed, 05 Apr 2023 09:09:22 +0200 > From: Mark Kettenis > > > Date: Wed, 5 Apr 2023 16:09:21 +1000 > > From: David Gwynne > > > > On Tue, Apr 04, 2023 at 10:59:56PM +1000, David Gwynne wrote: > > > > > > > > > > On 4 Apr 2023, at 20:37, Mark Kettenis wrote: > > > > > > > >> Date: Tue, 4 Apr 2023 09:49:40 +1000 > > > >> From: David Gwynne > > > >> > > > >> i did this when i was trying to figure out why TX wasn't working on the > > > >> nanopi r5s before figuring out that problem was because we didn't have > > > >> rkiovd. > > > >> > > > >> at the very least it should future proof dwqe against more phy setups, > > > >> and provides a decent example of how to interpret those fdt properties. > > > >> > > > >> ok? > > > > > > > > Oh wow. I always thought those properties are purely about enabling > > > > the internal delays of the PHY and came in addition to the delays > > > > configured in the MAC. But you're right, that isn't what the device > > > > tree bindings say and isn't what the Linux driver implements. So this > > > > is indeed the right thing to do. And it will make the rock-3a work > > > > once I correctly implement configuring the internal delay for > > > > rgephy(4). > > > > > > > > That said, I think that once again you are doing too much DT checking. > > > > Many of the options you're now erroring out on are optional. And > > > > there actually is a reasonable chance that the interface will work if > > > > they're absent, especially if the firmware already initializes the > > > > SoC-specific GRF registers. I don't think we should even print any > > > > warnings. > > > > > > having stared inside some boot loaders recently i have mixed feelings > > > about this. i guess they can only get better from here though, right? > > > > > > > If you make dwqe_rockchip_setup() specific to the rk3568 (no objection > > > > to that), you should probably rename it to dwqe_rk3568_setup(). > > > > > > alright. > > > > > > > And a style issue; please don't add parentheses around trivial > > > > function return values in code that doesn't already use that style. > > > > We moved away from that style years ago... > > > > > > yeah, sorry. muscle memory. > > > > > > i'll fix these up tomorrow and send it around again. > > > > this still works for me on the r5s. e25 doesnt use onboard gmac. > > Just realized that this risks leaving the task uninitialized. But > that risk is already there. > > ok kettenis@ BTW, I have an almost complete diff to wire up the MIIF_RXID/MIIF_TXID flags stuff in dwqe(4) that I'll finish once this diff is in. > > Index: if_dwqe_fdt.c > > === > > RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v > > retrieving revision 1.5 > > diff -u -p -r1.5 if_dwqe_fdt.c > > --- if_dwqe_fdt.c 26 Mar 2023 21:44:46 - 1.5 > > +++ if_dwqe_fdt.c 5 Apr 2023 06:06:57 - > > @@ -63,7 +63,7 @@ > > > > intdwqe_fdt_match(struct device *, void *, void *); > > void dwqe_fdt_attach(struct device *, struct device *, void *); > > -void dwqe_setup_rockchip(struct dwqe_softc *); > > +void dwqe_setup_rk3568(struct dwqe_softc *); > > void dwqe_mii_statchg_rk3568(struct device *); > > void dwqe_mii_statchg_rk3588(struct device *); > > > > @@ -138,7 +138,7 @@ dwqe_fdt_attach(struct device *parent, s > > > > /* Do hardware specific initializations. */ > > if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac")) > > - dwqe_setup_rockchip(sc); > > + dwqe_setup_rk3568(sc); > > > > /* Power up PHY. */ > > phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0); > > @@ -266,41 +266,62 @@ dwqe_reset_phy(struct dwqe_softc *sc, ui > > #define RK3568_GRF_GMACx_CON1(x) (0x0384 + (x) * 0x8) > > #define RK3568_GMAC_PHY_INTF_SEL_RGMII((0x7 << 4) << 16 | > > (0x1 << 4)) > > #define RK3568_GMAC_PHY_INTF_SEL_RMII ((0x7 << 4) << 16 | > > (0x4 << 4)) > > -#define RK3568_GMAC_TXCLK_DLY_ENA ((1 << 0) << 16 | (1 << 0)) > > -#define RK3568_GMAC_RXCLK_DLY_ENA ((1 << 1) << 16 | (1 << 1)) > > +#define RK3568_GMAC_TXCLK_DLY_SET(_v) ((1 << 0) << 16 | ((_v) > > << 0)) > > +#define RK3568_GMAC_RXCLK_DLY_SET(_v) ((1 << 1) << 16 | ((_v) > > << 1)) > > > > void dwqe_mii_statchg_rk3568_task(void *); > > > > void > > -dwqe_setup_rockchip(struct dwqe_softc *sc) > > +dwqe_setup_rk3568(struct dwqe_softc *sc) > > { > > + char phy_mode[32]; > > struct regmap *rm; > > uint32_t grf; > > int tx_delay, rx_delay; > > + uint32_t iface; > > > > grf = OF_getpropint(sc->sc_node, "rockchip,grf", 0); > > rm = regmap_byphandle(grf); > > if (rm == NULL) > > return; > > > > + if (OF_getprop(sc->sc_node, "phy-mode", > > + phy_mode, sizeof(phy_mode)) <= 0) > > + return; > > + > > tx_delay = OF_getpropint(sc->sc_node,
Re: [installer] default answer to "Is the disk partition already mounted?"
04.04.2023 19:58, Mikhail пишет: > In conversation with Klemens (kn@) new iteration has been born, it > follows to what Omar (op@) has suggested - we're not trying to change > the default answer for all cases, since no one complained for all these > years, but just flip default for installation mode, which should save > some keystrokes and back-and-forth movement for novice users. Feeback? This is OK kn if anyone wants to commit, else I'll take OKs. > > Next attempt (with input from kn@): > > diff /usr/src > commit - 3f266bdacd577948d2ae789cfe31cc2939a83cf9 > path + /usr/src > blob - 5919eeece326bed27a65b43e96be50e7c72471e1 > file + distrib/miniroot/install.sub > --- distrib/miniroot/install.sub > +++ distrib/miniroot/install.sub > @@ -2043,7 +2043,12 @@ install_disk() { > # Install sets from disk. > # Ask for the disk device containing the set files. > install_disk() { > - if ! ask_yn "Is the disk partition already mounted?" yes; then > + local _ismounted=yes > + > + # No partitions are mounted prior to regular installation. > + [[ $MODE == install ]] && _ismounted=no > + > + if ! ask_yn "Is the disk partition already mounted?" $_ismounted; then > ask_which "disk" "contains the $MODE media" \ > '$(bsort $(get_dkdevs))' \ > '$(get_dkdevs_uninitialized)' >
Re: acpithinkpad: don't setup non-existent temp sensors
On Tue, Apr 04, 2023 at 10:45:53PM -0500, joshua stein wrote: > acpithinkpad sets up a hard-coded number of temperature sensors and > doesn't check the result of aml_evalinteger when polling, so for all > invalid sensors it ends up reporting the value of the previous > successful sensor check resulting in this (for a machine with only a > TMP0 sensor): I see this on a X230 with coreboot: hw.sensors.acpithinkpad0.temp0=66.00 degC hw.sensors.acpithinkpad0.temp1=0.00 degC hw.sensors.acpithinkpad0.temp2=0.00 degC hw.sensors.acpithinkpad0.temp3=0.00 degC hw.sensors.acpithinkpad0.temp4=0.00 degC hw.sensors.acpithinkpad0.temp5=0.00 degC hw.sensors.acpithinkpad0.temp6=0.00 degC hw.sensors.acpithinkpad0.temp7=0.00 degC hw.sensors.acpithinkpad0.fan0=4326 RPM hw.sensors.acpithinkpad0.indicator0=Off (port replicator), UNKNOWN (It's in a proper dock, but .indicator0 always says "Off"; I think this used to work...) > This diff checks whether the TMP[0-8] node actually exists during > attach and if not, it doesn't create the sensor. It also checks the > return value during polling and sets the sensor to invalid if it > failed for some reason. hw.sensors.acpithinkpad0.temp0=65.00 degC hw.sensors.acpithinkpad0.temp1=0.00 degC hw.sensors.acpithinkpad0.fan0=4326 RPM hw.sensors.acpithinkpad0.indicator0=Off (port replicator), UNKNOWN Works for me, but looks like temp1 is actually a second sensor which doesn't work? Definitely warmer around here. Do you want me to test anything?
MALLOC_STATS: dump internal state and leak info via utrace(2)
Hi, This is work in progress. I have to think if the flags to kdump I'm introducing should be two or a single one. Currently, malloc.c can be compiled with MALLOC_STATS defined. If run with option D it dumps its state to a malloc.out file at exit. This state can be used to find leaks amongst other things. This is not ideal for pledged processes, as they often have no way to write files. This changes malloc to use utrace(2) for that. As kdump has no nice way to show those lines without all extras it normally shows, so add two options to it to just show the lines. To use, compile and install libc with MALLOC_STATS defined. Run : $ MALLOC_OPTIONS=D ktrace -tu your_program ... $ kdump -hu Feedback appreciated. -Otto Index: lib/libc/stdlib/malloc.c === RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v retrieving revision 1.280 diff -u -p -r1.280 malloc.c --- lib/libc/stdlib/malloc.c5 Apr 2023 06:25:38 - 1.280 +++ lib/libc/stdlib/malloc.c5 Apr 2023 08:23:42 - @@ -23,7 +23,7 @@ * can buy me a beer in return. Poul-Henning Kamp */ -/* #define MALLOC_STATS */ +#define MALLOC_STATS #include #include @@ -39,7 +39,9 @@ #include #ifdef MALLOC_STATS +#include #include +#include #include #endif @@ -243,10 +245,8 @@ static __dead void wrterror(struct dir_i __attribute__((__format__ (printf, 2, 3))); #ifdef MALLOC_STATS -void malloc_dump(int, int, struct dir_info *); +void malloc_dump(void); PROTO_NORMAL(malloc_dump); -void malloc_gdump(int); -PROTO_NORMAL(malloc_gdump); static void malloc_exit(void); #define CALLER __builtin_return_address(0) #else @@ -319,7 +319,7 @@ wrterror(struct dir_info *d, char *msg, #ifdef MALLOC_STATS if (mopts.malloc_stats) - malloc_gdump(STDERR_FILENO); + malloc_dump(); #endif /* MALLOC_STATS */ errno = saved_errno; @@ -2225,6 +2225,21 @@ aligned_alloc(size_t alignment, size_t s #ifdef MALLOC_STATS +static void +ulog(const char *format, ...) +{ + va_list ap; + char buf[100]; + int len; + + va_start(ap, format); + len = vsnprintf(buf, sizeof(buf), format, ap); + if (len > (int)sizeof(buf)) + len = sizeof(buf); + utrace("mallocdumpline", buf, len); + va_end(ap); +} + struct malloc_leak { void *f; size_t total_size; @@ -2280,20 +2295,20 @@ putleakinfo(void *f, size_t sz, int cnt) static struct malloc_leak *malloc_leaks; static void -dump_leaks(int fd) +dump_leaks(void) { struct leaknode *p; unsigned int i = 0; - dprintf(fd, "Leak report\n"); - dprintf(fd, " f sum #avg\n"); + ulog("Leak report\n"); + ulog(" f sum #avg\n"); /* XXX only one page of summary */ if (malloc_leaks == NULL) malloc_leaks = MMAP(MALLOC_PAGESIZE, 0); if (malloc_leaks != MAP_FAILED) memset(malloc_leaks, 0, MALLOC_PAGESIZE); RBT_FOREACH(p, leaktree, ) { - dprintf(fd, "%18p %7zu %6u %6zu\n", p->d.f, + ulog("%18p %7zu %6u %6zu\n", p->d.f, p->d.total_size, p->d.count, p->d.total_size / p->d.count); if (malloc_leaks == MAP_FAILED || i >= MALLOC_PAGESIZE / sizeof(struct malloc_leak)) @@ -2306,10 +2321,10 @@ dump_leaks(int fd) } static void -dump_chunk(int fd, struct chunk_info *p, void *f, int fromfreelist) +dump_chunk(struct chunk_info *p, void *f, int fromfreelist) { while (p != NULL) { - dprintf(fd, "chunk %18p %18p %4zu %d/%d\n", + ulog("chunk %18p %18p %4zu %d/%d\n", p->page, ((p->bits[0] & 1) ? NULL : f), B2SIZE(p->bucket), p->free, p->total); if (!fromfreelist) { @@ -2325,17 +2340,17 @@ dump_chunk(int fd, struct chunk_info *p, } p = LIST_NEXT(p, entries); if (p != NULL) - dprintf(fd, ""); + ulog(""); } } static void -dump_free_chunk_info(int fd, struct dir_info *d) +dump_free_chunk_info(struct dir_info *d) { int i, j, count; struct chunk_info *p; - dprintf(fd, "Free chunk structs:\n"); + ulog("Free chunk structs:\n"); for (i = 0; i <= BUCKETS; i++) { count = 0; LIST_FOREACH(p, >chunk_info_list[i], entries) @@ -2345,99 +2360,97 @@ dump_free_chunk_info(int fd, struct dir_ if (p == NULL && count == 0) continue; if (j == 0) - dprintf(fd, "%3d) %3d ", i, count); + ulog("%3d) %3d ", i, count); else - dprintf(fd, " "); +
Re: bgpd allow service name for ports
On Wed, Apr 05, 2023 at 10:09:46AM +0200, Claudio Jeker wrote: > Allow to use service names like 'bgp' for port definitions. > Adapted from pfctl/parse.y. [...] > +static int > +getservice(char *n) > +{ > + struct servent *s; > + > + s = getservbyname(n, "tcp"); > + if (s == NULL) > + s = getservbyname(n, "udp"); > + if (s == NULL) > + return (-1); I'd omit the parentheses here to keep the style in this file consistent on a per-function level. ok > + return s->s_port; > }
bgpd allow service name for ports
Allow to use service names like 'bgp' for port definitions. Adapted from pfctl/parse.y. -- :wq Claudio ? obj Index: parse.y === RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v retrieving revision 1.445 diff -u -p -r1.445 parse.y --- parse.y 5 Apr 2023 08:04:28 - 1.445 +++ parse.y 5 Apr 2023 08:05:14 - @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -161,6 +162,7 @@ static void add_roa_set(struct prefixse static struct rtr_config *get_rtr(struct bgpd_addr *); static int insert_rtr(struct rtr_config *); static int merge_aspa_set(uint32_t, struct aspa_tas_l *, time_t); +static int getservice(char *); static struct bgpd_config *conf; static struct network_head *netconf; @@ -247,6 +249,7 @@ typedef struct { %typeyesno inout restricted expires enforce %typevalidity aspa_validity %typeaddpathextra addpathmax +%typeport %typestring %type address %typeprefix addrspec @@ -692,12 +695,7 @@ rtropt : DESCR STRING { } currtr->local_addr = $2; } - | PORT NUMBER { - if ($2 < 1 || $2 > USHRT_MAX) { - yyerror("port must be between %u and %u", - 1, USHRT_MAX); - YYERROR; - } + | PORT port { currtr->remote_port = $2; } ; @@ -750,16 +748,10 @@ conf_main : AS as4number { memcpy(>sa, sa, la->sa_len); TAILQ_INSERT_TAIL(conf->listen_addrs, la, entry); } - | LISTEN ON address PORT NUMBER { + | LISTEN ON address PORT port { struct listen_addr *la; struct sockaddr *sa; - if ($5 < 1 || $5 > USHRT_MAX) { - yyerror("port must be between %u and %u", - 1, USHRT_MAX); - YYERROR; - } - if ((la = calloc(1, sizeof(struct listen_addr))) == NULL) fatal("parse conf_main listen on calloc"); @@ -1147,6 +1139,24 @@ network : NETWORK prefix filter_set { } ; +port : NUMBER{ + if ($1 < 1 || $1 > USHRT_MAX) { + yyerror("port must be between %u and %u", + 1, USHRT_MAX); + YYERROR; + } + $$ = $1; + } + | STRING{ + if (($$ = getservice($1)) == -1) { + yyerror("unknown port '%s'", $1); + free($1); + YYERROR; + } + free($1); + } + ; + inout : IN{ $$ = 1; } | OUT { $$ = 0; } ; @@ -1954,12 +1964,7 @@ peeropts : REMOTEAS as4number{ else curpeer->conf.flags &= ~PEERFLAG_NO_AS_SET; } - | PORT NUMBER { - if ($2 < 1 || $2 > USHRT_MAX) { - yyerror("port must be between %u and %u", - 1, USHRT_MAX); - YYERROR; - } + | PORT port { curpeer->conf.remote_port = $2; } | RDE EVALUATE STRING { @@ -5151,4 +5156,17 @@ merge_aspa_set(uint32_t as, struct aspa_ aspa->expires = expires; return 0; +} + +static int +getservice(char *n) +{ + struct servent *s; + + s = getservbyname(n, "tcp"); + if (s == NULL) + s = getservbyname(n, "udp"); + if (s == NULL) + return (-1); + return s->s_port; }
bgpd more parse.y cleanup
Rename family rule to af (more in line with pfctl parse.y). -- :wq Claudio Index: parse.y === RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v retrieving revision 1.443 diff -u -p -r1.443 parse.y --- parse.y 3 Apr 2023 10:48:00 - 1.443 +++ parse.y 5 Apr 2023 07:42:53 - @@ -243,7 +243,7 @@ typedef struct { %token STRING %token NUMBER %typeasnumber as4number as4number_any optnumber -%typeespah family safi restart origincode nettype +%typeespah af safi restart origincode nettype %typeyesno inout restricted expires enforce %typevalidity aspa_validity %typeaddpathextra addpathmax @@ -637,7 +637,7 @@ aspa_tas: as4number_any { $$->aid = AID_UNSPEC; $$->num = 1; } - | as4number_any family { + | as4number_any af { if (($$ = calloc(1, sizeof(*$$))) == NULL) fatal(NULL); $$->as = $1; @@ -1085,14 +1085,14 @@ network : NETWORK prefix filter_set { free($3); free($4); } - | NETWORK family RTLABEL STRING filter_set { + | NETWORK af RTLABEL STRING filter_set { struct network *n; if ((n = calloc(1, sizeof(struct network))) == NULL) fatal("new_network"); if (afi2aid($2, SAFI_UNICAST, >net.prefix.aid) == -1) { - yyerror("unknown family"); + yyerror("unknown address family"); filterset_free($5); free($5); YYERROR; @@ -1104,7 +1104,7 @@ network : NETWORK prefix filter_set { TAILQ_INSERT_TAIL(netconf, n, entry); } - | NETWORK family PRIORITY NUMBER filter_set { + | NETWORK af PRIORITY NUMBER filter_set { struct network *n; if (!kr_check_prio($4)) { yyerror("priority %lld out of range", $4); @@ -1115,7 +1115,7 @@ network : NETWORK prefix filter_set { fatal("new_network"); if (afi2aid($2, SAFI_UNICAST, >net.prefix.aid) == -1) { - yyerror("unknown family"); + yyerror("unknown address family"); filterset_free($5); free($5); YYERROR; @@ -1127,14 +1127,14 @@ network : NETWORK prefix filter_set { TAILQ_INSERT_TAIL(netconf, n, entry); } - | NETWORK family nettype filter_set { + | NETWORK af nettype filter_set { struct network *n; if ((n = calloc(1, sizeof(struct network))) == NULL) fatal("new_network"); if (afi2aid($2, SAFI_UNICAST, >net.prefix.aid) == -1) { - yyerror("unknown family"); + yyerror("unknown address family"); filterset_free($4); free($4); YYERROR; @@ -1553,7 +1553,7 @@ peeropts : REMOTEAS as4number{ } curpeer->conf.min_holdtime = $3; } - | ANNOUNCE family safi { + | ANNOUNCE af safi { uint8_t aid, safi; uint16_tafi; @@ -1988,7 +1988,7 @@ restart : /* nada */{ $$ = 0; } } ; -family : IPV4 { $$ = AFI_IPv4; } +af : IPV4 { $$ = AFI_IPv4; } | IPV6 { $$ = AFI_IPv6; } ;
Re: fill out more rk356x dwqe phy-mode handling
> Date: Wed, 5 Apr 2023 16:09:21 +1000 > From: David Gwynne > > On Tue, Apr 04, 2023 at 10:59:56PM +1000, David Gwynne wrote: > > > > > > > On 4 Apr 2023, at 20:37, Mark Kettenis wrote: > > > > > >> Date: Tue, 4 Apr 2023 09:49:40 +1000 > > >> From: David Gwynne > > >> > > >> i did this when i was trying to figure out why TX wasn't working on the > > >> nanopi r5s before figuring out that problem was because we didn't have > > >> rkiovd. > > >> > > >> at the very least it should future proof dwqe against more phy setups, > > >> and provides a decent example of how to interpret those fdt properties. > > >> > > >> ok? > > > > > > Oh wow. I always thought those properties are purely about enabling > > > the internal delays of the PHY and came in addition to the delays > > > configured in the MAC. But you're right, that isn't what the device > > > tree bindings say and isn't what the Linux driver implements. So this > > > is indeed the right thing to do. And it will make the rock-3a work > > > once I correctly implement configuring the internal delay for > > > rgephy(4). > > > > > > That said, I think that once again you are doing too much DT checking. > > > Many of the options you're now erroring out on are optional. And > > > there actually is a reasonable chance that the interface will work if > > > they're absent, especially if the firmware already initializes the > > > SoC-specific GRF registers. I don't think we should even print any > > > warnings. > > > > having stared inside some boot loaders recently i have mixed feelings about > > this. i guess they can only get better from here though, right? > > > > > If you make dwqe_rockchip_setup() specific to the rk3568 (no objection > > > to that), you should probably rename it to dwqe_rk3568_setup(). > > > > alright. > > > > > And a style issue; please don't add parentheses around trivial > > > function return values in code that doesn't already use that style. > > > We moved away from that style years ago... > > > > yeah, sorry. muscle memory. > > > > i'll fix these up tomorrow and send it around again. > > this still works for me on the r5s. e25 doesnt use onboard gmac. Just realized that this risks leaving the task uninitialized. But that risk is already there. ok kettenis@ > Index: if_dwqe_fdt.c > === > RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v > retrieving revision 1.5 > diff -u -p -r1.5 if_dwqe_fdt.c > --- if_dwqe_fdt.c 26 Mar 2023 21:44:46 - 1.5 > +++ if_dwqe_fdt.c 5 Apr 2023 06:06:57 - > @@ -63,7 +63,7 @@ > > int dwqe_fdt_match(struct device *, void *, void *); > void dwqe_fdt_attach(struct device *, struct device *, void *); > -void dwqe_setup_rockchip(struct dwqe_softc *); > +void dwqe_setup_rk3568(struct dwqe_softc *); > void dwqe_mii_statchg_rk3568(struct device *); > void dwqe_mii_statchg_rk3588(struct device *); > > @@ -138,7 +138,7 @@ dwqe_fdt_attach(struct device *parent, s > > /* Do hardware specific initializations. */ > if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac")) > - dwqe_setup_rockchip(sc); > + dwqe_setup_rk3568(sc); > > /* Power up PHY. */ > phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0); > @@ -266,41 +266,62 @@ dwqe_reset_phy(struct dwqe_softc *sc, ui > #define RK3568_GRF_GMACx_CON1(x) (0x0384 + (x) * 0x8) > #define RK3568_GMAC_PHY_INTF_SEL_RGMII ((0x7 << 4) << 16 | > (0x1 << 4)) > #define RK3568_GMAC_PHY_INTF_SEL_RMII ((0x7 << 4) << 16 | > (0x4 << 4)) > -#define RK3568_GMAC_TXCLK_DLY_ENA ((1 << 0) << 16 | (1 << 0)) > -#define RK3568_GMAC_RXCLK_DLY_ENA ((1 << 1) << 16 | (1 << 1)) > +#define RK3568_GMAC_TXCLK_DLY_SET(_v) ((1 << 0) << 16 | ((_v) > << 0)) > +#define RK3568_GMAC_RXCLK_DLY_SET(_v) ((1 << 1) << 16 | ((_v) > << 1)) > > void dwqe_mii_statchg_rk3568_task(void *); > > void > -dwqe_setup_rockchip(struct dwqe_softc *sc) > +dwqe_setup_rk3568(struct dwqe_softc *sc) > { > + char phy_mode[32]; > struct regmap *rm; > uint32_t grf; > int tx_delay, rx_delay; > + uint32_t iface; > > grf = OF_getpropint(sc->sc_node, "rockchip,grf", 0); > rm = regmap_byphandle(grf); > if (rm == NULL) > return; > > + if (OF_getprop(sc->sc_node, "phy-mode", > + phy_mode, sizeof(phy_mode)) <= 0) > + return; > + > tx_delay = OF_getpropint(sc->sc_node, "tx_delay", 0x30); > rx_delay = OF_getpropint(sc->sc_node, "rx_delay", 0x10); > > - if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-gmac")) { > - /* Program clock delay lines. */ > - regmap_write_4(rm, RK3568_GRF_GMACx_CON0(sc->sc_gmac_id), > - RK3568_GMAC_CLK_TX_DL_CFG(tx_delay) | > - RK3568_GMAC_CLK_RX_DL_CFG(rx_delay)); > - > - /* Use
Re: fill out more rk356x dwqe phy-mode handling
On Tue, Apr 04, 2023 at 10:59:56PM +1000, David Gwynne wrote: > > > > On 4 Apr 2023, at 20:37, Mark Kettenis wrote: > > > >> Date: Tue, 4 Apr 2023 09:49:40 +1000 > >> From: David Gwynne > >> > >> i did this when i was trying to figure out why TX wasn't working on the > >> nanopi r5s before figuring out that problem was because we didn't have > >> rkiovd. > >> > >> at the very least it should future proof dwqe against more phy setups, > >> and provides a decent example of how to interpret those fdt properties. > >> > >> ok? > > > > Oh wow. I always thought those properties are purely about enabling > > the internal delays of the PHY and came in addition to the delays > > configured in the MAC. But you're right, that isn't what the device > > tree bindings say and isn't what the Linux driver implements. So this > > is indeed the right thing to do. And it will make the rock-3a work > > once I correctly implement configuring the internal delay for > > rgephy(4). > > > > That said, I think that once again you are doing too much DT checking. > > Many of the options you're now erroring out on are optional. And > > there actually is a reasonable chance that the interface will work if > > they're absent, especially if the firmware already initializes the > > SoC-specific GRF registers. I don't think we should even print any > > warnings. > > having stared inside some boot loaders recently i have mixed feelings about > this. i guess they can only get better from here though, right? > > > If you make dwqe_rockchip_setup() specific to the rk3568 (no objection > > to that), you should probably rename it to dwqe_rk3568_setup(). > > alright. > > > And a style issue; please don't add parentheses around trivial > > function return values in code that doesn't already use that style. > > We moved away from that style years ago... > > yeah, sorry. muscle memory. > > i'll fix these up tomorrow and send it around again. this still works for me on the r5s. e25 doesnt use onboard gmac. Index: if_dwqe_fdt.c === RCS file: /cvs/src/sys/dev/fdt/if_dwqe_fdt.c,v retrieving revision 1.5 diff -u -p -r1.5 if_dwqe_fdt.c --- if_dwqe_fdt.c 26 Mar 2023 21:44:46 - 1.5 +++ if_dwqe_fdt.c 5 Apr 2023 06:06:57 - @@ -63,7 +63,7 @@ intdwqe_fdt_match(struct device *, void *, void *); void dwqe_fdt_attach(struct device *, struct device *, void *); -void dwqe_setup_rockchip(struct dwqe_softc *); +void dwqe_setup_rk3568(struct dwqe_softc *); void dwqe_mii_statchg_rk3568(struct device *); void dwqe_mii_statchg_rk3588(struct device *); @@ -138,7 +138,7 @@ dwqe_fdt_attach(struct device *parent, s /* Do hardware specific initializations. */ if (OF_is_compatible(faa->fa_node, "rockchip,rk3568-gmac")) - dwqe_setup_rockchip(sc); + dwqe_setup_rk3568(sc); /* Power up PHY. */ phy_supply = OF_getpropint(faa->fa_node, "phy-supply", 0); @@ -266,41 +266,62 @@ dwqe_reset_phy(struct dwqe_softc *sc, ui #define RK3568_GRF_GMACx_CON1(x) (0x0384 + (x) * 0x8) #define RK3568_GMAC_PHY_INTF_SEL_RGMII((0x7 << 4) << 16 | (0x1 << 4)) #define RK3568_GMAC_PHY_INTF_SEL_RMII ((0x7 << 4) << 16 | (0x4 << 4)) -#define RK3568_GMAC_TXCLK_DLY_ENA ((1 << 0) << 16 | (1 << 0)) -#define RK3568_GMAC_RXCLK_DLY_ENA ((1 << 1) << 16 | (1 << 1)) +#define RK3568_GMAC_TXCLK_DLY_SET(_v) ((1 << 0) << 16 | ((_v) << 0)) +#define RK3568_GMAC_RXCLK_DLY_SET(_v) ((1 << 1) << 16 | ((_v) << 1)) void dwqe_mii_statchg_rk3568_task(void *); void -dwqe_setup_rockchip(struct dwqe_softc *sc) +dwqe_setup_rk3568(struct dwqe_softc *sc) { + char phy_mode[32]; struct regmap *rm; uint32_t grf; int tx_delay, rx_delay; + uint32_t iface; grf = OF_getpropint(sc->sc_node, "rockchip,grf", 0); rm = regmap_byphandle(grf); if (rm == NULL) return; + if (OF_getprop(sc->sc_node, "phy-mode", + phy_mode, sizeof(phy_mode)) <= 0) + return; + tx_delay = OF_getpropint(sc->sc_node, "tx_delay", 0x30); rx_delay = OF_getpropint(sc->sc_node, "rx_delay", 0x10); - if (OF_is_compatible(sc->sc_node, "rockchip,rk3568-gmac")) { - /* Program clock delay lines. */ - regmap_write_4(rm, RK3568_GRF_GMACx_CON0(sc->sc_gmac_id), - RK3568_GMAC_CLK_TX_DL_CFG(tx_delay) | - RK3568_GMAC_CLK_RX_DL_CFG(rx_delay)); - - /* Use RGMII interface and enable clock delay. */ - regmap_write_4(rm, RK3568_GRF_GMACx_CON1(sc->sc_gmac_id), - RK3568_GMAC_PHY_INTF_SEL_RGMII | - RK3568_GMAC_TXCLK_DLY_ENA | - RK3568_GMAC_RXCLK_DLY_ENA); + if (strcmp(phy_mode, "rgmii") == 0) { + iface =