Apart from the style issue, I think this can be fine.

Why do you bother calling getcredhostid (which involves mutex lock/inlock) and arc4random second time while creating second interface?
Can't you just use already computed values from scb->oifp->if_hw_addr ?

I'm not 100% happy with hostid as well, I've just followed what already implemented in if_bridge(4).
I can just use arc4rand for those 2 bytes, that should help to mitigate possible collisions and simplify the code.
What do you think?

Using hostid should be fine as it adds protection against inter-jail MAC collisions. Just reuse it when possible instead of calling second time for it.

It might not make sense here, since it would reduce the available space for randomization, but just as a note, the FreeBSD Project has its own OUI space: 58:9C:FC:??:??:??

Well, I have many hosts having over housand of ngXXX interfaces, so yes, two bytes are needed at least.

In D15329#323193, @eugen_grosbein.net wrote:

Well, I have many hosts having over housand of ngXXX interfaces, so yes, two bytes are needed at least.

It's probably worth a little bit of thought as to what is the more common case, a zillion epair interfaces on one host (read: half a zillion vnet jails) or a much smaller number of epairs on a larger number of hosts. It sounds like you are well placed to assign your own addresses, whatever the default may be.

In D15329#323385, @wollman wrote:

It's probably worth a little bit of thought as to what is the more common case, a zillion epair interfaces on one host (read: half a zillion vnet jails) or a much smaller number of epairs on a larger number of hosts. It sounds like you are well placed to assign your own addresses, whatever the default may be.

Well, this is not network hot path and we now have jenkins_hash32() kernel function in all supported branches, so we could just allocate additional uint32_t key[3] array on stack of this function, copy there all 64 bits of (long)hostid plus (uint32_t)if_index and make use of jenkins_hash32() to get them mixed to 32-bit hash value. Then use it as-is for eaddr[1]-eaddr[4] similar to current manner or switch to using 58:9C:FC for upper half of MAC and use lower 24 bits of the hash for the rest of MAC resetting lowest bit to 0 for first interface and to 1 for second one.

In D15329#323510, @eugen_grosbein.net wrote:

In D15329#323385, @wollman wrote:

It's probably worth a little bit of thought as to what is the more common case, a zillion epair interfaces on one host (read: half a zillion vnet jails) or a much smaller number of epairs on a larger number of hosts. It sounds like you are well placed to assign your own addresses, whatever the default may be.

Well, this is not network hot path and we now have jenkins_hash32() kernel function in all supported branches, so we could just allocate additional uint32_t key[3] array on stack of this function, copy there all 64 bits of (long)hostid plus (uint32_t)if_index and make use of jenkins_hash32() to get them mixed to 32-bit hash value. Then use it as-is for eaddr[1]-eaddr[4] similar to current manner or switch to using 58:9C:FC for upper half of MAC and use lower 24 bits of the hash for the rest of MAC resetting lowest bit to 0 for first interface and to 1 for second one.

That would satisfy my objection, for sure. (Still need to set the LAA bit even if we use the official FreeBSD OUI, FWIW.)

In D15329#323510, @eugen_grosbein.net wrote:

In D15329#323385, @wollman wrote:

It's probably worth a little bit of thought as to what is the more common case, a zillion epair interfaces on one host (read: half a zillion vnet jails) or a much smaller number of epairs on a larger number of hosts. It sounds like you are well placed to assign your own addresses, whatever the default may be.

Well, this is not network hot path and we now have jenkins_hash32() kernel function in all supported branches, so we could just allocate additional uint32_t key[3] array on stack of this function, copy there all 64 bits of (long)hostid plus (uint32_t)if_index and make use of jenkins_hash32() to get them mixed to 32-bit hash value. Then use it as-is for eaddr[1]-eaddr[4] similar to current manner or switch to using 58:9C:FC for upper half of MAC and use lower 24 bits of the hash for the rest of MAC resetting lowest bit to 0 for first interface and to 1 for second one.

I like this approach. That's what I'll do (in pseudo code):

eaddr[0] = 0x02
eaddr[5] = 0x0a
eaddr[1-4] = hash( hostid and ifp->if_index )

Unfortunately, hostid is of type unsigned long and to correctly support 32 and 64 bit architectures, the key would be arranged in this way:

key[0] = (uint32_t)ifp->if_index;
key[1] = (uint32_t)(hostid & 0xffffffff);
if (sizeof(unsigned long) == 64)
    key[2] = (uint32_t)((hostid >> 32) & 0xfffffffff);
hash = jenkins_hash32(key, sizeof(unsigned long) == 64 ? 3 : 2, 0);

or alternatively:

key[0] = (uint32_t)ifp->if_index;
key[1] = (uint32_t)hostid & 0xffffffff;
if (sizeof(unsigned long) == 64)
    key[2] = (uint32_t)(hostid >> 32) & 0xfffffffff;
else
    key[2] = 0;
hash = jenkins_hash32(key, 3,  0);

I'm currently rebuilding everything to test the hash approach and then I'll update the patch. Do you see any other issue?

PS: theoretically, the eaddr can be re-arranged:

eaddr[0] = 0xa2; // 0xb2 for the other interface
eaddr[1-4] = hash(...)
eaddr[5] = random_value; // arc4rand maybe?

adding further 8 bit to ensure the etheraddr uniqueness, but I'm not sure about it.

sizeof counts in octets (bytes), not bits
and you do not need complexity of "if (sizeof...)" but simply cast hostid to uint64_t unconditionally first.

I've tested the patch on my laptop and I've found no issues.
I don't have commit bit for src, only for port, so it would be nice if one of you can commit it, now that the patch is approved.
In this way it would be possible to close PR176671 and PR184149.
thanks for all your feedback!