Re: installer: arm64: better disk variable handling

2023-04-05 Thread Andrew Hewus Fresh
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?"

2023-04-05 Thread Andrew Hewus Fresh
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

2023-04-05 Thread Klemens Nanni
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

2023-04-05 Thread Jason McIntyre
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

2023-04-05 Thread Ingo Schwarze
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

2023-04-05 Thread Alexander Bluhm
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

2023-04-05 Thread Klemens Nanni
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

2023-04-05 Thread Jason McIntyre
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

2023-04-05 Thread Klemens Nanni
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

2023-04-05 Thread Mark Kettenis
> 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?"

2023-04-05 Thread Klemens Nanni
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

2023-04-05 Thread Klemens Nanni
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)

2023-04-05 Thread Otto Moerbeek
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

2023-04-05 Thread Theo Buehler
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

2023-04-05 Thread Claudio Jeker
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

2023-04-05 Thread Claudio Jeker
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

2023-04-05 Thread 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@

> 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

2023-04-05 Thread 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.

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 =