Hello,

Amos Jeffries, le lun. 13 oct. 2025 17:50:27 +1300, a ecrit:
> On 13/10/2025 05:00, Joan Lledó wrote:
> > From: Joan Lledó
> > 
> > This adds a new main module to port dhcpdp to the Hurd.
> > 
> > * The Hurd module relies on libpcap to work with BPF filters.
> > * Addresses and routes are configured via ioctls to the stack.
> > * Only IPv4 over Ethernet is supported so far.
> 
> Might be better to go the other way and "only" support IPv6.
> 
> IPv4 addresses are a subset of IPv6 address space,

Yes, but DHCPv4 and DHCPv6 are really very different things, I don't
think dhcpcd treats them with common codepaths.

> > +#ifdef INET
> > +int
> > +if_route(unsigned char cmd, const struct rt *rt) {
> > +  int err;
> > +  struct dhcpcd_ctx *ctx;
> > +  struct kroute krt;
> > +
> > +  if_copykrt(&krt, rt);
> > +
> > +  ctx = rt->rt_ifp->ctx;
> > +
> > +  err = if_ioctl(ctx,
> > +        cmd == RTM_DELETE ? SIOCDELRT : SIOCADDRT, &krt, sizeof(krt));
> > +
> > +  return err;
> > +}
> 
> It is not clear why the above is IPv4-specific. Seems to me the logic is
> shared by all IP versions (past, present, or future).

The principle, yes, but the route structures are different, and the ipv6
stack could even be completely separate from the ipv4 stack.

> > +int
> > +if_addrflags(__unused const struct interface *ifp, __unused const struct 
> > in_addr *addr,
> > +    __unused const char *alias) {
> > +    /* TODO */
> > +    return 0;
> > +}
> > +#endif
> > +
> > +#ifdef INET6
> > +int
> > +if_address6(__unused unsigned char cmd, __unused const struct ipv6_addr 
> > *ia) {
> > +    /* TODO: We could add custom IOCTLs for this */
> 
> 
> FWIW, It may be better to refactor the existing IOCTLs to handle struct
> ifreq with non-IPv4 sin_family.

Better have something that works now with what we already have, than try
to make something perfect and never finish it...

> The struct ifreq is defined with sockaddr members.

Which unfortunately means that it's already too small to contain an
ipv6, so something really new would need to be designed with struct
sockaddr.

> > +    /* Get the MAC address */
> > +    kr = device_get_status (mach_dev, NET_ADDRESS, net_address, &count);
> > +    if (kr) {
> > +        logerrx("%s: cannot get device MAC address: %s", __func__, 
> > ifp->name);
> > +        goto eexit;
> > +    }
> > +
> > +    /* We expect 6 bytes. Don't allow partial or incomplete addresses */
> > +    if (count * sizeof (int) < ETH_HWADDR_LEN) {
> > +        logerrx("%s: insufficient MAC address data: %d bytes", __func__, 
> > count * sizeof(int));
> > +        goto eexit;
> > +    }
> > +
> > +    net_address[0] = (int)ntohl((uint32_t)net_address[0]);
> > +    net_address[1] = (int)ntohl((uint32_t)net_address[1]);
> 
> I expect this will have issues on 64-bit build where "int" is more than
> 32-bits long.

We don't have architectures where int is more than 32bit long.
The whole device_get/set_status RPCs principles would need to be
completely fixed in such a case.

> > +    /* TODO */
> > +    return 0;

Probably some of these should rather return an error to explicitly
express that it's unsupported.

Samuel

Reply via email to