From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f51.google.com (mail-lf0-f51.google.com [209.85.215.51]) by dpdk.org (Postfix) with ESMTP id 1397F1B2BD for ; Tue, 3 Oct 2017 08:23:44 +0200 (CEST) Received: by mail-lf0-f51.google.com with SMTP id c82so4620075lfc.6 for ; Mon, 02 Oct 2017 23:23:44 -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=uMjqAPaezs/bSBqVYnH3u/bOXGsyoNa2Wm3RNezhJSI=; b=l/pH9vxf5ZjkzbSc9RkNayoUtGAs8XtzX9ACQqVGJ5vFd1CqHSRwS8m+OW8hmUeSgU Zith62/+KpIbY/j2HH97zEo76Ez28p7rawOtSp4wSjOptxi7JE6jW2EaY4g86Ct9IscU iC4j2hKh2iV/WTZWYd2ZvM9RYyeAMIK95zfVdBgay4TlHY4/E9EvJAFxqtkJKy1VoO8Z z3vTCL70J5leek3OgCib0cAH4gnVHA1fTKAYbiA1BdXKUopHWICET3loULRXd1mVhA9U LpZZ9dV78ANugaw21LSp/CnIY9C2UFftFcfMSWlVJBxK7OVkkIUvNNFASelMV1RXeebZ SQtA== 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=uMjqAPaezs/bSBqVYnH3u/bOXGsyoNa2Wm3RNezhJSI=; b=igVkZGkARumzpwDaucPw32bi8xyGPRW1c+YDFHsTYoLaR4XPTpwoX9Uk+v7OeQWm+f f4jnhCLJv6ZYYw5osTCUFWhAzpLZsssCx7qlzShqp1uWggLOpScXSrB5Q0vCUM9aAlvG F2mZIP+nI0goyp5wEze+aJdzRBLHCZSFzgwXz6D8jFFh41J5Q3rjpBOASWTeZWTuiYd4 kYqKtk/7gJTggkr+597f9irmike8tGV2p8+hQwTJojUkGFR59MXovkKJPn9J5hA8di9z bahGhQt7+TLps2UXkpMvk5kFCMT4LYyyeOmRKUyyrH21NYh5ZuFNrd4XU/gEtY9S/oUL T7Wg== X-Gm-Message-State: AMCzsaXoUYnihqppcRJYCjROiSSI9b/Gwn6UFbDbBtafyut+xhbfReDI zmDcKdM8wSnO4AyBD+MFos9BSg== X-Google-Smtp-Source: AOwi7QDccU5+RbRd1dYRJrUlZKcCzz2PLM0ddm9KBMCiGwY/SQcbBYFLphc0Br9rpzeM6wpUzujkdA== X-Received: by 10.25.143.156 with SMTP id s28mr2549653lfk.236.1507011824415; Mon, 02 Oct 2017 23:23:44 -0700 (PDT) Received: from localhost (31-172-191-173.noc.fibertech.net.pl. [31.172.191.173]) by smtp.gmail.com with ESMTPSA id b15sm1885506lfl.88.2017.10.02.23.23.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 02 Oct 2017 23:23:43 -0700 (PDT) Date: Tue, 3 Oct 2017 08:23:42 +0200 From: Tomasz Duszynski To: Stephen Hemminger Cc: Tomasz Duszynski , dev@dpdk.org, mw@semihalf.com, dima@marvell.com, nsamsono@marvell.com, Jianbo.liu@linaro.org, Jacek Siuda Message-ID: <20171003062342.GA9598@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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20170929083800.3a71c9a2@xeon-e3> 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:23:45 -0000 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 ACK > > +static inline int > > +mrvl_get_bpool_size(int pp2_id, int pool_id) > No tab here please Good catch, thanks. > > Why does this need to be inlined? Is it in critical path? > > Right, in rx handler precisely. > > +{ > > + 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. > > ### [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) These were not triggered by the DPDK checkpatch wrapper. On the other hand using kernel checkpatch will show them. That makes we wonder which tool should actually be used? Anyway, will fix that in v3. > > 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", Personally I would go with kernel checkpatch, but DPDK checker complained about overly long lines thus such comments were split across lines. > > 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].t= c' > #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