Date: Fri, 22 Feb 2013 14:51:05 +0400 From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: Vijay Singh <vijju.singh@gmail.com> Cc: freebsd-net@freebsd.org Subject: Re: Possible optimization in ether_output() Message-ID: <51274D99.1070009@FreeBSD.org> In-Reply-To: <CALCNsJR-8wUztkBOhGPSMtSnC=0uusX60dLLn4_HSZ8TcGpDAg@mail.gmail.com> References: <CALCNsJR-8wUztkBOhGPSMtSnC=0uusX60dLLn4_HSZ8TcGpDAg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 20.02.2013 02:05, Vijay Singh wrote:
> Hi, this patch gives a modest performance improvement here @work.
> Please consider.
>
> [/u/vijay/bsd/CODE/cur/sys/net]# svn diff if_ethersubr.c
> Index: if_ethersubr.c
> ===================================================================
> --- if_ethersubr.c (revision 247012)
> +++ if_ethersubr.c (working copy)
> @@ -160,6 +160,7 @@
> struct pf_mtag *t;
> int loop_copy = 1;
> int hlen; /* link layer header length */
> + int use_lle_directly = 0;
>
> if (ro != NULL) {
> if (!(m->m_flags& (M_BCAST | M_MCAST)))
> @@ -184,7 +185,7 @@
> #ifdef INET
> case AF_INET:
> if (lle != NULL&& (lle->la_flags& LLE_VALID))
> - memcpy(edst,&lle->ll_addr.mac16, sizeof(edst));
> + use_lle_directly = 1;
> else
> error = arpresolve(ifp, rt0, m, dst, edst,&lle);
> if (error)
> @@ -222,7 +223,7 @@
> #ifdef INET6
> case AF_INET6:
> if (lle != NULL&& (lle->la_flags& LLE_VALID))
> - memcpy(edst,&lle->ll_addr.mac16, sizeof(edst));
> + use_lle_directly = 1;
> else
> error = nd6_storelladdr(ifp, m, dst, (u_char *)edst,&lle);
> if (error)
> @@ -317,9 +318,13 @@
> if (m == NULL)
> senderr(ENOBUFS);
> eh = mtod(m, struct ether_header *);
> - (void)memcpy(&eh->ether_type,&type,
> - sizeof(eh->ether_type));
> - (void)memcpy(eh->ether_dhost, edst, sizeof (edst));
> + eh->ether_type = type;
> + if (use_lle_directly) {
> + memcpy(eh->ether_dhost,&lle->ll_addr.mac16,
> + sizeof(eh->ether_dhost));
This is use-after-free since lle is returned unlocked by arpresolve().
This probably can be changed by supplying eh directly instead of edst,
but this is tricky, too.
I'm currently working on new arp stack implementation where such
problems are handled more efficient.
> + } else {
> + (void)memcpy(eh->ether_dhost, edst, sizeof (edst));
> + }
> if (hdrcmplt)
> (void)memcpy(eh->ether_shost, esrc,
> sizeof(eh->ether_shost));
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51274D99.1070009>