From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f54.google.com (mail-lf0-f54.google.com [209.85.215.54]) by dpdk.org (Postfix) with ESMTP id AD1F81B2D1 for ; Tue, 3 Oct 2017 08:33:59 +0200 (CEST) Received: by mail-lf0-f54.google.com with SMTP id l23so1597581lfk.10 for ; Mon, 02 Oct 2017 23:33:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=IbyVfD0pjKbdvpNJkBlA0hkiC+UUXfWtDbBJY1qSc90=; b=HIqFoinpiGxhjg/LHudFJkBziIQF5rmxw9wv+NYFWq3TtStMTu12IuL28ths6jbfLh ldYUAHodGnpKA2UFSA5UgRCQh097rdnTBOpJ+GfDKR0p6P+46e9s3pPmAylAcvFx6D2K 0dKmUEwIjLpuFL9equD6MI0miN18BBwLDoqweXLZokCBR2cRJ9moBSOGkdkRRSSa/Gcq Gx742L4Zzkc6WZGdgIxyDRg6AmiEAxg2qK7T2EAUrBI7wPOt0lcfsopqK3kgXrvvUatQ Eemyhf3ue5u6wlHgz08pGaIN238Gow+3QEwAcyg6JAuHgUKl9KDM3k/Bq5UQOCvUJV0p 6rTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=IbyVfD0pjKbdvpNJkBlA0hkiC+UUXfWtDbBJY1qSc90=; b=myHE/ezwP0ocAlRuqDWI5o4F3j/fLO5PpGOyTFG4He3/rMTgRIMZ1/DFWLd9KILP2n DKcifvMCl8GJawQRhSMz58HnArQAtJv4ovBtmDD8hY8Bba9ASBKMDHgEWD+AUXHsfUHr 9DT9GMpUqjVlTyL+SRdI5y6lDCr2C85L/QYwcWOzJsUiQRufhhVNIKfNP4/0xqAOKMd6 ipiyuKilJM7k9zT30RzBrVxLvwZuoAOEYcPM9h2cg83eYjq9VCcXMj7TQovIx5HRi1gW YXbFQzq3ADkUVcrqOuczJjpqqWc7D9Sif1q1WxtfmqN5f5MtIE5YaaTbHMdnm4k1So6g BAFQ== X-Gm-Message-State: AMCzsaWAnHPors+ef/TAb/84FAJzj5sBWZfBICzh9W0AuFhFBtaOUZIf c7SSjv/LksfYdAk2hVFy7xyhDtlLbmY= X-Google-Smtp-Source: AOwi7QD2bC7L9O+Tk70WDAcIhbRbXwoNYk8GUAA/M9p1y2D+F6nHD/LoXabWyg/f0Zi59/iCXjOqNA== X-Received: by 10.25.215.99 with SMTP id o96mr3270129lfg.33.1507012439281; Mon, 02 Oct 2017 23:33:59 -0700 (PDT) Received: from localhost (31-172-191-173.noc.fibertech.net.pl. [31.172.191.173]) by smtp.gmail.com with ESMTPSA id k10sm2732411ljb.97.2017.10.02.23.33.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 02 Oct 2017 23:33:58 -0700 (PDT) Date: Tue, 3 Oct 2017 08:33:57 +0200 From: Tomasz Duszynski To: Bruce Richardson Cc: Stephen Hemminger , Tomasz Duszynski , dev@dpdk.org, mw@semihalf.com, dima@marvell.com, nsamsono@marvell.com, Jianbo.liu@linaro.org, Jacek Siuda Message-ID: <20171003063357.GB9598@tdu> References: <1506418805-12117-1-git-send-email-tdu@semihalf.com> <1506594158-15721-1-git-send-email-tdu@semihalf.com> <1506594158-15721-3-git-send-email-tdu@semihalf.com> <20170929083800.3a71c9a2@xeon-e3> <20171002110839.GB1132@bricha3-MOBL3.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20171002110839.GB1132@bricha3-MOBL3.ger.corp.intel.com> User-Agent: Mutt/1.5.23.1 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v2 2/4] net/mrvl: add mrvl net pmd driver 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: , X-List-Received-Date: Tue, 03 Oct 2017 06:33:59 -0000 On Mon, Oct 02, 2017 at 12:08:39PM +0100, Bruce Richardson wrote: > On Fri, Sep 29, 2017 at 08:38:00AM -0700, Stephen Hemminger wrote: > > On Thu, 28 Sep 2017 12:22:36 +0200 > > Tomasz Duszynski wrote: > > > > > + > > > +struct mrvl_rxq; > > > +struct mrvl_txq; > > > > These forward decl should not be nececessary > > > +static inline int > > > +mrvl_get_bpool_size(int pp2_id, int pool_id) > > No tab here please > > > > Why does this need to be inlined? Is it in critical path? > > > > > > > +{ > > > + int i; > > > + int size =3D 0; > > > + > > > + for (i =3D mrvl_lcore_first; i <=3D mrvl_lcore_last; i++) > > > + size +=3D mrvl_port_bpool_size[pp2_id][pool_id][i]; > > > + > > > + return size; > > > +} > > > + > > > > Also, I prefer that the following restrictions from the kernel be > > also applied to DPDK code. > > > > +1 for LINE_SPACING (multiple blank lines), MULTILINE_DEREFERENCE and > SPLIT_STRING. [Is split string not already enforced?] ACK > > I'm a bit ambivilant about forcing a blank line after definitions, I > think it's a good idea in most cases, but I'm not sure it needs to be > enforced in all cases. Agree. In this case however adding a few extra blank lines after definitions will not hurt, plus it will improve coding style consistency across driver sources. > > /Bruce > > > ### [dpdk-dev] [PATCH v2 2/4] net/mrvl: add mrvl net pmd driver > > > > CHECK:LINE_SPACING: Please don't use multiple blank lines > > #452: FILE: drivers/net/mrvl/mrvl_ethdev.c:180: > > + > > + > > > > WARNING:LINE_SPACING: Missing a blank line after declarations > > #457: FILE: drivers/net/mrvl/mrvl_ethdev.c:185: > > + int n =3D sizeof(*bitmap) * 8 - __builtin_clz(*bitmap); > > + if (n >=3D max) > > > > CHECK:LINE_SPACING: Please don't use multiple blank lines > > #562: FILE: drivers/net/mrvl/mrvl_ethdev.c:290: > > + > > + > > > > WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer= 'priv->ppio_params.inqs_params.tcs_params[i].inqs_params' > > #880: FILE: drivers/net/mrvl/mrvl_ethdev.c:608: > > + rte_free(priv->ppio_params.inqs_params. > > + tcs_params[i].inqs_params); > > > > WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer= 'priv->ppio_params.inqs_params.tcs_params[i].inqs_params' > > #882: FILE: drivers/net/mrvl/mrvl_ethdev.c:610: > > + priv->ppio_params.inqs_params. > > + tcs_params[i].inqs_params =3D NULL; > > > > WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer= 'priv->ppio_params.inqs_params.tcs_params[tc].inqs_params[inq].size' > > #1330: FILE: drivers/net/mrvl/mrvl_ethdev.c:1058: > > + qinfo->nb_desc =3D priv->ppio_params.inqs_params. > > + tcs_params[tc].inqs_params[inq].size; > > > > WARNING:SPLIT_STRING: quoted string split across lines > > #1476: FILE: drivers/net/mrvl/mrvl_ethdev.c:1204: > > + RTE_LOG(ERR, PMD, "Mbuf size must be increased to %u bytes" > > + " to hold up to %u bytes of data.\n", > > > > WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer= 'priv->ppio_params.inqs_params.tcs_params[priv->rxq_map[rxq->queue_id].tc' > > #1500: FILE: drivers/net/mrvl/mrvl_ethdev.c:1228: > > + priv->ppio_params.inqs_params. > > + tcs_params[priv->rxq_map[rxq->queue_id].tc]. > > > > WARNING:MULTILINE_DEREFERENCE: Avoid multiple line dereference - prefer= 'q->priv->ppio_params.inqs_params.tcs_params[q->priv->rxq_map[q->queue_id]= =2Etc' > > #1532: FILE: drivers/net/mrvl/mrvl_ethdev.c:1260: > > + num =3D q->priv->ppio_params.inqs_params. > > + tcs_params[q->priv->rxq_map[q->queue_id].tc]. > > > > WARNING:SPLIT_STRING: quoted string split across lines > > #1902: FILE: drivers/net/mrvl/mrvl_ethdev.c:1630: > > + RTE_LOG(DEBUG, PMD, "\nport-%d:%d: bpool %d oversize -" > > + " remove %d buffers (pool size: %d -> %d)\n", > > > > WARNING:SPLIT_STRING: quoted string split across lines > > #2094: FILE: drivers/net/mrvl/mrvl_ethdev.c:1822: > > + "No room in shadow queue for %d packets!!!" > > + "%d packets will be sent.\n", > > > > CHECK:LINE_SPACING: Please don't use multiple blank lines > > #2294: FILE: drivers/net/mrvl/mrvl_ethdev.c:2022: > > + > > + > > > > CHECK:LINE_SPACING: Please don't use multiple blank lines > > #2595: FILE: drivers/net/mrvl/mrvl_ethdev.h:40: > > + > > + > > > > WARNING:LINE_SPACING: Missing a blank line after declarations > > #3253: FILE: drivers/net/mrvl/mrvl_qos.c:577: > > + uint8_t idx =3D port_cfg->tc[tc].inq[i]; > > + priv->rxq_map[idx].tc =3D tc; > > -- - Tomasz Duszy=C5=84ski