Page MenuHomeFreeBSD

net: Allow binding of unspecified address without address existance
ClosedPublic

Authored by roy_marples.name on Oct 19 2021, 4:49 PM.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

donner added a subscriber: donner.

The motivation behind the check can be understood from the viewpoint of a working system. It is an early dropout for a disconnected system.
But the PR253166 highlights, that this assumption is not valid during the early boot phase. So yes, remove this check.

This revision is now accepted and ready to land.Oct 20 2021, 6:59 AM

Has anyone checked what this was before the epoch work came in?

In D32563#735219, @bz wrote:

Has anyone checked what this was before the epoch work came in?

It was here even before your VIMAGE commits :)

In D32563#735219, @bz wrote:

Has anyone checked what this was before the epoch work came in?

The original check exists in BSD4.4 Lite :)
https://cgit.freebsd.org/src/tree/sys/netinet/in_pcb.c?id=df8bae1de4b67ccf57f4afebd4e2bf258c38910d#n91

So why do we think this is broken now? To me this initially (without looking at that code again after years) seems to be there to save us a lot of work if we cannot find the address later. So maybe this need a better explanation than just "it's broken because someone left a comment that says so"?

In D32563#735249, @bz wrote:

So why do we think this is broken now? To me this initially (without looking at that code again after years) seems to be there to save us a lot of work if we cannot find the address later. So maybe this need a better explanation than just "it's broken because someone left a comment that says so"?

We should be able to bind the unspecified address even if no addresses have been configured.
The socket is expected to work as addresses come and go.

emaste added subscribers: wollman, emaste.

XXX broken! was added in R10:59562606b9d3a. It seems to me @wollman discovered that the check is broken during the TAILQ work and added the XXX to highlight this, rather than an indication of something broken as a result of the TAILQ work.

I don't think this check saves us any work. I imagine it is exceedingly unlikely to have a system that has IPv4 or IPv6 enabled but has no addresses at all (incl loopback) configured. So as long as there is nothing in in_pcbbind_setup or in6_pcbbind that will fail on an empty container this LGTM.

I looked at NetBSD and OpenBSD's in_pcbbind, and NetBSD still has this case while OpenBSD removed it.

It looks like OpenBSD's removal was a side effect of other work. It was removed in:

From c6e49a3d6c2e71354c9e8c0d30443242e0c95e59 Mon Sep 17 00:00:00 2001
From: mpi <mpi@openbsd.org>
Date: Wed, 7 May 2014 08:26:38 +0000
Subject: [PATCH 154/254] Remove the last hacks concerning the global list of
 IPv4 addresses in the source address selection logic.

These hacks were only relevant for the NFS diskless boot code in order to
pick the local broadcast address of the only configured interface.  So, be
explicit and set this address directly.

Tested by florian@, ok henning@, beck@, chrisz@

I'll take care of this
--author=Roy Marples <roy@marples.name> yes?

I'll take care of this
--author=Roy Marples <roy@marples.name> yes?

Thanks and yes please.

In D32563#735382, @driesm.michiels_gmail.com wrote:

@emaste, MFC-able? (maybe 13?)

Yes I would expect to MFC to stable/13 after some time

Thanks for the digging. @emaste please be descriptive in the commit message.