On 18/05/16(Wed) 22:56, Martin Pieuchot wrote:
> Back in the old cool days you could simply call ether_output() with a
> stuffed sockaddr and it will do the L2 resolution for you... Well as
> long as you do not care about IPv6 it's true.
>
> During the last years I tried to reduce the number of places where
> ether_output() was called without passing it a "struct rtentry" as
> last argument. The reason for this move is to remove a potential
> route insertion in arpresolve(), done in read-only context (hot path).
New diff with ND bits. Please test and report back.
Index: net/if_ethersubr.c
===================================================================
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.236
diff -u -p -r1.236 if_ethersubr.c
--- net/if_ethersubr.c 18 May 2016 20:15:14 -0000 1.236
+++ net/if_ethersubr.c 23 May 2016 12:21:45 -0000
@@ -193,8 +193,12 @@ ether_output(struct ifnet *ifp, struct m
struct mbuf *mcopy = NULL;
struct ether_header *eh;
struct arpcom *ac = (struct arpcom *)ifp;
+ sa_family_t af = dst->sa_family;
int error = 0;
+ KASSERT(rt != NULL || ISSET(m->m_flags, M_MCAST|M_BCAST) ||
+ af == AF_UNSPEC || af == pseudo_AF_HDRCMPLT);
+
#ifdef DIAGNOSTIC
if (ifp->if_rdomain != rtable_l2(m->m_pkthdr.ph_rtableid)) {
printf("%s: trying to send packet on wrong domain. "
@@ -208,7 +212,7 @@ ether_output(struct ifnet *ifp, struct m
if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING))
senderr(ENETDOWN);
- switch (dst->sa_family) {
+ switch (af) {
case AF_INET:
error = arpresolve(ifp, rt, m, dst, edst);
if (error)
Index: netinet/if_ether.c
===================================================================
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.209
diff -u -p -r1.209 if_ether.c
--- netinet/if_ether.c 23 May 2016 09:23:43 -0000 1.209
+++ netinet/if_ether.c 23 May 2016 12:21:45 -0000
@@ -281,9 +281,8 @@ arpresolve(struct ifnet *ifp, struct rte
struct llinfo_arp *la = NULL;
struct sockaddr_dl *sdl;
struct rtentry *rt = NULL;
- struct mbuf *mh;
char addr[INET_ADDRSTRLEN];
- int error, created = 0;
+ int error;
if (m->m_flags & M_BCAST) { /* broadcast */
memcpy(desten, etherbroadcastaddr, sizeof(etherbroadcastaddr));
@@ -294,41 +293,20 @@ arpresolve(struct ifnet *ifp, struct rte
return (0);
}
- if (rt0 != NULL) {
- error = rt_checkgate(rt0, &rt);
- if (error) {
- m_freem(m);
- return (error);
- }
-
- if ((rt->rt_flags & RTF_LLINFO) == 0) {
- log(LOG_DEBUG, "%s: %s: route contains no arp"
- " information\n", __func__, inet_ntop(AF_INET,
- &satosin(rt_key(rt))->sin_addr, addr,
- sizeof(addr)));
- m_freem(m);
- return (EINVAL);
- }
+ error = rt_checkgate(rt0, &rt);
+ if (error) {
+ m_freem(m);
+ return (error);
+ }
- la = (struct llinfo_arp *)rt->rt_llinfo;
- if (la == NULL)
- log(LOG_DEBUG, "%s: %s: route without link "
- "local address\n", __func__, inet_ntop(AF_INET,
- &satosin(dst)->sin_addr, addr, sizeof(addr)));
- } else {
- rt = arplookup(&satosin(dst)->sin_addr, 1, 0, ifp->if_rdomain);
- if (rt != NULL) {
- created = 1;
- la = ((struct llinfo_arp *)rt->rt_llinfo);
- }
- if (la == NULL)
- log(LOG_DEBUG, "%s: %s: can't allocate llinfo\n",
- __func__,
- inet_ntop(AF_INET, &satosin(dst)->sin_addr,
- addr, sizeof(addr)));
+ if (!ISSET(rt->rt_flags, RTF_LLINFO)) {
+ log(LOG_DEBUG, "%s: %s: route contains no arp information\n",
+ __func__, inet_ntop(AF_INET, &satosin(rt_key(rt))->sin_addr,
+ addr, sizeof(addr)));
+ m_freem(m);
+ return (EINVAL);
}
- if (la == NULL || rt == NULL)
- goto bad;
+
sdl = satosdl(rt->rt_gateway);
if (sdl->sdl_alen > 0 && sdl->sdl_alen != ETHER_ADDR_LEN) {
log(LOG_DEBUG, "%s: %s: incorrect arp information\n", __func__,
@@ -336,6 +314,7 @@ arpresolve(struct ifnet *ifp, struct rte
addr, sizeof(addr)));
goto bad;
}
+
/*
* Check the address family and length is valid, the address
* is resolved; otherwise, try to resolve.
@@ -343,10 +322,9 @@ arpresolve(struct ifnet *ifp, struct rte
if ((rt->rt_expire == 0 || rt->rt_expire > time_second) &&
sdl->sdl_family == AF_LINK && sdl->sdl_alen != 0) {
memcpy(desten, LLADDR(sdl), sdl->sdl_alen);
- if (created)
- rtfree(rt);
return (0);
}
+
if (ifp->if_flags & IFF_NOARP)
goto bad;
@@ -355,7 +333,11 @@ arpresolve(struct ifnet *ifp, struct rte
* response yet. Insert mbuf in hold queue if below limit
* if above the limit free the queue without queuing the new packet.
*/
+ la = (struct llinfo_arp *)rt->rt_llinfo;
+ KASSERT(la != NULL);
if (la_hold_total < LA_HOLD_TOTAL && la_hold_total < nmbclust / 64) {
+ struct mbuf *mh;
+
if (ml_len(&la->la_ml) >= LA_HOLD_QUEUE) {
mh = ml_dequeue(&la->la_ml);
la_hold_total--;
@@ -396,14 +378,11 @@ arpresolve(struct ifnet *ifp, struct rte
}
}
}
- if (created)
- rtfree(rt);
+
return (EAGAIN);
bad:
m_freem(m);
- if (created)
- rtfree(rt);
return (EINVAL);
}
Index: netinet6/nd6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.179
diff -u -p -r1.179 nd6.c
--- netinet6/nd6.c 17 May 2016 08:29:14 -0000 1.179
+++ netinet6/nd6.c 23 May 2016 12:38:56 -0000
@@ -1507,20 +1507,15 @@ nd6_output(struct ifnet *ifp, struct mbu
struct mbuf *m = m0;
struct rtentry *rt = rt0;
struct llinfo_nd6 *ln = NULL;
- int created = 0, error = 0;
+ int error = 0;
if (IN6_IS_ADDR_MULTICAST(&dst->sin6_addr))
goto sendpkt;
- /*
- * next hop determination.
- */
- if (rt0 != NULL) {
- error = rt_checkgate(rt0, &rt);
- if (error) {
- m_freem(m);
- return (error);
- }
+ error = rt_checkgate(rt0, &rt);
+ if (error) {
+ m_freem(m);
+ return (error);
}
if (nd6_need_cache(ifp) == 0)
@@ -1532,42 +1527,17 @@ nd6_output(struct ifnet *ifp, struct mbu
* At this point, the destination of the packet must be a unicast
* or an anycast address(i.e. not a multicast).
*/
-
- /* Look up the neighbor cache for the nexthop */
- if (rt != NULL && (rt->rt_flags & RTF_LLINFO) != 0)
- ln = (struct llinfo_nd6 *)rt->rt_llinfo;
- else {
- /*
- * Since nd6_is_addr_neighbor() internally calls nd6_lookup(),
- * the condition below is not very efficient. But we believe
- * it is tolerable, because this should be a rare case.
- */
- if (nd6_is_addr_neighbor(dst, ifp)) {
- rt = nd6_lookup(&dst->sin6_addr, 1, ifp,
- ifp->if_rdomain);
- if (rt != NULL) {
- created = 1;
- ln = (struct llinfo_nd6 *)rt->rt_llinfo;
- }
- }
+ if (!ISSET(rt->rt_flags, RTF_LLINFO)) {
+ char addr[INET6_ADDRSTRLEN];
+ log(LOG_DEBUG, "%s: %s: route contains no ND information\n",
+ __func__, inet_ntop(AF_INET6,
+ &satosin6(rt_key(rt))->sin6_addr, addr, sizeof(addr)));
+ m_freem(m);
+ return (EINVAL);
}
- if (ln == NULL || rt == NULL) {
- if ((ND_IFINFO(ifp)->flags & ND6_IFF_PERFORMNUD) == 0) {
- char addr[INET6_ADDRSTRLEN];
-
- log(LOG_DEBUG, "%s: can't allocate llinfo for %s "
- "(ln=%p, rt=%p)\n", __func__,
- inet_ntop(AF_INET6, &dst->sin6_addr,
- addr, sizeof(addr)),
- ln, rt);
- m_freem(m);
- if (created)
- rtfree(rt);
- return (EIO); /* XXX: good error? */
- }
- goto sendpkt; /* send anyway */
- }
+ ln = (struct llinfo_nd6 *)rt->rt_llinfo;
+ KASSERT(ln != NULL);
/*
* Move this entry to the head of the queue so that it is less likely
@@ -1617,14 +1587,10 @@ nd6_output(struct ifnet *ifp, struct mbu
(long)ND_IFINFO(ifp)->retrans * hz / 1000);
nd6_ns_output(ifp, NULL, &dst->sin6_addr, ln, 0);
}
- if (created)
- rtfree(rt);
return (0);
sendpkt:
error = ifp->if_output(ifp, m, sin6tosa(dst), rt);
- if (created)
- rtfree(rt);
return (error);
}
@@ -1665,12 +1631,6 @@ nd6_storelladdr(struct ifnet *ifp, struc
m_freem(m);
return (EINVAL);
}
- }
-
- if (rt0 == NULL) {
- /* this could happen, if we could not allocate memory */
- m_freem(m);
- return (ENOMEM);
}
error = rt_checkgate(rt0, &rt);
