On Tue, Apr 19, 2016 at 11:06:31AM +0200, Martin Pieuchot wrote:
> This single cached route is a nice trick for having higher forwarding
> numbers w/ benchmarks. But as soon as you start forwarding packets to
> different end addresses, even using a single next hop, it becomes
> useless.
>
> Since this single cached route won't fly with multiple forwarding paths
> and it also makes some upcoming refactoring more complicated I'd like to
> kill it now.
>
> Note that with PF disable the kernel is currently doing 2 lookups per
> forwarded packet. One to check if the packet needs to be delivered
> locally and a second one, probably cached by ``ipforward_rt'', to find
> the next hop.
>
> My goal is to change the kernel to just do a single lookup per packet,
> so even without ``ipforward_rt'', the performances will be the same.
>
> Once that's done, I'll merge the multicast address lookup with this
> single route lookup, for obvious reasons.
>
> Ok?
Don't underestimate the effectivness of this single entry cache.
Anyway I agree with the path and because of that OK claudio@
In the end only doing one lookup will be a big gain lets hope it happens
sooner than later.
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.273
> diff -u -p -r1.273 ip_input.c
> --- netinet/ip_input.c 19 Apr 2016 08:23:13 -0000 1.273
> +++ netinet/ip_input.c 19 Apr 2016 08:55:59 -0000
> @@ -197,8 +197,6 @@ ip_init(void)
> mq_init(&ipsend_mq, 64, IPL_SOFTNET);
> }
>
> -struct route ipforward_rt;
> -
> void
> ipintr(void)
> {
> @@ -972,10 +970,6 @@ ip_slowtimo(void)
> ip_freef(fp);
> }
> }
> - if (ipforward_rt.ro_rt) {
> - rtfree(ipforward_rt.ro_rt);
> - ipforward_rt.ro_rt = NULL;
> - }
> splx(s);
> }
>
> @@ -1399,8 +1393,8 @@ ip_forward(struct mbuf *m, struct ifnet
> struct ip *ip = mtod(m, struct ip *);
> struct sockaddr_in *sin;
> struct rtentry *rt;
> + struct route ro;
> int error, type = 0, code = 0, destmtu = 0, fake = 0, len;
> - u_int rtableid = 0;
> u_int32_t dest;
>
> dest = 0;
> @@ -1414,29 +1408,18 @@ ip_forward(struct mbuf *m, struct ifnet
> return;
> }
>
> - rtableid = m->m_pkthdr.ph_rtableid;
>
> - rt = ipforward_rt.ro_rt;
> - sin = satosin(&ipforward_rt.ro_dst);
> - if (rt == NULL || ISSET(rt->rt_flags, RTF_MPATH) ||
> - ip->ip_dst.s_addr != sin->sin_addr.s_addr ||
> - rtableid != ipforward_rt.ro_tableid) {
> - if (ipforward_rt.ro_rt) {
> - rtfree(ipforward_rt.ro_rt);
> - ipforward_rt.ro_rt = NULL;
> - }
> - sin->sin_family = AF_INET;
> - sin->sin_len = sizeof(*sin);
> - sin->sin_addr = ip->ip_dst;
> - ipforward_rt.ro_tableid = rtableid;
> -
> - ipforward_rt.ro_rt = rtalloc_mpath(&ipforward_rt.ro_dst,
> - &ip->ip_src.s_addr, ipforward_rt.ro_tableid);
> - if (ipforward_rt.ro_rt == 0) {
> - icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, dest, 0);
> - return;
> - }
> - rt = ipforward_rt.ro_rt;
> + sin = satosin(&ro.ro_dst);
> + memset(sin, 0, sizeof(*sin));
> + sin->sin_family = AF_INET;
> + sin->sin_len = sizeof(*sin);
> + sin->sin_addr = ip->ip_dst;
> +
> + rt = rtalloc_mpath(sintosa(sin), &ip->ip_src.s_addr,
> + m->m_pkthdr.ph_rtableid);
> + if (rt == NULL) {
> + icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, dest, 0);
> + return;
> }
>
> /*
> @@ -1488,7 +1471,8 @@ ip_forward(struct mbuf *m, struct ifnet
> }
> }
>
> - error = ip_output(m, NULL, &ipforward_rt,
> + ro.ro_rt = rt;
> + error = ip_output(m, NULL, &ro,
> (IP_FORWARDING | (ip_directedbcast ? IP_ALLOWBROADCAST : 0)),
> NULL, NULL, 0);
> if (error)
> @@ -1501,7 +1485,7 @@ ip_forward(struct mbuf *m, struct ifnet
> goto freecopy;
> }
> if (!fake)
> - return;
> + goto freecopy;
>
> switch (error) {
>
> @@ -1559,9 +1543,10 @@ ip_forward(struct mbuf *m, struct ifnet
> if (mcopy)
> icmp_error(mcopy, type, code, dest, destmtu);
>
> - freecopy:
> +freecopy:
> if (fake)
> m_tag_delete_chain(&mfake);
> + rtfree(rt);
> }
>
> int
>
--
:wq Claudio
