In D58287#1336892, @ivy wrote:

In D58287#1336891, @pouria wrote:

With this patch, you're still able to do this.

okay, but you said:

We only going to disallow setting same /32 address on multiple interfaces.

so it seems like you intend to remove the ability to do this. have i misunderstood?

In D58287#1336891, @pouria wrote:

Same result.

In D58287#1336891, @pouria wrote:

With this patch, you're still able to do this.

In D58287#1336871, @ivy wrote:

you have to have an IPv4 address assigned to every interface that sends IPv4 traffic. for a router, this means any interface that routes IPv4 traffic, because of ICMP error generation. an IPv4 address on the outbound interface is also required for locally-originated outbound connections that aren't bind(2)ed to a specific IP address (this applies to hosts as well as routers).

if you don't do this, the outgoing packets will have a source address of 0.0.0.0. having an IPv4 address on the loopback is not enough, it also needs to be assigned to the outgoing interface or the source selection algorithm won't use it.

in "traditional" IPv4 setups, this isn't an issue because you always have at least one link net assigned to every interface that participates in IPv4. however, modern networks are more likely to route IPv4 using IPv6 nexthops, in which case there's no need for an IPv4 address on the interface because the nexthop resolution uses IPv6 ND. but you still need to assign an address to the interface for the source selection algorithm to be happy, and in nearly all cases, you will want to use the loopback address for that.

that means a practical FreeBSD router configuration in a modern network will look something like this:

ifconfig_lo0_alias0="1.2.3.4/32"
ifconfig_ix0="inet 1.2.3.4/32"
ifconfig_ix0_ipv6="inet6 auto_linklocal"
ifconfig_ix1="inet 1.2.3.4/32"
ifconfig_ix1_ipv6="inet6 auto_linklocal"
ifconfig_ix2="inet 1.2.3.4/32"
ifconfig_ix2_ipv6="inet6 auto_linklocal"
[ ... etc ... ]

To be honest, /usr/include/profile.h is a conventional place for this kerberos header and consumers expecting to find it here; for example take a look at net/freerdp3 port

i really don't like the filename /usr/include/profile.h, it's very generic and seems certain to conflict with something else in the future.

as an aside, for the ICMP case i believe you can work around this using the sysctl net.inet.icmp.reply_src=lo0 -- although i haven't tested this on a fully unnumbered interface, i would assume it works. but this doesn't affect outgoing non-ICMP connections, and it doesn't let you choose the source address per-interface, so it's not a replacement for the existing functionality.

In D58287#1336793, @pouria wrote:

Can you explain this please? The local address will be added as a loopback route anyway, with or without this patch.
Therefore, this should not break your use case.

yes i tested this!

yes i tested this!

yes i tested this!

yes i tested this!

In D58056#1336834, @ngie wrote:

In D58056#1336833, @kib wrote:

In D58056#1336831, @ngie wrote:

This change broke the build on armv7 (32-bit ARM). I addressed the definition/declaration mismatch in rG5cafd6213f14.

This is the supposed fix, I am waiting for tinderbox to finish.

diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c
index c867867f2618..c16e806e0bd7 100644
--- a/sys/kern/sys_generic.c
+++ b/sys/kern/sys_generic.c
@@ -2353,8 +2353,8 @@ kern_exterrctl(struct thread *td, u_int op, u_int flags, void *ptr)
 }
 
 int
-exterr_set(int eerror, int category, const char *mmsg, uintptr_t pp1,
-    uintptr_t pp2, int line)
+exterr_set(int eerror, int category, const char *mmsg, uint64ptr_t pp1,
+    uint64ptr_t pp2, int line)
 {
   struct thread *td;
 

I *just* committed that 😅...

In D58056#1336833, @kib wrote:

In D58056#1336831, @ngie wrote:

This change broke the build on armv7 (32-bit ARM). I addressed the definition/declaration mismatch in rG5cafd6213f14.

This is the supposed fix, I am waiting for tinderbox to finish.

diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c
index c867867f2618..c16e806e0bd7 100644
--- a/sys/kern/sys_generic.c
+++ b/sys/kern/sys_generic.c
@@ -2353,8 +2353,8 @@ kern_exterrctl(struct thread *td, u_int op, u_int flags, void *ptr)
 }
 
 int
-exterr_set(int eerror, int category, const char *mmsg, uintptr_t pp1,
-    uintptr_t pp2, int line)
+exterr_set(int eerror, int category, const char *mmsg, uint64ptr_t pp1,
+    uint64ptr_t pp2, int line)
 {
   struct thread *td;
 

In D58056#1336831, @ngie wrote:

This change broke the build on armv7 (32-bit ARM). I addressed the definition/declaration mismatch in rG5cafd6213f14.

This change broke the build on ARM64/GENERIC.

Some performance data. The test that pushes local port allocation to the corner run on CURRENT as base, then D58131 as unlock and then on D58131 and this revision combined as cache. Using virtual machine with 8 CPUs.

x base
+ unlock
* cache
+------------------------------------------------------------------------------+
|*                                                                             |
|*                                                                             |
|*                                                                             |
|*                                                                             |
|*                                                                             |
|*                                                                             |
|*    +                                                                        |
|*    ++                                                                       |
|*    ++                                                                       |
|*    +++++                     xx x    x x       xx          x  x            x|
|A    |A|                        |____________M__A______________|              |
+------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x  10         14.44         36.24         21.28        22.525     7.2199696
+  10          2.39          4.14         2.835         3.021    0.59897041
Difference at 95.0% confidence
        -19.504 +/- 4.81339
        -86.5882% +/- 3.35841%
        (Student's t, pooled s = 5.12283)
*  10          0.14          0.21          0.14         0.147   0.022135944
Difference at 95.0% confidence
        -22.378 +/- 4.79693
        -99.3474% +/- 0.153552%
        (Student's t, pooled s = 5.10531)

revert to original manpage format

In D58261#1336806, @alc wrote:

I'm surprised that we didn't see this on arm64.

I'm surprised that we didn't see this on arm64.

In D58287#1336725, @ivy wrote:

In D58287#1336724, @pouria wrote:

Assigning the same prefix (e.g. in your case /32) to two different interfaces in the same FIB shouldn't be allowed.

why not?

Whoops, thanks!

In D58287#1336793, @pouria wrote:

I'm confused how this patch might affect those BRASes. (I had those in my last ISP job for ADSL services. But I'm no longer working there.)
The loopback route is still here.
We only going to disallow setting same /32 address on multiple interfaces.
On BRAS, each user gets it's own tun and using same loopback address on multiple p2p ifps (tun) is still working since the in_addprefix() -> in_hasrtprefix() will take care of that.
For e.g:

tun0: flags=1008051<UP,POINTOPOINT,RUNNING,MULTICAST,LOWER_UP> metric 0 mtu 1500
options=4080000<LINKSTATE,MEXTPG>
inet 192.168.0.1 --> 192.168.0.2 netmask 0xffffff00
groups: tun
nd6 options=829<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL,STABLEADDR>
tun1: flags=1008051<UP,POINTOPOINT,RUNNING,MULTICAST,LOWER_UP> metric 0 mtu 1500
options=4080000<LINKSTATE,MEXTPG>
inet 192.168.0.1 --> 192.168.0.3 netmask 0xffffff00
groups: tun
nd6 options=829<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL,STABLEADDR>
[root@ftsr1] [~] # netstat -rn4
Routing tables

Internet:
Destination        Gateway            Flags         Netif Expire
default            172.23.1.254       UGS          vtnet0
127.0.0.1          link#3             UH              lo0
192.168.0.1        link#3             UHS             lo0
192.168.0.2        link#6             UH             tun0
192.168.0.3        link#7             UH             tun1

In D58287#1336725, @ivy wrote:

the same reason you put any address on a loopback interface: so it's available when the physical interface is down (or even not present, in case of virtual interfaces like netgraph). otherwise, as soon as the interface goes down, the address is removed from the IGP and you can't connect to the router.

Properly address the problem during installworld.

I can confirm that having the same local PtP (Point-to-Point, in contrast to P2P peer-2-peer) address has been a working feature since the 90s, when people were still operating modem banks.
I am pretty sure I used to have two outgoing connections for most of the 2000s with the same feature (and no need for it to be on loopback but also didn't care about the [icmp] error cases in that setup).

• Don't heap-allocate a std::vector
• Use mount_fusefs with memfs

Looks fine modulo the uz_flags comment and style and history nits.