svn commit: r285051 - head/sys/netinet
Gleb Smirnoff
glebius at FreeBSD.org
Tue Jul 28 12:42:29 UTC 2015
Ermal,
see comments inlined,
On Thu, Jul 02, 2015 at 06:10:42PM +0000, Ermal Luçi wrote:
E> Author: eri
E> Date: Thu Jul 2 18:10:41 2015
E> New Revision: 285051
E> URL: https://svnweb.freebsd.org/changeset/base/285051
E>
E> Log:
E> Avoid doing multiple route lookups for the same destination IP during forwarding
E>
E> ip_forward() does a route lookup for testing this packet can be sent to a known destination,
E> it also can do another route lookup if it detects that an ICMP redirect is needed,
E> it forgets all of this and handovers to ip_output() to do the same lookup yet again.
E>
E> This optimisation just does one route lookup during the forwarding path and handovers that to be considered by ip_output().
E>
E> Differential Revision: https://reviews.freebsd.org/D2964
E> Approved by: ae, gnn(mentor)
E> MFC after: 1 week
E>
E> Modified:
E> head/sys/netinet/ip_input.c
E>
E> Modified: head/sys/netinet/ip_input.c
E> ==============================================================================
E> --- head/sys/netinet/ip_input.c Thu Jul 2 17:30:59 2015 (r285050)
E> +++ head/sys/netinet/ip_input.c Thu Jul 2 18:10:41 2015 (r285051)
E> @@ -897,6 +897,7 @@ ip_forward(struct mbuf *m, int srcrt)
E> struct ip *ip = mtod(m, struct ip *);
E> struct in_ifaddr *ia;
E> struct mbuf *mcopy;
E> + struct sockaddr_in *sin;
E> struct in_addr dest;
E> struct route ro;
E> int error, type = 0, code = 0, mtu = 0;
E> @@ -925,7 +926,22 @@ ip_forward(struct mbuf *m, int srcrt)
E> }
E> #endif
E>
E> - ia = ip_rtaddr(ip->ip_dst, M_GETFIB(m));
E> + bzero(&ro, sizeof(ro));
E> + sin = (struct sockaddr_in *)&ro.ro_dst;
E> + sin->sin_family = AF_INET;
E> + sin->sin_len = sizeof(*sin);
E> + sin->sin_addr = ip->ip_dst;
E> +#ifdef RADIX_MPATH
E> + rtalloc_mpath_fib(&ro,
E> + ntohl(ip->ip_src.s_addr ^ ip->ip_dst.s_addr),
E> + M_GETFIB(m));
E> +#else
E> + in_rtalloc_ign(&ro, 0, M_GETFIB(m));
E> +#endif
E> + if (ro.ro_rt != NULL) {
E> + ia = ifatoia(ro.ro_rt->rt_ifa);
E> + ifa_ref(&ia->ia_ifa);
E> + }
E> #ifndef IPSEC
E> /*
E> * 'ia' may be NULL if there is no route for this destination.
E> @@ -934,6 +950,7 @@ ip_forward(struct mbuf *m, int srcrt)
E> */
E> if (!srcrt && ia == NULL) {
E> icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, 0, 0);
E> + RO_RTFREE(&ro);
E> return;
E> }
Here the ifa reference is leaked upon return.
But don't hurry with fixing that :) Actually you don't need to ifa_ref()
in this function. You acquired a reference on rtentry in in_rtalloc_ign()
and hold it until RO_RTFREE(). And the rtentry itself always holds a
reference on the ifa. So, there is no reason to put extra reference on
the ifa.
The ip_output() was already improved in r262747. And ip_forward() can
also be. The only place that touches ia after RO_RTFREE() is EMSGSIZE
handling, this can be moved up before RO_RTFREE().
Here is suggested patch. Ermal and Oliver, can you please test/benchmark
it?
--
Totus tuus, Glebius.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ip_forward.diff
Type: text/x-diff
Size: 2772 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20150728/861e6aae/attachment.bin>
More information about the svn-src-head
mailing list