In D56541#1296061, @kevans wrote:

In D56541#1296047, @jhb wrote:

In D56541#1296015, @kevans wrote:

In D56541#1295966, @jhb wrote:

So you may be interested in my other fixes where I had incorrectly labeled all kernel ports as dummy ports by mistake. See https://reviews.freebsd.org/D56524 and the related stack (and all the back and forth in the associated PR). I wonder if you even need this change anymore once that series of changes lands.

The context for this is the older review that I had tried to get input on: https://reviews.freebsd.org/D51782 -- I don't think your change addresses that, because I seem to be looking at a portal_group_port:

bool                                                                                                          
portal_group_port::is_dummy() const                                                                           
{                                                                                                             
        return (p_portal_group->is_dummy());                                                                  
}

Your out-of-band commentary combined with @asomers notes is what lead me to this path instead, since I couldn't get any traction there...

Oh, oof, there are "kernel" ports and there are "kernel ports". Having been down the long tunnel of the PR I referenced, I am thinking of kernel ports as the things managed by "kports" in ctld now, so physical ports on HBAs, and ioctl ports. Not a pre-existing portal_group port. For a pre-existing portal group port, I guess I'm surprised that we think the port is new. Hmm, looking at your other PR, is the issue that we do indeed add a port and a portal_group, but it has no portals so we think it is a dummy when it isn't? Ah, so it is, and I think I hadn't really understood the issue before now. I think I prefer getting the match right vs ignoring EEXIST errors (which is to say I think I prefer your original patch). If we were going to go the EEXIST route there's not much reason in enumerating non-physical kernel-enumerated ports at all, so I think fixing the match is closer to the code's intent. I might suggest naming the field pg_kernel_enumerated or some such to make it a bit clearer since we have kports so the notion of "kernel port" is a bit overloaded unfortunately.

FWIW, I personally much prefer my original patch as well, since I think it's pretty clear that 9215e501b9e7d3f42567 causing this was a side-effect rather than something intentional. I do think that discrepancy was 100% caused by the noted weirdness in how startup and shutdown works (before your patches) with now-conf::apply, but it'd be nice if we could fix the regression and maybe revisit that after.