From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 91172A0487 for ; Fri, 5 Jul 2019 15:37:58 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2DD7B1BED2; Fri, 5 Jul 2019 15:37:58 +0200 (CEST) Received: from mail-lf1-f67.google.com (mail-lf1-f67.google.com [209.85.167.67]) by dpdk.org (Postfix) with ESMTP id 508D51BE31 for ; Fri, 5 Jul 2019 15:37:56 +0200 (CEST) Received: by mail-lf1-f67.google.com with SMTP id h28so2029362lfj.5 for ; Fri, 05 Jul 2019 06:37:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=zDPaGIpzFZoxQ7AHCaeIscsku8sORVsj4Vt2rKaZkeg=; b=Enymi/5MiEfgLJNXzNKf8sZ7B51IOreebwu5TrvUC0ssKKql3tWYdn/dElWnujOF9f 8Kl7QVDLSmxGqfJIqt4JUtlbP5VIxeQnSz4pMReQYHwqebDybrmgjehPxV5S75mT7+X1 km3K3fRSYhtuHjfdx2iL9PuHeruzywILl9hvzueaBLJnPcURkl9cY+48hptg8JrN2ROm 1iLwrH2O5/phmHTDpSPkQrJJa0usLvoldhrn9vw4q7JFIm2k3XvOJMiJk27pbdSBHcur SNM7MITX6fylfuXsh9AhwDPiRP4GoBR7qP3mx6DXXtahLuGIX08TJR71uhirygIaUnIt TXwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=zDPaGIpzFZoxQ7AHCaeIscsku8sORVsj4Vt2rKaZkeg=; b=lRAl4BObocwaIuCxK9/T/PvB3RMwj79POkGTP7W3mCfnnyyl0Ip9dgrwPrfbXlvLl4 aeU3jo+dNu3vKcWE6XattVKR+mVtNJejUP8htYKUR1Zjdsec5icRRilpYMtky4r4kcZr naQKxfefL38BrIBYvfrmuggSFIWm8Wd7D/XXzgdxi6tYUGMwI7v94zCrNj5a+y+3KmGa oWPXYuIz5h83pgUMPsRmsWXGWvgR6iJ320ZFYypTB9FyGih4a2xKdlIUzfAqGyR/L9rO eM31De6OM4zZF3aLtiXj1XYTd87J8/cJfpXwHKWXdvm7mi2lhkU7IpV6DSfPdLGRCjYB NEEg== X-Gm-Message-State: APjAAAUD2dE4V2Qf+qvK5AywuRd+FaRs47v0tt7NM2l1TeVPceY1lFbf PGuCzFD2nFt/blP9HtWy/1AyydBiiyfIxMsRa9I= X-Google-Smtp-Source: APXvYqzEJSDs81T1nTDRGhKJ3tUmuLf7ES7Xe6Hso+8Kw+YXhgs54VXfD1zprrzB+M3lSQNdu0byMN5DZJD8NTQTpkk= X-Received: by 2002:ac2:418f:: with SMTP id z15mr1964655lfh.177.1562333875863; Fri, 05 Jul 2019 06:37:55 -0700 (PDT) MIME-Version: 1.0 References: <20190627093751.7746-1-ruifeng.wang@arm.com> <20190627082451.56719392@hermes.lan> <20190627213450.30082af6@hermes.lan> <185e012d-6f8a-66be-dc8c-a420065660fb@intel.com> <20190628083507.31eca1db@hermes.lan> <6a78a166-f19c-5444-0a1a-a74aa06463b1@intel.com> In-Reply-To: <6a78a166-f19c-5444-0a1a-a74aa06463b1@intel.com> From: Alex Kiselev Date: Fri, 5 Jul 2019 16:37:44 +0300 Message-ID: To: "Medvedkin, Vladimir" Cc: Stephen Hemminger , Honnappa Nagarahalli , "Ruifeng Wang (Arm Technology China)" , "bruce.richardson@intel.com" , "dev@dpdk.org" , "Gavin Hu (Arm Technology China)" , nd Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v3 1/3] lib/lpm: not inline unnecessary functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" =D0=BF=D1=82, 5 =D0=B8=D1=8E=D0=BB. 2019 =D0=B3. =D0=B2 13:31, Medvedkin, V= ladimir : > > Hi Stephen, > > On 28/06/2019 16:35, Stephen Hemminger wrote: > > On Fri, 28 Jun 2019 15:16:30 +0100 > > "Medvedkin, Vladimir" wrote: > > > >> Hi Honnappa, > >> > >> On 28/06/2019 14:57, Honnappa Nagarahalli wrote: > >>>> Hi all, > >>>> > >>>> On 28/06/2019 05:34, Stephen Hemminger wrote: > >>>>> On Fri, 28 Jun 2019 02:44:54 +0000 > >>>>> "Ruifeng Wang (Arm Technology China)" wrote: > >>>>> > >>>>>>>> Tests showed that the function inlining caused performance drop = on > >>>>>>>> some x86 platforms with the memory ordering patches applied. > >>>>>>>> By force no-inline functions, the performance was better than > >>>>>>>> before on x86 and no impact to arm64 platforms. > >>>>>>>> > >>>>>>>> Suggested-by: Medvedkin Vladimir > >>>>>>>> Signed-off-by: Ruifeng Wang > >>>>>>>> Reviewed-by: Gavin Hu > >>>>>>> { > >>>>>>> > >>>>>>> Do you actually need to force noinline or is just taking of inlin= e enough? > >>>>>>> In general, letting compiler decide is often best practice. > >>>>>> The force noinline is an optimization for x86 platforms to keep > >>>>>> rte_lpm_add() API performance with memory ordering applied. > >>>>> I don't think you answered my question. What does a recent version = of > >>>>> GCC do if you drop the inline. > >>>>> > >>>>> Actually all the functions in rte_lpm should drop inline. > >>>> I'm agree with Stephen. If it is not a fastpath and size of function= is not > >>>> minimal it is good to remove inline qualifier for other control plan= e functions > >>>> such as rule_add/delete/find/etc and let the compiler decide to inli= ne it > >>>> (unless it affects performance). > >>> IMO, the rule needs to be simple. If it is control plane function, we= should leave it to the compiler to decide. I do not think we need to worry= too much about performance for control plane functions. > >> Control plane is not as important as data plane speed but it is still > >> important. For lpm we are talking not about initialization, but runtim= e > >> routes add/del related functions. If it is very slow the library will = be > >> totally unusable because after it receives a route update it will be > >> blocked for a long time and route update queue would overflow. > > Control plane performance is more impacted by algorithmic choice. > > The original LPM had terrible (n^2?) control path. Current code is bett= er. > > I had a patch using RB tree, but it was rejected because it used the > > /usr/include/bsd/sys/tree.h which added a dependency. > > You're absolutely right, control plane performance is mostly depends on > algorithm. Current LPM implementation has number of problems there. One > problem is rules_tbl[] that is a flat array containing routes for > control plane purposes. Replacing it with a rb-tree solves this problem, > but there are another problems. For example, when you try to add a route > 10.0.0.0/8 while a number of subroutes are exist in the table (for > example 10.20.0.0/16), current implementation will load tbl_entry -> do > some checks (depth, ext entry) -> conditionally store new entry. Under > several circumstances it would take a lot time. But in fact it needs to > unconditionally rewrite only two ranges - from 10.0.0.0 to 10.19.255.255 > and from 10.21.0.0 to 10.255.255.255. And control plane could help us to > get this two ranges. The best struct to do so is lc-tree because it is > relatively easy to traverse subtree (described by 10.0.0.0/8) and get > subroutes. We are working on a new implementation, hopefully it will be > ready soon. Have you considered switching to this algorithm? http://www.nxlab.fer.hr/dxr/ --=20 Alex