dhcpcd-discuss

On 2019/11/10 12:06, Nathan Houghton wrote:
> Hi,
> 
> 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.

dhcpcd tracks LINK_STATE_UNKNOWN separately but it's already handled by the
earlier case in the switch, so LINK_STATE_IS_UP shouldn't cause any problem
for dhcpcd here, and it seems better to use the standard interface in case
any other "up" states are added.