On 07/05/15 14:12, Martin Lucina wrote:
Hi,
I've implemented support for IPv6 SLAAC in brlib (and by extension,
rumprun).
I'll be the first to admit I didn't know what SLAAC is without ddg'ing
it. But, awesome!
One thing I'm not sure about, I modeled the sending code on what
nfs_boot_sendrecv() does, am I right in understanding that if so_send()
completes successfully it takes ownership of the "nam" and "m" mbufs passed
to it?
I don't remember, but with my reading of both sosend() and
nfs_boot_sendrecv(), calling (*so_send()) always transfers ownership.
Historic code sometimes does have these weird interfaces where the
resource ownership is asymmetric depending on the return value, but I
don't like that style because it's very error-prone. Luckily, this
appears to not be one of those cases.
One problem is that as of very recently in NetBSD, "nam" is a "struct
sockaddr" instead of a "struct mbuf". So the code will not work after I
update src-netbsd. This is of course not your fault, but I suggest we
don't commit the code against the old interface at all. I'll try to
update src-netbsd tonight (I was planning to bundle in some other
changes with the update, but I think that plan is from several weeks ago :/)
-mato
diff --git a/brlib/libnetconfig/netconfig.c b/brlib/libnetconfig/netconfig.c
index 7a33b0e..43d6aa2 100644
--- a/brlib/libnetconfig/netconfig.c
+++ b/brlib/libnetconfig/netconfig.c
@@ -28,9 +28,13 @@
#include <sys/socketvar.h>
#include <net/if.h>
+#include <net/if_dl.h>
+#include <net/if_ether.h>
+#include <net/if_types.h>
#include <net/route.h>
#include <netinet/in.h>
+#include <netinet/icmp6.h>
#include <netinet6/in6.h>
#include <netinet6/in6_var.h>
@@ -317,6 +321,93 @@ rump_netconfig_ipv6_gw(const char *gwaddr)
return rv;
}
+extern int ip6_accept_rtadv;
ip6_var.h instead of manual extern?
+ */
+int
+rump_netconfig_auto_ipv6_oneshot(const char *ifname)
"oneshot" in the ipv4 version means that it does not properly implement
leases. Does that apply here too? I don't think so, right?
+ struct ifnet *ifp = NULL;
why = NULL?
+ int ifindex;
+ struct socket *rsso = NULL;
+ int rv = 0;
+ int hoplimit = 255;
+ struct mbuf *m = NULL,
+ *nam = NULL;
+ struct sockaddr_in6 *sin6;
+ char *buf;
+ struct nd_router_solicit *rs;
+ struct nd_opt_hdr *opt;
+ size_t rslen;
+ ifp = ifunit(ifname);
+ if (ifp == NULL)
+ return ENODEV;
ENXIO is the correct errno.
+ if (ifp->if_sadl->sdl_type != IFT_ETHER)
+ return EINVAL;
Might as well consistently "goto out" in case someone reorganizes the
code. I realize you'd have to add braces and assign rv, but c'est le code.
+ rv = socreate(PF_INET6, &rsso, SOCK_RAW, IPPROTO_ICMPV6, curlwp, NULL);
+ if (rv != 0)
+ goto out;
+ ifindex = ifp->if_index;
+ rv = so_setsockopt(curlwp, rsso, IPPROTO_IPV6, IPV6_MULTICAST_IF,
+ &ifindex, sizeof ifindex);
+ if (rv != 0)
+ goto out;
+ rv = so_setsockopt(curlwp, rsso, IPPROTO_IPV6, IPV6_MULTICAST_HOPS,
+ &hoplimit, sizeof hoplimit);
+ if (rv != 0)
+ goto out;
+
+ nam = m_get(M_WAIT, MT_SONAME);
+ sin6 = mtod(nam, struct sockaddr_in6 *);
I forget what the rules are exactly, but you might technically need to
m_pulldown() before you're allowed to mtod for such a large structure.
+ sin6->sin6_len = nam->m_len = sizeof (*sin6);
Don't you need to set nam->m_pkthdr.len?
+ sin6->sin6_family = AF_INET6;
+ netconfig_inet_pton6("ff02::2", &sin6->sin6_addr);
+
+ rslen = sizeof (struct nd_router_solicit) +
+ sizeof (struct nd_opt_hdr) + ETHER_ADDR_LEN;
+ m = m_gethdr(M_WAIT, MT_DATA);
+ m_clget(m, M_WAIT);
+ m->m_pkthdr.len = m->m_len = rslen;
+ m->m_pkthdr.rcvif = NULL;
+ buf = mtod(m, char *);
+ memset (buf, 0, rslen);
KASSERT() that rslen <= MCLBYTES is a good idea. Actually, can be a
CTASSERT()
+ rs = (struct nd_router_solicit *)buf;
+ rs->nd_rs_type = ND_ROUTER_SOLICIT;
+ buf += sizeof (*rs);
+ opt = (struct nd_opt_hdr *)buf;
Is "opt" *guaranteed* to be properly aligned? If yes, you can do it
that way. If no, you need to memcpy to the final space. Probably safer
to memcpy().
+ opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;
+ opt->nd_opt_len = 1; /* units of 8 octets */
+ buf += sizeof (*opt);
+ memcpy(buf, ifp->if_sadl->sdl_data + ifp->if_sadl->sdl_nlen,
+ ETHER_ADDR_LEN);
Use CLLADDR()
+ ip6_accept_rtadv = 1; /* XXX */
+ rv = rump_netconfig_ifup(ifname);
+ if (rv != 0)
+ goto out;
+ rv = (*rsso->so_send)(rsso, nam, NULL, m, NULL, 0, curlwp);
+ if (rv != 0)
+ goto out;
+ m = nam = NULL;
+ rv = 0;
So you fix these 4 lines to m = nam = NULL;
+out:
+ if (m)
+ m_freem(m);
+ if (nam)
+ m_freem(nam);
+ if (rsso)
+ soclose(rsso);
+ return rv;