Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Martijn van Duren
On Tue, 2022-11-22 at 11:25 +0100, Claudio Jeker wrote:
> On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote:
> > Need to query (and set $if, which might be used in route commands etc) I 
> > think.
> > 
> 
> I would prefer if people took a step back from configuring interfaces by
> MAC address. It feels like a overly specific hack is introduced for
> a case that should be handled in a different way.
> 
> Not all interfaces have MAC addresses. E.g. umb(4) would need a
> different identifier (most probably the IMEI). Some interfaces inherit
> the MAC address from an other interface (vlan, trunk).
> 
> This requires the use of interface groups to 'rename' these interfaces
> e.g. as 'green' and 'blue' or 'in' and 'out'. So that you can use these
> handles in pf.conf and other commands (rad comes to mind). Not all
> commands work with interface groups. route(8) is such an example but there
> are more commands needing this.
> 
> Btw. a lot of this can be done already now by using '!' in hostname.if
> It wont be pretty but it is also not a common use case.

Maybe you're right, but as theo said doing it via !-commands is just
horrendous. Best I can think of is letting people write their own script
and call it via `!/path/to/script $if`, which is guaranteed going to end
messy.

Here's a completely other idea that might be more generic and hopefully
doesn't cause this same level of confusion. What if instead of adding
a new extension we let people do that themselves and add support for
including other files? Diff below does this and must still be considered
a PoC. And yes I know sed, which isn't allowed. But the command above it
does the same thing, so I'm not going to apologise for it in a PoC.

One downside in the current parse_hn_line format is that spaces other
than a single SP are completely butchered, but I'm not expecting much
problems there.

In addition to $if which is available for '!'-commands I also added
support for $lladdr.

$ cat /etc/hostname.em0 
. /etc/hostname.$lladdr
$ cat /etc/hostname.88\:a4\:c2\:fb\:84\:77 
autoconf
up
$ doas sh ./netstart -n em0
{ ifconfig em0 || ifconfig em0 create; }
ifconfig em0 autoconf
ifconfig em0 up

martijn@

Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.229
diff -u -p -r1.229 netstart
--- netstart5 Nov 2022 12:06:05 -   1.229
+++ netstart23 Nov 2022 07:22:07 -
@@ -35,10 +35,38 @@ stripcom() {
done <$_file
 }
 
+LLGLOB='[[:xdigit:]][[:xdigit:]]:[[:xdigit:]][[:xdigit:]]'
+LLGLOB="$LLGLOB:$LLGLOB:$LLGLOB"
+set -A LLADDR_MAP -- $(
+   ifconfig | while IFS= read -- _line; do
+   if [[ "$_line" = +([[:alpha:]])+([[:digit:]]):* ]]; then
+   _if=${_line%:*}
+   elif [[ -n "$_if"
+&& "$_line" = +([[:space:]])lladdr\ $LLGLOB ]]; then
+   print "$_if,${_line#*lladdr }"
+   _if=
+   fi
+   done
+)
+
+# Find lladdr for if
+# Usage: if2lladdr if1
+if2lladdr() {
+   local _if=$1
+   for m in "${LLADDR_MAP[@]}"; do
+   if [[ "$_if" = "${m%,*}" ]]; then
+   print -- "${m#*,}"
+   break
+   fi
+   done
+   return 0
+}
+
 # Parse and "unpack" a hostname.if(5) line given as positional parameters.
 # Fill the _cmds array with the resulting interface configuration commands.
 parse_hn_line() {
-   local _af=0 _name=1 _mask=2 _bc=3 _prefix=2 _c _cmd _prev _daddr _dhcp 
_i
+   local _af=0 _name=1 _mask=2 _bc=3 _prefix=2 _c _cmd _prev _daddr _dhcp
+   local _i _file _line
set -A _c -- "$@"
set -o noglob
 
@@ -84,6 +112,17 @@ parse_hn_line() {
;;
'!'*)   _cmd=$(print -- "${_c[@]}" | sed 's/\$if/'$_if'/g')
_cmds[${#_cmds[*]}]="${_cmd#!}"
+   ;;
+   '.')unset _c[_af]
+   _file="$(print -- "${_c[@]}" | sed -e 's/\$if/'$_if'/g' \
+   -e 's/\$lladdr/'$(if2lladdr $_if)'/g')"
+   if [[ ! -f $_file ]]; then
+   print -u2 "${0##*/}: $_file: No such file or directory."
+   return
+   fi
+   while IFS= read -- _line; do
+   parse_hn_line $_line
+   done<"$_file"
;;
*)  _cmds[${#_cmds[*]}]="ifconfig $_if ${_c[@]}"
;;



Re: mbufs growing in 7.2

2022-11-22 Thread Greg Steuck
The watched kettle never boiled. No more crashes in over two weeks
(instead of two in the first week). I tried a loop of alternating iperf3
tcp and udp to no ill effect. I still see the growth in the metrics I
reported, yet the system remained stable.

I applied the patch below and am still collecting the metrics. I doubt
they are responsible for the original problem.

Thanks
Greg

Moritz Buhl  writes:

> Hi Greg, Hi Joe,
>
> dlg@ hinted to me that the ring might overwrite it's own starting
> position with the current code.
>
> Does this help?
> mbuhl
>
> Index: dev/pci/if_igc.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_igc.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 if_igc.c
> --- dev/pci/if_igc.c  2 Jun 2022 07:41:17 -   1.9
> +++ dev/pci/if_igc.c  8 Nov 2022 10:35:39 -
> @@ -978,7 +978,7 @@ igc_start(struct ifqueue *ifq)
>   mask = sc->num_tx_desc - 1;
>  
>   for (;;) {
> - if (free <= IGC_MAX_SCATTER) {
> + if (free <= IGC_MAX_SCATTER + 1) {
>   ifq_set_oactive(ifq);
>   break;
>   }
> @@ -1005,6 +1005,7 @@ igc_start(struct ifqueue *ifq)
>   /* Consume the first descriptor */
>   prod++;
>   prod &= mask;
> + free--;
>   }
>  
>   for (i = 0; i < map->dm_nsegs; i++) {



Re: Get rid of UVM_VNODE_CANPERSIST

2022-11-22 Thread Mark Kettenis
> Date: Tue, 22 Nov 2022 17:47:44 +
> From: Miod Vallat 
> 
> > Here is a diff.  Maybe bluhm@ can try this on the macppc machine that
> > triggered the original "vref used where vget required" problem?
> 
> On a similar machine it panics after a few hours with:
> 
> panic: uvn_flush: PGO_SYNCIO return 'try again' error (impossible)
> 
> The trace (transcribed by hand) is
> uvn_flush+0x820
> uvm_vnp_terminate+0x79
> vclean+0xdc
> vgonel+0x70
> getnewvnode+0x240
> ffs_vget+0xcc
> ffs_inode_alloc+0x13c
> ufs_makeinode+0x94
> ufs_create+0x58
> VOP_CREATE+0x48
> vn_open+0x188
> doopenat+0x1b4

Ah right, there is another path where we end up with a refcount of
zero.  Should be fixable, but I need to think about this for a bit.



Re: Get rid of UVM_VNODE_CANPERSIST

2022-11-22 Thread Alexander Bluhm
On Tue, Nov 22, 2022 at 05:47:44PM +, Miod Vallat wrote:
> > Here is a diff.  Maybe bluhm@ can try this on the macppc machine that
> > triggered the original "vref used where vget required" problem?
> 
> On a similar machine it panics after a few hours with:
> 
> panic: uvn_flush: PGO_SYNCIO return 'try again' error (impossible)

same here, took me 3:39 hours

panic: uvn_flush: PGO_SYNCIO return 'try again' error (impossible)
Stopped at  db_enter+0x24:  lwz r11,12(r1)
TIDPIDUID PRFLAGS PFLAGS  CPU  COMMAND
*316489  81760 21 0x2  01K c++
 186567  88345 21 0x2  00  c++
db_enter() at db_enter+0x20
panic(8f85a4) at panic+0x158
uvn_flush(0,9000,4000,) at uvn_flush+0x820
uvm_vnp_terminate(238827b8) at uvm_vnp_terminate+0x70
vclean(2820f822,e37a6514,e8123960) at vclean+0xdc
vgonel(e8123a24,b1c52908) at vgonel+0x70
getnewvnode(0,341a3,22f89509,909610) at getnewvnode+0x240
ffs_vget(56bf5c85,1000,0) at ffs_vget+0xcc
ufs_lookup() at ufs_lookup+0xb00
VOP_LOOKUP(e8123d20,e0ca3c38,238827b8) at VOP_LOOKUP+0x44
vfs_lookup(56) at vfs_lookup+0x4a8
namei() at namei+0x2a8
vn_open(0,e37a6514,e8123c30) at vn_open+0xc0
doopenat(2001,5a5ae178,fffef45c,ff9c,0,0) at doopenat+0x1b4
end trace frame: 0xe8123e90, count: 0
https://www.openbsd.org/ddb.html describes the minimum info required in bug
reports.  Insufficient info makes it difficult to find and fix bugs.
ddb{1}> 

> The trace (transcribed by hand) is
> uvn_flush+0x820
> uvm_vnp_terminate+0x79
> vclean+0xdc
> vgonel+0x70
> getnewvnode+0x240
> ffs_vget+0xcc
> ffs_inode_alloc+0x13c
> ufs_makeinode+0x94
> ufs_create+0x58
> VOP_CREATE+0x48
> vn_open+0x188
> doopenat+0x1b4

ddb{1}> trace
db_enter() at db_enter+0x20
panic(8f85a4) at panic+0x158
uvn_flush(0,9000,4000,) at uvn_flush+0x820
uvm_vnp_terminate(238827b8) at uvm_vnp_terminate+0x70
vclean(2820f822,e37a6514,e8123960) at vclean+0xdc
vgonel(e8123a24,b1c52908) at vgonel+0x70
getnewvnode(0,341a3,22f89509,909610) at getnewvnode+0x240
ffs_vget(56bf5c85,1000,0) at ffs_vget+0xcc
ufs_lookup() at ufs_lookup+0xb00
VOP_LOOKUP(e8123d20,e0ca3c38,238827b8) at VOP_LOOKUP+0x44
vfs_lookup(56) at vfs_lookup+0x4a8
namei() at namei+0x2a8
vn_open(0,e37a6514,e8123c30) at vn_open+0xc0
doopenat(2001,5a5ae178,fffef45c,ff9c,0,0) at doopenat+0x1b4
sys_open(e8123f58,aacc64d6,e8123f50) at sys_open+0x34
trap(0) at trap+0x8d4
trapagain() at trapagain+0x4
--- syscall (number 5) ---
End of kernel: 0xfffef3c0
end trace frame: 0xfffef3c0, count: -17

ddb{1}> show register
r0  0x144b6cpanic+0x15c
r10xe81237d0
r2  0xab9c60cpu_info+0x4c8
r3  0xab9c60cpu_info+0x4c8
r4  0xb0db_elf_line_at_pc.path+0x1bc
r5   0x1
r6 0
r70xe7cb9000
r8 0
r9 0
r100
r11   0x624940af
r12   0x7d635f61
r130
r14 0x3bddbsize+0xb
r15  0x5
r160x40e4598_end+0x35c4754
r17 0x1b
r18   0x9000tlbdsmsize+0x8f18
r19   0xe8123878
r200
r21   0xe8123884
r220
r23 0x10
r24   0x5000tlbdsmsize+0x4f18
r250
r26 0xb0c64cuvm+0x10
r270
r280
r29 0xab9f28cpu_info+0x790
r30 0x8f85a4acx100_txpower_maxim+0x7490
r31 0x1b
lr  0x848af0db_enter+0x24
cr0x48482602
xer   0x2000
ctr 0x4ab3e4openpic_splx
iar 0x848af0db_enter+0x24
msr   0x9032tlbdsmsize+0x8f4a
dar0
dsisr  0
db_enter+0x24:  lwz r11,12(r1)

ddb{1}> show panic
*cpu1: uvn_flush: PGO_SYNCIO return 'try again' error (impossible)

ddb{1}> show uvm
Current UVM status:
  pagesize=4096 (0x1000), pagemask=0xfff, pageshift=12
  500872 VM pages: 47375 active, 65511 inactive, 1 wired, 278921 free (13303 
zero)
  min  10% (25) anon, 10% (25) vnode, 5% (12) vtext
  freemin=16695, free-target=22260, inactive-target=0, wired-max=166957
  faults=205167249, traps=0, intrs=601834, ctxswitch=11667040 fpuswitch=1818402
  softint=1432338, syscalls=287584088, kmapent=11
  fault counts:
noram=0, noanon=0, noamap=0, pgwait=0, pgrele=0
ok relocks(total)=241266(242663), anget(retries)=141999836(0), 
amapcopy=24524315
neighbor anon/obj pg=9973263/96744262, gets(lock/unlock)=34934368/242837
cases: anon=140506255, anoncow=1493581, obj=30776338, prcopy=4156459, 
przero=28234615
  daemon and swap counts:
woke=0, revs=0, scans=0, obscans=0, anscans=0
busy=0, freed=0, reactivate=0, deactivate=0
pageouts=0, pending=0, nswget=0
nswapdev=1
swpages=589823, 

Re: Get rid of UVM_VNODE_CANPERSIST

2022-11-22 Thread Miod Vallat
> Here is a diff.  Maybe bluhm@ can try this on the macppc machine that
> triggered the original "vref used where vget required" problem?

On a similar machine it panics after a few hours with:

panic: uvn_flush: PGO_SYNCIO return 'try again' error (impossible)

The trace (transcribed by hand) is
uvn_flush+0x820
uvm_vnp_terminate+0x79
vclean+0xdc
vgonel+0x70
getnewvnode+0x240
ffs_vget+0xcc
ffs_inode_alloc+0x13c
ufs_makeinode+0x94
ufs_create+0x58
VOP_CREATE+0x48
vn_open+0x188
doopenat+0x1b4



Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Andrew Hewus Fresh
On Tue, Nov 22, 2022 at 03:37:20PM +, Klemens Nanni wrote:
> On Tue, Nov 22, 2022 at 08:09:11AM -0700, Theo de Raadt wrote:
> > Florian Obser  wrote:
> > > ifconfig(8) already knows about these (see -C option). Which made me
> > > think, it might be easier to just ask ifconfig(8).
> > > 
> > > $ ifconfig -Q 00:80:41:7b:f3:c3
> > > vio0
> > > 
> > > Would that be helpful?
> > 
> > I'm unsure about the rest of your proposal, where MAC works in place if
> > the IF argument.  Let's say we do this in ifcconfig.  Do we do it in route?
> > Or ten other commands?  I think that is the wrong way to go.
> > 
> > But this first idea is valid. We've now seen 3 monster functions trying to
> > do this task of convering MAC to IF in shell.  Here's code to do it in
> > ifconfig.
> > 
> > I've done it as -M
> 
> Better than [Q]uery.
> 
> > 
> > It fails if the same MAC is on multiple interfaces.  Go back to using
> > hostname.if# files, you heathen.
> 
> This reads like a viable approach, much cleaner than the netstart
> globbing attempts.
> 
> Using ifconfig -M, I can give the shell bits a try later this week.

I agree that offloading mapping of the unique identifier to ifconfig
makes the shell bits easier.  We currently need to be able to go both
directions, mostly to check existence of /etc/hostname.ure0 and
/etc/hostname.$lladdr when when trying to netstart ure0 so we can
complain that there are two configurations for the same interface.  We
can, of course, use the existing ifconfig parsing to look up the "lladdr
...".  Maybe this is a case of letting folks shoot themselves in the
foot.

I do think that referring to it as -M for MAC lookup makes me expect it
to return all interface names with that mac, not error if there is more
than one.  Unlike it it referred to a "unique identifier" that might be
a mac or maybe IMEI.  I could be wrong, but it might make swapping umb
devices from different providers easer.



Re: install.sub: fix softraid disks not being created before md_installboot()

2022-11-22 Thread ssnf
> Deliver steps to reproduce, installation logs, machine information,
> version details, ANYTHING technical.

Version: OpenBSD 7.2 release
Arch:i386
Image:   install72.iso
Machine: Thinkpad 380XD 32MB 233MHz Pentium MMX

Steps:
- Boot install CDROM
- (S)hell
- cd /dev
- sh MAKEDEV wd0
- fdisk -iy wd0
- disklabel -E wd0
- a\n\n\nRAID\nwq\n
- sh MAKEDEV wd1
- fdisk -iy wd1
- disklabel -E wd1
- a\n\n\nRAID\nwq\n
- bioctl -c C -l wd0a -k wd1a softraid0
- exit
- (I)nstall 
- Installation stops on md_installboot()
- "/dev/rwd1c" does not exist

Steps for successful install:
- Boot install CDROM
- (S)hell
- ed install.sub
- /bioctl
- s/\$/"$
- s/$/"
- wq
- cd /dev
- sh MAKEDEV wd0
- fdisk -iy wd0
- disklabel -E wd0
- a\n\n\nRAID\nwq\n
- sh MAKEDEV wd1
- fdisk -iy wd1
- disklabel -E wd1
- a\n\n\nRAID\nwq\n
- bioctl -c C -l wd0a -k wd1a softraid0
- exit
- (I)nstall 
- Installation successful, reboot

This e-mail could have ended here, but I have to point out several
issues that bothered me in your reply.

> I'm not saying the code is perfect, but so far you have failed to
> provide anything but long sentences about comments, shell behaviour
> and what MIGHT have been a user error... or whatever.

With all due respect I:
- described the issue:
- "My softraid keydisk did not get initialized during the
install process."

- explained in which function the error happened:
- "once it reached the installboot function, it would say
that /dev/rwd1c did not exist."
- This is where 99.99% of reports stop.

- precisely pointed at where the error came from
- "make_dev would not initialize my second disk on my softraid
volume"

- showed why the error exists:
- "without the double quotes, we get a multi-line result from
the $(bioctl|sed) command and only the first line is actually
passed to the make_dev function"

- and gave a solution:
- "When we place it inside the double quotes, the multi-line
result becomes a single line, and thus make_dev works as
expected."

Per the FAQ (https://www.openbsd.org/mail.html), include important
information.

And I did. I included ALL important information, even more than
necessary.

Did I deliver more information, when I was queried for more, just so
you could dismiss it as "long sentences about comments, shell
behaviour and what MIGHT have been a user error"? Fuck no. Read
through them, they are MEANINGFUL. They ARE the information you
requested.

I'm not a monkey who throws big sentences out there for no reason. My
sentences have meaning, just read through them. I'm not an idiot.

I'm very much the opposite, hence I prefer things to be simple, short
and to the point.

I'm here to fix a small, almost insignificant issue, not to discuss
about what should have/has been said. There is no point in discussing,
because the all the information is clearly there.

You know what? Fine, I don't care about it. Keep it that way and lets
move on, I'm not gonna be bothered by this anymore. This line has been
there since 2013, it should have bothered much more people at this
rate. Clearly a non-issue at this point.

I just hope next time I'm on this mailing list, I won't be received
with such indelicacy, as I'll provide actual lines of code, and not
just a DOUBLE-QUOTE fix for a obscure error that might even be user
error.

> I am very much aware of how the shell works, but still fail to see
> when/where/how this is a problem, because you are not being helpful.

I agree that you are aware of how the shell works, but you have clearly
not taken your time to read what I posted.

Yes. They were long, cohesive, and coherent sentences, but you just
decided to look at their sizes, not their contents. Of course they
won't be helpful that way.

You asked for more, and when you got it you dismissed it.

Your treatment was not even about 'not being enough', but rather
'it contains nothing useful', which is honestly absurd.

If you felt like my reply didn't provide you with enough info, just
ask for more. Don't discard what I wrote.

You are not being helpful.

> I have used plain disks, single-chunk and multi-chunk softraid
> volumes, sofraid volumes with passphrase and with 

Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Klemens Nanni
On Tue, Nov 22, 2022 at 08:09:11AM -0700, Theo de Raadt wrote:
> Florian Obser  wrote:
> 
> > On 2022-11-22 18:06 +10, David Gwynne  wrote:
> > >
> > > There are a few things to keep in mind if we're going to use lladdrs like 
> > > this.
> > >
> > > vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then 
> > > assume the lladdr of the parent interface when that is configured.
> > >
> > > Clonable devices (eg, egre, vport, aggr, etc) tend to generate random
> > > lladdrs when they're created. If you want a consistent lladdr for
> > > these you configure that in /etc/hostname.vportX or whatever you're
> > > using.
> > 
> > ifconfig(8) already knows about these (see -C option). Which made me
> > think, it might be easier to just ask ifconfig(8).
> > 
> > $ ifconfig -Q 00:80:41:7b:f3:c3
> > vio0
> > 
> > Would that be helpful?
> 
> I'm unsure about the rest of your proposal, where MAC works in place if
> the IF argument.  Let's say we do this in ifcconfig.  Do we do it in route?
> Or ten other commands?  I think that is the wrong way to go.
> 
> But this first idea is valid. We've now seen 3 monster functions trying to
> do this task of convering MAC to IF in shell.  Here's code to do it in
> ifconfig.
> 
> I've done it as -M

Better than [Q]uery.

> 
> It fails if the same MAC is on multiple interfaces.  Go back to using
> hostname.if# files, you heathen.

This reads like a viable approach, much cleaner than the netstart
globbing attempts.

Using ifconfig -M, I can give the shell bits a try later this week.

Nits inline.

> 
> Index: ifconfig.8
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.385
> diff -u -p -u -r1.385 ifconfig.8
> --- ifconfig.826 Oct 2022 17:06:31 -  1.385
> +++ ifconfig.822 Nov 2022 15:04:09 -
> @@ -40,6 +40,7 @@
>  .Sh SYNOPSIS
>  .Nm ifconfig
>  .Op Fl AaC
> +.Op Fl M Ar lladdr
>  .Op Ar interface
>  .Op Ar address_family
>  .Oo
> @@ -88,6 +89,11 @@ This is the default, if no parameters ar
>  Print the names of all network pseudo-devices that
>  can be created dynamically at runtime using
>  .Nm Cm create .
> +.It Fl M Ar lladdr
> +Scan the interface list for the MAC address
> +.Ar lladdr
> +and print the name of that interface.
> +If the MAC address is found on multiple interfaces, print nothing.
>  .It Ar interface
>  The
>  .Ar interface
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.457
> diff -u -p -u -r1.457 ifconfig.c
> --- ifconfig.c26 Oct 2022 17:06:31 -  1.457
> +++ ifconfig.c22 Nov 2022 15:02:20 -
> @@ -368,6 +368,8 @@ void  wg_status(int);
>  void setignore(const char *, int);
>  #endif
>  
> +void findmac(char *);
> +
>  /*
>   * Media stuff.  Whenever a media command is first performed, the
>   * currently select media is grabbed for this interface.  If `media'
> @@ -759,6 +761,7 @@ main(int argc, char *argv[])
>   int create = 0;
>   int Cflag = 0;
>   int gflag = 0;
> + int Mflag = 0;
>   int found_rulefile = 0;
>   int i;
>  
> @@ -795,6 +798,12 @@ main(int argc, char *argv[])
>   Cflag = 1;
>   nomore = 1;
>   break;
> + case 'M':
> + if (argv[1] == NULL)
> + usage();
> + findmac(argv[1]);
> + exit(1);

Above main() already uses return, no need for exit here, I think:

return findmac(argv[1]);

> + break;
>   default:
>   usage();
>   break;
> @@ -6614,7 +6623,7 @@ __dead void
>  usage(void)
>  {
>   fprintf(stderr,
> - "usage: ifconfig [-AaC] [interface] [address_family] "
> + "usage: ifconfig [-AaC] [-M lladdr] [interface] [address_family] "
>   "[address [dest_address]]\n"
>   "\t\t[parameters]\n");
>   exit(1);
> @@ -6782,3 +6791,30 @@ setignore(const char *id, int param)
>   /* just digest the command */
>  }
>  #endif
> +
> +void
> +findmac(char *mac)
> +{
> + struct ifaddrs *ifap, *ifa;
> + char *ifnam = NULL;

Both *mac and *ifnam can be const.

> +
> + if (getifaddrs() != 0)
> + err(1, "getifaddrs");
> +
> + for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
> + struct sockaddr_dl *sdl = (struct sockaddr_dl *)ifa->ifa_addr;
> +
> + if (sdl != NULL && sdl->sdl_alen &&
> + (sdl->sdl_type == IFT_ETHER || sdl->sdl_type == IFT_CARP)) {
> + if (strcmp(ether_ntoa((struct ether_addr *)LLADDR(sdl)),
> + mac) == 0) {
> + if (ifnam)  /* 

Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Theo de Raadt
Florian Obser  wrote:

> On 2022-11-22 18:06 +10, David Gwynne  wrote:
> >
> > There are a few things to keep in mind if we're going to use lladdrs like 
> > this.
> >
> > vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then 
> > assume the lladdr of the parent interface when that is configured.
> >
> > Clonable devices (eg, egre, vport, aggr, etc) tend to generate random
> > lladdrs when they're created. If you want a consistent lladdr for
> > these you configure that in /etc/hostname.vportX or whatever you're
> > using.
> 
> ifconfig(8) already knows about these (see -C option). Which made me
> think, it might be easier to just ask ifconfig(8).
> 
> $ ifconfig -Q 00:80:41:7b:f3:c3
> vio0
> 
> Would that be helpful?

I'm unsure about the rest of your proposal, where MAC works in place if
the IF argument.  Let's say we do this in ifcconfig.  Do we do it in route?
Or ten other commands?  I think that is the wrong way to go.

But this first idea is valid. We've now seen 3 monster functions trying to
do this task of convering MAC to IF in shell.  Here's code to do it in
ifconfig.

I've done it as -M

It fails if the same MAC is on multiple interfaces.  Go back to using
hostname.if# files, you heathen.

Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.385
diff -u -p -u -r1.385 ifconfig.8
--- ifconfig.8  26 Oct 2022 17:06:31 -  1.385
+++ ifconfig.8  22 Nov 2022 15:04:09 -
@@ -40,6 +40,7 @@
 .Sh SYNOPSIS
 .Nm ifconfig
 .Op Fl AaC
+.Op Fl M Ar lladdr
 .Op Ar interface
 .Op Ar address_family
 .Oo
@@ -88,6 +89,11 @@ This is the default, if no parameters ar
 Print the names of all network pseudo-devices that
 can be created dynamically at runtime using
 .Nm Cm create .
+.It Fl M Ar lladdr
+Scan the interface list for the MAC address
+.Ar lladdr
+and print the name of that interface.
+If the MAC address is found on multiple interfaces, print nothing.
 .It Ar interface
 The
 .Ar interface
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.457
diff -u -p -u -r1.457 ifconfig.c
--- ifconfig.c  26 Oct 2022 17:06:31 -  1.457
+++ ifconfig.c  22 Nov 2022 15:02:20 -
@@ -368,6 +368,8 @@ voidwg_status(int);
 void   setignore(const char *, int);
 #endif
 
+void   findmac(char *);
+
 /*
  * Media stuff.  Whenever a media command is first performed, the
  * currently select media is grabbed for this interface.  If `media'
@@ -759,6 +761,7 @@ main(int argc, char *argv[])
int create = 0;
int Cflag = 0;
int gflag = 0;
+   int Mflag = 0;
int found_rulefile = 0;
int i;
 
@@ -795,6 +798,12 @@ main(int argc, char *argv[])
Cflag = 1;
nomore = 1;
break;
+   case 'M':
+   if (argv[1] == NULL)
+   usage();
+   findmac(argv[1]);
+   exit(1);
+   break;
default:
usage();
break;
@@ -6614,7 +6623,7 @@ __dead void
 usage(void)
 {
fprintf(stderr,
-   "usage: ifconfig [-AaC] [interface] [address_family] "
+   "usage: ifconfig [-AaC] [-M lladdr] [interface] [address_family] "
"[address [dest_address]]\n"
"\t\t[parameters]\n");
exit(1);
@@ -6782,3 +6791,30 @@ setignore(const char *id, int param)
/* just digest the command */
 }
 #endif
+
+void
+findmac(char *mac)
+{
+   struct ifaddrs *ifap, *ifa;
+   char *ifnam = NULL;
+
+   if (getifaddrs() != 0)
+   err(1, "getifaddrs");
+
+   for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
+   struct sockaddr_dl *sdl = (struct sockaddr_dl *)ifa->ifa_addr;
+
+   if (sdl != NULL && sdl->sdl_alen &&
+   (sdl->sdl_type == IFT_ETHER || sdl->sdl_type == IFT_CARP)) {
+   if (strcmp(ether_ntoa((struct ether_addr *)LLADDR(sdl)),
+   mac) == 0) {
+   if (ifnam)  /* same MAC on multiple ifp */
+   exit(1);
+   ifnam = ifa->ifa_name;
+   }
+   }
+   }
+   if (ifnam)
+   printf("%s\n", ifnam);
+   exit(0);
+}



Re: interface hooks to pf(4) should be using PF_LOCK()/PF_UNLOCK()

2022-11-22 Thread Klemens Nanni
On Sun, Nov 06, 2022 at 07:12:31PM +0100, Alexandr Nedvedicky wrote:
> Hello,
> 
> diff below is the first step to make pfioctl() _without_ NET_LOCK().
> Currently pf_if.c seems to be the only blocker which prevents us
> from removing all NET_LOCK()/NET_UNLOCK() calls we have in pf(4).
> 
> diff below passed very basic smoke test. OK to commit?

OK kn



Re: Get rid of UVM_VNODE_CANPERSIST

2022-11-22 Thread Mark Kettenis
> Date: Tue, 22 Nov 2022 15:08:30 +0100
> From: Martin Pieuchot 
> 
> On 18/11/22(Fri) 21:33, Mark Kettenis wrote:
> > > Date: Thu, 17 Nov 2022 20:23:37 +0100
> > > From: Mark Kettenis 
> > > 
> > > > From: Jeremie Courreges-Anglas 
> > > > Date: Thu, 17 Nov 2022 18:00:21 +0100
> > > > 
> > > > On Tue, Nov 15 2022, Martin Pieuchot  wrote:
> > > > > UVM vnode objects include a reference count to keep track of the 
> > > > > number
> > > > > of processes that have the corresponding pages mapped in their VM 
> > > > > space.
> > > > >
> > > > > When the last process referencing a given library or executable dies,
> > > > > the reaper will munmap this object on its behalf.  When this happens 
> > > > > it
> > > > > doesn't free the associated pages to speed-up possible re-use of the
> > > > > file.  Instead the pages are placed on the inactive list but stay 
> > > > > ready
> > > > > to be pmap_enter()'d without requiring I/O as soon as a newly process
> > > > > needs to access them.
> > > > >
> > > > > The mechanism to keep pages populated, known as UVM_VNODE_CANPERSIST,
> > > > > doesn't work well with swapping [0].  For some reason when the page 
> > > > > daemon
> > > > > wants to free pages on the inactive list it tries to flush the pages 
> > > > > to
> > > > > disk and panic(9) because it needs a valid reference to the vnode to 
> > > > > do
> > > > > so.
> > > > >
> > > > > This indicates that the mechanism described above, which seems to work
> > > > > fine for RO mappings, is currently buggy in more complex situations.
> > > > > Flushing the pages when the last reference of the UVM object is 
> > > > > dropped
> > > > > also doesn't seem to be enough as bluhm@ reported [1].
> > > > >
> > > > > The diff below, which has already be committed and reverted, gets rid 
> > > > > of
> > > > > the UVM_VNODE_CANPERSIST logic.  I'd like to commit it again now that
> > > > > the arm64 caching bug has been found and fixed.
> > > > >
> > > > > Getting rid of this logic means more I/O will be generated and pages
> > > > > might have a faster reuse cycle.  I'm aware this might introduce a 
> > > > > small
> > > > > slowdown,
> > > > 
> > > > Numbers for my usual make -j4 in libc,
> > > > on an Unmatched riscv64 box, now:
> > > >16m32.65s real21m36.79s user30m53.45s system
> > > >16m32.37s real21m33.40s user31m17.98s system
> > > >16m32.63s real21m35.74s user31m12.01s system
> > > >16m32.13s real21m36.12s user31m06.92s system
> > > > After:
> > > >19m14.15s real21m09.39s user36m51.33s system
> > > >19m19.11s real21m02.61s user36m58.46s system
> > > >19m21.77s real21m09.23s user37m03.85s system
> > > >19m09.39s real21m08.96s user36m36.00s system
> > > > 
> > > > 4 cores amd64 VM, before (-current plus an other diff):
> > > >1m54.31s real 2m47.36s user 4m24.70s system
> > > >1m52.64s real 2m45.68s user 4m23.46s system
> > > >1m53.47s real 2m43.59s user 4m27.60s system
> > > > After:
> > > >2m34.12s real 2m51.15s user 6m20.91s system
> > > >2m34.30s real 2m48.48s user 6m23.34s system
> > > >2m37.07s real 2m49.60s user 6m31.53s system
> > > > 
> > > > > however I believe we should work towards loading files from the
> > > > > buffer cache to save I/O cycles instead of having another layer of 
> > > > > cache.
> > > > > Such work isn't trivial and making sure the vnode <-> UVM relation is
> > > > > simple and well understood is the first step in this direction.
> > > > >
> > > > > I'd appreciate if the diff below could be tested on many 
> > > > > architectures,
> > > > > include the offending rpi4.
> > > > 
> > > > Mike has already tested a make build on a riscv64 Unmatched.  I have
> > > > also run regress in sys, lib/libc and lib/libpthread on that arch.  As
> > > > far as I can see this looks stable on my machine, but what I really care
> > > > about is the riscv64 bulk build cluster (I'm going to start another
> > > > bulk build soon).
> > > > 
> > > > > Comments?  Oks?
> > > > 
> > > > The performance drop in my microbenchmark kinda worries me but it's only
> > > > a microbenchmark...
> > > 
> > > I wouldn't call this a microbenchmark.  I fear this is typical for
> > > builds of anything on clang architectures.  And I expect it to be
> > > worse on single-processor machine where *every* time we execute clang
> > > or lld all the pages are thrown away upon exit and will need to be
> > > read back from disk again for the next bit of C code we compile.
> > > 
> > > Having mmap'ed reads go through the buffer cache should indeed help
> > > here, but that smells like UBC and even if it isn't, it is something
> > > we don't have and will be tricky to get right.
> > > 
> > > The discussions we had during h2k22 made me understand this thing a
> > > bit better.  Is there any reason why we can't keep a reference to the
> > > vnode and have the pagedaemon drop 

Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Vitaliy Makkoveev
On Tue, Nov 22, 2022 at 06:28:31AM -0700, Theo de Raadt wrote:
> Vitaliy Makkoveev  wrote:
> 
> > On Tue, Nov 22, 2022 at 11:25:55AM +0100, Claudio Jeker wrote:
> > > On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote:
> > > > Need to query (and set $if, which might be used in route commands etc) 
> > > > I think.
> > > > 
> > > 
> > > I would prefer if people took a step back from configuring interfaces by
> > > MAC address. It feels like a overly specific hack is introduced for
> > > a case that should be handled in a different way.
> > > 
> > 
> > [skip]
> > 
> > > 
> > > Btw. a lot of this can be done already now by using '!' in hostname.if
> > > It wont be pretty but it is also not a common use case.
> > 
> > Agree.
> 
> 
> Come on, that's just some incomplete claim.
> 
> I do not think you have tried this.
> 
> Try it.  I have actually tried it.  The results were so incredibly
> disgusting that I gave up and used another machine.
> 

As dlg@ pointed, we have collisions with clonable devices, which inherit
lladdr or could generate already existing lladdr. We need to handle
this, for example exclude clobable devices from hostname.lladdr logic.



Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Theo de Raadt
Vitaliy Makkoveev  wrote:

> As dlg@ pointed, we have collisions with clonable devices, which inherit
> lladdr or could generate already existing lladdr. We need to handle
> this, for example exclude clobable devices from hostname.lladdr logic.

Noone is proposing deleting the existing support for 'hostname.if'.

hostname.mac will be an addition to identify SOME network interfaces.

Noone is suggesting that users use only hostname.MAC

Specific exclusions could be handled, for instance rejecting 00:00:00:00:00:00
only working on the first or last device with a specific MAC, or rejecting
operation if a MAC exists multiple times --> and thus forcing people back
to using hostname.IF for those cases

But if no such tests were added (to the already bloated) /etc/netstart,
the 'number of people harmed' will probably be zero.





Re: Get rid of UVM_VNODE_CANPERSIST

2022-11-22 Thread Martin Pieuchot
On 18/11/22(Fri) 21:33, Mark Kettenis wrote:
> > Date: Thu, 17 Nov 2022 20:23:37 +0100
> > From: Mark Kettenis 
> > 
> > > From: Jeremie Courreges-Anglas 
> > > Date: Thu, 17 Nov 2022 18:00:21 +0100
> > > 
> > > On Tue, Nov 15 2022, Martin Pieuchot  wrote:
> > > > UVM vnode objects include a reference count to keep track of the number
> > > > of processes that have the corresponding pages mapped in their VM space.
> > > >
> > > > When the last process referencing a given library or executable dies,
> > > > the reaper will munmap this object on its behalf.  When this happens it
> > > > doesn't free the associated pages to speed-up possible re-use of the
> > > > file.  Instead the pages are placed on the inactive list but stay ready
> > > > to be pmap_enter()'d without requiring I/O as soon as a newly process
> > > > needs to access them.
> > > >
> > > > The mechanism to keep pages populated, known as UVM_VNODE_CANPERSIST,
> > > > doesn't work well with swapping [0].  For some reason when the page 
> > > > daemon
> > > > wants to free pages on the inactive list it tries to flush the pages to
> > > > disk and panic(9) because it needs a valid reference to the vnode to do
> > > > so.
> > > >
> > > > This indicates that the mechanism described above, which seems to work
> > > > fine for RO mappings, is currently buggy in more complex situations.
> > > > Flushing the pages when the last reference of the UVM object is dropped
> > > > also doesn't seem to be enough as bluhm@ reported [1].
> > > >
> > > > The diff below, which has already be committed and reverted, gets rid of
> > > > the UVM_VNODE_CANPERSIST logic.  I'd like to commit it again now that
> > > > the arm64 caching bug has been found and fixed.
> > > >
> > > > Getting rid of this logic means more I/O will be generated and pages
> > > > might have a faster reuse cycle.  I'm aware this might introduce a small
> > > > slowdown,
> > > 
> > > Numbers for my usual make -j4 in libc,
> > > on an Unmatched riscv64 box, now:
> > >16m32.65s real21m36.79s user30m53.45s system
> > >16m32.37s real21m33.40s user31m17.98s system
> > >16m32.63s real21m35.74s user31m12.01s system
> > >16m32.13s real21m36.12s user31m06.92s system
> > > After:
> > >19m14.15s real21m09.39s user36m51.33s system
> > >19m19.11s real21m02.61s user36m58.46s system
> > >19m21.77s real21m09.23s user37m03.85s system
> > >19m09.39s real21m08.96s user36m36.00s system
> > > 
> > > 4 cores amd64 VM, before (-current plus an other diff):
> > >1m54.31s real 2m47.36s user 4m24.70s system
> > >1m52.64s real 2m45.68s user 4m23.46s system
> > >1m53.47s real 2m43.59s user 4m27.60s system
> > > After:
> > >2m34.12s real 2m51.15s user 6m20.91s system
> > >2m34.30s real 2m48.48s user 6m23.34s system
> > >2m37.07s real 2m49.60s user 6m31.53s system
> > > 
> > > > however I believe we should work towards loading files from the
> > > > buffer cache to save I/O cycles instead of having another layer of 
> > > > cache.
> > > > Such work isn't trivial and making sure the vnode <-> UVM relation is
> > > > simple and well understood is the first step in this direction.
> > > >
> > > > I'd appreciate if the diff below could be tested on many architectures,
> > > > include the offending rpi4.
> > > 
> > > Mike has already tested a make build on a riscv64 Unmatched.  I have
> > > also run regress in sys, lib/libc and lib/libpthread on that arch.  As
> > > far as I can see this looks stable on my machine, but what I really care
> > > about is the riscv64 bulk build cluster (I'm going to start another
> > > bulk build soon).
> > > 
> > > > Comments?  Oks?
> > > 
> > > The performance drop in my microbenchmark kinda worries me but it's only
> > > a microbenchmark...
> > 
> > I wouldn't call this a microbenchmark.  I fear this is typical for
> > builds of anything on clang architectures.  And I expect it to be
> > worse on single-processor machine where *every* time we execute clang
> > or lld all the pages are thrown away upon exit and will need to be
> > read back from disk again for the next bit of C code we compile.
> > 
> > Having mmap'ed reads go through the buffer cache should indeed help
> > here, but that smells like UBC and even if it isn't, it is something
> > we don't have and will be tricky to get right.
> > 
> > The discussions we had during h2k22 made me understand this thing a
> > bit better.  Is there any reason why we can't keep a reference to the
> > vnode and have the pagedaemon drop it when it drops the pages?  Is
> > there a chance that we run out of vnodes this way?  I vaguely remember
> > beck@ saying that we can have tons of vnodes now.
> 
> Maybe that isn't the best idea.  So here is what may be an actual fix
> for the "macppc panic: vref used where vget required" problem we're
> seeing that prompted us to look at 

Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Theo de Raadt
Claudio Jeker  wrote:

> On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote:
> > Need to query (and set $if, which might be used in route commands etc) I 
> > think.
> > 
> 
> I would prefer if people took a step back from configuring interfaces by
> MAC address. It feels like a overly specific hack is introduced for
> a case that should be handled in a different way.
> 
> Not all interfaces have MAC addresses. E.g. umb(4) would need a
> different identifier (most probably the IMEI). Some interfaces inherit
> the MAC address from an other interface (vlan, trunk).
> 
> This requires the use of interface groups to 'rename' these interfaces
> e.g. as 'green' and 'blue' or 'in' and 'out'. So that you can use these
> handles in pf.conf and other commands (rad comes to mind). Not all
> commands work with interface groups. route(8) is such an example but there
> are more commands needing this.


The point of hostname.MAC is you'll be able to

1) set the address
2) set the group

and then

pf.conf will use the group


But that's no different than doing this in a hostname.IF file!

We've been encouraging people to use the group egress, as well as
predefined groups since they were invented!.  In this case, people will
start to use those more, but not because it is a MAC, but because the IF
name is unstable.  They can establish a stable group name, if they know
what interface-instance to assign a group to.

But otherwise, using groups to identify the specific interface position
is completely unrelated to setting the configuration on these interfaces
in the first place.

Claudio, you really should *SHOW* a working configuration using !, if
you believe in it so much.  And do it without a helper shell script,
because where are you going to slap that, into /etc, come on.  What I
came up before had 3 lines of ! command that were 70-90 characters long,
and each line had to to run ifconfig to COMPARE AT THE MAC, and then run
ifconfig to set a configuration; another example did the MAC and then
ran ifconfig multipel times but that single line wrapped the screen
multiple times.

It was a demo of how rough this problem is.  So maybe, show how you do !
commands in such files, so that everyone can see it is ugly as sin and
the wrong thing.



Re: lladdr support for netstart/hostname.if (was: Re: Locking network card configuration)

2022-11-22 Thread Theo de Raadt
Miod Vallat  wrote:

> I'm a bit late to the thread, but whatever its outcome, things have to
> work correctly on older sparc64 hardware, where the default behaviour
> for on-board and Sun-branded expansion card interfaces is to use the
> same MAC address.
> 
> This hints that hostname. should have priority over
> hoshname. for the latter will be ambiguous on these
> systems.

Don't use hostname.MAC in that case.

Noone is proposing removing hostname.IF0 support.



Just like noone is proposing deleting sed because awk can do the job.




Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Theo de Raadt
Vitaliy Makkoveev  wrote:

> On Tue, Nov 22, 2022 at 11:25:55AM +0100, Claudio Jeker wrote:
> > On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote:
> > > Need to query (and set $if, which might be used in route commands etc) I 
> > > think.
> > > 
> > 
> > I would prefer if people took a step back from configuring interfaces by
> > MAC address. It feels like a overly specific hack is introduced for
> > a case that should be handled in a different way.
> > 
> 
> [skip]
> 
> > 
> > Btw. a lot of this can be done already now by using '!' in hostname.if
> > It wont be pretty but it is also not a common use case.
> 
> Agree.


Come on, that's just some incomplete claim.

I do not think you have tried this.

Try it.  I have actually tried it.  The results were so incredibly
disgusting that I gave up and used another machine.



Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Theo de Raadt
Claudio Jeker  wrote:

> Not all interfaces have MAC addresses. E.g. umb(4) would need a
> different identifier (most probably the IMEI). Some interfaces inherit
> the MAC address from an other interface (vlan, trunk).


Then don't use hostname.MAC for those interfaces.

The other mechanism will remain.


And if that is the case, what is the fuss?


There is no other solution available for interfaces that attach out
of order.  Any attempt to solve this in subr_config.c to provide
stable device names / interface names is going to be a bigger mess.

 



Re: lladdr support for netstart/hostname.if (was: Re: Locking network card configuration)

2022-11-22 Thread Miod Vallat
I'm a bit late to the thread, but whatever its outcome, things have to
work correctly on older sparc64 hardware, where the default behaviour
for on-board and Sun-branded expansion card interfaces is to use the
same MAC address.

This hints that hostname. should have priority over
hoshname. for the latter will be ambiguous on these
systems.



Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Vitaliy Makkoveev
On Tue, Nov 22, 2022 at 11:25:55AM +0100, Claudio Jeker wrote:
> On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote:
> > Need to query (and set $if, which might be used in route commands etc) I 
> > think.
> > 
> 
> I would prefer if people took a step back from configuring interfaces by
> MAC address. It feels like a overly specific hack is introduced for
> a case that should be handled in a different way.
> 

[skip]

> 
> Btw. a lot of this can be done already now by using '!' in hostname.if
> It wont be pretty but it is also not a common use case.

Agree.



Re: mg: handle prefix argument in shell-command{,-on-region}

2022-11-22 Thread Omar Polo
anyone?

On 2022/11/09 09:00:03 +0100, Omar Polo  wrote:
> bump
> 
> On 2022/10/25 14:30:51 +0200, Omar Polo  wrote:
> > On 2022/10/13 12:25:00 +0200, Omar Polo  wrote:
> > > shell-command (M-!) and shell-command-on-region (M-|) works by
> > > displaying the output of the command in a new buffer, but in emacs
> > > using a prefix argument (C-u) allows to operate on the current buffer.
> > > 
> > > diff belows adds that for mg.  I can finally C-u M-! got diff RET when
> > > composing mails :)
> > > 
> > > A possible drawback is that now the *Shell Command Output* buffer
> > > gains an undo history.  linsert is also possibly slower than addline
> > > but on the plus side we're no more limited to BUFSIZ long lines.
> > > 
> > > ok/comments/improvements?
> > 
> > Here's a slightly tweaked version that adds a missing parens around a
> > return value and uses ssize_t for some vars in preadin.  it also changes
> > the size read(2) from BUFSIZ-1 to BUFSIZ since we no longer need to NUL
> > terminate it.
> > 
> > This has been more useful than I originally expected.  I wanted it to
> > include diffs and the like more easily, now i'm using it also for all
> > sorts of stuff that mg doesn't do out-of-the-box (like using C-u M-|
> > sort RET instead of M-x sort-lines.)
> > 
> > If it were for me, M-| and M-! would operate by default on the buffer
> > and with C-u on a scratch one, but this is what emacs does and i'm
> > probably several decades too late :)


diff 214e94d3085276f4e5c6b416bfd54b5d50a7bf91 
cde294b6d4634ab98c7926103d373202007e23c2
commit - 214e94d3085276f4e5c6b416bfd54b5d50a7bf91
commit + cde294b6d4634ab98c7926103d373202007e23c2
blob - 21c5174f52d21103b9cd15942620eb746b5069b2
blob + 1bee60b37ceea3f9ec3ceb779727f877a7f15851
--- usr.bin/mg/region.c
+++ usr.bin/mg/region.c
@@ -26,14 +26,13 @@ static char leftover[BUFSIZ];
 
 #define TIMEOUT 1
 
-static char leftover[BUFSIZ];
-
 static int getregion(struct region *);
 static int iomux(int, char * const, int, struct buffer *);
 static int preadin(int, struct buffer *);
 static voidpwriteout(int, char **, int *);
 static int setsize(struct region *, RSIZE);
-static int shellcmdoutput(char * const[], char * const, int);
+static int shellcmdoutput(char * const[], char * const, int,
+   struct buffer *);
 
 /*
  * Kill the region.  Ask "getregion" to figure out the bounds of the region.
@@ -419,14 +418,11 @@ piperegion(int f, int n)
 piperegion(int f, int n)
 {
struct region region;
+   struct buffer *bp = NULL;
int len;
char *cmd, cmdbuf[NFILEN], *text;
char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
 
-   /* C-u M-| is not supported yet */
-   if (n > 1)
-   return (ABORT);
-
if (curwp->w_markp == NULL) {
dobeep();
ewprintf("The mark is not set now, so there is no region");
@@ -452,7 +448,13 @@ piperegion(int f, int n)
 
region_get_data(, text, len);
 
-   return shellcmdoutput(argv, text, len);
+   if (n > 1) {
+   bp = curbp;
+   killregion(FFRAND, 1);
+   kdelete();
+   }
+
+   return (shellcmdoutput(argv, text, len, bp));
 }
 
 /*
@@ -462,12 +464,12 @@ shellcommand(int f, int n)
 int
 shellcommand(int f, int n)
 {
-
+   struct buffer *bp = NULL;
char *cmd, cmdbuf[NFILEN];
char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
 
if (n > 1)
-   return (ABORT);
+   bp = curbp;
 
if ((cmd = eread("Shell command: ", cmdbuf, sizeof(cmdbuf),
EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
@@ -475,36 +477,43 @@ shellcommand(int f, int n)
 
argv[2] = cmd;
 
-   return shellcmdoutput(argv, NULL, 0);
+   return (shellcmdoutput(argv, NULL, 0, bp));
 }
 
-
 int
-shellcmdoutput(char* const argv[], char* const text, int len)
+shellcmdoutput(char* const argv[], char* const text, int len,
+struct buffer *bp)
 {
-
-   struct buffer *bp;
+   struct mgwin *wp;
char*shellp;
-   int  ret;
+   int  ret, special = 0;
 
-   bp = bfind("*Shell Command Output*", TRUE);
-   bp->b_flag |= BFREADONLY;
-   if (bclear(bp) != TRUE) {
-   free(text);
-   return (FALSE);
+   if (bp == NULL) {
+   special = 1;
+   bp = bfind("*Shell Command Output*", TRUE);
+   bp->b_flag &= ~BFREADONLY;  /* disable read-only */
+   wp = popbuf(bp, WNONE);
+   if (wp == NULL || bclear(bp) != TRUE) {
+   free(text);
+   return (FALSE);
+   }
+   curbp = bp;
+   curwp = wp;
}
 
shellp = getenv("SHELL");
 
ret = pipeio(shellp, argv, text, len, bp);
-
if (ret == TRUE) {
eerase();
-   if (lforw(bp->b_headp) == bp->b_headp)
+   if 

Re: interface hooks to pf(4) should be using PF_LOCK()/PF_UNLOCK()

2022-11-22 Thread David Gwynne



> On 22 Nov 2022, at 18:49, Alexandr Nedvedicky  wrote:
> 
> Hello,
> 
> this change is required to unhook pf(4) from NET_LOCK().
> therefore I'd like to get it in.

ok dlg@

> 
> On Mon, Nov 07, 2022 at 04:51:59AM +1000, David Gwynne wrote:
>> 
>> 
>>> On 7 Nov 2022, at 4:12 am, Alexandr Nedvedicky  wrote:
>>> 
>>> Hello,
>>> 
>>> diff below is the first step to make pfioctl() _without_ NET_LOCK().
>>> Currently pf_if.c seems to be the only blocker which prevents us
>>> from removing all NET_LOCK()/NET_UNLOCK() calls we have in pf(4).
>>> 
>>> diff below passed very basic smoke test. OK to commit?
>>> 
>>> 
>>> thanks and
>>> regards
>>> sashan
>>> 
>>> 8<---8<---8<--8<
>>> diff --git a/sys/net/pf_if.c b/sys/net/pf_if.c
>>> index e23c14e6769..e86d85aa2c4 100644
>>> --- a/sys/net/pf_if.c
>>> +++ b/sys/net/pf_if.c
>>> @@ -53,10 +53,17 @@
>>> 
>>> #include 
>>> 
>>> +#include 
>>> +#include 
>>> +#include 
>> 
>> do we need this chunk?
>> 
> 
>chunk above is required. It got sucked as a dependency for
>pfvar_priv.h We need pfvar_priv.h because it provides definitions
>for PF_LOCK(). With PF_LOCK() we are also getting definition of pf_pdesc,
>which requires header files above.
> 
> thanks and
> regards
> sashan



Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Claudio Jeker
On Tue, Nov 22, 2022 at 09:25:08AM +, Stuart Henderson wrote:
> Need to query (and set $if, which might be used in route commands etc) I 
> think.
> 

I would prefer if people took a step back from configuring interfaces by
MAC address. It feels like a overly specific hack is introduced for
a case that should be handled in a different way.

Not all interfaces have MAC addresses. E.g. umb(4) would need a
different identifier (most probably the IMEI). Some interfaces inherit
the MAC address from an other interface (vlan, trunk).

This requires the use of interface groups to 'rename' these interfaces
e.g. as 'green' and 'blue' or 'in' and 'out'. So that you can use these
handles in pf.conf and other commands (rad comes to mind). Not all
commands work with interface groups. route(8) is such an example but there
are more commands needing this.

Btw. a lot of this can be done already now by using '!' in hostname.if
It wont be pretty but it is also not a common use case.
-- 
:wq Claudio

> On 22 November 2022 08:37:05 Florian Obser  wrote:
> 
> > On 2022-11-22 18:06 +10, David Gwynne  wrote:
> > > 
> > > There are a few things to keep in mind if we're going to use lladdrs like 
> > > this.
> > > 
> > > vlan interfaces start with their lladdr as 00:00:00:00:00:00 and
> > > then assume the lladdr of the parent interface when that is
> > > configured.
> > > 
> > > Clonable devices (eg, egre, vport, aggr, etc) tend to generate random
> > > lladdrs when they're created. If you want a consistent lladdr for
> > > these you configure that in /etc/hostname.vportX or whatever you're
> > > using.
> > 
> > ifconfig(8) already knows about these (see -C option). Which made me
> > think, it might be easier to just ask ifconfig(8).
> > 
> > $ ifconfig -Q 00:80:41:7b:f3:c3
> > vio0
> > 
> > Would that be helpful?
> > 
> > Or would you need
> > 
> > # ifconfig 00:80:41:7b:f3:c3 inet 192.0.2.2/24
> > 
> > to work?
> > 
> > I think you want to query,no?
> > 
> > > 
> > > "Platform deficient" systems like arm SBCs don't always do a good job
> > > of providing lladdrs for their ethernet interfaces. I'm working on one
> > > now that has rge(4) and it comes up with an lladdr of
> > > 00:00:00:00:00:00. I have another one where the drivers fall back to
> > > randomly generating an lladdr if none is set by u-boot/edk/etc.
> > > 
> > > I don't think any of these are showstoppers, but do need to be considered.
> > > 
> > 
> > --
> > I'm not entirely sure you are real.
> 



Re: reorder_kernel: suport ro /usr like library_aslr does

2022-11-22 Thread Thomas de Grivel
Hello,

I use read only /usr also,
I don't like a script changing mount options without a warning.
If it's read-only reorder_kernel should fail.
I just put a symlink from /usr/share/relink to /var/relink and everything
works fine...
With mount_mfs we could have a temporary directory but where has gone mfs ?

Le mar. 8 nov. 2022 à 12:11, Klemens Nanni  a écrit :

> More read-only filesystems mean less fsck during boot after crashes.
> Especially on crappy machines like the Pinebook Poop, I keep /usr
> read-only and run with this diff so I still get a relinked kernel.
>
> rc's reorder_libs() already does the same remount dance, but for
> multiple directories/filesystems.
>
> My diff works as expected with read-write and read-only /usr as well as
> interrupting /usr/libexec/reorder_kernel runs with ^C, i.e. a failed run
> will correctly mount /usr ro again if it was ro before the run.
>
> Feedback? OK?
>
> Index: reorder_kernel.sh
> ===
> RCS file: /cvs/src/libexec/reorder_kernel/reorder_kernel.sh,v
> retrieving revision 1.13
> diff -u -p -r1.13 reorder_kernel.sh
> --- reorder_kernel.sh   7 Nov 2022 15:55:56 -   1.13
> +++ reorder_kernel.sh   8 Nov 2022 11:02:42 -
> @@ -27,13 +27,32 @@ LOGFILE=$KERNEL_DIR/$KERNEL/relink.log
>  PROGNAME=${0##*/}
>  SHA256=/var/db/kernel.SHA256
>
> -# Silently skip if on a NFS mounted filesystem.
> -df -t nonfs $KERNEL_DIR >/dev/null 2>&1
> +# Silently skip if on NFS, otherwise remount the filesystem read-write.
> +DEV=$(df -t nonfs $KERNEL_DIR 2>/dev/null | awk 'NR == 2 { print $1 }')
> +MP=$(mount -t ffs | grep ^$DEV)
> +RO=false
> +if [[ $MP == *read-only* ]]; then
> +   MP=${MP%% *}
> +   mount -u -w $MP
> +   RO=true
> +fi
> +
> +restore_mount() {
> +   if $RO; then
> +   # Close the logfile to unbusy $MP by switching back to
> console.
> +   exec 1>/dev/console
> +   exec 2>&1
> +   mount -u -r $MP
> +   fi
> +}
>
>  # Install trap handlers to inform about success or failure via syslog.
>  ERRMSG='failed'
> -trap 'trap - EXIT; logger -st $PROGNAME "$ERRMSG" >/dev/console 2>&1' ERR
> -trap 'logger -t $PROGNAME "kernel relinking done"' EXIT
> +trap 'trap - EXIT
> +   logger -st $PROGNAME "$ERRMSG" 2>/dev/console
> +   restore_mount' ERR
> +trap 'logger -t $PROGNAME "kernel relinking done"
> +   restore_mount' EXIT
>
>  # Create kernel compile dir and redirect stdout/stderr to a logfile.
>  mkdir -m 700 -p $KERNEL_DIR/$KERNEL
>
>

-- 
 Thomas de Grivel
 https://www.kmx.io


Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Stuart Henderson

Need to query (and set $if, which might be used in route commands etc) I think.

--
 Sent from a phone, apologies for poor formatting.

On 22 November 2022 08:37:05 Florian Obser  wrote:


On 2022-11-22 18:06 +10, David Gwynne  wrote:


There are a few things to keep in mind if we're going to use lladdrs like this.

vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then 
assume the lladdr of the parent interface when that is configured.


Clonable devices (eg, egre, vport, aggr, etc) tend to generate random
lladdrs when they're created. If you want a consistent lladdr for
these you configure that in /etc/hostname.vportX or whatever you're
using.


ifconfig(8) already knows about these (see -C option). Which made me
think, it might be easier to just ask ifconfig(8).

$ ifconfig -Q 00:80:41:7b:f3:c3
vio0

Would that be helpful?

Or would you need

# ifconfig 00:80:41:7b:f3:c3 inet 192.0.2.2/24

to work?

I think you want to query,no?



"Platform deficient" systems like arm SBCs don't always do a good job
of providing lladdrs for their ethernet interfaces. I'm working on one
now that has rge(4) and it comes up with an lladdr of
00:00:00:00:00:00. I have another one where the drivers fall back to
randomly generating an lladdr if none is set by u-boot/edk/etc.

I don't think any of these are showstoppers, but do need to be considered.



--
I'm not entirely sure you are real.




Re: interface hooks to pf(4) should be using PF_LOCK()/PF_UNLOCK()

2022-11-22 Thread Alexandr Nedvedicky
Hello,

this change is required to unhook pf(4) from NET_LOCK().
therefore I'd like to get it in.


On Mon, Nov 07, 2022 at 04:51:59AM +1000, David Gwynne wrote:
> 
> 
> > On 7 Nov 2022, at 4:12 am, Alexandr Nedvedicky  wrote:
> > 
> > Hello,
> > 
> > diff below is the first step to make pfioctl() _without_ NET_LOCK().
> > Currently pf_if.c seems to be the only blocker which prevents us
> > from removing all NET_LOCK()/NET_UNLOCK() calls we have in pf(4).
> > 
> > diff below passed very basic smoke test. OK to commit?
> > 
> > 
> > thanks and
> > regards
> > sashan
> > 
> > 8<---8<---8<--8<
> > diff --git a/sys/net/pf_if.c b/sys/net/pf_if.c
> > index e23c14e6769..e86d85aa2c4 100644
> > --- a/sys/net/pf_if.c
> > +++ b/sys/net/pf_if.c
> > @@ -53,10 +53,17 @@
> > 
> > #include 
> > 
> > +#include 
> > +#include 
> > +#include 
> 
> do we need this chunk?
> 

chunk above is required. It got sucked as a dependency for
pfvar_priv.h We need pfvar_priv.h because it provides definitions
for PF_LOCK(). With PF_LOCK() we are also getting definition of pf_pdesc,
which requires header files above.

thanks and
regards
sashan



Re: lladdr support for netstart/hostname.if

2022-11-22 Thread Florian Obser
On 2022-11-22 18:06 +10, David Gwynne  wrote:
>
> There are a few things to keep in mind if we're going to use lladdrs like 
> this.
>
> vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then assume 
> the lladdr of the parent interface when that is configured.
>
> Clonable devices (eg, egre, vport, aggr, etc) tend to generate random
> lladdrs when they're created. If you want a consistent lladdr for
> these you configure that in /etc/hostname.vportX or whatever you're
> using.

ifconfig(8) already knows about these (see -C option). Which made me
think, it might be easier to just ask ifconfig(8).

$ ifconfig -Q 00:80:41:7b:f3:c3
vio0

Would that be helpful?

Or would you need

# ifconfig 00:80:41:7b:f3:c3 inet 192.0.2.2/24

to work?

I think you want to query,no?

>
> "Platform deficient" systems like arm SBCs don't always do a good job
> of providing lladdrs for their ethernet interfaces. I'm working on one
> now that has rge(4) and it comes up with an lladdr of
> 00:00:00:00:00:00. I have another one where the drivers fall back to
> randomly generating an lladdr if none is set by u-boot/edk/etc.
>
> I don't think any of these are showstoppers, but do need to be considered.
>

-- 
I'm not entirely sure you are real.



Re: lladdr support for netstart/hostname.if (was: Re: Locking network card configuration)

2022-11-22 Thread David Gwynne



> On 22 Nov 2022, at 16:13, Andrew Hewus Fresh  wrote:
> 
> On Mon, Nov 21, 2022 at 04:56:07PM +0100, Martijn van Duren wrote:
>> On Sun, 2022-11-20 at 19:35 -0700, Theo de Raadt wrote:
>>> Steve Litt  wrote:
>>> 
 Vitaliy Makkoveev said on Mon, 21 Nov 2022 03:48:21 +0300
 
>> On 20 Nov 2022, at 18:06, Odd Martin Baanrud 
>> wrote:
>> 
>> Hello,
>> 
>> I have a Raspberry Pi 4 with 2 USB NIC’s attached.
>> One via USB3 (ure0), and the other via USB2 (ure1).
>> Since they are connected to different USB interfaces, I thaught they
>> would get configured the same way on reboot. But that’s not the case.
>> They became swapped on reboot.
>> Is there a way to “lock” the configuration I want?
>> So the USB3 NIC always become ure0, and the USB2 ure1.
>> 
>> Regards, Martin
>> 
> 
> You could parse ifconfig(8) output to determine which names network
> interfaces received. But unfortunately, you can’t rename interfaces.
 
 During your parsing you could assign each one to an environment
 variable such that, for instance, $lan contains the network card name
 of the LAN one, and $wan contains the network name of the one going to
 the Internet. Unfortunately, this would probably mean changing a lot of
 existing shellscripts, but it's doable.
>>> 
>>> But that is not the problem.
>>> 
>>> hostname.* installs addresses on an interface, based upon the name of that
>>> interface.
>>> 
>>> So it is too late for what you suggest.
>>> 
>>> Unless the suggestion is have each hostname.* do a !command to a script 
>>> which
>>> does the assigning.  That is pretty crazy.
>>> 
>>> pf.conf is not the problem either, because that can be entirely written 
>>> using
>>> egress and groups.
>>> 
>>> 
>>> 
>>> There is a problem with device attachment -> naming a device at that
>>> moment -> using that name in netstart.. but I am not sure how we could
>>> solve this without creating bigger problems for everyone else in the
>>> other non-hot-plug configurations, which is the majority of users with
 1 network device.
>>> 
>>> We also hit this problem with disks, and we worked around it with the
>>> DUID subsystem.
>>> 
>>> 
>>> I suppose there is some argument that we should support hostname.MAC
>>> files
>>> 
>> I don't have a usecase for this myself, but it seemed like a nice
>> exercise and might get the ball rolling. I also don't have much
>> experience with our rc/netstart shellcode, so I'm expecting this diff
>> should be taken as a starting-point iff we want something like this.
>> 
>> I've chosen to error out on missing lladdr, duplicate lladdr and when
>> there's a hostname.if for both the lladdr and the if variant. This means
>> that there's smaller chance for order confusion or doubly applied
>> configs. Downside is that if someone decided to backup their hostname.if
>> to hostname.lladdr that will break their setup. However, I don't expect
>> people to backup their config files in this manner, but you never know.
>> 
>> Errors:
>> On duplicate lladdr (in this case em0 and iwx0 in trunk0):
>> $ doas sh /usr/src/etc/netstart 88:a4:c2:fb:84:77 
>> netstart: /etc/hostname.88:a4:c2:fb:84:77: unique if for lladdr not found.
>> 
>> On missing lladdr:
>> $ doas sh /usr/src/etc/netstart 88:a4:c2:fb:84:76 
>> netstart: /etc/hostname.88:a4:c2:fb:84:76: unique if for lladdr not found.
>> 
>> And having both hostname.if and hostname.lladdr installed:
>> $ doas sh ./netstart 00:11:22:33:44:55
>> netstart: /etc/hostname.00:11:22:33:44:55: duplicate config found in 
>> /etc/hostname.vio0.
>> $ doas sh ./netstart vio0 
>> netstart: /etc/hostname.vio0: duplicate config found in 
>> /etc/hostname.00:11:22:33:44:55.
>> 
>> Two omissions I considered but didn't implement:
>> 1) I didn't test if the lladdr is owned by one of `ifconfig -C`
>>   interfaces. Not sure if this is an upside or downside.
>> 2) Allowing /etc/netstart if1 and parsing the hostname.lladdr1 and vice
>>   versa.
> 
> 
> I got interested in this, and looked at it a bit.  My diff is also a bit
> preliminary, but a couple of things.
> 
> First, I only parse ifconfig output once and save the LLADDR_MAP to look
> up later.  This makes the lookup functions a bit simpler.  Also, the
> glob now uses xdigit, which seems more correct, unless there's something
> I am missing about mac addresses.
> 
> I also thought the error message for `netstart $lladdr` when
> /etc/hostname.$lladdr doesn't exist, but /etc/hostname.$if does was poor
> (it claimed duplicate configs which wasn't true) so I thought the
> easiest solution was to implement your #2 there and allow it to start
> the $if when you specify the $lladdr.
> 
> Unfortunately, I then looked at the clock and realized it's time for
> bed, but I figured you might be interested in another take, even if it's
> probably incomplete.  In any case, tomorrow is dinner with friends, so
> it will be Wednesday before I again have