On 2020/10/02 01:35, Klemens Nanni wrote:
> A single commit from Roy Marples in current HEAD fixes dhcpcd when used
> on pppoe(4) interfaces; naddy saw this breakage as well which caused
> sthen to revert to 9.1.4 which apparently works fine for naddy.
>
> I've been experiencing these with 9.1.4 already, neither the 9.2.0
> update (nor the revert obviously) fixed it for me, so I here's the same
> update but with upstream's fix included.
>
> Latest HEAD works for me as well, what's left to debug now is why 9.1.4
> works for naddy but not for me - I'd move this discussion over on
> dhcpcd's list, though.
>
> The commit fails to apply as is due to the src/if.c hunk which I fixed
> manually, otherwise I'd probably have used PATCHFILES instead.
>
> naddy, does this work for you?
> Feedback? OK?
>
Please use standard ports-tree patches for the individual files rather
than a handrolled jumbo patch. (no need for linux/sun parts I think).
The code in git head ("BSD: struct if_data->ifi_link_state is the single
source of truth") looks better but this is an easier patch until a new
release is made.
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/net/dhcpcd/Makefile,v
> retrieving revision 1.80
> diff -u -p -r1.80 Makefile
> --- Makefile 8 Sep 2020 15:02:32 -0000 1.80
> +++ Makefile 1 Oct 2020 23:04:41 -0000
> @@ -2,7 +2,7 @@
>
> COMMENT= DHCPv4/IPv4LL/IPv6RS/DHCPv6 quad stack client
>
> -DISTNAME= dhcpcd-9.1.4
> +DISTNAME= dhcpcd-9.2.0
> EPOCH= 0
>
> CATEGORIES= net
> Index: distinfo
> ===================================================================
> RCS file: /cvs/ports/net/dhcpcd/distinfo,v
> retrieving revision 1.48
> diff -u -p -r1.48 distinfo
> --- distinfo 8 Sep 2020 15:02:32 -0000 1.48
> +++ distinfo 1 Oct 2020 23:04:46 -0000
> @@ -1,2 +1,2 @@
> -SHA256 (dhcpcd-9.1.4.tar.xz) = X+Ez5Ul9ivbSa9bmuN1IqxLRJNbMTO/m3mU2/5f3aCA=
> -SIZE (dhcpcd-9.1.4.tar.xz) = 249648
> +SHA256 (dhcpcd-9.2.0.tar.xz) = /LLRlnLURbv9OGeP3uT1Vu+Wej6mvYEJLRBUXfLLlmY=
> +SIZE (dhcpcd-9.2.0.tar.xz) = 250584
> Index: patches/patch-pppoe_fix
> ===================================================================
> RCS file: patches/patch-pppoe_fix
> diff -N patches/patch-pppoe_fix
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-pppoe_fix 1 Oct 2020 23:26:21 -0000
> @@ -0,0 +1,113 @@
> +$OpenBSD$
> +
> +Cherry-picked (and manually fixed src/if.c hunk), remove with next release:
> +
> +From e675d4dde0d865b6ffc7de733623a52852d284ff Mon Sep 17 00:00:00 2001
> +From: Roy Marples <r...@marples.name>
> +Date: Tue, 22 Sep 2020 13:09:03 +0100
> +Subject: BSD: Detect initial link state in ifa_data
> +
> +Not all interfaces report media state to get the link state.
> +However, link state is available from getifaddrs(3) ifa_data
> +for AF_LINK addresses.
> +
> +Testing shows that link state is also sent correctly via
> +route(4) messages for the same interface.
> +
> +This makes pppoe(4) interfaces more reliable on FreeBSD and OpenBSD.
> +
> +Index: src/if-bsd.c
> +--- src/if-bsd.c.orig
> ++++ src/if-bsd.c
> +@@ -363,11 +363,32 @@ if_carrier(struct interface *ifp)
> + return LINK_UNKNOWN;
> +
> + strlcpy(ifmr.ifm_name, ifp->name, sizeof(ifmr.ifm_name));
> +- if (ioctl(ifp->ctx->pf_inet_fd, SIOCGIFMEDIA, &ifmr) == -1 ||
> +- !(ifmr.ifm_status & IFM_AVALID))
> ++ if (ioctl(ifp->ctx->pf_inet_fd, SIOCGIFMEDIA, &ifmr) == -1)
> + return LINK_UNKNOWN;
> +
> ++ if (!(ifmr.ifm_status & IFM_AVALID))
> ++ return LINK_UNKNOWN;
> ++
> + return (ifmr.ifm_status & IFM_ACTIVE) ? LINK_UP : LINK_DOWN;
> ++}
> ++
> ++int
> ++if_carrier_ifadata(struct interface *ifp, void *ifadata)
> ++{
> ++ int carrier = if_carrier(ifp);
> ++ struct if_data *ifdata;
> ++
> ++ if (carrier != LINK_UNKNOWN || ifadata == NULL)
> ++ return carrier;
> ++
> ++ ifdata = ifadata;
> ++ switch (ifdata->ifi_link_state) {
> ++ case LINK_STATE_DOWN:
> ++ return LINK_DOWN;
> ++ case LINK_STATE_UP:
> ++ return LINK_UP;
> ++ }
> ++ return LINK_UNKNOWN;
> + }
> +
> + static void
> +Index: src/if-linux.c
> +--- src/if-linux.c.orig
> ++++ src/if-linux.c
> +@@ -475,6 +475,13 @@ if_carrier(struct interface *ifp)
> + }
> +
> + int
> ++if_carrier_ifadata(struct interface *ifp, __unused void *ifadata)
> ++{
> ++
> ++ return if_carrier(ifp);
> ++}
> ++
> ++int
> + if_getnetlink(struct dhcpcd_ctx *ctx, struct iovec *iov, int fd, int flags,
> + int (*cb)(struct dhcpcd_ctx *, void *, struct nlmsghdr *), void *cbarg)
> + {
> +Index: src/if-sun.c
> +--- src/if-sun.c.orig
> ++++ src/if-sun.c
> +@@ -240,6 +240,13 @@ err:
> + }
> +
> + int
> ++if_carrier_ifadata(struct interface *ifp, __unused void *ifadata)
> ++{
> ++
> ++ return if_carrier(ifp);
> ++}
> ++
> ++int
> + if_mtu_os(const struct interface *ifp)
> + {
> + dlpi_handle_t dh;
> +Index: src/if.c
> +--- src/if.c.orig
> ++++ src/if.c
> +@@ -682,7 +682,7 @@ if_discover(struct dhcpcd_ctx *ctx, struct ifaddrs **i
> +
> + ifp->active = active;
> + if (ifp->active)
> +- ifp->carrier = if_carrier(ifp);
> ++ ifp->carrier = if_carrier_ifadata(ifp, ifa->ifa_data);
> + else
> + ifp->carrier = LINK_UNKNOWN;
> + TAILQ_INSERT_TAIL(ifs, ifp, next);
> +Index: src/if.h
> +--- src/if.h.orig
> ++++ src/if.h
> +@@ -160,6 +160,7 @@ int if_domtu(const struct interface *, short int);
> + #define if_getmtu(ifp) if_domtu((ifp), 0)
> + #define if_setmtu(ifp, mtu) if_domtu((ifp), (mtu))
> + int if_carrier(struct interface *);
> ++int if_carrier_ifadata(struct interface *, void *);
> + int if_pollinit(struct interface *ifp);
> +
> + #ifdef ALIAS_ADDR
>
