dhcpcd-discuss

Hi Nathan

On 10/11/2019 20:06, Nathan Houghton wrote:
> OS info: OpenBSD 6.6 stable
> DHCPCD version: dhcpcd-8.0.6 (installed from OpenBSD ports tree)
>
> Recently I found that dhcpcd was having issues detecting a vlan 
> interface entering the "up" state after unplugging and re-plugging the 
> Ethernet cable.
>
> After some debug, I found that RTM_IFINFO handler in src/if-bsd.c was 
> treating the OpenBSD specific LINK_STATE_HALF_DUPLEX and 
> LINK_STATE_FULL_DUPLEX values of "ifi_link_state" as "LINK_DOWN" states, 
> due to them missing in the switch statement.
>
> --------8<--- OpenBSD's net/if.h [1] -----------
> #define LINK_STATE_UP        4    /* link is up */
> #define LINK_STATE_HALF_DUPLEX    5    /* link is up and half duplex */
> #define LINK_STATE_FULL_DUPLEX    6    /* link is up and full duplex */
> --------8<--------------
>
> Please find a patch that addresses this issue below.
>
> cat patches/patch-src_if-bsd_c
> --------8<--------------
> $OpenBSD$
>
> Index: src/if-bsd.c
> --- src/if-bsd.c.orig
> +++ src/if-bsd.c
> @@ -1068,6 +1068,10 @@ if_ifinfo(struct dhcpcd_ctx *ctx, const struct 
> if_msgh
>                  link_state = LINK_UNKNOWN;
>                  break;
>          case LINK_STATE_UP:
> +#if defined(__OpenBSD__)
> +       case LINK_STATE_HALF_DUPLEX:
> +       case LINK_STATE_FULL_DUPLEX:
> +#endif
>                  link_state = LINK_UP;
>                  break;
>          default:
> --------8<--------------
>
> I saw that there was also a LINK_STATE_IS_UP macro defined in OpenBSD's 
> "/usr/include/net/if.h" [1], but since dhcpcd seemed to already be 
> tracking the "UNKNOWN" state independently from up / down, I went with 
> the approach above. I BCCed Stuart Henderson (the OpenBSD dhcpcd port 
> maintainer), hoping he will chime in if he prefers a different approach.

The patch looks fine and is comitted here with a minor change incase any 
other OS picks up this OpenBSD feature:
https://roy.marples.name/cgit/dhcpcd.git/commit/src/if-bsd.c?id=c7312d1852168efe3307188e6d2874b7d73d8c9d

I track LINK_STATE_UNKNOWN differently from the BSD LINK_STATE_IS_UP 
macro because it's *wrong*, especially for RTM_IFINFO messages. The 
rationale being if we get a RTM_IFINFO message then the driver by nature 
*must* support link state changes and thusly *must* send a correct UP or 
DOWN state at some point.

This is important as many wireless drivers love to send 
LINK_STATE_UNKNOWN as they are coming up and dhcpcd needs to ignore that 
as it's not possible to get the SSID we're connected to at this point.
Why is this important? Well, some drivers are equally quite spammy and 
just set UP UP UP UP UP. NetBSD grew code to avoid this being sent to 
userland, but I don't think other BSD's have this and thus the current 
state of affairs.

Thanks for the patch!

Roy