From owner-svn-src-head@freebsd.org Wed Jul 29 13:01:01 2015 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6C30D9ADA11; Wed, 29 Jul 2015 13:01:01 +0000 (UTC) (envelope-from ermal.luci@gmail.com) Received: from mail-yk0-x22c.google.com (mail-yk0-x22c.google.com [IPv6:2607:f8b0:4002:c07::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 1BC901EAA; Wed, 29 Jul 2015 13:01:01 +0000 (UTC) (envelope-from ermal.luci@gmail.com) Received: by ykay190 with SMTP id y190so6617400yka.3; Wed, 29 Jul 2015 06:01:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=/pZeb3pk0CSivxTfmUXcFfqj9owekwJZvA0JBIT9P90=; b=K2AbHQl4qClFKcAAgJZh3/5X62cThNEkiLq4V8nlWiBIK7LZtLbPaytB1HaM7cyD6M KuePJT3/TKBgwDerA8KVdx4rdBtzZtNRjiS9+HDgb29tdjjIIJzqV8/tEJSa29D/5LIP Ci70DOD6eY1eEOOALz2SppqWRaKLLfgy4uKKj3dYlIch75g9TtMDjQJoBHwi6Zoa9Vvi 0lysf8JWgI2Dtc3BPEKQ1D/27cJPcvy1NybZEJFxfEeDamnmDg/1Lv2WKcCPdhh/6bPA d4QnpvRvil5mxCoD1ZwFfws8prVCQd6vtpnU3XR7Xe4fvsBhFdX0UkMeDscs8A/BpXYq FIfg== MIME-Version: 1.0 X-Received: by 10.129.82.129 with SMTP id g123mr44926177ywb.9.1438174859999; Wed, 29 Jul 2015 06:00:59 -0700 (PDT) Sender: ermal.luci@gmail.com Received: by 10.129.83.139 with HTTP; Wed, 29 Jul 2015 06:00:59 -0700 (PDT) In-Reply-To: <20150728124220.GW72729@FreeBSD.org> References: <201507021810.t62IAgCc003272@repo.freebsd.org> <20150728124220.GW72729@FreeBSD.org> Date: Wed, 29 Jul 2015 15:00:59 +0200 X-Google-Sender-Auth: 2Jcz1FIyvlixB826pH2vbxn1CVQ Message-ID: Subject: Re: svn commit: r285051 - head/sys/netinet From: =?UTF-8?Q?Ermal_Lu=C3=A7i?= To: Gleb Smirnoff Cc: =?UTF-8?Q?Olivier_Cochard=2DLabb=C3=A9?= , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jul 2015 13:01:01 -0000 On Tue, Jul 28, 2015 at 2:42 PM, Gleb Smirnoff wrote: > Ermal, > > see comments inlined, > > On Thu, Jul 02, 2015 at 06:10:42PM +0000, Ermal Lu=C3=A7i 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 durin= g > 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> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > 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 =3D 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 =3D 0, code =3D 0, mtu =3D 0; > E> @@ -925,7 +926,22 @@ ip_forward(struct mbuf *m, int srcrt) > E> } > E> #endif > E> > E> - ia =3D ip_rtaddr(ip->ip_dst, M_GETFIB(m)); > E> + bzero(&ro, sizeof(ro)); > E> + sin =3D (struct sockaddr_in *)&ro.ro_dst; > E> + sin->sin_family =3D AF_INET; > E> + sin->sin_len =3D sizeof(*sin); > E> + sin->sin_addr =3D 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 !=3D NULL) { > E> + ia =3D 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 =3D=3D 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. > > Gleb, the improvement on the ifa_ref not needed is something to look at but the ifa_ref here is not lost since ia =3D=3D NULL, no? Maybe i am missing something else. Also can we put this on a review? > > 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. > --=20 Ermal